diff --git a/agent-team/agent_team/coordinator.py b/agent-team/agent_team/coordinator.py index 48abc23..e4399df 100644 --- a/agent-team/agent_team/coordinator.py +++ b/agent-team/agent_team/coordinator.py @@ -1151,15 +1151,47 @@ class Coordinator: ) continue - # No pending question: the task settled. Distinguish parked vs done, - # and say WHERE it got to and WHAT is blocking it. + # ``built`` == the task reached the P3 build subgraph (it produced a + # candidate diff). This distinguishes a build/verify-stage state from a + # plan/review escalation, which the messages below key off so a + # successfully-built task is never reported as "the plan could not be + # auto-approved". status = values.get("status") + run_id = str(values.get("run_id") or "").strip() + built = bool(values.get("candidate_diff") or values.get("diff_hash")) + + # IN-PROGRESS, not settled: the VERIFY node SUSPENDS (async CI-wait) + # awaiting a dispatched run's terminal conclusion. Its interrupt + # payload is NOT question-shaped, so ``pending_question`` returns None + # even though the graph is still suspended โ€” without this branch the + # task is misread as a settled PARK and gets the "could not be + # auto-approved / re-assign" escalation copy. Detect the suspend (graph + # still interrupted + a dispatched run + a built diff) and report + # honest progress instead. + suspended = bool(getattr(snap, "interrupts", None)) or bool( + getattr(snap, "next", None) + ) + if question is None and suspended and built and run_id: + self._emit( + f"๐Ÿ› ๏ธ {label} โ€” plan approved; diff built and dispatched to CI " + f"(run `{run_id}`). Awaiting verification โ€” I'll post the " + "result when the run finishes.", + thread_ts=root_ts, + ) + continue + + # No pending question and not awaiting CI: the task settled. Distinguish + # parked vs done, and say WHERE it got to and WHAT is blocking it. phase = str(values.get("current_phase") or "unknown") # When a task parks, current_phase is the terminal "parked" โ€” not # useful. Infer the phase it was IN when it escalated, so the human - # sees WHERE it died (review > plan > clarify by what state exists). + # sees WHERE it stopped (verify > review > plan > clarify by what state + # exists). A built diff means it reached the build/verify subgraph, so + # that takes precedence over the plan/review escalation phases. if phase == "parked": - if values.get("review_verdicts"): + if built: + phase = "verify" + elif values.get("review_verdicts"): phase = "review" elif values.get("plan"): phase = "plan" @@ -1181,6 +1213,20 @@ class Coordinator: "Re-assign it to try again.", thread_ts=root_ts, ) + elif status == TaskStatus.PARKED.value and built: + # The diff WAS built and dispatched, but the build/verify gate did + # not pass (CI BLOCKed / no authenticated pass). This is NOT a + # "plan could not be auto-approved" escalation โ€” say what actually + # happened so the human looks at the CI run, not the plan. + blocker = self._summarize_build_blocker(values) + self._emit( + f"โš ๏ธ PARKED โ€” {label}\n" + f"โ€ข Reached phase: {phase}\n" + f"โ€ข What's blocking it: {blocker}\n" + "โ€ข The diff was built and dispatched, but the verification " + "gate did not pass. Review the CI run, then re-assign to retry.", + thread_ts=root_ts, + ) elif status == TaskStatus.PARKED.value: blocker = self._summarize_blocker(values) self._emit( @@ -1522,6 +1568,30 @@ class Coordinator: "requesting changes without converging)." ) + @staticmethod + def _summarize_build_blocker(values: "dict[str, Any]") -> str: + """Human-readable reason a BUILT task parked at the build/verify gate. + + Surfaces the CI conclusion the verifier gated on (``ci_results``) so the + human sees WHY verification failed โ€” a failing/no-pass run, or no + authenticated result at all โ€” rather than the plan-review escalation text + :meth:`_summarize_blocker` produces (which is wrong for a built diff). + """ + ci = values.get("ci_results") + if isinstance(ci, dict): + conclusion = str(ci.get("conclusion") or "").strip() + if conclusion: + run_id = str(ci.get("run_id") or values.get("run_id") or "").strip() + where = f" (run {run_id})" if run_id else "" + return ( + f"the CI verify run concluded '{conclusion}'{where}; the " + "pure-code gate did not pass." + ) + return ( + "the verification gate did not pass (no authenticated CI pass was " + "available for the dispatched diff)." + ) + @staticmethod def _verify_pass_verdict(values: "dict[str, Any]") -> "dict[str, Any] | None": """Return the verify-stage PASS verdict if the task reached the P3 PASS terminus. diff --git a/agent-team/tests/test_coordinator.py b/agent-team/tests/test_coordinator.py index 07b7d2a..341e1cd 100644 --- a/agent-team/tests/test_coordinator.py +++ b/agent-team/tests/test_coordinator.py @@ -1499,6 +1499,88 @@ def test_parked_message_infers_phase_when_current_phase_is_parked( assert "Reached phase: parked" not in msgs[0] +def test_followups_built_park_reports_verify_not_plan_escalation( + db_path: Path, monkeypatch: Any +) -> None: + # A task that BUILT a diff and parked at the verify/CI gate must report the + # build/verify stage + the CI conclusion โ€” NOT the plan-review escalation + # copy ("the plan could not be auto-approved"), which is wrong once a diff + # exists. Regression for the issue-#60 sibling notification bug. + from agent_team import coordinator as coord_mod + from agent_team.task_model import TaskStatus + + msgs: list[str] = [] + coord = _make_coordinator(db_path) + coord._notify = msgs.append + coord.setup() + monkeypatch.setattr( + coord_mod.graph_mod, "pending_question", lambda _g, *, thread_id: None + ) + + class _Snap: + values = { + "status": TaskStatus.PARKED.value, + "current_phase": "parked", + "task": "add a smoke-test file in agent-team/tests", + "candidate_diff": "diff --git a/x b/x\n", + "diff_hash": "abc", + "run_id": "r1", + "ci_results": {"run_id": "r1", "conclusion": "failure"}, + # Stale review verdicts must NOT win the phase inference for a built task. + "review_verdicts": [{"verdict": "request_changes", "findings": "old"}], + } + + monkeypatch.setattr(coord._graph, "get_state", lambda _cfg: _Snap()) + coord._post_resume_followups([_resume_result("beef0003cafe")]) + assert len(msgs) == 1 + m = msgs[0] + assert "Reached phase: verify" in m # build/verify, not "review" + assert "could not be auto-approved" not in m # the misleading copy is gone + assert "verification gate did not pass" in m + assert "failure" in m # the CI conclusion is surfaced + + +def test_followups_awaiting_ci_reports_in_progress_not_parked( + db_path: Path, monkeypatch: Any +) -> None: + # The VERIFY node SUSPENDS (async CI-wait) with a non-question interrupt, so + # pending_question is None even though the graph is still suspended. The + # notifier must report in-progress ("awaiting verification"), NOT a settled + # PARK โ€” this is the exact mislabel that posted "could not be auto-approved" + # to Slack for a healthy, building task. + from agent_team import coordinator as coord_mod + from agent_team.task_model import TaskStatus + + msgs: list[str] = [] + coord = _make_coordinator(db_path) + coord._notify = msgs.append + coord.setup() + monkeypatch.setattr( + coord_mod.graph_mod, "pending_question", lambda _g, *, thread_id: None + ) + + class _Snap: + interrupts = ("await-ci",) # graph still suspended on the CI-wait interrupt + next = ("verify_node",) + values = { + "status": TaskStatus.PARKED.value, # stale carried-over while suspended + "current_phase": "parked", + "task": "add a smoke-test file in agent-team/tests", + "candidate_diff": "diff --git a/x b/x\n", + "diff_hash": "abc", + "run_id": "run-42", + } + + monkeypatch.setattr(coord._graph, "get_state", lambda _cfg: _Snap()) + coord._post_resume_followups([_resume_result("d00d0004beef")]) + assert len(msgs) == 1 + m = msgs[0] + assert "awaiting verification" in m.lower() + assert "run-42" in m + assert "could not be auto-approved" not in m + assert "parked" not in m.lower() + + # --------------------------------------------------------------------------- # # CI-watcher sweep in tick (ยง3.3.2 Decision 2) โ€” async resume-on-CI-complete # --------------------------------------------------------------------------- #