adam/orchestrator
Archived
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(agent-team): accurate Slack status for build/verify states (no false 'plan could not be auto-approved')

Browse source 
The resume-followup notifier mislabeled build/verify-stage tasks. Two cases,
both surfaced live while smoke-testing the #60 builder fix:

1. AWAITING CI (in-progress): the VERIFY node suspends via interrupt() for the
   async CI-wait. Its interrupt payload is not question-shaped, so
   pending_question() returns None and the notifier treated the still-suspended
   task as a settled PARK -> posted '⚠️ PARKED ... the plan could not be
   auto-approved ... re-assign' for a task that had actually approved the plan,
   built a diff, and dispatched it to CI. Now detect the suspend (graph
   interrupted + dispatched run_id + a built diff) and post an honest
   'diff built and dispatched to CI (run X); awaiting verification' notice.

2. SETTLED build/verify PARK: phase inference keyed off review_verdicts and
   reported 'review' for a task that reached verify; the copy hardcoded 'the
   plan could not be auto-approved'. Now a built diff (candidate_diff/diff_hash)
   makes the inferred phase 'verify' and the copy reflect a verification/CI-gate
   failure, surfacing the CI conclusion via _summarize_build_blocker.

Plan/review escalation parks (no candidate_diff) are unchanged.

- coordinator.py: _post_resume_followups awaiting-CI branch + build-aware phase
  inference + build-aware PARKED copy; add _summarize_build_blocker.
- tests: built-park reports verify + CI conclusion (not auto-approve copy);
  awaiting-CI suspend reports in-progress, not parked.
This commit is contained in:
Adam Moussa 2026-06-24 14:36:00 -04:00
parent e42b910a80
commit d12880ed09
2 changed files with 156 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

78
agent-team/agent_team/coordinator.py
View file

@ -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.

82
agent-team/tests/test_coordinator.py
View file

@ -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
# --------------------------------------------------------------------------- #