init_db's own schema_meta write was ON CONFLICT DO NOTHING, and the daemon
(Coordinator.setup) calls init_db, never migrate() — so on an existing ledger
the column was ensured but schema_version was never advanced (observed live:
kind column present, schema_meta stuck at 3). migrate() already upserts the
version correctly but was effectively dead code (no production caller).
init_db now ends by calling migrate(conn), which steps the version and runs any
version-gated steps. Idempotent — re-running the create/ensure statements is
harmless. Regression test: an existing v3-stamped DB run through init_db now
reports schema_version == SCHEMA_VERSION (4) and has the kind column. 1491 passed.
_review_round_index counted EVERY review_verdicts entry, including the synthetic
human-gate verdict graph._apply_plan_decision folds in on a "request changes"
(reviewer == "human_plan_gate"). That inflated the count so a revised plan could
escalate prematurely without a fresh adversarial review.
Now counts only reviewer-authored verdicts: a new _is_reviewer_verdict excludes
entries tagged reviewer=="human_plan_gate" (read from the verdict dict's own
field — no graph.py import). A human request_changes now grants the revised plan
a fresh reviewer-round budget. Termination still bounded by MAX_PLAN_GATE_VISITS
(each request_changes consumes one gate visit). 1490 passed.
_revision_count read verdict.get("decision"), but verdicts are keyed "verdict"
(both reviewer and synthetic human-gate), so the count was always 0 and the
plan_node MAX_PLAN_REVISIONS self-park was dead code. Now reads "verdict" first
(fallback "decision"), matching _format_review_feedback's precedence; the
existing .strip().upper()==_REQUEST_CHANGES compare covers both request_changes
and REQUEST_CHANGES. Counts reviewer + human request_changes.
Cap composition: the review-loop round cap and MAX_PLAN_GATE_VISITS govern the
live loops; the planner MAX_PLAN_REVISIONS is now a correct backstop (was inert),
not a behavior change to the gate. Tests drive the real "verdict" key and prove
the previously-dead park fires. 1487 passed.
Security-review follow-up (LOGIC-01/02/05, all confirmed correctness).
- New transport-neutral `decisions.normalize_decision(raw, *, allow_abandon)` is
the single source of truth: approve-allowlist→approve; abandon-allowlist→abandon
ONLY when allow_abandon; everything else (prose, empty, abandon-verbs when
disallowed) → request_changes with the full reply as notes; idempotent on an
already-formed decision dict. slack_adapter.map_plan_decision is now a thin
wrapper (default allow_abandon=True, no caller churn).
- LOGIC-01/02: graph._parse_decision now delegates to normalize_decision (was:
any unrecognized verb → abandon → FAILED). The graph is now the universal safe
backstop, so EVERY writer that bypassed the listener mapping — operator CLI
answer_on_behalf (raw), Coordinator.submit_answer (raw), the recovery sweep —
loops back on prose instead of silently FAILing the task. Explicit abandon
still abandons (preserves the confirmed-button path).
- LOGIC-05: the Slack FREE-TEXT reply path maps with allow_abandon=False, so a
bare "cancel"/"stop"/"abandon" typed in-thread → request_changes (never
terminal abandon); abandon stays reachable only via the confirm-guarded button.
Tests: graph unrecognized→loops-back (not FAILED), operator raw-prose→request_
changes, free-text destructive verbs→request_changes vs button→abandon,
normalizer idempotency. 1484 passed.
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).
Hermetic e2e tests driving the WHOLE stack composed together — real
build_graph(plan_gate=True) + real Coordinator + real SQLite ledger + real
review_loop router, with only the LLM nodes stubbed — through the daemon API
(start_task/submit_answer/tick), never nodes directly.
Flows: approve settles at BUILD; request-changes via RAW PROSE through the real
SlackListener -> _resolve_payload -> map_plan_decision proves the prose maps to
request_changes (NOT FAILED) and the notes reach the planner; abandon -> FAILED;
repeated request_changes terminates at MAX_PLAN_GATE_VISITS -> PARKED; and a
legacy (no-kind) ledger migrates in place then routes clarify vs plan_decision
correctly. 1459 passed.
Turn a human's Slack interaction at the plan gate into a structured decision the
graph can route, with the kind-aware mapping that closes a silent-FAIL hazard.
- KIND-AWARE NORMALIZATION (load-bearing): map_plan_decision() in slack_adapter
maps a reply to {"decision","notes"} — approve ∈ {approve,approved,yes,ok,lgtm,
ship}; abandon ∈ {abandon,reject,cancel,stop,kill}; EVERYTHING ELSE →
request_changes with the full reply as notes (never accidental abandon). Wired
in the listener's _resolve_payload for plan_decision rows ONLY (clarify passes
through). Without this, arbitrary change-notes hit the graph's
unrecognized-verb→FAILED path and silently fail the task. Anti-FAIL tests
assert prose → request_changes (!= abandon) at both the mapper and the
end-to-end listener seam; a regression test guards clarify pass-through.
New find_open_question_kind_by_channel_ref (anti-replay, status='open') powers
the thread-reply fallback's kind lookup.
- BUTTONS + MODAL: build_plan_decision_blocks() renders Approve (primary) /
Request changes / Abandon (danger+confirm); question_id double-anchored in
message metadata AND each button value ("<verb>:<question_id>"). Approve/abandon
submit via the existing @app.action(.*); request_changes has a dedicated
handler that AUTHORIZES before views_open (proven by test) and opens a notes
modal (private_metadata carries the id) → view_submission → request_changes +
notes. Free-text reply stays the always-available equal path. AUTHZ-01 ordering
preserved.
- No manifest change (views.open needs no extra scope).
1454 passed (1412 + 42).
Connect the graph plan-gate (B2a) to the durable ledger + Slack presentation.
- setup() passes build_graph(plan_gate=True) only on the wired review path
(plan_gate flag ANDed with review_node present); P1/stub paths force it off.
- _post_resume_followups detects a settled interrupt by payload
kind == PLAN_DECISION_KIND (NOT status, which still reads 'parked' at the
gate per B2a) and posts the decision gate: opens a pending_questions row with
kind='plan_decision' (24h deadline, threaded, channel_ref = root ts) and
presents _summarize_plan + findings + reply instructions, truncated to a
~2700-char Slack budget. A clarify/legacy interrupt keeps the existing path.
- Single-open-gate invariant: the opener skips if any open row already exists
for the thread (one row, one presentation).
- Expiry: _park posts a plan-decision-specific recovery notice (re-assign /
force-resume) for an expired gate row.
- Resume path unchanged: the decision answer flows through submit_answer →
ResumeWorker → plan_gate_node with no resume-worker special-casing.
notify_question has no kind param in this tree, so the opener calls
ledger.post_question(kind=...) + ledger.set_channel_ref directly; the clarifier
path still uses notify_question unchanged.
Tests: gate row+presentation+threading, approve/request_changes/abandon via
submit_answer, single-open-gate skip, expiry notice. 1412 passed.
Replace the terminal review-cap PARK with a resumable human decision gate,
opt-in via build_graph(plan_gate=True) (default False → all existing P1/P2/P3
wiring unchanged).
- plan_gate_node interrupt()s mirroring the clarifier contract (same payload
keys → existing pending_question() extractor + turn-guarded ResumeWorker drive
it with zero special-casing) plus a kind="plan_decision" discriminator and the
plan + latest review findings as context.
- Decision contract {"decision": approve|request_changes|abandon, "notes": ...}:
approve → the same terminal state an auto-approved plan reaches (ACTIVE/BUILD);
request_changes → append a synthetic human verdict to review_verdicts (so the
planner's _format_review_feedback surfaces the notes) and loop back to PLAN;
abandon / unrecognized → terminal FAILED (safe default, never accidental
approve).
- Bounded termination: MAX_PLAN_GATE_VISITS=3 combined ceiling on plan_gate_visits
(new channel on PipelineState + TaskRecord); on exhaustion the gate goes
terminal PARKED ("revision ceiling reached") WITHOUT interrupting. Proven by a
loop-past-ceiling test.
Notes for the coordinator wiring (B2b): while suspended at the gate the status
channel still reads 'parked' (carried over from review_node's escalate branch) —
the load-bearing "awaiting decision, not terminal" signal is the live pending
interrupt + kind="plan_decision", NOT the status channel.
Tests: interrupt-at-cap, approve/request_changes(notes)/abandon routing,
ceiling-terminates, auto-approve still bypasses the gate. 1404 passed.
The plan-review gate (coming next) needs to tell its decision questions apart
from clarifier questions in the durable ledger. Add a `kind` column to
pending_questions (values 'clarify' | 'plan_decision').
- Fresh DBs: `kind TEXT NOT NULL DEFAULT 'clarify'` (+ CHECK) in the DDL.
- Live ledger: idempotent additive migration (SCHEMA_VERSION 3→4) — a guarded
ALTER (PRAGMA table_info) run from both migrate() and init_db; legacy rows
take the 'clarify' default, never null. (SQLite can't add a CHECK via ALTER,
so the migrated column is NOT NULL DEFAULT only; value constraint is enforced
on fresh DBs by the CHECK and on all writes by the typed helper.)
- ledger.post_question gains a keyword-only `kind="clarify"` (backward
compatible — existing callers unchanged); PendingQuestion.from_row reads it.
Tests: fresh-DB column+default, idempotent init_db, legacy-DB backfill to
'clarify', plan_decision round-trip. 1396 passed.
The planner's single-shot Claude call intermittently failed with "Reached
maximum number of turns (1)" — it needs slightly more headroom than the
clarifier to finish emitting its JSON. Phase A of the planner-reliability plan:
- plan_node now invokes with max_turns=4 (allowed_tools stays []; the extra
turns buy completion, not exploration).
- build_plan_prompt instructs the model to use no tools and return only JSON
(a tool_use would consume the single turn before the plan is emitted).
- plan_node auto-retries the model call exactly once on a TRANSIENT failure
(turn-cap exhaustion or an empty reply), and fails fast on DETERMINISTIC ones
(malformed JSON, missing/blank phases) — a retry would just reproduce those.
review_loop_llm.py is intentionally GPT-4.1 cross-family (no claude_invoke), so
it gets no turn-budget change. verifier_llm.py does use claude_invoke but is P3
build/verify scope — left for a follow-up.
Tests: max_turns passthrough; retry on turn-cap and on empty; no retry on
malformed JSON; the no-tools prompt line. 1387 passed.
