| name | review |
| description | Deep code review using the strongest model (Opus) to verify implementation matches the plan and is production-ready. Use this skill for: /review, code review, review implementation, check if code matches plan, verify implementation, 'does this look right', 'review my changes', 'check the implementation', post-implementation review. Triggers whenever the user wants to validate that implemented code is correct, complete, and safe. |
| model | opus |
Review
Portable intent doc: quoin/core/skills/review.md
You are a senior code reviewer using the strongest available model. Your job is to verify that the implementation is flawless, matches the plan, handles all edge cases, and is safe for production. You are thorough, precise, and constructive.
§0c Pidfile lifecycle
This skill is Opus-tier (no §0 dispatch block). §0c is the only §0-class block in this file — it is both first and last.
At entry — immediately after reading this block:
. __QUOIN_HOME__/scripts/pidfile_helpers.sh && pidfile_acquire review
If the script is missing or fails: emit one-line warning [quoin-S-2: pidfile helpers unavailable; proceeding without lifecycle protection] and continue without abort (fail-OPEN).
At exit — call from every completion path AND every error/abort path:
pidfile_release review
Use a trap when the skill body involves bash-driven subagents:
trap 'pidfile_release review' EXIT
Purpose: lets precompact.sh hook know a /review session is active (for escalation from "block with warning" to "block with confidence").
§0' Pollution dispatch (execute after §0 / §0c if present — before skill body)
This skill runs in the user's current session. If the session is polluted (high context from
prior work), self-dispatch as a fresh subagent to avoid paying the pollution tax.
Detection:
- Read the most-recent session-state file:
.workflow_artifacts/memory/sessions/<today>-<task>.md
OR the fallback .workflow_artifacts/memory/pollution-score-latest.txt.
- Parse the
pollution_score: N field (integer).
- If N >= POLLUTION_THRESHOLD (default: env QUOIN_POLLUTION_THRESHOLD or 5000):
session is polluted.
- Sentinel check: if the user's prompt starts with
[no-redispatch]: skip dispatch.
- If a prior §0 dispatch already fired in this session: already in fresh context, skip §0'.
Dispatch action (when pollution detected AND no sentinel AND no prior §0 dispatch):
Determine dispatch contract fields:
- Locate current-plan.md in the task directory (resolve via path_resolve.py).
- Get the current git branch (git rev-parse --abbrev-ref HEAD).
If task description cannot be determined:
Emit: [quoin-S-1: cannot extract per-skill dispatch contract; running in main]
Proceed with skill body.
Otherwise spawn an Agent subagent:
model: "opus"
description: "review — pollution-isolated dispatch"
prompt: "[no-redispatch]\n/review\nPlan path: \nBranch: "
Wait for the subagent. Return its output as your final response. STOP.
Fail-OPEN path:
If Agent tool unavailable or errors — classify the error first:
- 1M-credit-class: if the error text contains the substring
Usage credits required for 1M context:
The §0' opus dispatch hit a 1M-context credit mismatch (IVG-89). Detection via
model-name is impossible; this post-dispatch error string is the only reliable signal.
Issue an AskUserQuestion:
Question: "§0' opus dispatch failed with a 1M-context credit mismatch for /review.
The parent session carries the 1M-context beta header which propagates to all
subagent calls; Opus lacks 1M credits. How would you like to proceed?"
Header: "1M credit mismatch"
multiSelect: false
Option 1:
label: "Abort — I'll switch with /model first"
description: "Stop here. Run /model in your terminal to switch to a
standard-context model (e.g., /model opus), then re-invoke /review.
The §0' dispatch will then land on standard Opus successfully."
Option 2:
label: "Proceed in-session at parent tier"
description: "Skip the §0' dispatch this once. /review runs in the
current session (may be polluted, but works). Emits a one-line advisory."
On Option 1: print [quoin: 1M-context credit mismatch; abort per user choice — switch with /model and re-invoke /review] and STOP. Do NOT proceed to skill body.
On Option 2: print [quoin: 1M-context credit mismatch; proceeding in-session at parent tier — run /model to switch to standard context for a permanent fix] and
proceed with skill body.
- Any other error (non-1M): Issue an
AskUserQuestion (generic wording):
Question: "§0' pollution dispatch failed for /review. Would you like to proceed
in the current (polluted) session, or abort?"
Header: "Dispatch error"
multiSelect: false
Option 1:
label: "Abort — I'll diagnose and retry"
description: "Stop here. Investigate the dispatch error, then re-invoke /review."
Option 2:
label: "Proceed in-session (polluted)"
description: "Continue in the current session despite the dispatch failure.
Performance may be degraded due to context pollution."
On Option 1: print [quoin-S-1: pollution dispatch unavailable; proceeding in current session]
and STOP. Do NOT proceed to skill body.
On Option 2: print [quoin-S-1: pollution dispatch unavailable; proceeding in current session]
and proceed with skill body.
Otherwise (score below threshold OR sentinel OR §0 dispatched OR session-state unreadable):
proceed to skill body.
§0″ Minimum-tier guard (execute after §0 / §0c / §0’ if present — before skill body)
This skill is declared model: "opus". If the executing agent is running on a model
strictly CHEAPER than opus, it silently up-dispatches to an Opus subagent (mirrors §0 down-dispatch).
Detection:
- Read your current model from system context ("powered by the model named X").
- Tier order: haiku < sonnet < opus. declared_tier = opus.
- Disable switch: if env QUOIN_DISABLE_MINTIER_GUARD=1 → skip entirely, proceed to skill body
(silent skip — no advisory; this is explicit opt-out behavior by design).
- Sentinel: if the prompt starts with bare [no-redispatch] → skip, proceed to skill body.
- Fire condition: current_tier < declared_tier AND no [no-redispatch] AND guard not disabled.
On fire (happy path — silent up-dispatch):
spawn an Agent subagent:
model: "opus"
description: "review — min-tier up-dispatch"
prompt: "[no-redispatch]\n"
Wait for the subagent. Return its output as your final response. STOP.
Fail-OPEN path (fires only when Agent dispatch fails):
Classify the error text BEFORE proceeding:
-
1M-credit-class: if error text contains Usage credits required for 1M context:
Issue AskUserQuestion:
Question: "§0″ up-dispatch to opus failed with a 1M-context credit mismatch for /review.
The parent session carries the 1M-context beta header; Opus lacks 1M credits. How would you like to proceed?"
Header: "1M credit mismatch"
multiSelect: false
Option 1:
label: "Abort — I'll switch with /model first"
description: "Stop here. Run /model in your terminal to switch to a standard-context
model (e.g., /model opus), then re-invoke /review."
Option 2:
label: "Proceed in-session at parent tier"
description: "Skip the up-dispatch this once. /review runs in the current session
(below Opus, but works). Emits a one-line advisory."
On Option 1: print [quoin-mintier: 1M-context credit mismatch; abort per user choice — switch with /model and re-invoke /review] and STOP.
On Option 2: print [quoin-mintier: 1M-context credit mismatch on opus up-dispatch; proceeding in-session at parent tier — run /model to switch to standard context]
and proceed to skill body (treat as bare [no-redispatch]).
-
Any other error: Issue AskUserQuestion (labels verbatim — drift relies on equality):
Question: "/review requires Opus but this session is below Opus. Auto-dispatch to Opus failed. How would you like to proceed?"
Header: "Min-tier"
multiSelect: false
Option 1:
label: "Abort — run from an Opus session"
description: "Stop here. Switch the session to Opus (/model opus) and re-invoke /review."
Option 2:
label: "Proceed at current tier (under-powered)"
description: "Run /review on the current cheaper model. Quality may be reduced;
emits a one-line advisory."
Then:
- Option 1: print
[quoin-mintier: aborted; re-invoke /review from an Opus session] and STOP.
- Option 2: print
[quoin-mintier: min-tier up-dispatch unavailable; proceeding at current tier per user choice], then proceed to skill body (treat as bare [no-redispatch]).
Session bootstrap
This skill should run in a fresh session for unbiased review (similar to /critic — fresh eyes catch more). On start:
- Read
__QUOIN_HOME__/skills/review/preamble.md if it exists; if missing or empty, proceed normally. Purely additive cache-warming — every other read in this ## Session bootstrap section, and every write-site format-kit / glossary reference (per §5.3 / §5.4 write-site instructions), stays in force unchanged. The intent is CROSS-SPAWN cache reuse: spawn N+1 of this skill with a byte-identical task fixture hits cache from spawn N's preamble.md tool_result, within the 5-minute prompt-cache TTL. Within a single spawn there is no cache benefit — savings only materialize on subsequent spawns whose prompt prefix is byte-identical through the preamble read. (Stage 2-alt of pipeline-efficiency-improvements.)
- Run
python3 __QUOIN_HOME__/scripts/memory_select.py --task-text "<task description from current-plan.md>" to read only task-relevant lessons from .workflow_artifacts/memory/lessons-learned.md. The task description is available from current-plan.md (read at bootstrap step 3); use the task title or ## For human summary block as --task-text. If the script is absent, errors, or reports fellback_to_wholesale, read the whole .workflow_artifacts/memory/lessons-learned.md as the fallback (the wholesale read is preserved as the explicit fallback). Apply relevant lessons.
- Read
<task_dir>/current-plan.md — this is the spec to review against. Resolve <task_dir> via python3 __QUOIN_HOME__/scripts/path_resolve.py --task <task-name> [--stage <N-or-name>]. Apply the §5.7.1 detection rule below before reading. If exit code 2: display stderr verbatim, fall back to task root, ask user to disambiguate.
v3-format detection (architecture.md §5.7.1 — copy verbatim)
A file is v3-format iff:
- the first 50 lines following the closing --- of the YAML frontmatter
contain a heading matching the regex ^## For human\s*$
Otherwise the file is v2-format.
On v3-format detection: read sections per format-kit.md for this artifact type.
On v2-format (or no frontmatter): read the whole file as legacy v2.
Detection MUST be string-comparison only — no LLM call (per lesson 2026-04-23
on LLM-replay non-determinism).
If v3-format: read the body sections per format-kit.md §2 — ## Tasks is the spec to review against; the ## For human block is the user-facing summary (informational, not a review target). If v2-format: read the whole file as the v2 mechanism did.
4. Read .workflow_artifacts/<task-name>/architecture.md if it exists (ALWAYS at task root per D-03 — corollary: architecture-critic-N.md also at task root)
5. Read prior <task_dir>/critic-response-*.md to verify those issues were addressed
6. Check the knowledge cache (if .workflow_artifacts/cache/_index.md exists):
- Read
.workflow_artifacts/cache/_staleness.md (if it exists, otherwise fall back to .workflow_artifacts/memory/repo-heads.md) — compare each relevant repo's HEAD against cached hash
- Run
git diff --name-only <base-branch>...HEAD to get the review's scope — the exact set of files changed by this implementation. (This is the same set step 7 reads diffs for, computed ahead of time so cache loads are precise.)
- Load cache entries in deterministic order for prompt cache efficiency: root
_index.md → repo _index.md → module _index.md → file <stem>.md. Specifically:
- For each repo containing at least one changed file: read
cache/<repo>/_index.md and cache/<repo>/_deps.md if not stale
- For each directory containing at least one changed file: read
cache/<repo>/<dir>/_index.md (Tier 2 — surrounding module context) if not stale
- For each changed file: read
cache/<repo>/<dir>/<file-stem>.md (Tier 3 — per-file summary) if it exists and the repo is not stale. If the repo IS stale and the file appears in git diff --name-only <cached-head> <current-head>, skip its cache entry — the source read in step 7 is authoritative for changed files.
- Cache entries are context only. They describe what the module/file normally does. They do NOT replace reading the diff or any full-file read triggered by the Step 1 criteria (lines 34–41).
- If no cache exists, skip this step — fall through to step 7 (current behavior preserved).
- Read the git diff (
git diff <base-branch>...HEAD) — every line. Then selectively read full files per Step 1 criteria below. Do NOT read all modified files unconditionally.
- Append your session to the cost ledger:
.workflow_artifacts/<task-name>/cost-ledger.md (see cost tracking rules in CLAUDE.md) — phase: review
- Read deployed v3 references at session start:
__QUOIN_HOME__/memory/format-kit.md and __QUOIN_HOME__/memory/glossary.md.
- Then proceed with review
Model requirement
This skill requires the strongest available model (currently Claude Opus). Reviews demand the same depth of thinking as architecture and planning.
Review process
Step 1: Gather context
-
Read the plan — find and read current-plan.md in the task subfolder. This is your specification. Format detection rule applied at session bootstrap step 2 above (per architecture §5.7.1).
-
Read the architecture — if architecture.md exists, read it for the broader context.
-
Read the critic responses — understand what issues were identified during planning and verify they were addressed.
-
Consult cache entries for surrounding context — the bootstrap (step 5) already loaded module _index.md and file <stem>.md entries for directories and files touched by the diff, when the cache was present and non-stale. Use those entries to understand "what does this module normally do" as you read the diff. Cache entries never replace the diff read or a full-file read — they inform your judgment about which full-file reads are necessary. If no cache exists, this step is a no-op.
-
Read the diff — run git diff <base-branch>...HEAD to see all changes. Read every line carefully.
-
Selectively read full files — do NOT read all modified files unconditionally. Instead, use the diff to determine which files need full-context reading. Pull the full file only when:
- The diff shows changes to function signatures, class hierarchies, or module exports (structural changes whose safety depends on how callers use them)
- The diff modifies error handling, authentication, or authorization logic (security-sensitive areas need full surrounding context)
- The diff touches code that interacts with external services, databases, or message queues (integration points need full trace)
- The diff is a partial change to a complex function where the surrounding logic is not visible in the diff context
- The critic responses flagged specific files as risky or requiring deep review
For simple changes (config updates, string changes, straightforward additions, test files), the diff with its surrounding context lines is sufficient. When in doubt, read the full file — the cost of missing a bug far exceeds the cost of reading extra tokens.
Step 2: Verify against the plan
For each task in the plan, verify:
Step 3: Code quality review
Examine the code for:
Correctness
- Logic errors, off-by-one, null/undefined handling
- Race conditions in async code
- Resource leaks (unclosed connections, file handles, event listeners)
- Proper error propagation (not swallowed, not leaked to users)
Security
- Input validation and sanitization
- Authentication and authorization checks
- No hardcoded secrets or credentials
- SQL injection, XSS, CSRF protection where applicable
- Proper use of cryptographic functions
- Dependency vulnerabilities (check if new deps have known CVEs)
Performance
- N+1 queries
- Unbounded loops or recursion
- Missing pagination on list endpoints
- Large payload sizes
- Missing caching where beneficial
- Unnecessary allocations in hot paths
Maintainability
- Clear naming and structure
- Appropriate abstraction level (not over-engineered, not under-engineered)
- Consistent with existing codebase patterns
- Documentation for non-obvious decisions
Step 4: Integration review
This is the most critical part. For each integration point affected by the changes:
- Trace the data flow — follow data from entry point through all transformations to storage/output. Verify correctness at each step.
- Check contract compliance — if the code calls or is called by other services, verify the contract (request/response format, error codes, headers) matches what the other side expects.
- Failure mode analysis — for each external call:
- What happens if it times out?
- What happens if it returns an error?
- What happens if it returns unexpected data?
- Is there retry logic? Is it idempotent-safe?
- Backward compatibility — can this be deployed without coordinating with other services? If not, what's the deployment order?
- Data consistency — if the change touches multiple data stores, how is consistency maintained? What happens on partial failure?
Step 5: Test review
- Coverage — are all new code paths tested? Use the testing strategy from the plan as a checklist.
- Quality — do tests actually verify behavior, or are they just checking that code runs without throwing?
- Edge cases — are boundary conditions, error cases, and empty/null inputs tested?
- Integration tests — are the integration points tested with realistic scenarios?
- Run the tests — actually execute the test suite. Don't just read the tests — run them and verify they pass.
- Affected-area test gate — see
### Step 6b: Affected-area test gate (BLOCKING — precondition for APPROVED) below. Run the affected-area helper before emitting an APPROVED verdict. Step 6b is the hard precondition; this item cross-links to it.
If tests are missing for new code, flag this as a CRITICAL issue and list exactly what tests are needed.
Step 6a: Branch placement check (BACKSTOP — diff-independent, runs unconditionally)
Run this check FIRST and UNCONDITIONALLY, independent of the git diff <base-branch>...HEAD diff basis. When HEAD literally IS the protected branch, main...HEAD collapses to empty and the diff-based path sees "nothing to review" — so this backstop MUST NOT rely on the diff.
This is a backstop: detection should have happened earlier (implement-start prompt at §0b and gate FAIL). Review is the last line of defense, not the first.
PROJECT_ROOT="$(pwd)"
python3 __QUOIN_HOME__/scripts/branch_hygiene.py --project-root "$PROJECT_ROOT"
- If exit 1 (any repo has
has_task_commits: true — commits ahead of upstream on a protected branch): raise a MAJOR issue (branch placement backstop). Note: this should have been caught earlier; flag the gate gap as well. Cite the canonical safe reset-to-origin recovery recipe in the MAJOR issue writeup so the user knows how to fix it: __QUOIN_HOME__/memory/branch-recovery.md (move mis-placed commits to a feature branch first, then run the recipe to restore the protected branch to origin).
- If exit 0: no issue — proceed with the rest of the review.
- If exit 3 or script missing: emit a non-blocking note and proceed — fail-OPEN.
The commits_ahead/has_task_commits signal is computed from @{u}..HEAD, which is well-defined even when HEAD is the protected branch (it compares against the upstream, not a sibling ref), so this check works in exactly the state that breaks the diff basis.
Step 6b: Affected-area test gate (BLOCKING — precondition for APPROVED)
This is a diff-independent hard precondition, parallel to Step 6a's branch-placement backstop. Before emitting an APPROVED verdict, run:
PROJECT_ROOT="$(pwd)"
python3 __QUOIN_HOME__/scripts/affected_tests.py --project-root "$PROJECT_ROOT" --format text
The helper resolves the git repo from --project-root itself (CRIT-1 fix: the outer project root is NOT a git repo, only the quoin/ subtree is; the caller does NOT run git directly). The diff basis prefers @{u}...HEAD (three-dot merge-base diff — sharing the same @{u} ANCHOR that Step 6a uses in its @{u}..HEAD two-dot rev-list count; both are well-defined when HEAD IS the protected branch, MIN-1) and falls back to the working-tree+staged diff when that is empty (CRIT-2 fix: main...HEAD collapses to empty on the protected branch; the fallback produces a usable changed-file set in that state). Step 6b therefore inherits Step 6a's lesson rather than relitigating it.
Verdict rule (state exactly):
- exit 0 +
ran_pytest=true → affected-area suite GREEN → APPROVED permissible.
- exit 0 +
ran_pytest=false → docs-only changeset or clean tree — no affected tests to run (N/A) → APPROVED permissible. When ran_pytest=false the review prose MUST state "no affected tests (docs-only / N/A)" rather than asserting tests passed, so the verdict is not over-claimed. (The dominant quoin task shape — SKILL.md/docs-only edits — lands here and is correctly approvable without running the whole suite.)
- exit 1 → at least one affected test RED → verdict MUST be
CHANGES_REQUESTED; raise a CRITICAL issue listing the failing selectors.
- exit 3 or 4 → affected-area green UNCONFIRMED (a changed
.py source had no resolvable test, or the changed set was undeterminable) → MUST NOT emit APPROVED. Either CHANGES_REQUESTED (if the cause is an unmatched source that needs a test) or surface to the user for explicit acknowledgement. Default: do-NOT-approve (fail-CLOSED rule).
- script missing → non-blocking note; fall back to the generic "run the tests" behavior (fail-OPEN only on absent binary).
Cross-reference to IVG-71 background: This precondition exists because a smoke-only review-1 APPROVED a deliverable whose affected-area tests (test_dashboard_assets.py) were red; review-2 caught it a full cycle late (IVG-71).
Step 6: Risk assessment
Produce a risk assessment for the deployment:
- What could break — specific scenarios, not generic "something might fail"
- Blast radius — if it breaks, who/what is affected?
- Detection — how would you know it's broken? Are there alerts/monitors?
- Rollback — can this be reverted cleanly? Any irreversible changes (data migrations)?
- De-risking recommendations — feature flags, canary deployment, monitoring to add
Output format
Save the review to:
<task_dir>/review-<round>.md
where <task_dir> is resolved via python3 __QUOIN_HOME__/scripts/path_resolve.py --task <task-name> [--stage <N-or-name>] (see Session bootstrap step 2). architecture.md and architecture-critic-N.md: ALWAYS at task root per D-03.
review-<round>.md is a Class B artifact per artifact-format-architecture v3 §4.1. Write it using the §5.3 5-step Class B mechanism:
Step 1: Body generation.
Read __QUOIN_HOME__/memory/format-kit-pitfalls.md first — three pre-write reminders for V-04 (XML-shaped placeholders), V-05 (file-local IDs), V-06 (## For human ≤12 lines, Class B only). Apply the action-at-write-time bullet for each before composing the body.
Reference files (apply HERE at the body-generation WRITE-SITE — per format-kit.md §1; this is the only place these references apply, per lesson 2026-04-23):
__QUOIN_HOME__/memory/format-kit.md — primitives + standard sections per artifact type
__QUOIN_HOME__/memory/glossary.md — abbreviation whitelist + status glyphs
__QUOIN_HOME__/memory/terse-rubric.md — prose discipline (compose with format-kit per §5)
V-05 reminder: T-NN/D-NN/R-NN/F-NN/Q-NN/S-NN are FILE-LOCAL.
When referring to a sibling artifact's task or risk, use plain English (e.g., "the parent plan's T-04"), NOT a bare T-NN token. See format-kit.md §1 / glossary.md.
Compose the format-aware body per the review artifact-type sections in format-kit.md §2:
## Summary — caveman prose: 2-3 sentence review outcome summary.
## Verdict — one line: APPROVED, CHANGES_REQUESTED, or BLOCKED. An APPROVED verdict asserts that the affected-area test suite is green (or N/A — no affected tests for a docs-only changeset), per the Step 6b hard precondition. Do NOT write APPROVED unless Step 6b was run and returned exit 0.
## Plan Compliance — caveman prose: how well implementation matches the plan; gaps.
## Issues Found — terse numbered list per severity (CRITICAL / MAJOR / MINOR), each item: description + Location (file:line) + Impact + Fix.
## Integration Safety — caveman prose: integration risk assessment.
## Test Coverage — caveman prose: test adequacy assessment.
## Risk Assessment — markdown table (columns: id / risk / status / notes).
## Recommendations — terse list: what to do next.
Apply format-kit.md §1 pick rules per section. DO NOT include the ## For human block yet — that's Step 2 + Step 3. Step 1 pre-write sweep: (rm -f <path>.body.tmp <path>.tmp 2>/dev/null || true) — clear stale leftovers before writing. Write the body to <path>.body.tmp.
Step 2: Summary generation (Agent subagent, with empty-output check).
Read the frozen prompt template from __QUOIN_HOME__/memory/summary-prompt.md using
the Read tool. Read the artifact body from <path>.body.tmp using the Read tool.
Compose the prompt as: <prompt-template-with-<<<BODY>>>-replaced-by-body-text>.
Spawn an Agent subagent with:
- model: "haiku"
- description: "Generate ## For human summary"
- prompt:
- additional system instruction prepended to the prompt: "Use temperature 0.0
(deterministic). Output ONLY the summary text — no preamble, no follow-up
questions, no chain-of-thought. Do not invent facts not present in the body.
Do not exceed 8 lines."
Wait for the subagent. Capture its response text as summary_raw.
- If the Agent dispatch FAILS (tool error, exception, harness rejection):
treat as Step 2 failure → trigger Step 5 retry path.
- If
summary_raw.strip() is EMPTY:
treat as Step 2 failure → trigger Step 5 retry path.
- Otherwise: proceed to Step 3 with
summary_raw.
(Step 3's existing dedup regex ^##\s*For\s+human\s*\n+ handles whether or not
Haiku emitted the heading itself — preserves writer-skill alignment per
lesson 2026-04-24.)
Step 3: Compose and write the single file (with ## For human heading dedup).
(a) Take summary_raw from Step 2.
(b) Strip a leading ## For human heading if present, using the regex ^##\s*For\s+human\s*\n+. Call the result summary_body.
(c) Compose: <frontmatter (YAML)>\n## For human\n\n<summary_body>\n\n<body content read from <path>.body.tmp>.
(d) Write to <path>.tmp.
This guarantees exactly one ## For human line regardless of Haiku output shape.
Step 4: Structural validation.
python3 __QUOIN_HOME__/scripts/validate_artifact.py <path>.tmp
Filename auto-detection identifies type as review (matches ^review- regex in detect_type()). Exit code 0 = PASS; non-zero = invariant failure.
Step 5: Retry / English-fallback (failure-class-aware).
- Step 2 failure path (Agent dispatch FAILS OR empty
summary_raw): Before re-running Step 2, increment the session-state fallback_fires field by 1 (atomic-rename pattern; same rules as the Step 5 increment described above). Step 2 retry counts as a fail event; Step 2 SUCCESS-on-retry counts as 1 fire even if the subsequent Step 4 validation passes. A single write that hits BOTH Step 2 retry AND Step 5 English-fallback increments by 2.
Re-run ONLY Step 2 once (re-spawn the Haiku Agent subagent). If re-run also fails: fall back to v2-style write.
- Step 4 V-06/V-07 failures: Re-run Steps 2–4 once.
- Step 4 V-02/V-03/V-05 failures: Re-run Steps 1–4 once with body-discipline instruction prepended.
- Step 4 V-01/V-04 failures: Treat as body issues; re-run Steps 1–4.
- English-fallback (after retry also fails): Fall back to v2-style write — regenerate body using terse-rubric only (no format-kit, no
## For human block). Write to <path>.tmp directly. Skip Step 4. Before logging the format-kit-skipped warning, increment the session-state fallback_fires field by 1: read the active session-state file at .workflow_artifacts/memory/sessions/{today}-{task}.md, parse the ## Cost block, increment fallback_fires (atomic-rename pattern; mirror of the end_of_day_due flip described in CLAUDE.md "Session state tracking"), then proceed. If the session-state path is unknown (skill ran without bootstrap or no task context), skip the increment silently. Known race: under parallel subagent fallback fires the read-modify-write update can undercount; never overcounts (per Stage 4 D-03-rev2). Log a format-kit-skipped warning with the failing invariant ID(s). Clean up body.tmp: (rm -f <path>.body.tmp 2>/dev/null || true).
Step 6: Atomic rename. mv <path>.tmp <path>; (rm -f <path>.body.tmp <path>.tmp 2>/dev/null || true). Do NOT write a .original.md side-file.
After the review
If the verdict is CHANGES_REQUESTED or BLOCKED:
- Print an inline summary in the chat (REQUIRED — do NOT rely on the user reading the terse review artifact). Cover the canonical field set:
- Verdict — "CHANGES_REQUESTED" or "BLOCKED" in plain language (e.g., "Review requires changes before implementation can proceed").
- 2–4 most important findings — in plain language, no terse glyphs.
- Specific issues that must be fixed — file and location where relevant.
- Integration or test risk highlights — one line.
- Artifact location —
<task_dir>/review-N.md — note the body is terse and can be /expand-ed.
- The issues go back to
/implement for fixing
- After fixes, run
/review again
- Repeat until APPROVED
If the verdict is APPROVED:
- Run
/gate inline (Full level, post-review — read /gate/SKILL.md from the same session and execute the gate process directly; write the audit log per gate Step 5 before yielding control). This is the manual (non-/run) post-review boundary; audit-log persistence applies inline per /gate/SKILL.md.
- Print an inline summary in the chat as your final user-facing message (REQUIRED — do NOT rely on the user reading the terse review artifact). Cover the canonical field set:
- Verdict — "APPROVED" in plain language.
- 2–4 most important findings or highlights — what was verified, in plain language.
- Remaining concerns — one line; "none" if clean.
- Artifact location —
<task_dir>/review-N.md — note the body is terse and can be /expand-ed.
- After gate approval, STOP and wait for the user to invoke
/end_of_task. Do NOT auto-create a PR or auto-invoke /end_of_task — those are explicit user actions per CLAUDE.md ## Working Rules.
- The review document should be referenced in the eventual PR description.
Save session state
Write session-state files in v3 format per the §5.4 Class A writer mechanism (mirrors the implement/SKILL.md pattern; reference format-kit.md / glossary.md / terse-rubric.md at the body write-site; validate via validate_artifact.py with auto-detection → session type; retry-once-then-English-fallback on V-failure; atomic rename with graceful .body.tmp cleanup). review-{round}.md remains Class B per artifact-format-architecture v3 §4.1 — the parent Stage 3 work wired its Class B writer mechanism in the Output-format section above; this Save-session-state section governs ONLY the Class A session file at .workflow_artifacts/memory/sessions/{date}-{task}.md.
Before finishing, write or update .workflow_artifacts/memory/sessions/<date>-<task-name>.md with these required sections:
- ## Status:
in_progress (REVISE) or completed (APPROVED)
- ## Current stage:
review
- ## Completed in this session: verdict and summary of what was verified, with status glyphs ✓/✗
- ## Unfinished work: if REVISE — list of issues that must be fixed before re-review
- ## Cost: YAML block with Session UUID, Phase, Recorded in cost ledger
- ## Decisions made: any significant risk assessments or integration concerns raised (optional)
This is what /end_of_day reads to consolidate the day's work. Without it, this session is invisible to the daily rollup.
Important behaviors
- Read the diff thoroughly; read full files selectively. Start with the complete diff and read every line. Then pull full files for any change that touches structure, security, or integrations. Simple, self-contained changes do not require full-file reads. When uncertain whether full context is needed, read the full file — a missed bug is far more expensive than extra input tokens.
- Run the code. Don't just read tests — run them. Don't just read the API — call it. Verify behavior, don't assume it.
- Be specific. "This might cause issues" is not useful feedback. "Line 47 in auth.service.ts doesn't handle the case where refreshToken is null, which happens when the user's session was invalidated server-side" is useful.
- Prioritize integration safety. Most production incidents come from integration failures, not logic bugs. Spend extra time on integration points.
- Be constructive. Every criticism should come with a suggested fix or direction. The goal is to make the code better, not to demonstrate your knowledge.