The apply pipeline's draft PRs were titled `agent-apply: <task_id>
(diff <hash>)` with a flat body — neither within Sea Haven PR
conventions. That format was deliberate injection-hardening (only
sanitized tokens, never model free-text; §4.6).
Thread the approved plan's title through dispatch as an optional
`pr_title` input, sanitized box-side (single line, no control chars,
70-char cap, capitalized) and RE-VALIDATED in the workflow as defense-
in-depth, with the hardened `agent-apply: <task_id>` title as the
fallback when empty/unsafe. Provenance (task id, diff hash, head) moves
to the PR body. gh consumes both as argv data, never shell-interpolated.
Adds SYNC_WEB=1 to /sh-deploy-r720's script: build agent-team/web/dist on the Mac
(box Node too old for Vite), rsync it with --delete (clears stale hashed bundles),
force a status-service restart, and verify the served index.html references the
freshly-built bundle. Also stops the verify step false-failing on the known benign
draft-pr-monitor 'gh'-missing traceback (strips that block before judging the
journal; is-active/NRestarts/threads remain the authoritative crash-loop signals).
Adds .github/workflows/ci-web.yaml, a thin caller over the org
Sea-Haven-Industries/.github ci-typescript-frontend.yaml reusable workflow,
scoped to agent-team/web changes. Runs build (tsc -b + vite build = typecheck)
and vitest on PRs to main. Distinct job id ci-web (status 'ci-web / ci') so it
does not collide with the Python 'ci / ci' context. format:check/lint/test:e2e
steps toggled off until Prettier/ESLint/Playwright are wired.
Verified locally: npm ci + npm run build + npm test (36/36) all green.
The box-side dispatch smoke test failed at `git clone` (exit 128, "could not
read Username"): GitHub's git smart-HTTP transport authenticates an installation
token via BASIC auth (username x-access-token), not Bearer. Bearer is the REST
API form (minting + workflow_dispatch + run-list all use it correctly) but git
rejects it.
app_branch_pusher now sets the http.extraHeader to
`Authorization: Basic <base64("x-access-token:" + token)>`. Verified on the box:
Bearer -> exit 128, Basic -> clone OK. _scrub now also redacts the base64
credential blob (it decodes to the token). Test updated to assert the Basic form
and that the raw token never appears literally in the header.
End-to-end validation surfaced that auto-dispatch parked every task at
"empty declared_scope": dispatch_node required plan["scope"], but the planner
emits only summary+phases (never a scope) and config.allowed_scope defaults to
None, so no node ever populated it. (The earlier dispatch test was
operator-initiated with an explicit scope; the auto planner->build->dispatch
path was never exercised until box-side App dispatch went live.)
Fix: when no planner-/operator-declared scope is present, dispatch_node derives
declared_scope from the candidate diff's own touched paths (ci_gate.diff_touched_paths).
This supplies the missing scope without relaxing any CI trust control — the
apply/verify workflow still INDEPENDENTLY re-checks the materialized diff against
the denylist + '..'-escape + this scope + the diff-hash binding, and the
agent-apply environment's required reviewer remains the human gate. A non-empty
diff that parses to zero touched paths still parks (fail closed).
Tests: the three old park-on-missing-scope cases now assert scope-from-diff
dispatch; added a park case for a diff with no parseable paths. 1527 pass.
The App private key is placed under ~/.ssh (already mode 700) rather than
~/.sea-haven (which holds the synced engineering-handbook). Update the env path,
the secure-copy block, and the rotation/incident-response rm to ~/.ssh.
App agent-team-apply: app_id 4119505, installation 141992144 (not secrets — only
the .pem is). A 2026-06-24 test mint confirmed contents:write + actions:write +
repository_selection:selected. The box gets its own freshly-generated private key
(the CI-side AGENT_APPLY_APP_PRIVATE_KEY Actions secret is write-only and cannot
be re-exported); secure-copy it to the box and delete the Mac copy after.
- app_run_locator: default a missing status_code to None (fail closed -> raise)
rather than 200, so a malformed response object can never be treated as a
successful run list.
- app_run_locator: drop the unused `_now` parameter (dead/misleading — the floor
is derived solely from since_iso) and simplify the redundant two-step `_sleep`
indirection to a single resolution.
- github_app._parse_expires_at: normalise a naive parsed datetime to aware UTC so
an offset-less expires_at cannot raise a bare TypeError in TokenProvider.token
(bypassing the fail-closed GitHubAppError contract).
- coordinator._app_dispatch_seams: use the module-level Path import instead of a
redundant inline one.
Follow-ups (left to respect the plan's "leave the gh _default_* seams untouched,
additive only"): the floor/skew/poll scaffold is duplicated between
app_run_locator and _default_run_locator, and the GitHub REST header dict is
rebuilt in several places — both worth a later shared helper.
Full suite 1526 passing; ruff clean.
Two defense-in-depth fixes surfaced by /sh-security-review (both were
unverified — no exploit — but cheaply strengthen the credential contract):
- dispatcher: wrap the requests.post/get in app_workflow_dispatcher and
app_run_locator in try/except that re-raises DispatcherError with the
exception TYPE only (`from None`). The no-token-in-a-propagating-exception
guarantee is now enforced by code, not by requests' incidental behavior.
- github_app: parse expires_at BEFORE caching the token and raise GitHubAppError
(scrubbed) on a malformed value, so a parse failure fails closed without
leaving a half-written cache (token set, expiry None) behind a bare ValueError.
Tests: +3 (transport-error scrub for both HTTP seams; malformed-expiry fail-closed
with no half-written cache). Full suite 1526 passing; ruff clean.
The P3 dispatcher's default seams shell out to gh/git, but the R720 box has
no gh and a read-only PAT with no Actions scope — so dispatch_apply_verify
returned no run_id and every task parked at verify ("dispatch unresolved").
Add a GitHub-App auth path: the box mints short-lived (~1h) installation
access tokens from the App private key and uses them for the three dispatch
seams, removing the gh dependency.
- agent_team/github_app.py (new): mint_installation_token (RS256 App JWT,
iss=app_id, iat backdated 60s, exp 9 min; POST /access_tokens) + a lazy
TokenProvider that caches and re-mints near expiry. Secret-safe: the JWT
and token are never logged, never in an exception message, never persisted.
- dispatcher.py: app_branch_pusher / app_workflow_dispatcher / app_run_locator
(additive; gh/git _default_* left untouched). Push auth rides a host-scoped
http.extraHeader via GIT_CONFIG_* env (token never in argv/ps); the REST
run locator maps id->databaseId / created_at->createdAt into select_run_id
and surfaces 4xx promptly instead of silently exhausting the poll window.
- coordinator.py: default_dispatch_node_factory binds the App seams when
AGENT_TEAM_GH_APP_ID / _INSTALLATION_ID / _PRIVATE_KEY are all set; partial
or unreadable config logs one warning and falls back to gh-default (never
raises at serve-start).
- requirements.txt: pin PyJWT, cryptography, requests (App seams + CI fetcher).
- DEPLOY-R720.md / README.md: App dispatch config, permission/scope audit,
env-precedence check, key rotation/revocation + incident response.
Tests: +18 (test_github_app.py new; dispatcher/coordinator additions) covering
JWT claims, cache/re-mint, token-scrub-on-error, REST field mapping + run-name
correlation, and the partial-env inert fallback. Full suite 1523 passing.
The clarifier (ClaudeClarifier._turn) called claude_invoke with no max_turns,
inheriting the single-shot default (1). When the model's one turn did not
terminate in a final result the SDK raised 'Reached maximum number of turns (1)'
and, with no salvageable text, the call failed and crashed the clarify node —
leaving the task wedged at clarify with NO question posted to Slack (the human
never sees a clarifier prompt). Observed live on the R720.
Same single-shot flake the planner hit and fixed in PR #58 (_PLANNER_MAX_TURNS=4);
the clarifier never got the headroom. Give it the same: pass max_turns=4 (tools
stay off — still a fast reasoning->JSON completion).
- clarifier_llm.py: _turn passes max_turns=_CLARIFIER_MAX_TURNS (=4).
- tests: clarifier passes max_turns headroom to the invoke seam.
Full suite 1505 passed; ruff clean.
The real #60 root cause: failsafe_production_p3_wiring never bound a diff_builder,
so the build node fell back to builders.default_diff_builder (Claude). The Claude
agentic builder (max_turns + read-only tools) then exhausted its turn cap reading
files before it could emit a diff — 'Reached maximum number of turns (8)' live.
The intended builder already exists: builders_llm.as_diff_builder() routes the
build through DeepSeek fast_coder as a SINGLE model completion (with the
orchestrator's retrieval context + defensive _extract_diff + fail-safe no-op).
A single completion has no agent turn loop, so it CANNOT exhaust turns. Confirmed
get_fast_coder() works in the daemon environment.
- coordinator.py: failsafe_production_p3_wiring (P3-configured branch) now binds
builders_llm.as_diff_builder() as the diff_builder. Without it the build node
silently used the turn-exhausting Claude fallback.
- builders.py: default_diff_builder reverted to a safe SINGLE-SHOT, TOOL-LESS
fallback (max_turns=4, no allowed_tools/budget) — tools are what consumed the
turns; this fallback is no longer the live builder. Keeps _extract_unified_diff.
- tests: failsafe binds a real (callable) diff_builder when P3 is configured;
default_diff_builder is single-shot + tool-less.
Full suite 1504 passed; ruff clean.
The #60 max_turns/tools fix stopped the build-node crash but exposed the next
gap: with tools enabled the agentic builder reads files and its final text is
NARRATION (observed live: candidate_diff = 'Let me read the key source files to
get exact signatures bef…'), not a unified diff. That non-diff dispatched to CI,
could not be applied, and the task parked at verify.
- builders.py: default_diff_builder now extracts the unified diff from the
response via _extract_unified_diff — prefers a fenced ```diff block, else
slices from the first 'diff --git' header, dropping surrounding prose. A reply
with NO diff header raises BuildError so narration fails closed (the node fails
the task with a clear reason) instead of dispatching a bogus diff.
- builders.py: _render_build_prompt now instructs the model that its FINAL
message must be ONLY the unified diff in a single ```diff fenced block, no
narration before/after.
- tests: extract from fenced/bare diff with narration; reject narration-only
(BuildError); default_diff_builder returns the clean diff from a narrated
response and raises on a prose-only reply.
Full suite 1503 passed; ruff clean.
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.
The backend still flags the P3 nodes (build/verify/dispatch) gated:true, but in
the live R720 deployment they are wired and active — so that flag is a stale
'P3 opt-in' hint, not a real 'inactive' signal. Stop deriving the dimmed/'inert'
treatment from stage.gated / meta.gated in both the Board columns and the DAG
nodes; activity is conveyed by live task counts + status colors. Human gates are
unaffected (still keyed off topology kind 'gate').
Typecheck + 36 tests + build all green.
Discovered while smoke-testing the #60 builder fix on the R720: a task that
reaches the human plan-gate and is APPROVED never built. The gate's
GATE_APPROVE_ROUTE was hard-wired to END in build_graph, so plan_gate_node set
phase=BUILD/status=ACTIVE and the graph terminated WITHOUT entering the build
subgraph — the task wedged at phase=build with no build, no error. Only the
reviewer's auto-approve path (review -> build_node) reached the builder; every
human-gate-approved plan silently dead-ended.
The P3 splice only repoints the REVIEW node's build route to BUILD_NODE; the
plan_gate edges are independent and were never updated, so even with P3 fully
wired the gate approve went to END. _apply_plan_decision's 'settle exactly as
an auto-approved plan' intent was broken by the edge map.
- graph.py: when build_verify (P3) is wired, point GATE_APPROVE_ROUTE at
BUILD_NODE (mirroring REVIEW's BUILD_ROUTE: BUILD_NODE); keep END when P3 is
inert (P2 approved-plan terminus). LangGraph resolves the forward reference
to BUILD_NODE at compile().
- tests: gate-approve with P3 wired traverses BUILD -> VERIFY -> DONE on an
authenticated CI pass, and parks at VERIFY (never fabricates a pass) when the
CI fetcher is inert. Existing P2 gate-approve -> END behavior unchanged.
GPT-4.1 cross-family review verdict: SHIP-WITH-FIXES, no BLOCKs (read-only,
XSS-safe, no-backend, gate-vs-inert all confirmed). Applied:
- a11y: aria-label on the Board/Pipeline Tabs; drawer collapsible section labels
are now semantic <h3> headings.
- Bundle: function-based rollup manualChunks splits react/flow/markdown/radix/
vendor so the 614 kB single chunk is gone (largest now ~142 kB react); app
code drops to ~30 kB. Clears the 500 kB Vite warning. CSS splits too.
(Skipped GPT's .prose-sm finding — incorrect; index.css uses a hand-written .md
block, not Tailwind Typography. Roving-tabindex left out by design for this
read-only view.)
Typecheck + 36 tests + build all green.
Fresh-context deep review verdict was SHIP-WITH-FIXES (all hard constraints held:
read-only, inert gate panel, XSS posture, no backend, gate-vs-inert semantics).
Resolved:
- Board columns: replace magic calc(100vh-220px) ScrollArea cap with a proper
flex chain (h-full/min-h-0) so card lists size to real column height at any
window size / wrapped toolbar.
- Valid ARIA: stage row is role=list, each column a role=listitem wrapping a
role=listbox of task-card options (no malformed listbox nesting).
- TopBar health dot uses status-active/status-failed tokens (no hardcoded Tailwind
palette colors).
- Stale TaskList reference in badge.tsx comment.
Typecheck + 36 tests + build all green.
isGate() now keys off topology node kind 'gate' (clarify) or the 'gate' stage,
not stage.gated — which actually means INERT (disabled P3 nodes build/verify/
dispatch). Inert columns get a dimmed + 'inert' treatment instead of a false
'human gate' lock. Verified against the live R720 dashboard (10 stages) via a
dev-proxy smoke test; typecheck + 36 tests + build all green.
Adds the styling foundation for the UI redesign (vibe-kanban / Magentic-UI /
Langflow references): Tailwind CSS + shadcn/ui primitives, HSL design tokens
(index.css) ported from theme.css, @/ alias, and the App.tsx Board|Pipeline tabs
shell. Extends StateResponse with the backend's existing stages[] for the Board.
theme.css kept transiently until components migrate to Tailwind. No backend
changes. Component fan-out (Board, MapNode, TaskDrawer, TopBar) follows.
The Plane-2 builder (default_diff_builder) called claude_invoke with no
overrides, inheriting the subscription invoker's single-shot defaults
(max_turns=1, allowed_tools=[]). Diff synthesis is agentic, so the call
died with 'Reached maximum number of turns (1)' and every task failed at
phase=build.
- invoker.py: thread allowed_tools through subscription_invoker and
_collect_subscription_text (default None -> []), so callers can opt in;
single-shot reasoning nodes are unchanged.
- builders.py: default_diff_builder now passes max_turns=8, a read-only
tool allowlist (Read/Grep/Glob), and budget_usd=4.0. No write tools --
the builder returns the diff as data and performs no repo writes (D2/D11).
- Tests: builder agentic-config passthrough; invoker allowed_tools thread +
tool-less default guard (so future nodes must opt in explicitly).
Closes#60
Two dashboard-SPA refinements (frontend only):
1. Retry loops (plan<->review, build<->verify) no longer draw a backward arc over
the forward edge (the 'circular arrows'). Loop-backs are excluded from the
default render and from the dagre layout; instead the source node shows a small
↺ chip ('can send work back to ...'), and the actual return arc is drawn only
when a selected task ACTUALLY looped it (computed from its timeline), highlighted
on that task's path.
2. Q&A / Review Verdicts / Plan render human-readably instead of JSON blobs:
react-markdown (no rehype-raw -> raw HTML escaped, XSS-safe) renders findings/
answers/summary; verdicts as cards (badge + round + outcome), Q&A as per-turn
cards (string + dict shapes), plan as summary + phase/step lists.
16 frontend tests pass (incl. loopback default-off/on-when-looped, markdown bold,
and a no-raw-HTML XSS guard); typecheck + build clean.
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.
- run-team.py: add the 'dispatch <thread_id>' operator command (P3 option-b).
The read-only box parks at DISPATCH; this completes it with a just-in-time
WRITE token: reads candidate_diff + scope from the checkpoint (or --diff/--scope
files), pushes the head branch + fires workflow_dispatch via dispatch_apply_verify,
prints the located run_id, and (--write-back) writes it into the task checkpoint
so VERIFY binds. +2 tests.
- OPERATOR-RUNBOOK: fix the misleading 'systemctl show -p Environment' check (it
does NOT show EnvironmentFile= vars) -> use /proc/<MainPID>/environ +
_p3_env_is_configured(); document the operator-initiated dispatch flow + the
fine-grained-token write-probe caveat.
Suite green, ruff clean. Branch only; not merged.
The no-write-token detector's test fixtures + a doc comment contained contiguous
'-----BEGIN ... PRIVATE KEY-----' literals that tripped the repo's gitleaks
pre-push backstop (a false positive on a secret-DETECTOR's own test data). Build
the PEM markers at runtime so the source carries no contiguous literal; the
runtime values are still full PEM blocks (what the detector under test sees).
No behavior change; 39 no-write-token tests pass.
High-recall /sh-security-review fan-out + proof-or-kill verifier found two
confirmed HIGH; both now closed (verified empirically against the working tree):
- LOGIC-RACE-01 (HIGH, CWE-835): the build-loop budget was structurally dead
(verifier read a shared wiring-time VerifierConfig.build_loops, always 0, so
the max_build_loops park never fired -> a perpetually-failing task looped
BUILD->DISPATCH->VERIFY forever, force-pushing + firing a CI run each round).
Threaded build_loops through durable PipelineState/TaskRecord; verifier reads
state.get('build_loops',0), writes the incremented count back on each FAIL, and
PARKS at max_build_loops. Parks after exactly N failures, never unbounded.
- SEC-01 (HIGH, CWE-532) + SEC-02 (MED, CWE-214): p3_rollback.sh echoed the live
App JWT to stdout in default dry-run and passed it as a gh argv literal. Added
redact_secrets (Bearer/Authorization/ghX_/PEM masking) through run_or_plan; the
App uninstall now uses curl -H @<0600 tempfile> (JWT never on argv), shredded
after. Empirical: app/incident/all dry-runs leak 0 JWT occurrences.
- SEC-03 (MED, CWE-798): assert_no_write_token now applies the PEM regex + the
configured App-ID to env/config VALUES (not just files) — an App private key
under a benign env name is caught.
- SEC-04 (LOW) + P3-IAC-08 (LOW): tightened the box GITHUB_TOKEN fallback /
value-scan; staged-only WARN on the live workflow revert.
Suite: 1382 passed, ruff clean. Branch only; not merged/deployed.
NOTE: re-verifier flagged SEC-01 as open by grepping COMMITTED blobs (the fix was
uncommitted working-tree state); independently confirmed closed empirically.
GPT-4.1 cross-family review (APPROVE, no critical/high) raised two MEDIUMs on the
rollback tooling; addressed both:
- require_keys: each restore_* asserts its required baseline keys up front and
refuses a PARTIAL (silently-weaker) restore. environment accepts ids OR logins
(equivalent); a missing protection.full now REFUSES the enforce_admins-only
degrade unless P3_ROLLBACK_ALLOW_PARTIAL=1 is set (loud DEGRADED warning).
- out-of-band ACK: an --apply that needs a MANUAL App neutralise (no APP JWT, or
action=out-of-band) refuses unless P3_ROLLBACK_OOB_ACK=1 — so the App is never
left un-neutralised without a conscious operator sign-off; with the ack the
other surfaces still restore.
Documented both env vars in usage. +3 tests (required-key refuse, partial-protection
ack, oob ack). Suite: 1362 passed, ruff clean.
