implement-review-verify

Imported from xdevs23/workflow-skills (skills/implement-review-verify/SKILL.md). Install upstream with npx skills add xdevs23/workflow-skills --skill implement-review-verify. Copyright stays with the author.

Implement → Review → Verify → Fix — a workflow for code changes

A reusable, project-agnostic shape for landing a non-trivial CODE change with confidence. It is the code-implementation counterpart to the document-oriented loops (verify-loop, find-gaps, research-loop): those prove a spec; this builds against a settled design and adversarially checks the result before it is accepted. Scoped commits provide immutable review snapshots, not approval to merge or push.

The orchestrator runs it as a Workflow() (deterministic fan-out/sequence). The phases are fixed; the breadth inside each scales to the change.

Execution context — orchestrator versus stage

The orchestrator owns the process. Skill selection/loading, workflow construction, stage launches, barriers, retries and final completion checks are orchestrator responsibilities. An instruction to use this workflow does not require each stage to launch another copy.

A stage owns only its assignment. Implement, review, verify or fix as assigned, then return its result to the enclosing workflow. Never launch workflows or subagents, invoke another process indirectly through a skill or shell command, or repeat the pre-phase. The enclosing workflow owns the remaining stages; their checks are not already passed just because it exists.

Load a matching skill for instructions when required and available, but apply only the stage-relevant instructions. Orchestration sections address the caller. Before launch, the orchestrator supplies any required stage instructions that the stage cannot load, respecting its input boundaries: execution hygiene is appropriate for unbriefed seats, design briefing is not. Do not grant extra tools or broaden a reader's source access merely to load a skill. If required stage instructions remain inaccessible, report that specific limitation rather than pretending they were read.

Missing Workflow, Agent or Skill tools alone are not an authority contradiction or a reason to stop a fully briefed stage. Missing capabilities needed for the actual assignment, missing authorization, or genuinely contradictory applicable requirements still block the affected work and must be reported honestly. This role boundary does not override higher-priority instructions. Scope inherited project/global workflow mandates to orchestrators at their source; a child prompt is not a workaround for an explicitly conflicting instruction.

Include the stage boundary in every prompt, including template-less cold spec review and the Git-object-only roaster. Keep orchestration tools unavailable to stages; loading instructions is not permission to recursively execute the process they describe.

When to use it

Reach for this when at least one is true:

• the change touches shared infrastructure other code depends on (a queue, an executor, a base class, a wire format);
• it carries subtle invariants — ordering, idempotency, concurrency, dedup, a "complete only after X lands" guarantee — where a plausible-looking implementation can be quietly wrong;
• the user explicitly asked for "a workflow" / "with reviewers" / a thorough pass.

Do NOT use it for one-off mechanical edits, a rename, or pure research — the overhead (multiple agents reading the codebase) isn't worth it. For those, just do the edit, or use a single agent.

Before phase 1 — settle the design AND pin acceptance criteria

Settle the design first. The implementer builds against a decided design; it does not invent scope. If the design isn't settled, stop and settle it with the user (or run a design/research loop) first.

ACCEPTANCE CRITERIA ARE MANDATORY. Before you launch, state them explicitly — numbered, checkable, one per behaviour that must hold — and make sure the spec doc carries them too. The concern reviewers return verdicts per criterion; the additional seats retain their distinct contracts. This is not ceremony: without pinned criteria, "review" degrades to vibes, each seat invents its own bar, and nothing the fixer receives can be triaged against anything. No criteria, no launch.

Pre-phase — cold spec review (2 unbriefed seats, BEFORE any implementation)

Since a settled spec is already the precondition for launching, review the SPEC before reviewing the code. Two seats, from DIFFERENT model families, each given only "review the spec at <path>" plus repo access and the run's hygiene floor (git safety, where scratch goes, run checks bare, no background waits, and that no seat edits an authority document) — no briefing, no framing, no orchestrator summary, because the absence of briefing is what makes them see what the author stopped seeing. The hygiene floor is not a briefing: it says nothing about the spec, the review taxonomy or what the author meant. The main run's shared PINS block is not handed to these seats, because its authority tiers and findings contract are exactly that framing:

• a gap-finder (agentType:'gap-finder') — what the spec fails to say: unhandled cases, undefined behaviour, assumptions stated nowhere. Its usual scope fence comes from the artifact itself here: the spec's own stated scope is the fence, taken from the doc rather than handed over by you, which is what keeps the seat unbriefed;
• a soundness reviewer — a plain unbriefed strong seat on the other family, asked whether the spec's requirements are mutually satisfiable and whether each acceptance criterion is actually checkable as written. This seat is deliberately template-less: any fixed role prompt would be a briefing.

These seats must PROBE, not just read. Most of the yield comes from rendering, recomputing, fetching and measuring the spec's claims against reality — a read-only adversarial pass catches roughly a third of what a probing pass catches. Say so in both prompts, and name the artifacts they may exercise. A duty to probe is not a briefing and does not break the rule above: it says nothing about what the spec contains or what the author meant. Rank the three yield factors honestly, because the ranking decides what you protect when you trim:

1. Unbriefedness matters most. A briefing smuggles in the author's frame; an unbriefed seat reads what the spec SAYS — which is exactly what the implementing agents will read.
2. Empirical duty second. A seat told to verify against the artifact finds what no amount of careful reading finds.
3. Vendor diversity third — cheap insurance that mainly widens minor-finding coverage, with one specific exception that earns it: it catches the orchestrator glossing the same ruling two contradictory ways.

Typical yield is around three blockers per spec, of exactly the class that is catastrophic to discover mid-implementation. Name the three classes in the prompts, because naming them makes them findable:

1. JOINT IMPOSSIBILITY — two constraints, each satisfiable alone, unsatisfiable together. Authors check constraints pairwise; nobody checks the conjunction. This class is found by COMPUTATION, not by reading.
2. MISSING PRODUCTION CONTRACT — the spec assumes an artifact exists without saying how it is produced, sized, or kept in sync.
3. REALITY DRIFT — the world moved under a recorded assumption.

Discovered here they cost an edit; discovered in phase 4 they cost the run.

Their output is advisory to the orchestrator, who triages it against the recorded rulings and amends the spec. Amend the SPEC DOC — never patch the finding into a prompt, or the spec and the prompts immediately disagree.

Run the pre-phase as its OWN short run, and let it end there. A running script cannot pause while a person edits a document, so a pre-phase bolted onto the front of the main script launches the implementer against the unamended spec and the whole yield is advisory to nobody. Two runs: one that returns the two cold seats, then the triage-and-amend, then the main workflow against the amended doc — which the seats below read from disk (law 9), so no prompt needs rewriting.

Spec discipline: trivial work gets no spec, and having a spec is exactly what makes the two cold seats worth it. Do not manufacture a spec to justify the seats, and do not skip the seats when a spec exists.

Anti-re-litigation needs a technical decision record and a PRIVATE source record. The committed spec records decisions, constraints and rejected alternatives with their reasons, never conversational quotations. Treat user messages as confidential: verbatim directives may be kept only in untracked, ignored artifacts unless committing them is explicitly authorized. Point authority-aware seats at that private record to verify fidelity without copying it into tracked docs, tests, code or commit messages. A broad commit instruction does not authorize including private records. Keep workflow scripts containing private text untracked too.

The root builds that private record from the actual conversation: the directives themselves plus the qualifications, surrounding context and examples that give them meaning, each with its source and order so later statements can be told from earlier ones. Label a summary or an applicable project requirement as such — neither substitutes for available verbatim evidence, and neither is relabeled as a human quotation. Never selectively omit, truncate or rewrite the original evidence to make a spec or implementation pass; only a later, actual human decision may supersede an earlier one, and only with its provenance recorded — an assistant's own spec edit never does. The record is fixed for the duration of a review cycle; a new directive invalidates the reviews and approvals it affects. A necessary part of the record being unavailable or incomplete is an explicit limitation that blocks acceptance — it is never license to fall back on trusting the spec.

The shape

Four phases: Implement → Review → Verify → Fix — after the cold spec review above. Cold alternatives joins Review. The mandatory roaster overlaps Fix on the pre-fix commit plus approved fix list; its findings join the next Verify pass against the resulting snapshot.

Phase 1 — Implement (1 agent, sequential — agentType:'implementer')

ONE implementer (agents/implementer.md), working sequentially on the real tree. One agent — not a fan-out — because a coupled change mutates shared files and parallel writers collide. Multi-implementer fan-out on a coupled change is explicitly rejected: it produced file collisions and consistency drift. Parallel implementers are allowed only across genuinely disjoint trees/repos — and even then the reviews can be one barrier covering both. Brief it with:

• what is already on disk (if part of the work exists), file by file, told to REUSE not rebuild;
• the settled design and its decisions, stated as authoritative, plus the acceptance criteria;
• the invariants in plain language (the ordering rule, the idempotency rule, …);
• a self-check: run the relevant test subset before reporting done, and FIX what it added that fails.

Prompt scrutiny / abort — exactly one trigger. The implementer checks the prompt against the spec and the code before editing, and there is a single abort condition: a human verbatim directive directly contradicted by either authority document or by this prompt — directive-versus-spec and directive-versus-prompt are the same trigger. The AUTHORITY DOCUMENTS are the human's verbatim directives and the spec; the prompt is UNTRUSTED relative to the spec (law 8), but that ranking does not exempt the prompt from the directive ranked above both. Then everything else falls out:

• prompt vs spec, with no directive on either side → an ordinary MUST-FIX finding, not an abort. The prompt loses, the seat proceeds against the spec, and it reports the conflict rather than silently picking a side;
• the prompt asserts a plainly false premise about the tree ("module X already exists") → VERIFIED-AND-REPORTED. Every factual claim the prompt makes about the tree is CHECKED against the tree before anything is built on it; a false one is not merely disregarded but corrected — build to the TRUE state of the tree, and flag the premise as a must-fix in the report. That beats both stopping and trusting, and it is what "untrusted" is supposed to buy. It is not a contradiction with a directive, so it must not fire the marker;
• a tree that does not yet satisfy the spec → the NORMAL starting condition. Treating it as a contradiction deadlocks the run (law 10).

None of those three emits the marker. Only a contradiction with a human directive on at least one side emits HARD-FLAG:. Caught before any edit, it stops with the tree UNMODIFIED; caught after some edits already landed, it stops further writes that would extend the conflict and reports the existing changes as-is, without reverting them. No extra gate beyond that timing. One trigger, one marker, one disposition — a taxonomy with two abort classes and one marker leaves a class undetectable, and a class with three dispositions deadlocks. Same rule for scope: touch only what the task needs, and flag anything beyond the ruled scope as an invention rather than building it.

It reports what changed and the test result, commits only its own scoped changes after checks, then returns the full immutable snapshot SHA and clean status with quoted Git evidence. A failed check or commit is an incomplete stage, never a fabricated successful snapshot.

Writer commits are snapshots, not integration permission

Start each writer in a clean isolated worktree at the supplied full SHA. Before edits, inspect HEAD, the index and working-tree status; unrelated or pre-existing changes are an anomaly, not permission to absorb or discard them. Only the implementer and fixer may stage explicit paths for their own scoped changes, inspect the staged diff, and create NEW commits after checks. No broad add, amend, reset, rebase, merge, cherry-pick, branch switching, history rewriting or push. Honor project commit-message rules and normal hooks/signing. If hooks change content, rerun proof on the final committed contents before claiming success.

Return startSha, full snapshotSha, clean and quoted Git/test evidence. Use git rev-parse --verify HEAD^{commit} and git status --porcelain=v1 --untracked-files=all. Scratch and local TODO.md remain ignored and untracked; clean status is not permission to commit them. Genuine no-ops reuse their starting SHA without an empty commit. Readers and the verifier independently check snapshots; a writer's self-report is not proof by itself.

All other seats remain Git-read-only. The root pins the starting commit as args.baseSha; use full object IDs, not HEAD or moving branch names, as review identity. Immutable commits avoid an extra checkout, archive or copy. Acceptance and integration still happen separately.

Phase 2 — Review (N agents, parallel VERDICT seats, split BY CONCERN)

Independent reviewers, run in parallel, each owning a DISTINCT lens, each via its own agentType. This phase is a genuine barrier — the finding verifier needs every selected reader before consolidation. The standing seats:

• Correctness (agents/reviewer-correctness.md) — bugs, races, broken invariants, the failure modes the change introduces. Name the hazards in the prompt: "check the guard semantics around X" beats "find bugs". Tell it to say plainly "I found nothing" rather than invent issues. This seat also owns ASSERTION GRANULARITY (law 16): it READS the assertions and checks that each invariant is pinned at the granularity the rule binds at, never aggregated over the artifact — a class the gate structurally cannot catch, because the aggregate assertion is green. When the work must PRESERVE AN INVENTORY — every fact, row, entry or capability carried from a source into a new artifact — this seat also owns TRUNCATION-WITH-ELLIPSIS: under content pressure the characteristic failure is to COMPRESS, truncating an entry with an ellipsis, collapsing a list, or folding content behind a disclosure device, and the result still reads as complete and well-formed. The check is an explicit inventory diff against the source, item by item, treating any collapse or truncation device as a FAILURE rather than a formatting choice. It is a seat check for the same reason as the one above: it needs a reader holding both artifacts side by side, and nothing a gate can run goes red.
• Separation of concerns / cleanliness (agents/reviewer-cleanliness.md) — does logic sit in the right layer? Did a special-case leak into shared/generic code? Dead code left by the rework? Naming — including a PLAIN-LANGUAGE lens: identifiers and prose in plain words, no coined metaphor vocabulary, because a coined vocabulary makes the work unreadable to the person who owns the thing it describes. (NOT bugs — that's the other seat's job.)
• Spec compliance (agents/reviewer-spec-compliance.md) — checks explicit requirements FORWARD into the implementation: missing or incorrect required behaviour. The spec, not the orchestrator's description, is its reference. It receives NO implementer report. Inverse-spec owns the reverse authorization map, excess scope and decisions missing from the spec.
• Duplicate checker (agents/duplicate-checker.md) — "one decision path, recorded once": second enforcement sites, parallel decision paths, truth re-derived or re-recorded twice, logic copied instead of shared. Cheap, narrow, and catches a class nothing else does.

A seat earns its place by having a DISTINCT FAILURE-DETECTION MODE, not by adding redundancy. Three identical reviewers are worth less than three different lenses. Add a fifth lens (security, performance) only when the change actually has that surface.

Concern-reviewer output: per-criterion verdicts, never bare lists. These seats return PASS / AT-RISK / FAIL against each stated acceptance criterion, every verdict backed by file:line receipts, plus its findings rated must-fix / should-fix / nit. A bare findings list lets a reviewer hedge; a verdict is a claim someone can refute. Receipts are the only currency that survives triage.

Only the three code-lens verdict seats receive the implementer's report as UNTRUSTED CLAIMS. The code-lens seats (correctness, cleanliness, duplication) get it explicitly as a list of CLAIMS TO VERIFY against the actual tree, never as a source they may review by reading: holding the claim in hand is what lets a seat catch a claim that is false, which it cannot do if it never saw the claim. The SPEC-COMPLIANCE seat does not receive it at all. The seat that judges the code against the AUTHORITY DOCUMENT must not be handed the implementer's account of what it did — its whole job is the spec versus the tree, and an account of the work is precisely the framing that makes a missing requirement look answered. One briefed verifier plus one cold judge beats both all-briefed and all-cold. This is a rule about WHICH INPUT a seat gets, and it is a different thing from the cold-every-round rule in phase 4, which is about CROSS-ROUND state and applies to every seat here including this one.

And a FINDING IS A DEFECT — nothing else. The verdict rows, the coverage notes, the record of what was run, the criteria that passed: all of those ride in the seat's report, never in its findings array, because mixing coverage with defects obscures what actually needs correction. Every source finding carries a FILE, cited repo-relative, so verification can trace the claim to the tree. Coverage gaps without a code location belong in the report and must still be checked by the verifier. Concern reviewers suggest WHO CAN CLOSE IT using their existing actionability lanes; the verifier validates those suggestions before dispositioning.

Every source finding and report goes to the finding verifier. A lane or severity assigned by a reviewer does not authorize a fix; only the verifier's checked, consolidated approval does.

Additional review seats — parallel with the verdict reviewers

• Quality (agents/quality.md) — a broad, deliberately unbriefed read of the diff and touched-file context. No spec, directives, project docs, implementer report, or shared authority briefing. Its ignorance is the mechanism; use only the hygiene floor and diff.
• Inverse-spec (agents/reviewer-inverse-spec.md) — maps the COMPLETE branch diff's choices back to exact authorizing words. Owns excess scope, missing spec decisions, deletion/simplification proposals and estimated savings. Spec compliance owns the other direction: whether explicit requirements are implemented correctly.
• Project rule reader (agents/project-rule-reader.md) — reads complete changed files against applicable project/global rules, including violations beside the diff. Its cleanup findings are preserved without expanding this unit's repair scope.
• Cold alternatives (agents/cold-alternatives.md) — only the diff, surrounding code and required invariants, never the implementer's report. Proposes materially simpler shapes or explains why the current shape is right.

All selected Review seats are REQUIRED results. Read them against a stable tree and await ALL of them before verification. The roaster is the explicit exception to this scheduling: it runs in Fix against immutable Git objects, never against the writer's moving filesystem. Quality can legitimately return a short no-findings report. Other seats keep their own output contracts; the inverse reviewer owes an authorization map, the rule reader a coverage report. Do not impose acceptance-criterion verdicts on these distinct roles just to make their report shapes identical.

Phase 3 — Verify and consolidate (1 read-only agentType:'finding-verifier')

The verifier receives ALL reports available for the current cycle, including quality and cold alternatives. The initial verification precedes roasting; each concurrent roast is consumed after its fix pass, before completion or another correction is approved. It checks claims against the code, settled spec, applicable rules and recorded instructions, resolves conflicts using evidence, and merges duplicate defects into ONE fix list. It preserves every source ID: consolidation is never permission to drop a finding. It also checks report-level coverage gaps and independently verifies prior fix claims.

This is ordinary workflow work, not a root checkpoint. The root is an exception handler. A verifier is neither a rubber stamp nor a new source of design authority. Corrections already authorized by the record can proceed regardless of which seat found them; a new necessary choice cannot proceed merely because a reviewer or verifier prefers it.

One explicit decision per consolidated group:

• approve-fix — verified defect and already-authorized correction. Supply evidence, authority references with EXACT QUOTES, the correction, constraints and an acceptance check.
• reject — false positive or unsupported objection, with concrete counterevidence. Duplicates are MERGED with all source IDs, not silently rejected or discarded.
• needs-decision — a choice without which the assigned work cannot satisfy the existing requirements. Establish the impossibility, name the question and alternatives; return to the root.
• root-action — a demonstrated impossibility or required investigation the verifier cannot complete. A proposed spec edit alone is not a blocker: implement and review the spec as written, retaining non-blocking suggestions in the report or as record, not as prerequisites.
• cleanup — verified work outside this unit's repair scope, with concrete cleanup entries and receipts retained for the root's end-of-run handoff.
• record — genuinely non-blocking observations, retained in the ledger. Never use it to dispose of a confirmed must-fix or CRITICAL violation.

Every inverse-spec source finding carries CRITICAL severity unconditionally, regardless of the label it arrived with (law 15): record and cleanup are never available for one — an inverse-spec finding is about a choice made IN this unit's own diff, never work outside its repair scope — and reject still needs concrete counterevidence against the finding itself, never against an edited spec.

Reviewer lanes and severity are claims to verify, not queue permissions. Every source ID must belong to exactly one decision group. The SCRIPT checks coverage, unknown IDs, duplicate IDs and approval payloads before mutation. Missing reports or invalid handoffs stop the run; missing evidence is never an implicit rejection or a clean empty queue.

Only approvals enter the fixer list. Unsettled necessary decisions, required root actions and unresolved report-level limitations prevent fixing, including otherwise approved work in that unit. Routine rejections and successful consolidation remain in the workflow record and final summary; they do not interrupt the root one by one. Every decision on an inverse-spec source finding, however it resolves, stays visible to the root in that summary: an approve-fix or a well-evidenced reject does not need to interrupt the cycle, but the root still owes each one an explicit resolution — correcting the spec to state an existing decision faithfully, or asking the human about a genuinely unsettled one — and neither a later spec edit nor a completed run closes it on its own.

Phase 4 — Fix and roast concurrently

Launch ONE fixer and ONE mandatory roaster together after the approved list is finalized. Capture the pre-fix SHA before starting either. The roaster receives the immutable base and snapshot SHAs plus the approved list, using its Bash-only Git-object prompt. It reads files with git show SHA:path, lists with git ls-tree, searches with git grep at that SHA, and compares with git diff --no-ext-diff --no-textconv BASE_SHA SNAPSHOT_SHA --. No source-tree Read/Grep/Glob, filesystem scripts, builds, symlink following or external diff helpers. It never substitutes HEAD. Receipts include the snapshot SHA and file:line.

The fix list is PLANNED WORK, not proof. The roast should avoid repeating assigned defects, but may flag inadequate corrections, interactions and uncovered weaknesses. Both tasks must settle before the next verification or before control returns on failure. The roaster cannot be dropped, including when the approved list is empty and the fixer runs proof only. A missing, failed or wrong-snapshot roast leaves the run incomplete.

The fixer receives ONLY the consolidated approved list, with its source IDs, evidence, authority and boundaries. Raw reports are not extra work orders. It:

• independently rechecks each approved correction before acting;
• returns exactly one fixed / rejected / blocked disposition per approved key;
• applies the approved outcome within its bounds, never broadening scope or editing a spec or other authority document to make the correction legal after the fact;
• returns disagreements with counterevidence to the ROOT, not automatically to the human and not to another automatic fix attempt. A blocked mechanism stays untouched;
• runs full checks BARE AFTER ITS LAST WRITE, commits completed scoped corrections, then returns the clean snapshot SHA, quoted Git/check output and proofPassed. The next independent pass must still verify the commit and any claimed closure.

With an EMPTY approved list the fix pass owes PROOF ONLY and may not edit or create an empty commit. It returns the original SHA. A failing check is reported for independent triage, not permission to invent a repair. The concurrent roast is still mandatory and must be processed by Verify; a green proof alone cannot complete the run.

The Review → Verify → Fix loop

Ordinary detection stays cold. After a new writer commit, Review uses fresh seats with their original input boundaries and the new SHA. No findings history or fixer explanations go to those readers. The roaster is deliberately informed by the current approved fix list; quality stays unbriefed. On an unchanged proof-only snapshot, reuse the already-journaled ordinary reviews rather than rerunning them; only the new roast needs verification. The verifier receives persisted prior decisions and pending fixes as UNTRUSTED context, because it must reconcile repeats and check closure. No agent relies on private memory.

Source identity is deterministic, consolidation is semantic. The script assigns IDs by round, seat and finding index. The verifier groups the same defect across sources using code evidence; it never groups merely by file or similarity of prose. Every group preserves its source IDs. The script uses these IDs for exact coverage checks, not a hand-written normalizer that tries to decide whether two claims mean the same thing.

A fix claim is not closure. After a committed fix, run fresh Review → Verify and include the preceding roast with its ORIGINAL snapshot receipts and IDs. Verify those claims against the POST-FIX snapshot; a resolved criticism is a rejection with evidence, not another fix. The verifier checks each pending key's acceptance condition against the CURRENT tree and returns closed / unresolved, with evidence, exactly once per pending key. An unresolved attempted fix is bounded non-convergence and returns to the root. Reviewer silence alone never establishes closure. A corrected defect may involve a caller or shared implementation rather than the file the reviewer cited; touched-file bookkeeping is a cross-check, not semantic proof.

The loop exits when fresh verification has no further approved work and the current proof passes; when a necessary decision/root action or fixer disagreement needs resolution; when a required check fails; or when the code-fix budget is spent and verification still finds approved work. A final independent read/verification is allowed after the last fix pass so its roast and closures are never silently omitted. New approved findings may start another fix pass within that budget. Never turn a rejected or blocked correction into an endless internal argument. Return each exception with the decisions and evidence that make it actionable; the root decides whether the human must break the tie.

Every post-write exit retains explicit unverified state. Until independent closure, keep pending fixed keys in unverified and treeUnreviewed: true. A later review that confirms some fixes but cannot close another records both facts separately. A budget exit must not turn the fixer's own claim into a verified result. An empty proof-only run cannot invalidate the reviewed tree by editing it. Any unprocessed roast is separately reported in unverifiedRoasts, even if the concurrent writer failed or disagreed. A successful result requires all launched roasts to have been processed. This does not add a second roast of the final commit merely to repeat the just-completed post-fix independent review.

Rule violations and local cleanup records

Bound review and repair separately. Ordinary verdict seats review the change and what it touches. The rule reader reads EVERY changed file IN FULL against the applicable project and global rules, including violations beside the diff. Cite the code, exact rule and source. A confirmed rule violation is CRITICAL, never a nit; matching house style or pre-existing status cannot excuse it. CRITICAL expresses rule compliance, not an assumed level of operational impact, which is reported separately.

The verifier approves authorized corrections in this unit's repair scope. Unrelated existing violations become concrete cleanup entries: issue, rule citation, code receipts, source finding IDs and the required correction. Existing entries are updated rather than duplicated. The root records this consolidated handoff in the project's TODO.md in the SAME RUN, before reporting the task finished, including when the workflow exits with unresolved work. Schedule those cleanup units promptly; recording an issue is not fixing it or permission to defer it indefinitely. Do not force unrelated cleanup into the current fix loop or interrupt the root for each entry separately. If recording is blocked, report the incomplete handoff explicitly.

TODO.md stays UNTRACKED by default, not merely unstaged. Creating or updating a local cleanup record is not permission to version it. Track and commit it only when the user explicitly requests that. Before writing, inspect any existing file and check its Git tracking status with git ls-files --error-unmatch -- TODO.md. For an untracked file, ensure Git ignores it; prefer a repo-local /TODO.md entry in the exclude file located by git rev-parse --git-path info/exclude unless an existing ignore rule already covers it. Inspect and preserve that exclude file; do not rewrite tracked .gitignore just for this local default. Never stage or commit TODO content through a broad add/commit operation.

If TODO.md is already tracked, do not silently delete it or remove it from the index. Honor a recorded explicit request to track and commit it; otherwise report the tracking conflict to the root for direction before writing cleanup entries into it. The read-only reviewers and verifier never edit TODO files or Git excludes; this handoff belongs to the root.

The enum-locked handoff and executable example below implement this contract. The design and rejected alternatives are recorded in docs/workflow-finding-verification.md.

Root completion checks — timing, size and project-defined integration

A completed Review/Verify/Fix cycle proves that cycle, not permission to integrate or delete its worktree. The root performs the checks below before accepting the unit. These are root responsibilities, not extra workflow seats or a second implementer pre-check.

Root question-premise check

Before presenting any question, trade-off, limitation or acceptance request to the human, the root checks its premises first. Identify the proposed question in plain terms, the premise it rests on, and the exact directive/context reference and any related spec-compliance or inverse-spec finding it touches. Then check the record against that premise: when it challenges the premise, investigate the mismatch before asking anything, identify the unsupported scope, and report a discovered implementation deviation from the requested result plainly — never present the consequence of an invented mechanism as though it were a new choice the human must make. A choice the record already settles is never asked again; only a choice it leaves genuinely unresolved is presented as a decision request. Every entry the run returns in inverseSpecDecisions gets this treatment: the root either corrects the spec to state the existing decision faithfully or, after this check, asks the human about the part that is genuinely unsettled.

This is a root PROMPT obligation, not a script gate. An executable test can confirm the instruction above is wired into the root's prompt and that exceptions and inverseSpecDecisions reach the root intact and unretired; it cannot prove a future model actually performed the conversational premise check correctly.

Post-run timing review

After every run, including an incomplete run, inspect the actual per-stage durations in the harness results or journal. Include retries and distinguish executed work from cached replay. Name the largest time sink and whether it was necessary reasoning/generation, machine waiting, repeated source discovery, repeated checks, or rework. Parallel durations overlap: do not add agent elapsed times and call the total workflow wall time. If timing data is unavailable, report that limitation rather than inventing durations.

Remove avoidable cost at its source: reusable prepared artifacts, narrower assignments, missing task context, or redundant checks. Preserve cold-review input boundaries and required checks after the last write; do not improve timing by deleting reviewers or trusting stale proof. Apply improvements within authorized scope and report any broader follow-up. No fixed agent-duration ceiling, automatic model escalation, extra polling loop or provider-specific concurrency limit is introduced.

Size report and the 20:1 acceptance gate

Measure the final candidate against its unit spec before integration, using immutable inputs: record the merge-base SHA, candidate SHA, spec path and spec blob ID. Read the spec at that candidate, not a moving working file. For bundle/patch delivery the comparison base is the project's declared reconstruction base; do not silently substitute a convenient newer base.

• Spec lines: count non-blank lines in the unit spec. This is a line count, not a Markdown interpretation; include its technical content, not a private conversation record.
• Code added/deleted: sum the added and deleted line counts from git diff --no-ext-diff --no-textconv --no-renames --numstat BASE_SHA CANDIDATE_SHA -- over implementation files. Use added lines as the numerator, never net added-minus-deleted.
• Test added/deleted: report separately for paths containing tests/, test/, .test. or _test.. Treat the repository root as a path boundary so root-level test directories count.
• Exclude documentation (*.md), lockfiles, generated files and binaries from implementation counts. List excluded paths and the reason for each; lockfile/generated classification must come from concrete project conventions or Git attributes, not an agent's wish to reduce the number. State additional project-specific test/doc patterns explicitly. Keep the same classification and rename setting on every measurement; report deleted totals and test totals alongside the ratio. Disabling rename detection makes accounting reproducible (a moved file counts as delete/add); explain large moves rather than silently changing the measurement.
• Ratio: code added / non-blank spec lines, displayed to one decimal. Compare unrounded counts: above 20:1 blocks acceptance/merge; exactly 20:1 does not breach the size gate. The size gate passing is not proof of correctness or permission to skip another check.

The root obtains the counts from Git and the pinned spec, retaining receipts. Use established libraries/tools for machine-readable Git data, not a hand-written diff or Markdown parser. Missing measurements or an empty required spec leave acceptance incomplete, never a zero ratio. For the separate no-spec targeted-patch path, report the ratio as not applicable and the code/test counts anyway; do not manufacture a spec or reclassify a spec-governed unit to evade the gate.

This arithmetic helper classifies a measured, spec-governed unit; it does not collect counts or authorize integration. The root supplies the verified counts and handles the result:

const assessSize = ({ specLines, codeAdded }) => {
  if (!Number.isSafeInteger(specLines) || specLines <= 0 ||
      !Number.isSafeInteger(codeAdded) || codeAdded < 0) {
    throw new Error('Verified code counts and a non-empty unit spec are required')
  }
  return {
    specLines, codeAdded, ratio: (codeAdded / specLines).toFixed(1) + ':1',
    status: codeAdded > 20 * specLines ? 'root-review-required' : 'within-limit',
  }
}

A breach is first a root diagnosis, not an automatic request for permission or a fixer retry. Read the inverse-spec review and the candidate against the existing requirements. Identify unnecessary mechanisms, duplication and concrete deletion/simplification savings; also identify real missing spec detail or a prerequisite foundation. Correct excess code through the normal approved-fix/review path. Only the root may clarify genuinely missing spec detail, consistent with existing authority; new scope still needs authorization. Never pad the spec to lower the ratio, or use a later amendment to retroactively authorize unsupported code. A spec suggestion alone does not stop the implementation/reviewer cycle; this gate applies to the finished unit.

After any code/spec change, repeat affected verification and measure the new pinned candidate. If the justified implementation still exceeds 20:1, keep acceptance blocked unless the user explicitly approves that remaining size. Before asking once, present the ratio, inverse-spec conclusions, savings already taken or rejected with reasons, real spec gaps and the remaining size traced to requirements. Keep any verbatim approval private/untracked; record only the technical disposition in commit-bound artifacts. Approval is specific to the measured candidate and spec, not a reusable waiver. Do not repeat the request without materially new evidence.

Integration and worktree cleanup belong to the project

The project chooses its integration/delivery contract: a PR, direct merge, Git bundle, patch file, or another explicit handoff. Record the chosen route, destination and completion evidence before integration; if no route is established, leave a verified candidate and report that integration is pending. Never infer merge/push/network-send permission from snapshot commits. The size gate applies before accepting the candidate for any route, not only direct merges. An explicitly requested draft/review artifact may expose an unresolved gate, but must be labeled unaccepted; producing or sending it does not waive the gate.

Keep temporary worktrees inside an ignored project-local directory, such as .cache/worktrees/. Never delete a worktree merely because the workflow finished. First verify that it is clean, inspect ignored/untracked contents for material to preserve, and verify the project's handoff:

• Direct merge: confirm the candidate commit is contained in the intended target branch. For squash/rebase integration, verify the resulting content and the project's recorded mapping.
• PR: creating a PR is not proof of merge. Verify the durable source branch/candidate and project-selected completion condition; retain the branch while the PR still needs it.
• Bundle: verify the bundle with Git, its advertised candidate and prerequisites, and its durable destination. If delivery is required, verify receipt too; a local bundle is not proof it arrived. A bundle depending on this soon-to-be-deleted checkout is not a durable handoff.
• Patch: verify reconstruction from the declared base yields the intended candidate tree, including binary/mode/deletion changes where relevant. Verify the durable patch destination and any required delivery receipt. A patch left only inside the removed worktree is not saved.

The verification is its own tool call, whose successful result the root reads before issuing any removal in a separate call. Never chain checks and deletion with &&, use forced removal, or treat a failed/unknown check as success. Worktree removal does not authorize deleting its branch or handoff artifacts. If evidence or preservation is incomplete, retain the worktree and report what remains. Respect the project's deletion authorization in addition to these checks.

The QUALITY GATE — three different things, and only two of them BLOCK

A gate is not a review seat, and the two words are not interchangeable. Say which of the three a given check is, because only two of them stop the run:

• BLOCKING — committed TOOLS invoked as gate steps. The repo's own check scripts (tests, lint, format), plus scans of the same objective kind: a banned-vocabulary scanner, an incoming conflict-marker sweep. These are exit-code gates — they pass or they fail and nobody adjudicates the result.
• BLOCKING — SCRIPT-LEVEL contract checks. The orchestrator SCRIPT throws on a protocol violation: the fail-fast retry helper (law 4) and the deliverable-proof check below. These stop the run deliberately, and the decision lives in the script — never delegated to a downstream agent to rediscover, for the same reason the structural abort does not (law 10).
• RECORDING — SEATS. Quality and cleanliness emit findings for independent verification, not directly into a fix queue. Their judgments are claims, not exit-code gates. A missing required report or a verified unresolved decision still prevents the next stage.

The boundary is the whole taxonomy in one line: MECHANICAL AND OBJECTIVE goes in the GATE as a TOOL; JUDGMENT goes in the REVIEW as a SEAT. A gate that only reports is a seat wearing the wrong name, and a seat that stops the run is a gate — either way the run's exit reason is a lie about which mechanism decided it.

THE COMPLETION-CLAIM RULE: a fixer's completion claim is only valid off a BARE RERUN AFTER ITS LAST WRITE, with the tails quoted VERBATIM. A claim resting on a run from before the last edit is not evidence — the edit it is offered as proof of is precisely what that run never saw. And piping a check through head or grep is itself an offense rather than a style question, because it hides the failure the gate exists to surface.

GATE TOOLS ARE VERSIONED AND MATERIALIZED. A gate tool lives in a REPOSITORY and is materialized into every tree the gate runs in (a link or copy placed at tree creation). A tool kept as a loose file at one workspace root fails not-found in every OTHER tree, and every run then hand-substitutes it — a failure that is silent in the worst way, because it presents as a broken gate rather than as a missing tool, so each run debugs the gate instead of installing the tool.

GATES EXECUTE INSIDE THE FIX PHASE — the fixer runs them bare after its own last write. A failing required check returns a failed proof, never permission to invent an unapproved fix. Do not place checks after the completed workflow and still claim its proof covered them. Likewise, do not ask reviewers to judge a criterion a later stage has not yet produced.

THE RECORDING SEATS RIDE AS TEMPLATE CONSTANTS, not as per-script prose. Anything retyped per run erodes — audits find the standing quality and cleanliness lenses silently absent from the large majority of a fleet's scripts, each omission individually reasonable when it was made. A constant resists that; retyping does not. This is in genuine tension with law 5(a) — editing a shared constant busts every cache key — and the two coexist by a rule about WHEN, not whether: the constant is authored once and then left alone. When a resume needs one seat re-run, bump that seat's OWN prompt, never the constant (law 5a stands unchanged). Erosion is the larger cost, because a busted cache costs one run while a dropped seat costs every run after it.

Why this shape (the rationale that makes it work)

• Sequential implement, parallel review. Implementation has write-conflicts; review is read-only and independent — so the parallelism goes in the review phase, not the build.
• Reviewers split by concern, not by file. Different lenses find different classes of problem; pointing them all at "review everything" wastes them on overlap.
• Adversarial correctness review is the point. Brief the correctness reviewer to try to break the change — name the hazards and ask "is this actually wrong?". That's what catches the plausible-but-broken implementation that tests written by the implementer won't.
• Verification precedes mutation. One read-only verifier checks and consolidates every source; the separate fixer rechecks approved corrections and returns disagreements to the root. Fresh review and verification attest the changed tree rather than trusting its author's claim.
• The biggest wall-clock win is killing redundant stages, not parallelizing bad ones.

Laws

Non-negotiable across every run of this skill.

1. EXPLICIT model AND effort on every stage — never inherited. Two silent-downgrade paths: a custom agentType resolving its own default, and a cached resume. Either can quietly land a stage on the cheapest tier while the run looks healthy.
2. Cross-family review. Prefer a reviewer from a DIFFERENT model family/vendor than the implementer. A same-family reviewer shares the author's blind spots and will nod at exactly the assumption you needed challenged. (Vendor-neutral rule; pick per the project's own model policy.)
3. Effort policy. High for implementation, fixing, and code review. Low/medium for mechanical or repetitive stages (list-checking, formatting sweeps). Reserve the top efforts for genuinely hard reasoning — they overthink routine work and cost wall-clock for nothing.
4. FAIL-FAST. An agent returning null or a sub-minimal result retries the SAME agent (3 attempts total), then the helper throws and no downstream stage runs. The main loop records the incomplete run with any pending unverified writes; it never treats a failure as an empty review. Set the floor below the shortest legitimate result for each seat. Quality may return a short no-findings report; concern reviewers still owe their per-criterion verdict block. The floor is a PROSE test and it cannot prove an artifact, so a stage whose deliverable is FILES is ACCEPTED ON a deliverable-proof marker — a long narration clears any floor with nothing on disk. The floor still rides underneath that marker, because it catches a different failure and costs nothing; it is simply not what decides acceptance there (see the acceptance section below). The law guards EVERY required reader, including adversaries: the verifier consumes them all. A missing report is incomplete verification, never a harmless gap in a finished fix.
5. Cache-busting on resume. A resume replays a cached result for an identical (prompt, opts), so retrying a POISONED stage verbatim just replays the same bad output. Edit that ONE stage's prompt or label to bust its key, and leave every good stage's key untouched so it replays free. Two corollaries. (a) Shared prompt text is a GLOBAL cache-buster: editing a shared authority block on resume busts every key that embeds it, so every already-settled seat re-runs. Bump ONLY the seats that must re-run, via a short revision marker prepended to their individual prompts — never by touching the shared constant. (b) A poisoned result is itself CACHED, including a hard-flagged one: fixing the underlying cause and resuming returns the same bad result unless that seat's prompt is bumped. The fix is always a prompt edit, never a re-invoke.
6. Barrier discipline. Review readers run concurrently on a stable clean snapshot, then Verify consolidates their results. Fix awaits that approval. Only the Git-object-only roaster overlaps the fixer, reading the captured pre-fix SHA and approved list. Await both tasks; process the roast on the resulting snapshot before completion. These are data dependencies.
7. Premise drift — read the authority, not a relayed gloss. Point authority-aware stages at the private, ignored/untracked directive record and the current spec path (law 9). Preserve exact source wording in that private record, never in commit-bound artifacts without explicit permission. Technical specs record decisions and constraints, not conversational appendices.
8. AUTHORITY ARCHITECTURE — state the hierarchy in authority-aware prompts. Quality and cold spec reviewers receive only their hygiene/diff inputs; cold alternatives gets invariants, not the shared authority briefing. For other seats the three tiers are: owner/human verbatim directives > the spec > this prompt, with the prompt explicitly labelled UNTRUSTED relative to both, and "a prompt-vs-spec conflict is itself a must-fix finding". The AUTHORITY DOCUMENTS are the top two tiers only — the directives and the spec. The prompt is not one, which is what makes a prompt-vs-spec conflict an ordinary finding rather than the hard flag of law 10 — but the human veto still reaches the prompt. A prompt that directly contradicts a directive is the same hard-flag class as a spec that does: being untrusted RELATIVE TO THE SPEC does not exempt the prompt from the directive ranked above both. Anything the orchestrator adds beyond the spec is labelled "ORCHESTRATOR SCOPING — this added scope loses to the spec on conflict", which makes it structurally attackable by every seat; the spec itself never outranks a directive, including a spec the orchestrator amended. This exists because the orchestrator's own errors are the dominant error class — a mis-stated criterion, a gloss that contradicts another gloss of the same ruling, a "verbatim" appendix that isn't, or an assignment overriding a directive it disagrees with — and this hierarchy is the only mechanism in the run that catches them. A specification gains no decision authority merely by being written, even when the orchestrator wrote it: it can be corrected to state an existing directive faithfully, never used to authorize a new one. Untrusted means VERIFIED, not ignored: every factual claim the prompt makes about the tree is checked against the tree, and a FALSE one is verified-and-reported — build to the true state, flag the premise as a must-fix — which beats both trusting it and stopping on it (law 10).
9. SPECS ARE LIVING DOCUMENTS, READ FROM DISK. Every spec-consuming prompt names it by PATH and instructs: "read the current on-disk revision in full; it is the authority, not this prompt's description of it." Never cite a revision number, never restate the spec's content in the prompt. This is what prevents drift between a prompt's stale summary and the doc. Do not change authority during a review/fix cycle; return for the authority edit and invalidate affected review/approval calls before continuing. An amendment cannot make a cached approval current. Corollary: authority documents RETRACT a contradicted sentence in place. Never append an acknowledgement beside a sentence it contradicts: layered addenda manufacture diverging premises, and seats then flag the contradiction forever, correctly.
10. HARD-FLAG SEMANTICS. A hard flag (agent stops, script aborts) is reserved for a contradiction that puts a human verbatim directive on at least one side — directive-vs-spec, or directive-vs-this-prompt — two texts that cannot both be true (law 8). The prompt being UNTRUSTED relative to the spec does not exempt it from the directive ranked above both: an assignment overriding a directive is the same conflict class as a spec that does, hard-flagged the same way. A tree that does not yet satisfy a coherent spec is the NORMAL precondition of review-and-fix and yields ordinary findings; so does an untrusted prompt that merely conflicts with the SPEC with no directive on either side, or one asserting a false premise about the tree — those are verified-and-reported, built to the truth (law 8), never an abort. Getting this wrong deadlocks the run: the fixer that would resolve the finding can never run, because the flag aborts before it. One trigger, one marker, one disposition — a second abort class with no marker of its own is undetectable, and a single event with three dispositions is the deadlock in another costume. And the structural abort lives in the SCRIPT, which checks every consumed stage result for the marker and returns — never delegated to a downstream agent to rediscover. Every required report is consumed by verification; a failed or hard-flagged reader stops the cycle before fixing.
11. ENUM-LOCK ANY VOCABULARY THE SCRIPT BRANCHES ON. If control flow keys off severity, lock it in the output schema as an enum (must-fix / should-fix / nit) with validation-retry — and the same for every other vocabulary the loop switches on: the actionability lane (fixer-actionable / orchestrator-only / later-phase / not-a-defect) and the disposition (fixed / rejected / blocked), verifier action (approve-fix / reject / needs-decision / root-action / cleanup / record) and closure (closed / unresolved). A seat emitting one word against a check testing for another silently disables the phase and the run reports success — the worst possible failure mode, because it looks like a green run.
12. GROUNDED MEANS OBSERVED. Code-reading that concludes "it should work" loses to empirical observation every time. Verify against real output: real builds, real requests, real rendered results. Mechanical gates RECOMPUTE from the artifacts; an item's self-report is only a truncation-and-dishonesty detector, never evidence.
13. END-OF-RUN COMPLETENESS PASS. Per-item checks structurally CANNOT see a missing item. Every fan-out over a work-list ends with one pass whose only question is "which item is missing entirely?". Absences are the worst defect class to ship, and they are invisible to exactly the checks that look most thorough.
14. HARNESS TOOLS BEAT PER-AGENT IMPROVISATION. When several seats each hand-roll the same invocation (gate runs, server boots, probe walks), commit a one-command tool and put the exact invocation in every prompt with hand-rolling forbidden. Measured effect: seat turn-counts roughly halved. Extra rule for models without prompt caching, which re-pay their full input every turn: point them at tool DUMPS and keep their exploration short-context, since long ad-hoc exploration is disproportionately expensive exactly there.
15. IMPLEMENT THE SPEC AS WRITTEN; only the ORCHESTRATOR edits it. A suggested spec change does not block implementation, fixing or the normal reviewer cycle. Report the suggestion and its evidence to the root without changing the spec or making its amendment a prerequisite. Non-blocking suggestions belong in report material, or record when dispositioning a supplied finding. A preference for different requirements is not an impossibility. If the assigned work genuinely cannot satisfy the applicable requirements, report the concrete impossibility and block rather than inventing requirements or claiming completion. Contradictions between authority documents retain the existing law-10 hard flag; the spec-versus-instructions pre-check already exists and does not need another gate. Reviewers retain their usual checks. ONLY THE ORCHESTRATOR MAY EDIT A SPEC OR OTHER AUTHORITY DOCUMENT. If the root amends one, record the technical rationale, retract contradicted text in place (law 9), and invalidate affected cached reviews and approvals. Never retroactively authorize unsupported implementation. Every inverse-spec finding is CRITICAL regardless of the severity or lane it arrived with; the finding verifier, the fixer and the root all ignore that supplied categorization and must dispose of it explicitly — never leave it implicitly closed. The root resolves it by correcting the spec to state an existing human decision fa

Truncated - read the full file at https://github.com/xdevs23/workflow-skills/blob/1a8b4db13cb81ba5f8c94e1e52bf1e2c1cc85782/skills/implement-review-verify/SKILL.md.