diff --git a/agent-team/DEPLOY-R720.md b/agent-team/DEPLOY-R720.md index 76a6e09..016d4e5 100644 --- a/agent-team/DEPLOY-R720.md +++ b/agent-team/DEPLOY-R720.md @@ -118,6 +118,23 @@ The unit runs `python3 run-team.py serve` from secrets from `EnvironmentFile=/home/adam/secrev.env`. `Restart=on-failure` keeps it up across transient faults; `journalctl -u` is the live log. +> **⚠️ This deploy carries a ledger migration (SCHEMA_VERSION → 4).** It adds a +> `kind` column to `pending_questions` (values `clarify` | `plan_decision`, +> existing rows default to `clarify`) for the plan-review decision gate. The +> migration is an **additive, idempotent in-place `ALTER TABLE`** run on startup +> (`init_db` / `migrate`, guarded so a second run is a no-op — it never recreates +> the table), so it applies in place against the live box ledger. **Take the +> ledger backup (§1 / the deploy script step) BEFORE restart** — it is the +> migration's safety net (see Rollback, §6). After restart, confirm the column +> landed and the daemon came up clean: +> ```bash +> sqlite3 ~/orchestrator/agent-team/state/agent_team.sqlite \ +> "PRAGMA table_info(pending_questions);" | grep kind # expect a 'kind' row +> sqlite3 ~/orchestrator/agent-team/state/agent_team.sqlite \ +> "SELECT schema_version FROM schema_meta WHERE id=1;" # expect 4 +> journalctl -u agent-team-coordinator.service -e | tail # no migration/import errors +> ``` + ## 4b. WS0–WS5 rollout — UPDATE an already-deployed box The steps above (§1–4) are the **first-time** P1 provision. To bring an @@ -286,6 +303,24 @@ it without a full snapshot restore: back it up first, then wipe. cp ~/orchestrator/agent-team/state/agent_team.sqlite{,.bak} # back up rm ~/orchestrator/agent-team/state/agent_team.sqlite* # wipe (then re-run init-db) ``` + +**Ledger-migration rollback (the schema-v4 `kind` migration).** The deploy takes +a dated ledger backup (`~/agent_team.sqlite.bak-`, written by the +`/sh-deploy-r720` flow / `scripts/deploy-r720.sh`) **before** restart — that +backup is the migration's safety net. The `kind` migration is additive and +idempotent, but **if the `init_db`/`migrate` step fails, or the deploy is rolled +back to pre-v4 code after the migration ran, restore the ledger from that backup +BEFORE restarting the coordinator** (old code does not expect the new column to +matter, but restoring guarantees a clean, pre-migration ledger): +``` +sudo systemctl stop agent-team-coordinator.service +cp ~/agent_team.sqlite.bak- ~/orchestrator/agent-team/state/agent_team.sqlite +rm -f ~/orchestrator/agent-team/state/agent_team.sqlite-wal \ + ~/orchestrator/agent-team/state/agent_team.sqlite-shm # drop stale WAL/SHM +sudo systemctl start agent-team-coordinator.service +``` +Restore the ledger backup BEFORE the coordinator restarts — never start the +daemon against a half-migrated or suspect ledger. Note the secrets in `~/secrev.env` are NOT removed by rollback - leave them, or strip the four agent-team keys if you are decommissioning entirely. diff --git a/agent-team/README.md b/agent-team/README.md index 95729af..d7b126e 100644 --- a/agent-team/README.md +++ b/agent-team/README.md @@ -2,15 +2,28 @@ The durable, human-gated agentic SDLC pipeline for the R720 (`sh-secrev` VM), design: `../docs/r720-agent-team-design.md`. A task flows INTAKE → CLARIFY (the -human gate) → PLAN → REVIEW, and (P3) → BUILD → DISPATCH → VERIFY → draft +first human gate) → PLAN → REVIEW, and (P3) → BUILD → DISPATCH → VERIFY → draft PR. Every stage is durable and resumable (LangGraph + a SQLite checkpointer); -the human gate suspends on `interrupt()` and resumes on a real answer. +both human gates suspend on `interrupt()` and resume on a real answer. + +There are now **two human gates**: the **clarifier** (CLARIFY asks question-sets +until confident) and the **plan-decision gate** (the dead-end when PLAN ⇄ REVIEW +cannot auto-converge). When the review loop hits its revision cap (or the planner +salvages only a partial plan), the pipeline no longer terminally PARKs — it +suspends on a resumable `interrupt()` and the coordinator posts the plan + +reviewer findings to Slack `#agent-team`, threaded under the task root, for the +owner to decide. ``` INTAKE → CLARIFY (Claude, human gate) → PLAN (Claude) → REVIEW (GPT-4.1) ▲ │ └── loop-back ───┤ - approve/escalate → END + approve → BUILD/END + review-cap / partial plan + → PLAN-DECISION GATE (human) + approve → BUILD/END + request changes → PLAN + abandon → FAILED (P3 box path — gated behind the C1 re-review): approve → BUILD (DeepSeek) → DISPATCH (push branch, trigger CI, capture run_id, suspend) → [CI-watcher resumes on terminal @@ -64,7 +77,8 @@ agent-team/ operator_cli.py nodes/ # pipeline stages + their model bindings clarifier.py + clarifier_llm.py # human gate (Claude) - planner.py # plan (Claude) + planner.py # plan (Claude); per-call max_turns=4 + + # classified retry-once (reliability fix) review_loop.py + review_loop_llm.py # adversarial review (GPT-4.1 via orchestrator) builders.py + builders_llm.py # candidate diff (DeepSeek) — INERT, proposes only verifier.py + verifier_llm.py # ci_gate sole PASS authority; LLM = fix-proposer; @@ -102,12 +116,34 @@ snake_case (`agent_team/`), per the engineering handbook. ## Key design points -- **Durable human gate (§3.3.1).** The `pending_questions` ledger is the single - source of truth for the question lifecycle. Every race (duplicate answers, - transport redelivery, answer-vs-timeout) resolves via one atomic - compare-and-set against `status`, inside a `BEGIN IMMEDIATE` transaction — - first-answer-wins (`rowcount == 1`), late/duplicate ignored. The LangGraph - `SqliteSaver` checkpointer shares the same DB file. +- **Durable human gates (§3.3.1).** The `pending_questions` ledger is the single + source of truth for the question lifecycle, with a `kind` discriminator + (`clarify` | `plan_decision`) marking which gate a row belongs to (schema v4, + idempotent additive migration). Every race (duplicate answers, transport + redelivery, answer-vs-timeout) resolves via one atomic compare-and-set against + `status`, inside a `BEGIN IMMEDIATE` transaction — first-answer-wins + (`rowcount == 1`), late/duplicate ignored. **Single-open-gate invariant:** a + thread holds at most one open question at a time (the clarifier row is answered + before the plan stage runs), so clarifier and plan-decision gates can never be + open simultaneously for one thread. The LangGraph `SqliteSaver` checkpointer + shares the same DB file. +- **Plan-review decision gate.** When PLAN ⇄ REVIEW cannot auto-converge + (review-revision cap) or only a partial plan is salvaged, the coordinator posts + the plan (`_summarize_plan`) + reviewer findings (`_summarize_blocker`) to Slack + `#agent-team` and the task owner decides via three verbs — **Approve** (settle + the plan → BUILD), **Request changes** (loop back to the planner with the notes + folded into review feedback), **Abandon** (FAILED). The decision arrives via + Block Kit buttons, a notes modal, or a free-text thread reply; **free-text + prose that isn't a recognized approve/abandon verb defaults to request-changes** + (carrying the full reply as the notes) so a change request can never be + misread as an accidental approve or abandon. Bounded by `MAX_PLAN_GATE_VISITS` + (= 3) so the human loop always terminates. +- **Planner reliability.** The planner's single-shot Claude call runs with + `max_turns=4` (tools stay disabled) so it has room to finish emitting its JSON + rather than exhausting the default 1-turn budget mid-reply, plus a classified + retry-once: a *transient* failure (turn-cap exhaustion or an empty reply) is + retried exactly once; a *deterministic* failure (malformed JSON, missing + phases) fails fast. - **Fail-safe model seams.** Every node treats model output as untrusted and fails SAFE: garbage never clears the 98% clarifier gate, never auto-approves a plan, never fabricates a build success, and the verifier's `ci_gate` is the diff --git a/docs/provisioning/OPERATOR-RUNBOOK.md b/docs/provisioning/OPERATOR-RUNBOOK.md index cec3cbf..21c1999 100644 --- a/docs/provisioning/OPERATOR-RUNBOOK.md +++ b/docs/provisioning/OPERATOR-RUNBOOK.md @@ -124,6 +124,70 @@ Jira ticket per the ladder) rather than starving silently. --- +## Incident 2b — Plan-review decision gate (PLAN ⇄ REVIEW dead-end) + +A second human gate opens when the plan↔review loop **cannot auto-converge** (the +review-revision cap is hit) or the planner produced only a partial plan. Instead +of terminally parking, the coordinator suspends on a resumable `interrupt()` and +posts the **plan + reviewer findings** to Slack `#agent-team`, **threaded under +the task root**, with three decision verbs. The ledger row carries +`kind = 'plan_decision'` (a clarifier row is `kind = 'clarify'`); both are +ordinary `pending_questions` rows, so the same `list` / `show` / `force-resume` +verbs apply. + +**The three decision paths (all equivalent — pick whichever is handy):** + +| Path | How | Effect | +|---|---|---| +| **Buttons** | Block Kit *Approve* / *Request changes* / *Abandon* on the gate message | Approve & Abandon submit immediately; *Request changes* opens a notes modal | +| **Modal** | the *Request changes* button → a one-field "What should change?" modal | submits `request_changes` with your notes | +| **Free-text reply** | reply in the gate thread | parsed kind-aware (below) | + +**What each decision does:** +- **Approve** — settles the plan (advances to BUILD / continues the pipeline). +- **Request changes (+ notes)** — loops back to the **planner**, folding the notes + into the review feedback so the re-plan addresses them. +- **Abandon** — fails the task (terminal `FAILED`). + +**Free-text mapping (the safe-default rule).** A thread reply is normalized +(lowercase/strip) and matched against small allowlists: +`approve ∈ {approve, approved, yes, ok, lgtm, ship}`; +`abandon ∈ {abandon, reject, cancel, stop, kill}`. **Anything else — any other +prose, including empty/whitespace — maps to *request changes*, carrying the full +reply as the notes.** So typing change notes in the thread (e.g. "use pytest +fixtures instead") requests changes; it can never be misread as an accidental +approve or abandon. + +**Single-open-gate invariant.** A thread holds **at most one open question at a +time** — the clarifier row is already answered before the plan stage runs, so a +clarifier-answer and a plan-decision can never be open simultaneously for one +thread. If a second open row for a thread ever appears, treat it as a bug +(the coordinator logs + skips opening it) and inspect with `list --all`. + +**Ceiling behavior.** The human request-changes loop is bounded by +`MAX_PLAN_GATE_VISITS` (graph constant, currently **3**, distinct from the +planner's `MAX_PLAN_REVISIONS`). When the gate-visit ceiling is exhausted the +node does **not** re-open the gate — it returns terminal **PARKED** with a +"revision ceiling reached" note, so the loop always terminates. + +**Expiry / recovery (gate goes unanswered).** A `plan_decision` row uses the +**same 24h deadline window as the clarifier**. If unanswered, the deadline sweep +**expires** the row → the task **PARKS**, and the coordinator posts a +`⌛ PLAN DECISION EXPIRED` lifecycle notice naming the task + the recovery path, +threaded under the task root. Recover the same way as a parked clarifier: + +```bash +python3 run-team.py list --parked +python3 run-team.py show # kind == "plan_decision", status == expired +python3 run-team.py force-resume --confirm # reopens the expired gate for re-delivery +``` + +Then answer it (Slack buttons / modal / free-text reply, or CLI `answer`). If the +task is better restarted from scratch, **re-assign** it instead. There is no +auto-retry — a PARKED plan gate stays PARKED until an operator acts. + +--- + ## Incident 3 — Failed human-in-the-loop resume **Symptoms:** an answer was submitted (Slack or CLI) but the graph did not