docs(agent-team): document the plan-review decision gate (Phase C)
README: two human gates (clarifier + plan-decision), the approve/request-changes/ abandon verbs, free-text-defaults-to-request-changes, MAX_PLAN_GATE_VISITS, and the planner max_turns reliability fix. OPERATOR-RUNBOOK: how the gate appears in Slack, the three decision paths (buttons/modal/free-text), the single-open-gate invariant, ceiling→PARKED, and the 24h expiry→PARKED→recovery (re-assign / force-resume). DEPLOY-R720: the pending_questions.kind ledger migration (SCHEMA_VERSION→4, idempotent additive ALTER on startup) + rollback (restore the ledger backup before restart if the migration fails).
This commit is contained in:
parent
082e45bf88
commit
f46d691e36
3 changed files with 145 additions and 10 deletions
|
|
@ -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-<date>`, 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-<date> ~/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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 <qid> # kind == "plan_decision", status == expired
|
||||
python3 run-team.py force-resume <qid> --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
|
||||
|
|
|
|||
Reference in a new issue