Review-fix cycle for implementation plans. Sits between writing-plans and executing-plans. Launches proportional parallel reviewers, merges issues, fixes with research, and loops until clean or convergence. Invoked by writing-plans after saving the plan doc.
Resources
1Install
npx skillscat add daniel-ospina/agent-infra/plan-review Install via the SkillsCat registry.
⛔ This skill MUST be read in full — not skimmed. Formal review gates depend on its workflow.
Skipping steps silently bypasses quality checks. Missing gates = undetected breakages.
Verifier gate: dispatches AI reviewers. Pipeline auto-advances when clean.
Cold-class seam (#512): reviewer/eval dispatches are cache-cold one-shot traffic — an operator who exports
COLD_CLASS_PROVIDER=veniceopts them into the venice leg (--provider venice --model deepseek-v4-flash; same model id — venice serves cold prompts with cache reads). Unset (default) = inert. Interactive/warm traffic never routes venice (docs/providers.md §8).
Canonical:agent-infra/skills/plan-review/SKILL.md— git-tracked source of truth. Pi reads via~/.pi/agent/skills; consumers hard-link intooperations/skills.
Plan Review
Automated review-fix cycle for implementation plans. Ensures plan quality before execution begins.
When to Use
- Invoked automatically by
writing-plansafter saving a Standard or Complex plan doc - Can also be invoked manually:
plan-review docs/plans/<plan>.md - Skip for Micro tier — micro plans are inline issue comments, not docs
Arguments
plan-review <plan-doc-path> [--issue <number>] [--epic <path>] [--tier standard|complex]
Proportional Review Cycles
Reviewer counts and cycle caps: proportional-gates §Review Cycles — never restated here. Tier crosswalk: micro → Low, standard → Low-Medium, complex → High.
| Risk tier | Reviewers dispatched |
|---|---|
| Micro | not applicable — plan-review does not run for Micro (see When to Use) |
| Low-Medium (small plan, existing patterns) | #1 Structural & Efficiency, #2 Integration |
| Medium-High (large plan, some novelty) | #1–#2 + #3 UX Coherence |
| High (novel architecture, first-of-kind) | #1–#3 + #4 Failure Mode Auditor |
Reviewer #5 (Duplication & Architecture, #688) is conditional and additive — dispatched alongside the tier's set whenever the plan introduces a new component, a new write path, a new shared-state owner, or a new definition of an existing vocabulary, and again on the final cycle. It is orthogonal to the risk tiers, not a fifth one; see the Reviewer #5 section in Phase 1. #5 is never a replacement for another reviewer.
Proportional dispatch: the tier sets the count; the agent decides whether the plan's size and novelty justify reporting up a tier, notes the decision, and a reviewer sub-agent validates it.
Adversarial domain — declared threat surface (bound: 2 cycles, orthogonal to the rows above). When the scoping comment carries an ### Adversarial Threat Surface declaration (gate/enforcement code whose correctness is "an attacker cannot make it fail open"), the plan review is bounded by that surface, not by reviewer exhaustion: cap 2 cycles, acceptance = every declared threat class covered by a test + green CI, residuals filed from cycle 1, not chased. A fresh reviewer returning THREAT SURFACE COVERED (all declared classes covered, no in-scope bypass reproduced) is a clean exit for this domain — a literal NO ISSUES FOUND is not required, and when the merge rests on threat-list coverage the PR body must disclose it ([ADVERSARIAL-BOUND] cycles=<N> threats=<K> covered=<K> residuals=<#N,…|none>). Statement of record: AGENTS.md §Hard Cap.
Input Resolution
Before starting the first cycle:
- Read the plan doc in full
- If
--issueprovided: fetch issue spec viagh issue view <N>and derive epic path from**Epic:** docs/epics/... - If
--epicprovided: read the epic doc - Collect codebase context: read key files referenced in the plan's
Files:sections - Resolve research brief: derive path from epic or plan doc frontmatter
- Research Resolution Gate (#2092): if research brief exists, pre-read content once. Inject as
## Verified Research Context (author-provided)header into every reviewer sub-agent prompt. If no brief exists, omit the section. - Tortoise — prior state and premises: query the graph for what is already believed about the plan's subject (
tortoise_search,tortoise_recallmode=state,tortoise_belief_timeline) and inject the result as## Prior State (Tortoise). ADVISORY, never blocking: an empty or unreachable graph is not a finding, it means no priors (how-to-use-tortoise). When the plan rests on a genuinely contested design choice, deliberate it on the graph — options/criteria/findings → IMPL/NAND →tortoise_compute_confidence— and review the ranking (tortoise-decide).
Review Cycle
Phase 1: Review (N parallel agents, per risk tier)
↓
Phase 2: Merge & Dedup
↓
Phase 3: Fix (1 agent + research)
↓
Phase 4: Cycle Status → [clean|issues|stalled]
↓
Phase 5: Final Verification (Claude)Self-Healing Protocol (NEW in v3.1.0)
When review finds issues, the agent attempts to resolve them autonomously before pausing:
| Issue severity | Action |
|---|---|
| P2 (improvement) | Fix inline immediately (surgical fix). Note in changelog. Re-review. Do NOT pause. |
| P1 (important gap) | Research + fix inline. Note in changelog. Re-review. Do NOT pause. |
| P0 (structural flaw), fixable in < 5 lines | Research, then fix inline. Note in changelog. Re-review. Do NOT pause. |
| P0, needs substantial work | Research, review carefully, run through issue-workflow, return to plan-review cycle. Do NOT pause unless the fix fails. |
| P0, requires human input (data loss, security, ontology choice, significant architectural change, cost >$10/mo, legal/compliance) | Research, then pause with a structured question: present context, options, analysis, and a recommendation in plain language. |
Stall detection: A fingerprint persisting across 2+ cycles is precisely what the fingerprint-stall detector measures — do not hand-diagnose it here. Follow the 3-layer stuckness algorithm in Phase 4: recurrence ≥ stall_threshold (default 0.8) → escalate to a human. Filing a GitHub issue is a remedy for a stalled issue, never an alternative to the detector — filing it and continuing is how a stalled loop silently runs to its cycle cap.
Phase 1 — Review (Parallel Agents)
Launch the proportional reviewer count (N) for the risk tier (proportional-gates §Review Cycles) in parallel via Pi task. Each receives the full plan doc, issue spec (if available), epic doc (if available), and research context (if resolved). Each returns ISSUE: blocks or NO ISSUES FOUND.
Every reviewer reads whichever of ## Verified Research Context and ## Prior State (Tortoise) are present before judging — an absent section means none was available, not a finding. Treat them as strong but fallible: where a reviewer doubts a finding it verifies the claim itself rather than inheriting it.
Reviewer #1 — Structural & Efficiency (merged, all 6 dimensions):
You are reviewing an implementation plan for structural correctness and efficiency. Your job is to find issues — NOT to fix them.
PLAN DOC: <full plan>
ISSUE SPEC: <body + issue-scoping comment, or "none">
EPIC DOC: <epic content, or "none">
CHECK THESE DIMENSIONS:
1. SPEC COVERAGE (skip if no issue spec):
- Does the plan address every requirement in the issue spec?
- Are there gaps — requirements in the issue but absent from the plan?
- Are there extras — plan tasks that go beyond what the issue requested?
- **Deferred/gated work:** if the plan (or its scope record) defers any task pending data, prove-out, approval, or a future event, a REAL re-check mechanism must exist — a scheduled job, a dated gate, an automated trip, or a named owner + concrete trigger. "Defer until X" with no mechanism = the work silently rots; flag it (P1).
- Is the plan ultimately going to deliver the outcomes the issue/epic expects?
2. STEP COHERENCE:
- Do any steps contradict each other?
- Are dependencies between steps explicit and correctly ordered?
- Does any step depend on something not yet built?
- Are there circular dependencies?
3. EPIC ALIGNMENT (skip if no epic doc):
- Does the plan's data model match the epic's?
- Does the plan's migration approach match the epic's phases?
- Does the plan respect the epic's component boundaries?
- Are there any silent divergences from the epic architecture?
4. PARALLELIZABILITY (Complex tier emphasis but always checked):
- Are there tasks sequenced that have no actual dependency?
- Could any tasks be merged without losing clarity?
- Are there unnecessary ordering constraints?
5. PLAN QUALITY (Complex tier emphasis but always checked):
- YAGNI: does the plan build things not needed for the stated goal?
- DRY: does the plan duplicate logic across tasks?
- Are there redundant verification steps?
- Is complexity proportional to the tier?
- Does the plan cite its research inputs (research brief / Pattern Research) as the basis for its key decisions?
6. GOOD > EASY (design quality — always checked):
- Does any design decision choose the EASY path over the GOOD one? Easy paths accumulate into brittle systems; good paths cost more upfront but pay back in reliability, extensibility, and user satisfaction.
- Flag decisions that optimize for implementation convenience over outcome quality: shortcuts on error handling, schema changes that skip migrations, duplicated logic instead of a shared abstraction, hardcoded config instead of proper configuration, quick hacks over maintainable patterns.
- Each GOOD > EASY flag MUST name the Good alternative AND its cost (effort, time, risk). If you cannot name the Good alternative, it is a preference — omit it.
- System design: does the plan account for duplication, scalability, and where shared state lives? Check against Reviewer #5's D1–D9/A1–A6 rather than re-deriving them.
For each issue, return EXACTLY:
ISSUE:
severity: P0|P1|P2
dimension: spec-coverage|step-coherence|epic-alignment|parallelizability|plan-quality|good-easy
location: [Task N, Step M] or [Header section name]
description: <what's wrong>
suggestion: <what to fix>
Severity: P0=structural flaw, P1=important gap, P2=improvement
If no issues: NO ISSUES FOUNDReviewer #2 — Integration:
You are reviewing an implementation plan for integration completeness.
PLAN DOC: <full plan>
CODEBASE CONTEXT: <contents of key files referenced in the plan>
CHECK THESE DIMENSIONS:
1. INTERFACE & INTEGRATION IMPACT:
- Does the plan identify ALL systems that touch or are touched by this change?
- API contracts: if API shape changes, are all consumers accounted for?
- Shared types: if TypeScript types change, are all importers updated?
- Edge functions: if DB schema changes, are edge functions updated?
- SSR pages: if data shape changes, are server-rendered pages updated?
- RLS policies: if table access patterns change, are RLS policies updated?
- Frontend consumers: if API responses change, are components updated?
2. EDGE CASE COVERAGE:
- Are failure modes addressed (what happens when X fails)?
- Are auth boundaries checked (who can access what)?
- Are concurrency issues considered?
- Are empty/null states handled?
3. TEST COVERAGE:
- Does every integration surface have a test?
- Do tests verify behavior, not just implementation?
- Are edge cases covered by tests?
4. SURFACE MAP QUALITY (skip if no Integration Surface Map in plan):
- Are all boundaries from the feature spec captured in the map?
- Are test layers correctly assigned per surface? Specifically flag:
* SQL business logic tested with TS mocks instead of pgTAP
* External API calls without contract tests
* Auth boundaries without integration tests
- Are failure modes enumerated per surface (at least 2 per surface)?
- Does the surface map cover the user journey steps (if Journey Test Map exists)?
For each issue, return EXACTLY:
ISSUE:
severity: P0|P1|P2
dimension: interface-impact|edge-cases|test-coverage|surface-map-quality
location: [Task N, Step M] or [Header section name]
description: <what's wrong>
suggestion: <what to fix>
P0=structural flaw, P1=important gap, P2=improvement
If no issues: NO ISSUES FOUNDReviewer #3 — UX Coherence:
You are reviewing an implementation PLAN TEXT for UX coherence issues. You are NOT reviewing HTML prototypes — you are reviewing the plan document itself.
PLAN DOC: <full plan>
ISSUE SPEC: <body, or "none">
EPIC DOC: <epic content, or "none">
CHECK THESE DIMENSIONS:
1. UX PATTERN CONSISTENCY:
- Does the plan describe UI interactions consistently?
- Are the same UX patterns named the same way across the plan?
- Are there conflicting UX approaches described in different tasks?
2. UI STATE COVERAGE:
- Does the plan account for loading, empty, error, and edge-case states?
- Are transitions between states described?
- Are there states mentioned in one task but forgotten in another?
3. USER FLOW GAPS:
- Are there missing steps in user journeys described by the plan? Is the whole user journey considered?
- Does the user have a clear path through every described flow?
- Has information architecture been considered to provide what's needed without bloat/distractions?
- Are there dead ends or unreachable states?
4. UX ASSUMPTIONS
- Are UX decisions described without rationale?
- Are accessibility considerations mentioned where relevant?
5. INFORMATION ARCHITECTURE:
- Is information architecture considered across the user journey — is each screen or step showing the right information, in the right order?
- Does any single step load more information than the user needs to act on it?
For each issue, return EXACTLY:
ISSUE:
severity: P0|P1|P2
dimension: ux-coherence|information-architecture
location: [Task N, Step M] or [Header section name]
description: <what's wrong>
suggestion: <what to fix>
P0=structural flaw, P1=important gap, P2=improvement
If no issues: NO ISSUES FOUNDReviewer #4 — Failure Mode Auditor:
You are conducting a failure-mode gap analysis on an implementation plan. Identify failure modes NOT caught by the plan's listed tests.
TESTING STRATEGY: <extracted from plan>
VERIFICATION PLAN: <extracted from plan>
ACCEPTANCE CRITERIA: <from issue body, or "none">
For each failure mode NOT covered, return:
- The failure scenario
- Why existing tests miss it
- What test would catch it
Consider these failure families:
1. Race conditions / concurrency
2. Partial failures (one system down, others up)
3. Data inconsistency (write succeeds, subsequent read fails)
4. Time-based failures (cron jobs, token expiry, TTL)
5. Auth boundary violations (wrong role accessing data)
6. Input edge cases (empty, oversized, malformed)
7. State corruption (interrupted multi-step operations)
8. Resource exhaustion (rate limits, memory, connections)
For each gap found, return:
ISSUE:
severity: P0|P1|P2
dimension: failure-mode-gap
location: Testing Strategy
description: <failure mode + why existing tests miss it>
suggestion: <test that would catch it>
If no gaps: NO ISSUES FOUNDReviewer #5 — Duplication & Whole-Architecture (#688) — CONDITIONAL, not proportional:
Dispatched in addition to the proportional set above — when the plan introduces a new component, a new write path, a new shared-state owner, or a new definition of an existing vocabulary. Not triggered by plan length. Also dispatched on the FINAL cycle when the plan's data-model / interface / E2E steps introduced surfaces the earlier architecture review never saw.
Read skills/reviewers/duplication-architecture/SKILL.md in full and run checks D1–D9 (duplication) and A1–A6 (whole-architecture). Do not paraphrase those checks from memory — the skill is the specification. Condensed prompt:
You are reviewing an implementation plan for (a) duplication of something that
already exists and (b) whole-architecture soundness with this change in place.
PLAN: <full plan>
REPO: <repo root> TORTOISE: <graph reachable? state which sources you checked>
DUPLICATION (D1–D9 — read the reviewer skill; do not work from memory):
- D2: discover writers BY FUNCTION, NOT BY NAME. Writers doing one job
routinely share no vocabulary. Separate writers from drivers (a driver
delegates; a writer emits the persistence statement). Read the whole family
for the odd member — the sibling that does it differently is the defect.
Check reader gates too: a reader querying a subset can mis-classify records
every writer produced correctly.
- D3: if this adds a second writer of state already written, that is the
finding. Enumerate the COMPLETE set and show your search predicate
(`search:` / `excluded:` / `asserted:`). A number you assert is not evidence.
- D4: for each duplicated vocabulary/invariant, state whether a test asserts
the definitions agree. If none does, THAT ABSENCE IS THE FINDING.
- D7: for silent divergence, invert the burden — name the exception, log, or
test that would surface it. Any ABSENT → the divergence is silent → fires.
- D9: if divergence has already run, require the retrofit audit and name the
owner (`retrofit_audit_required:`).
- Do NOT use "this adds no new component" as an escape. Extending an existing
writer with a new field is exactly the shape of the defect (D6: recurrence).
ARCHITECTURE (A1–A6): is the WHOLE coherent with this component added, not
just this component internally sound? A6: any claimed invariant that is NOT
actually enforced is a finding.
VERDICT — required for every near-duplicate, three-valued:
unify — consolidate into the existing implementation
keep separate — with a STATED reason (else it is an open issue)
unify-contract-keep-drivers — one shared contract, genuinely distinct drivers
A `keep separate` with no reason, or a duplication report with no verdict, is
an open finding. "No duplication found" is invalid if a source went unchecked.
ADVISORY, not blocking. Tortoise is ONE source among several — never the only
one, never required. Unreachable or stale sources lower confidence; they must
never be reported as "no duplicates found".
OUTPUT — use the block below. It carries the skill's required fields (Evidence /
Writers / Shared contract / Verdict / Confidence). `confidence: high|medium|low`
is required on **every** finding, not only some; the skill defines further
per-check fields — include them when present. A finding that is incomplete by
the skill's own definition is an open issue, not a pass.
ISSUE:
severity: P0|P1|P2
dimension: duplication-architecture
location: <plan section>
verdict: unify | keep separate | unify-contract-keep-drivers
writers: <ALL writers of the state, path:line> # D3
shared_contract: <the contract that must be declared, or ABSENT> # D3
evidence: <source — file path, component name, or graph query> # required
confidence: high|medium|low # required
description: <what duplicates what, or what is incoherent>
suggestion: <what to fix, and which mechanism prevents recurrence>
A `keep separate` verdict with no stated reason is an OPEN finding.
If clean: NO ISSUES FOUND — CLEAN
If a source was unavailable: NO ISSUES FOUND — DEGRADED (<source> unavailable)Disposition — controller-level, and #5 is NOT part of the cycle loop. Reviewer #5 is advisory; its findings must not be merged into Phase 2, must not affect convergence, and must not trigger a re-dispatch of any reviewer. (One format retry is allowed if its output is unparseable or missing verdicts — that is a retry for malformed output, not a re-review on its findings.) Phase 4 step 2's Issues found → Phase 2-3 applies to the proportional reviewers only.
| Result | Action |
|---|---|
NO ISSUES FOUND — CLEAN |
Record verdicts in the plan doc. |
NO ISSUES FOUND — DEGRADED (<source>) |
Record + name the unavailable source. Not clean, not blocking. |
ISSUES: with a verdict |
Record each verdict. unify → fold into the plan. keep separate / unify-contract-keep-drivers → record the reason. |
ISSUES: with no verdict |
Invalid. Re-dispatch once; if it repeats, record ⚠️ unverdict findings and proceed. |
⚠️ Both tokens contain the substring NO ISSUES FOUND. Never test #5's result with a substring match — …— DEGRADED is not a pass. Match the full token.
Phase 2 — Merge & Dedup
- Parse all
ISSUE:blocks from reviewer outputs - Dedup: same location + similar description → keep higher severity
- Sort: P0 > P1 > P2, then structural > integration > UX > failure > good-easy
- If zero issues → plan is clean, proceed to Phase 5
Phase 2.5 — Confidence Scoring (NEW — CPI-7)
After merge-dedup, before Phase 3 (Fix): dispatch isolated confidence scorer sub-agents via Pi task for each issue.
Rubric (same as code-review Step 5):
Score on a scale from 0-100:
a. 0: Not confident. False positive or irrelevant.
b. 25: Somewhat confident. Might be real but could not verify.
c. 50: Moderately confident. Real issue but may be a nitpick.
d. 75: Highly confident. Very likely a real issue that matters.
e. 100: Absolutely certain. Confirmed by evidence.
Return ONLY the number.Filter: Issues scoring < 50 are tagged [LOW-CONFIDENCE] P2 — surfaced for human judgment, not sent to fixer. Issues scoring >= 50 proceed to Phase 3 (Fix). Low-confidence issues do NOT count as unresolved for convergence purposes.
Phase 3 — Fix (1 Agent with Research)
Dispatch a fixer sub-agent via Pi task:
You are fixing issues found in an implementation plan. For each issue, apply the minimum change needed.
PLAN DOC: <full plan content>
ISSUES TO FIX (all severities): <merged issue list>
RULES:
1. RESEARCH FIRST (MANDATORY): if an issue involves external systems, library capabilities, or unfamiliar APIs → use web_search to verify the correct approach before writing the fix. This is NOT optional — skip only if the issue is purely internal (typo, formatting, in-repo convention). Research protocol: (a) context7 first for library docs, (b) Perplexity queries for best practices and pitfalls (as many as needed to verify the approach), (c) inject findings as context. No query cap — mistakes cost more than queries.
2. SURGICAL EDITS: change only what the issue requires. Do not improve surrounding text.
3. PRESERVE STRUCTURE: keep task numbering, step format, header structure intact.
4. LOG CHANGES: for each fix, note what changed and why. Include whether research was performed and what sources were consulted.
5. GOOD > EASY RESOLUTION (MANDATORY): For every `good-easy` flag, EITHER fix the plan to the Good alternative OR record an explicit deferral in the plan doc:
`Deferred: <easy path chosen> — Good alternative: <name> — Cost: <effort/time/risk> — Rationale: <why deferred — time-box, external constraint, dependency>`
A deferral without a named Good alternative + cost + rationale is not a deferral — it is an unresolved flag. Keep it open and surface it to the human.
Return the COMPLETE updated plan doc, followed by:
CHANGELOG:
| # | Issue | Severity | Location | Fix Applied | Research? |
|---|-------|----------|----------|-------------|-----------|
SUMMARY:
- Fixes applied: N
- Research queries: N
- New content introduced: yes/noWrite the updated plan to disk. If the fixer introduced substantial new content (new tasks, architectural decisions) → next cycle needed.
Phase 4 — Gate Loop — MANDATORY
Each review cycle dispatches FRESH task sub-agents. The reviewers have no memory
of prior cycles, no investment in defending prior fixes. This prevents confirmation bias.
For each cycle:
- Dispatch all N reviewers in parallel via
tasktool (freshpi -psessions), plus Reviewer #5 when its trigger fires (new component / new write path / new shared-state owner / new vocabulary definition) or on the final cycle. #5 is dispatched alongside the proportional set, never instead of it — N does not drop because #5 fired. If the trigger does not fire, the cycle runs the proportional N only, and the cycle log records#5: not triggered. - Parse responses. Every proportional reviewer returning
NO ISSUES FOUND→ exit clean (adversarial domain:THREAT SURFACE COVEREDfrom a fresh reviewer substitutes — see the Adversarial domain subsection above); issues found → Phase 2-3. Reviewer #5 is parsed separately (see its disposition table):ISSUESfrom #5 does not enter Phase 2-3, does not trigger a re-dispatch, and does not affect convergence. Match its full token —NO ISSUES FOUND — DEGRADEDis not clean. - After fixes applied, go to step 1 (repeat cycle)
Why task sub-agents: pi -p spawns a fresh session. The reviewer has no context
of the original plan draft, no awareness of what was "just fixed." It evaluates the
current plan text with fresh eyes — the closest available proxy for an independent reviewer.
Exit conditions — ALL must be true before proceeding to Phase 5:
Last cycle's proportional reviewers all returned "NO ISSUES FOUND" (verbatim, not paraphrased)
Reviewer #5 (if dispatched) was parsed against its full token and dispositioned per the table above —
NO ISSUES FOUND — CLEAN,— DEGRADED (<source>)recorded as a caveat, orISSUES:recorded with its verdicts. None of these blocks the cycle; all three satisfy this box. Substring-matchingNO ISSUES FOUNDand reading— DEGRADEDas clean fails this box.⚠️ This box gates proceeding, not cleanliness — the two are different predicates and this is the one place a reader mid-loop consults. All three dispositions satisfy it, but only
NO ISSUES FOUND — CLEANyields a clean exit.ISSUES:(and a— DEGRADEDrecorded as a caveat) yields an escalation exit: proceed to Phase 5 with the surviving issues documented and carried into the escalation payload, and never report it as clean or complete. PerAGENTS.md§Hard Cap, an exit that leaves issues unresolved is an escalation exit, not a completion.If cycle 1 found any issues → at least 1 re-review cycle completed
Cycle log posted: each cycle's issues and fixes documented
Adversarial domain only: a fresh reviewer returned
THREAT SURFACE COVERED(every declared threat class test-covered, no in-scope bypass reproduced) — this substitutes for the first box
No hard cap. The loop continues until clean exit or convergence. The safety cap is the tier's Max Cycles row in proportional-gates §Review Cycles — if reached, escalate to human (runaway prevention, not a quality gate). The adversarial domain's own bound is 2 (above) — the skill's own bound for that domain, not a cap imposed by AGENTS.md.
Half-budget research rule. Once half the governing budget, rounded up, is spent — the tier's Max Cycles (proportional-gates §Review Cycles), or the adversarial domain's 2 when that domain applies (above) — every surviving issue needs research backing before the next fix: the fixer cites the source, or records internal-only, no external source when Phase 3's skip condition applies (purely internal: typo, formatting, in-repo convention). An unresearched re-fix does not count as a fix.
Stuckness detection (3-layer algorithm):
a. Fingerprint-stall: Hash each surviving issue's location + severity (SHA256) — the defect's stable identity, never its dimension/description/suggestion. Wording is the part a fresh, memoryless reviewer varies; hashing it made an identical defect re-described in new words hash differently, so recurrence read ~0 on a cycle where nothing was fixed — the most common real stall, invisible to the primary detector. Recurrence is |current ∩ prev| / |prev| — "what fraction of last cycle's issues came back?". |prev| is the correct denominator and the only one: it is the stall signal, so a cycle that repeats all 10 prior issues must score 1.0. (A symmetric denominator — max(|current|, |prev|) — inverts this: repeating all 10 prior issues plus 5 new ones scores 10/15 = 0.67, below threshold, so the loop reads as ordinary churn on a cycle where nothing was resolved. Do not use it.) stall_threshold defaults to 0.8 — the same value code-review/references/fixer-loop.md reads from STALL_THRESHOLD. If recurrence ≥ stall_threshold → escalate to human with stuck issues and attempted fixes. Do NOT auto-exit.
b. Honest-stuck: Track issues_per_cycle. This is an INDEPENDENT signal, not a refinement of recurrence — it fires when the count is non-decreasing for 3 consecutive cycles, regardless of recurrence. Gating it behind recurrence ≥ stall_threshold was itself a regression: a loop that churns one-for-one (every issue fixed, a new one appearing in its place) has low recurrence but is not converging, and the gate left it undiagnosed until the cycle cap. Exit reason = honest-stuck. Escalate to human. Do NOT auto-exit — remaining issues must be acknowledged by a human before proceeding.
⚠️ Fire if recurrence ≥ stall_threshold OR issues_per_cycle is non-decreasing for 3 consecutive cycles. Either is sufficient on its own. The two tests exist because they catch different pathologies — recurrence catches the same issues coming back, the count test catches the loop not shrinking, however caused. Recurrence is used only to pick the label (honest-stuck when the count is not shrinking, else fingerprint-stall); it is never a precondition for the count signal.
c. Zero-progress: Track whether the plan doc was modified each cycle. If the plan doc is unchanged for 2 consecutive cycles, the fixer is making zero progress. Exit reason = zero-progress. Escalate to human. Do NOT auto-exit — remaining issues must be acknowledged by a human before proceeding. (Previously mapped onto fingerprint-stall, which made three distinct conditions — identical issues recurring, new issues outpacing fixes, and the fixer writing nothing at all — indistinguishable in the persisted record.)
Convergence rule: If cycle N issues are a strict subset of cycle N-1 issues (no new dimensions or locations, only previously-flagged items remain), the reviewer is in a refinement loop — fixes are shrinking the problem space but not eliminating it. Log convergence and escalate to human: present remaining issues with attempted fixes. Do NOT auto-exit — remaining issues must be acknowledged by a human before proceeding.
Cycle-status YAML: Write operations/logs/cycle-status.yaml on loop exit:
exit_reason: <clean|fingerprint-stall|honest-stuck|zero-progress|cycle-cap|adversarial-capped|convergence>
cycles: <N>
issues_per_cycle: <json array>
plan_modified_per_cycle: <json array of booleans>
detector_fired: <fingerprint-stall|honest-stuck|zero-progress|''> # which layer fired, or empty
fingerprint_recurrence_last_cycle: <0.0-1.0|null> # the predicate's actual input; null on a zero-progress exit, which fires before the scanWhy
detector_firedandfingerprint_recurrence_last_cycleare recorded: without them, "why did this loop exit?" cannot be answered after the fact — the predicate's inputs are gone and only the verdict survives. That is what made a real non-convergent run undiagnosable.
Progress report: After each cycle, output: "Plan review cycle N: X issues found across N reviewers, Y fixed, Z remaining."
FORBIDDEN — these bypass the quality gate entirely:
❌ Run review → get issues → fix → declare done without re-dispatching reviewers
This IS skipping the review. Fixing without re-reviewing = no review.❌ Self-declare "I addressed the feedback" as completion
Only "NO ISSUES FOUND" from all fresh reviewers is a valid exit signal — except in the adversarial domain, where a fresh reviewer'sTHREAT SURFACE COVEREDsubstitutes (see "Adversarial domain" above).❌ Re-review in the same conversation context
Confirmation bias makes same-context re-review unreliable.
Always usetaskfor fresh sessions.
Phase 5 — Final Verification
After plan is clean (Phase 4 says clean), dispatch the tier's reviewers (the same N as the review cycles, proportional to plan risk) via Pi task — one per Phase 1 prompt, not concatenated — to re-review the final plan.
If any reviewer finds issues:
- Fix them (Phase 3)
- Re-verify (Phase 5 again)
- Max 2 additional cycles
- On 3rd failure → surface to user
Append a summary line to the plan comment when double-gate runs:
🔍 Final verification: found N residuals — resolved in M fix cycles- Or:
🔍 Final verification: clean
Exit & Signature
On exit, append to the bottom of the plan doc:
<!-- plan-review: cycles=N, status=clean|capped|stalled, version=2.3.0 -->Clean exit: Return control to writing-plans for Execution Handoff. Do NOT pause or ask the user for confirmation. The review cycle IS the quality gate — if it passed, proceed immediately.
Capped/Stalled exit: Before halting, the orchestrator (you, the main session) makes one deep-fix attempt:
- Read all remaining issues
- Research each one (web_search for correct approaches)
- Fix the plan directly with researched solutions
- Re-run the review cycle (Phase 1-4)
- If still capped/stalled → THEN output "Requires Human Input" with full context
This gives one orchestrator-level recovery before waking the human.
## Plan Review — Requires Human Input
**Status:** capped after N cycles | stalled at cycle N
**Remaining:** P0: X, P1: Y, P2: Z
### Unresolved Issues
1. **[P0] [dimension]** — [location]
[description]
Fixer attempted: [what was tried]Announce
At invocation: "Running plan-review on [plan-doc-path] with N parallel reviewers (proportional to plan risk). Capped per plan risk tier (proportional-gates §Review Cycles) — escalates to human at cap. Does not auto-exit with remaining issues."
Continue following the workflow as mandated by this skill. Do not skip steps.