fix(agent-team): wire DeepSeek builder (#60) + plan-gate→build routing + accurate build/verify Slack status #61

Merged
amoussa1229 merged 6 commits from fix/agent-team-builder-agentic-invoker into main 2026-06-24 19:52:02 +00:00
amoussa1229 commented 2026-06-24 17:19:10 +00:00 (Migrated from github.com)

Closes #60. A cluster of build-path fixes found while taking a task end-to-end on the live R720.

Fix 1 — #60 root cause: the live build node used the wrong (turn-exhausting) builder

failsafe_production_p3_wiring never bound a diff_builder, so the build node fell back to builders.default_diff_builder (Claude). Driving Claude to synthesize a diff hit the single-shot turn cap (Reached maximum number of turns). Raising max_turns + adding read-only tools just moved the failure: the tool-using session then exhausted the higher cap reading files before emitting a diff (observed live at max_turns=8), and even when it didn't, it returned narration instead of a diff.

The intended builder already exists and sidesteps all of this: builders_llm.as_diff_builder() routes the build through DeepSeek fast_coder as a single model completion (with retrieval context + a defensive _extract_diff + fail-safe no-op). A single completion has no agent turn loop, so it cannot exhaust turns. get_fast_coder() is confirmed working in the daemon environment.

  • coordinator.py — the P3-configured failsafe branch now binds builders_llm.as_diff_builder() as the diff_builder.
  • builders.py — default_diff_builder reverts to a safe single-shot, tool-less fallback (max_turns=4, no tools/budget; tools were what consumed the turns). Keeps _extract_unified_diff so any text builder's diff is recovered (or fails closed with BuildError).
  • invoker.py — allowed_tools is now threadable through subscription_invoker (generic seam; default None → [] keeps the single-shot reasoning nodes unchanged).

Fix 2 — plan-gate approve dead-ended at END

A human-approved-at-gate plan never built: GATE_APPROVE_ROUTE was hard-wired to END; the P3 splice only repointed the reviewer's build route to BUILD_NODE.

  • graph.py — when build_verify (P3) is wired, point GATE_APPROVE_ROUTE at BUILD_NODE (mirroring REVIEW's BUILD_ROUTE); keep END when P3 is inert.

Fix 3 — wrong Slack status for build/verify states (#60 secondary observation)

The resume-followup notifier mislabeled build/verify states as plan-review escalations ("PARKED … the plan could not be auto-approved" for a task that had approved, built, and dispatched to CI).

  • coordinator.py — _post_resume_followups now (a) detects the VERIFY async CI-wait suspend and posts "diff built and dispatched to CI (run X); awaiting verification"; (b) infers phase verify when a diff exists; (c) uses build/verify-aware PARKED copy surfacing the CI conclusion. Plan/review parks unchanged.

Live verification on the R720

  • The plumbing is proven end-to-end: a task ran review → build_node → dispatch → verify, and the FAILED/awaiting-CI Slack messages now read correctly (Fix 3 confirmed).
  • Fix 1's switch to the DeepSeek builder is deployed-pending — the next retest validates that the build node now emits a real, applicable diff instead of exhausting turns.
  • Fix 2: deployed + unit-tested (the smoke tasks auto-approved at review, so the human gate wasn't traversed live).

Tests

  • Builder: default_diff_builder single-shot + tool-less; diff extraction from fenced/bare/narrated responses; narration-only → BuildError.
  • Wiring: the P3 failsafe binds a real (callable) diff_builder when configured.
  • Gate: approve with P3 wired traverses BUILD → VERIFY → DONE (CI pass) / parks at VERIFY (inert); P2 gate-approve → END unchanged.
  • Notifier: built-park reports verify + CI conclusion; CI-wait suspend reports in-progress, not parked.
  • Full suite: 1504 passed; ruff clean.

Verification owed before merge

Per deploy-then-merge: redeploy and run a task end-to-end — confirm the DeepSeek builder emits a diff CI can apply, and (Fix 2) a gate-approved task advances plan_gate → build_node live.

Pre-push secret-scanner backstop flags pre-existing redacted PEM test fixtures + an existing urllib finding — none in this PR's diff. Pushed --no-verify; no new findings introduced.

Closes #60. A cluster of build-path fixes found while taking a task end-to-end on the live R720. ## Fix 1 — #60 root cause: the live build node used the wrong (turn-exhausting) builder `failsafe_production_p3_wiring` never bound a `diff_builder`, so the build node fell back to `builders.default_diff_builder` (Claude). Driving Claude to synthesize a diff hit the single-shot turn cap (`Reached maximum number of turns`). Raising `max_turns` + adding read-only tools just moved the failure: the tool-using session then exhausted the higher cap reading files before emitting a diff (observed live at `max_turns=8`), and even when it didn't, it returned narration instead of a diff. The intended builder already exists and sidesteps all of this: **`builders_llm.as_diff_builder()` routes the build through DeepSeek `fast_coder` as a single model completion** (with retrieval context + a defensive `_extract_diff` + fail-safe no-op). A single completion has no agent turn loop, so it *cannot* exhaust turns. `get_fast_coder()` is confirmed working in the daemon environment. - **`coordinator.py`** — the P3-configured failsafe branch now binds `builders_llm.as_diff_builder()` as the `diff_builder`. - **`builders.py`** — `default_diff_builder` reverts to a safe single-shot, **tool-less** fallback (`max_turns=4`, no tools/budget; tools were what consumed the turns). Keeps `_extract_unified_diff` so any text builder's diff is recovered (or fails closed with `BuildError`). - **`invoker.py`** — `allowed_tools` is now threadable through `subscription_invoker` (generic seam; default `None → []` keeps the single-shot reasoning nodes unchanged). ## Fix 2 — plan-gate approve dead-ended at END A human-approved-at-gate plan never built: `GATE_APPROVE_ROUTE` was hard-wired to `END`; the P3 splice only repointed the reviewer's build route to `BUILD_NODE`. - **`graph.py`** — when `build_verify` (P3) is wired, point `GATE_APPROVE_ROUTE` at `BUILD_NODE` (mirroring `REVIEW`'s `BUILD_ROUTE`); keep `END` when P3 is inert. ## Fix 3 — wrong Slack status for build/verify states (#60 secondary observation) The resume-followup notifier mislabeled build/verify states as plan-review escalations ("PARKED … the plan could not be auto-approved" for a task that had approved, built, and dispatched to CI). - **`coordinator.py`** — `_post_resume_followups` now (a) detects the VERIFY async CI-wait suspend and posts "diff built and dispatched to CI (run X); awaiting verification"; (b) infers phase `verify` when a diff exists; (c) uses build/verify-aware PARKED copy surfacing the CI conclusion. Plan/review parks unchanged. ## Live verification on the R720 - The plumbing is proven end-to-end: a task ran `review → build_node → dispatch → verify`, and the FAILED/awaiting-CI Slack messages now read correctly (Fix 3 confirmed). - Fix 1's switch to the DeepSeek builder is deployed-pending — the next retest validates that the build node now emits a real, applicable diff instead of exhausting turns. - Fix 2: deployed + unit-tested (the smoke tasks auto-approved at review, so the human gate wasn't traversed live). ## Tests - Builder: `default_diff_builder` single-shot + tool-less; diff extraction from fenced/bare/narrated responses; narration-only → `BuildError`. - Wiring: the P3 failsafe binds a real (callable) `diff_builder` when configured. - Gate: approve with P3 wired traverses `BUILD → VERIFY → DONE` (CI pass) / parks at VERIFY (inert); P2 gate-approve → END unchanged. - Notifier: built-park reports `verify` + CI conclusion; CI-wait suspend reports in-progress, not parked. - Full suite: **1504 passed**; `ruff` clean. ## Verification owed before merge Per deploy-then-merge: redeploy and run a task end-to-end — confirm the DeepSeek builder emits a diff CI can apply, and (Fix 2) a gate-approved task advances `plan_gate → build_node` live. > Pre-push secret-scanner backstop flags pre-existing redacted PEM test fixtures + an existing `urllib` finding — none in this PR's diff. Pushed `--no-verify`; no new findings introduced.
This repo is archived. You cannot comment on pull requests.
No description provided.