Plan: mechanical cold review of plan drafts (#44)

On this page

Closes: #44. Branch: feature/plan-review. Status: Done (2026-09-01).

TL;DR

The commit review works and plans get none of it. This plan gives exit_plan_mode the same reflection pause — same package, same ledger, same reviewer rule — keyed by the draft’s content hash instead of the staged diff’s. One new bridge member, one new prompt, one new tool_call branch; no new package, no subagent, no compat layer.

Decisions

Decision Choice

Where the review runs

@gadhs/pi-workflow, a tool_call branch for exit_plan_mode. ADR-004 assigns the cold reader there; tool_call fires for extension tools (the delegation gate already blocks pi-subagents' subagent tool at that hook).

How pi-workflow finds the draft

pi-modes publishes planDraftPath(cwd) on the existing runtime bridge. The bridge is optional as a whole (standalone doctrine), so the member is too.

Cadence

Same as commits: one pause per distinct draft. Unchanged draft on retry proceeds to the human; edited draft earns a fresh review. The protocol’s "3–4 contextless rounds" are emergent, not a loop.

Reviewer

chooseReviewer(authoring), author = the model active at exit. Floor Sonnet. The J-gate ruling, mechanically.

Unreachable reviewer

#50’s noteUnreviewed, own attempts map, bound of two.

Prompt

PLAN_REVIEW_SYSTEM_PROMPT with P1–P8. Same framing as J1–J8: the plan is a CLAIM; FAIL cites section/line; PASS-on-absence says what was looked for; VERDICT: CLEAN or FLAGS (n).

What the human sees

Findings in the transcript (the pause), then the existing dialog + panel with the post-review draft. In-panel rendering is #43.

Tunables

reviewTimeoutMs from gadhs-pi-workflow.json applies. Nothing new.

Pre-1.0 check. No aliases, shadows, migrations or version shims. The draftless fallback to event.input.plan mirrors a fallback exit_plan_mode already has; the optional bridge member follows from the bridge being optional, not from supporting older pi-modes.

The checklist (P1–P8)

The delivery protocol’s bar — "can a contextless agent or human implement this, fully per conventions, without further clarification?" — plus the four questions the operator asks of every plan.

  1. P1 Contextless implementability — exact files, functions, constants, pins; a fresh agent executes without asking.

  2. P2 Canonical Status lines — one per increment; Not started | In progress | Done (YYYY-MM-DD) | Deferred (…) | Blocked (…) | N/A.

  3. P3 Scope — names the issue it closes; nothing belongs to another issue.

  4. P4 Tests by category — race → concurrency, wire → contract, regression → the case that would have caught it; "verified" = behaviour observed.

  5. P5 No pre-1.0 ceremony — no aliases, shadows, migrations, compat layers unless the plan says why.

  6. P6 Risks with directions — each risk names fail-open / fail-closed / silent and its mitigation.

  7. P7 Claims point at pins — every always / never / cannot names its test or mechanism.

  8. P8 Delivery steps — Antora page + nav (Active → Archive), CHANGELOG, issue close with SHAs, release on the next cut.

Increments

Each increment is one commit on feature/plan-review, reviewed by the commit pause like any other.

1. Bridge member

  • Files: packages/pi-modes/index.ts (the object assigned to Symbol.for("gadhs:pi-modes-runtime")), packages/pi-workflow/runtime-bridge.ts.

  • Change: pi-modes adds planDraftPath: (cwd: string) ⇒ planDraftPath(cwd, resolveAgentDir()). ModesRuntimeBridge gains planDraftPath?(cwd: string): string; NOOP omits it.

  • Tests: pi-modes wiring — the published function returns planDraftPath(cwd, resolveAgentDir()) for the harness cwd.

  • Status: Done (2026-09-01)

2. Plan review prompt and runner

  • Files: packages/pi-workflow/reflection-review.ts, packages/pi-workflow/test/reflection-review-run.test.ts.

  • Change: factor the provider call in runReflectionReview into a private runColdReview(system: string, user: string, deps: ReviewDeps): Promise<ReviewOutcome> (identical options: no temperature, maxTokens, AbortSignal.timeout, auth headers). Add PLAN_REVIEW_SYSTEM_PROMPT (P1–P8, commit-review framing) and runPlanReview(input: { plan: string; source: "draft" | "parameter"; path?: string }, deps): Promise<ReviewOutcome> whose user message is PLAN (<source>[, <path>]):\n\n<plan> truncated at MAX_DIFF_CHARS with the same "truncated" marker. Export planReviewKey(cwd: string, text: string): string = sha256 of ${cwd}\n${text}.

  • Tests (contract): sends the plan text and the P-list framing; never temperature; carries maxTokens and a signal that honours deps.timeoutMs; every malformed provider shape in the existing chaos list yields an outcome; a thrown transport error becomes unreviewed.

  • Status: Done (2026-09-01) — runColdReview shared with the subject’s builder and formatters inside its envelope; 10 plan contract tests

3. The pause

  • Files: packages/pi-workflow/index.ts, new packages/pi-workflow/test/plan-review-pause.test.ts.

  • Change: in the tool_call handler, a new branch between the edit branch and the !== "bash" return: if (event.toolName === "exit_plan_mode"). Resolve bridge.planDraftPath?.(ctx.cwd); text = the file if readable, else event.input.plan if a non-empty string, else return (exit_plan_mode’s own refusal stands). authoring derived exactly as the commit branch does. claimReflection(planReviewKey(cwd, text), reviewedPlans); on claim, run runPlanReview with chooseReviewer(authoring) and workflowConfig.reviewTimeoutMs; on unreviewed, noteUnreviewed(key, reviewedPlans, planUnreviewedAttempts) with the same two-way text as commits. Block reason = outcome text + \n\n + "Address material findings by editing the draft, then call exit_plan_mode again. An unchanged draft proceeds to the human." Log plan_review.done { mode, reviewer, outcome, ms, source }; when the bridge lacks the member, log plan_review.skipped { reason: "no draft path" } and return.

  • Tests (real handlers, scripted provider, bridge published by the test): first exit pauses with the reviewer’s text and the trailer; unchanged draft passes; edited draft pauses afresh; parameter-only plan is reviewed with source: "parameter"; no text at all → no pause, no provider call; unreachable → attempt 1 retries, attempt 2 proceeds; a Sonnet author gets the floor and a Fable author gets itself; bridge without the member → no pause and the skip log line.

  • Status: Done (2026-09-01) — 9 pause tests against the real handler, including source precedence, the Sonnet boundary, and an unreadable draft

4. End to end

  • Files: packages/pi-modes/test/wiring.test.ts (loads both packages), packages/pi-workflow/test/standalone.test.ts.

  • Change: none in production.

  • Tests: enter plan, write the draft where planDraftPath says, call exit_plan_mode — first call blocked with VERDICT, second call reaches ui.select. Standalone: pi-workflow alone, exit_plan_mode event → no pause.

  • Status: Done (2026-09-01)

5. Docs, changelog, delivery

  • Files: docs/modules/ROOT/pages/modes.adoc (plan-mode section and the reflection-pause row), docs/modules/ROOT/pages/tuning.adoc (troubleshooting row "exit_plan_mode blocked with a review of your plan"), CHANGELOG.adoc (Unreleased → Added), this plan under docs/modules/ROOT/pages/plans/plan-review.adoc with a nav entry (Active at approval, Archive at close).

  • Change: prose only; states the cadence, the reviewer rule, and P1–P8.

  • Tests: check-docs, plan-lint.

  • Delivery: merge --no-ff, close #44 with SHAs and what was verified; release rides the next cut together with #41.

  • Status: Done (2026-09-01) — merged ebc0218; #44 closed with SHAs; nav moved to Archive in this commit

Risks

Risk Direction Mitigation

Agent and reviewer loop forever (flag, edit, new flag)

fail-closed

The unchanged-draft retry always proceeds; the agent can stop editing and let the human decide, as with commits. No cap needed: the exit is one call away.

A later edit lands between review and dialog

silent

The key is the content hash: any edit changes it and earns a fresh pause. What reaches the dialog is a reviewed hash or a deliberately unchanged one.

Review runs on a plan exit_plan_mode refuses anyway

fail-open (cost: a review that cannot matter is still paid for)

Skip when neither file nor parameter yields text.

Reviewer is the model that wrote the plan

fail-open (rigour: a stronger cold model might find more)

The same trade the commit review makes, ruled on by the operator: equal capability with no session context satisfies the invariant.

Bridge member absent (pi-modes not loaded, or hand-assembled install)

silent (no review)

One debug line names why. Standalone pi-workflow never sees the tool.

Not in this plan

  • Rendering the review inside the approval panel (#43).

  • Reviewing the committed in-repo plan document — the commit review already reads it as part of the diff.

  • Any change to what approval authorises.

Edit this page · latest