plan-review

Dynamic-workflow alternative (opt-in). A Workflow-tool port of this skill lives at [workflows/plan-review.workflow.js](./workflows/plan-review.workflow.js) — deterministic fan-out, journaled resume, schema-validated output. Run it with Workflow({ scriptPath: "plugins/soleur/skills/plan-review/workflows/plan-review.workflow.js", args: ... }). The prose skill below stays the default; the two coexist during calibration. Known divergence: the port's code-simplicity lens string and its consolidation prompt still carry the older "is this over-architected? what should be cut?" framing, not the per-mechanism requirement question mandated below — a run driven from that script asks the weaker question. See [knowledge-base/project/specs/feat-review-workflow-prototype/spec.md](../../../../knowledge-base/project/specs/feat-review-workflow-prototype/spec.md).

Plan Review

Have @agent-dhh-rails-reviewer @agent-kieran-rails-reviewer @agent-code-simplicity-reviewer review this plan in parallel.

Prompt code-simplicity-reviewer per mechanism, not per plan. Its question is "which requirement does this mechanism satisfy, and does a simpler mechanism already satisfy it?" — never "is this plan good?". A reviewer asked to assess quality optimizes the design it was handed; asked to justify each mechanism against a requirement, it can recommend deleting one. Feed it the Property List and Cut List from the plan's ## Research Insights section so "which requirement" has something to check against; if the plan carries neither, report that absence as the first finding rather than inferring a requirement. (code-simplicity-reviewer renders ### Goal Verification per its own plan-review carve-out — without that carve-out this instruction is silently no-opped.) Why: #7418 / ADR-176 — a six-agent plan-review panel returned eight P0s and every one of them made the proposed cursor better, with code-simplicity-reviewer in that panel; the same agent class, asked post-implementation whether the design was necessary at all, said delete it — at roughly ten times the cost to act on.

When the plan declares Brand-survival threshold: single-user incident in its ## User-Brand Impact section, also include @agent-soleur:engineering:review:architecture-strategist and @agent-soleur:product:spec-flow-analyzer in the parallel batch — the 3-agent baseline catches overengineering and convention drift; the 5-agent panel catches blast-radius and flow gaps.

When consolidating a 5-agent panel's findings, treat the simplification panel (DHH + code-simplicity) and the correctness panel (Kieran + architecture-strategist + spec-flow) as orthogonal axes. When BOTH panels fire on the same scope, prefer delete over fix — a feature that simultaneously triggers "too complex, remove" and "has 4 specific bugs" is over-architected; cutting it dissolves the bugs. Many "paper-resolution" findings (FRs added without implementation) vanish when the cuts land. Why: 2026-05-11 #2720 plan v1→v2 — 953→829 lines, 4 P0 issues dissolved when the matrix-split cut landed; see knowledge-base/project/learnings/2026-05-11-five-agent-plan-review-panel-and-architectural-false-trails.md.

Named CEO/design/devex panel (relevance-gated)

The eng panel above reviews for engineering quality (simplicity, convention, correctness, blast-radius, flow). A plan can be engineering-immaculate and still be a product / market / design / developer-experience mistake. Alongside (never replacing) the eng panel, include a named panel of the existing domain leaders — spawned only when the plan is relevant to their lens:

| Panel axis | Agent(s) | Lens (reviews the finished plan for) | |---|---|---| | CEO / business | @agent-soleur:product:cpo + @agent-soleur:marketing:cmo | cpo: product strategy, positioning, scope-vs-roadmap fit. cmo: market/GTM implications, brand-voice, messaging risk. | | design | @agent-soleur:product:design:ux-design-lead | user-flow completeness, UX decay, design-taste risk in user-facing surfaces. | | devex / eng-strategy | @agent-soleur:engineering:cto | developer/operator experience, maintenance/DX cost, build-vs-buy, ongoing engineering strategy — distinct from architecture-strategist's blast-radius/structural lens. Give cto a devex lens string, not a structural one, so the two do not duplicate. |

Relevance gate — INDEPENDENT of the plan's own ## Domain Review verdict. The named panel exists to catch what plan Phase 2.5 got wrong, so it MUST NOT trust that phase's verdict as its trigger (a UI plan mis-judged Product: NONE would inherit the exact miss). Compute activation from an independent read of the plan's actual content; use the ## Domain Review + Product/UX Gate tier only as a hint, never as the sole gate:

1. Mechanical UI-surface scan (independent): scan the plan's ## Files to Create + ## Files to Edit against the UI-surface glob superset (components/**/*.tsx, app//page.tsx, app//layout.tsx, plus the shared UI-term list). Any hit → force ux-design-lead + cpo active regardless of the Domain Review verdict.
2. Fresh relevance read (independent): judge relevance from the plan body (Overview, Files, User-Brand Impact), not by parsing a "Domains relevant: none" line — product/scope language → cpo; market/GTM/brand/user-copy language → cmo; user-facing/flow/visual language → ux-design-lead; code/infra/tooling Files-to-Edit → cto.
3. Threshold bias: when the plan declares Brand-survival threshold: single-user incident, bias toward activating (stakes are high — the eng panel already escalated).
4. If none activate (trivial non-engineering docs plan), only the eng panel runs — today's behavior is preserved.

Every named-panel reviewer is prompted for structured advisory only — NO AskUserQuestion. cpo/cmo default to orchestrator mode and will otherwise emit AskUserQuestion, which hangs a headless one-shot (a Task subagent cannot answer it, and the agent .md body does not reach anonymous Task spawns). The prohibition MUST live in the prompt text, mirroring the plan Phase 2.5 Product/UX gate's "Output a structured advisory — do not use AskUserQuestion."

Standing panel checks (every review, no relevance gate)

Run these against the plan regardless of change class. They are cheap, textual, and each has exactly one right answer, so all findings here classify Mechanical and auto-apply.

1. Acceptance criteria must not depend on unrelated concurrent processes

(cq-ac-must-not-depend-on-concurrent-sessions). Read every entry under ## Acceptance Criteria and ask: could a process this plan never mentions — a sibling worktree, another agent session, ambient machine state, whatever else happens to be running — flip this from true to false without a single line of the diff changing? If yes, the criterion measures the machine rather than the change, and it must be rewritten as a deterministic test over a synthetic fixture.

The tell is an AC that asserts the ABSENCE of an ambient signal: "the run fires no contention banner", "no sibling process is detected", "/tmp has N MB free", "the suite completes in under N minutes". Each is unfalsifiable when it passes (you learn only that nothing else ran) and uninformative when it fails.

Rewrite by moving the property into a fixture the plan controls. "Fires no contention banner" becomes "with a synthetic procfs containing zero sibling runs, tc_preamble emits no SIBLING_RUN_DETECTED" — which is a claim about the code, runs in milliseconds, and cannot be invalidated by a colleague starting a build.

Why this is worth a standing check: on 2026-08-11 an AC of exactly this shape ("the sanctioned full-gate run fires neither banner") cost a second ~45-minute full-gate re-run afte