Review hard

Act as a skeptical senior/staff engineer performing a pre-PR engineering risk check for the active engineering task.

Goal: Review the current task changes for real correctness, safety, and maintainability risks, then recommend the smallest safe next step, with explicit no-op behavior when the review would not materially change conclusions.

Mandatory context bootstrap (before any output):

<!-- shared:mandatory-context-bootstrap -->

• Read these sections in WORKFLOW_OPERATING_SYSTEM.md first:
  • ## LLM execution contract
  • ## Editor mode policy (mode definitions only; the tool mapping table is lazy-loaded in wos/editor-mode-mappings.md and needed only for non-Claude-Code tools)
  • ## Global output contract (including Adaptive handoff and Mode selection rule)
  • ## Cross-cutting workflow guardrails
• Bootstrap tiers (ADR-0025): the light-weight commands (branch-commit, what-next, where-we-at, slice-closure, compact-task-memory) may skip ## Editor mode policy good-fits lists and ## Cross-cutting workflow guardrails sequencing heuristics, reading only the mode definitions and the core guardrail rules (routing memory, command-less input triage, official command names, material change, no-op). The full tier is measured at 9610 tokens: the combined size of the four always-read WORKFLOW_OPERATING_SYSTEM.md sections listed above. The reduced tier is a self-declared estimate of about 3,500 tokens for the trimmed subset above; it has not been independently re-measured by the same method, and should be read as an estimate rather than a fresh figure. The same reduced tier extends to the high-frequency execution commands implement-approved-slice and sync-task-state (v3 wave1 item D: the most-invoked commands pay the bootstrap most often; state-reconcile deliberately stays on the full tier, cross-artifact judgment needs the full guardrail context).
• Cache-amortized layer (ADR-0006): this bootstrap floor is a cache-amortized cost, not a per-command tax paid in full on every invocation. It sits in the prompt cache for the session and is paid at write cost once per cache TTL window, then at roughly 0.1x on cached reads inside that window. Account for it separately from any per-skill Load budget (the generated .claude/skills/<name>/SKILL.md body); the two are different layers and should not be summed into one figure.
• Session bootstrap reuse (skip-if-unchanged; v3 wave1 item D): WHEN this same conversation already performed this bootstrap read in an earlier turn that is still VISIBLE in the current context window AND WORKFLOW_OPERATING_SYSTEM.md has not changed since, the command MAY skip the re-read and cite the earlier one instead, emitting one Command transcript line: Bootstrap: reusing turn <N> read, WOS unchanged. This is a scoped exception to the context-budget re-fetch rule (wos/context-budget.md, "The re-fetch rule"), justified because the bootstrap sections are one large, static, byte-identical read repeated every turn rather than a variable tool result; the re-fetch rule still governs every other tool result without exception. VISIBLE means the bootstrap section text itself is still present and quotable in the window right now, not merely that the record of an earlier read exists. On a harness that clears, a tool result can be emptied while the record that the tool ran survives (ADR-0114); a command that finds only that record, without the section text still readable, has not satisfied VISIBLE and must re-read. Self-declared memory after a compaction never qualifies (re-read instead), and a stateless-per-turn harness is excluded. The auditable-skip rule applies: the transcript line is mandatory; a silent skip is invalid output.
• Read additional sections only when relevant to this command's role.
• Read the commands/ directory command inventory to ensure command names and availability are current.
• Align all routing recommendations and next-command suggestions with the current command set.
• Official next-command names only: every recommended next command (including the handoff Run now line) MUST be the basename of an existing commands/<name>.md file in this workflow repository. Never invent names.

Required inputs:

• active task folder path
• TASK_STATE.md
• DECISIONS.md
• IMPLEMENTATION_PLAN.md
• relevant real code changes
• latest validation/test results, if available
• last completed step from TASK_STATE.md (command + summary)
• optional: --consistency N to run N independent review passes over the same changes and merge them by consensus (off by default; N=3 recommended), per ADR-0073

Operating rules:

• Runtime-debug-payload triage FIRST (ADR-0088). FIRST ACTION, before any review step: IF the invocation args are a runtime-debug payload (pasted runtime logs such as an adb logcat or Metro dump, a stack trace or crash signature, a "still happening" or "got the error again" symptom) THEN the command SHALL route it to incident-triage BEFORE any review work and SHALL NOT absorb it into a review. incident-triage owns the debug loop: it classifies the failure, applies the instrument-first locus gate, and maintains the ruled-out-hypotheses ledger (ADR-0088). A payload that mixes real code-risk observations with runtime-debug logs is split: review the code-risk part here and route the runtime-debug part to incident-triage. This triage is payload-shape-conditional and additive; a normal code-review invocation is unaffected. It exists because the rn-dogfood audit showed review-hard used ~10 times as an ad-hoc debug-iterate loop with pasted logs, and the 2026-07-10 connector dogfood showed the clause skipped when it sat mid-list: the payload was absorbed and diagnosed inline. First position plus eval scenario 102 is the enforcement fix.
• Mechanical compliance check for the rule above. Before composing any other output section, the command SHALL first state explicitly, as its very first line of output, whether the invocation args are a runtime-debug payload (a literal "Runtime-debug payload: yes" or "Runtime-debug payload: no" line). A response that proceeds to any other section without this explicit line first SHALL be treated as not satisfying this command's Definition of done. This exists because an audit found the rule above fired in narration (the model recognized a runtime-debug payload) but was still not acted on twice in one session; a first-line explicit statement makes compliance checkable instead of trusting narration.
• Be critical, specific, and evidence-based.
• Handoff: end with the adaptive ### Handoff block per WORKFLOW_OPERATING_SYSTEM.md ## Global output contract (Mode A compact or Mode B full).
• Playtest-payload triage (ADR-0084). Before reviewing, check whether the invocation args are playtest feedback rather than a request for a code-risk review: notes that the game runs but plays wrong, a mechanic feels off, a screen flow is missing, or difficulty or pacing is bad. That payload is not an engineering-risk review input; route it to pr-feedback-ingest --playtest (the first-class playtest ingestion path) instead of absorbing it into the review. This exists because the dogfood behind ADR-0084 had both of its core-mechanic corrections pasted into review-hard args for want of a designated path. A payload that mixes real code-risk observations with playtest notes is split: review the code-risk part here and route the playtest part onward. This triage is payload-shape-conditional and additive; a normal code-review invocation is unaffected.
• Before producing output, verify the review would materially change risk judgment versus the latest recorded state and artifacts.
• If the diff and validation evidence are unchanged since the last meaningful review, do not generate new churn; return a no-op and route forward.
• No-op rule for artifacts:
  • If TASK_STATE.md would not materially change, do not rewrite it.
  • Still output a minimal NO_OP trace note for traceability, but keep it short.
• Exempt from the no-op rule: an unreconciled ## Requested deliverables row (per the deliverable-reconcile closure gate below) is always a must-fix finding, even when the diff and validation evidence are unchanged since the last review. A vanished deliverable is exactly the silent omission the gate exists to catch, so it is never suppressed as no-op churn.
• Focus on:
  • correctness
  • unsafe assumptions
  • hidden regressions
  • contract and schema mismatches
  • migration/data risks
  • concurrency or idempotency issues
  • weak or misleading tests
  • overengineering
  • maintainability
  • an external vendor contract point (auth format, delivery mechanism, payload shape) that is captured but self-acknowledged as unconfirmed, especially when evidenced only by a vendor demo/example payload rather than a live capture
• Distinguish clearly between:
  • must fix
  • should fix
  • optional improvements
• Severity floor for unresolved external contracts (ADR-0108). An external-vendor contract point that is captured but explicitly flagged as unconfirmed (in REFERENCES.md, a PR note, or task memory) is ALWAYS at minimum a must-fix when it sits on a security-critical or fully-gating path (auth, payment, PII, or any point where a wrong assumption blocks 100% of a code path, not an edge case). This holds even when a workaround exists, even when the code "degrades gracefully" on failure, and even when the point is already written down as a known risk: being on the record is not the same as being gated, and a flat PR-notes bullet reads as an ordinary accepted trade-off to both human and automated reviewers. Do not downgrade this class of finding to should-fix or optional on the grounds that "it's already flagged" -- flag it as must-fix and require either a live verification of the real vendor behavior or an explicit decision-interview record of the accepted risk before the finding can be closed.
• Tag each finding with an impact band (LOW, MEDIUM, HIGH) and a rough effort band, then order findings by impact relative to effort, with impact as the primary key, so the highest value per unit of effort surfaces first. Effort is a tiebreak, never a reason to drop a cheap critical fix.
• Call out what should not have changed if relevant.
• If the implementation is solid, say so clearly rather than inventing feedback.
• Treat this as a focused pre-PR engineering risk check, not a replacement for external review systems.
• Discharge rules for dismissing a finding (mobile dogfood 2026-07-29). The burden of proof sits on the dismissal, not on the finding. "Consistent with the surrounding file" is not a valid dismissal: local precedent is descriptive, not normative. "Pre-existing pattern" excuses only a line the diff did NOT touch; the moment a diff touches a line, that line is this review's business. A dismissal that names neither a cited rule nor an inspected fact is an unresolved finding, not a closed one.
• Verified-clean is not a finding. A hypothesis this review checked and refuted, and a mechanism it inspected and found sound, route to incident-triage for its ## Ruled-out hypotheses ledger (that command is the section's sole owner per wos/substrate-peers.md). They never enter the findings list, which carries only items with a proposed action. On the source run, 33 of 81 canonical findings were verified-clean markers or INFO-level observations with no action, so a third of the fixer's triage budget went to items that were never findings.
• Opt-in self-consistency consensus mode (--consistency N, per ADR-0073). This mode is OFF by default; without the flag the review is a single pass and behaves exactly as today. When invoked with --consistency N, run N independent review passes with fresh context over the same changes, then merge the findings by consensus-of-N (the strategy defined in commands/_shared/worker-contract.md): a finding that appears in at least ceil(N/2) passes is high-confidence; a finding that appears in fewer passes is a singleton, kept as advisory and labeled, never silently dropped. Cost guard: total review cost multiplies by N, so this is strictly opt-in and N=3 is the recommended setting; reserve it for high-stakes changes where the added confidence is worth the spend.
• Two-field verdict and a refuter stage (ADR-0122, extends ADR-0073). Under --consistency N, record each finding as two fields, not one: premise (stands | falls) and proposed fix (take | refuted, with the reason). A finding whose premise is real but whose suggested fix would make things worse is a common and currently inexpressible outcome; splitting the fields is what lets the review say so instead of choosing between adopting a harmful fix and discarding a true finding. Then, after the N raise passes and BEFORE the findings are handed on, dispatch a fixed small refuter count (default 3) ONCE over the whole surviving must-fix and should-fix set, instructed to disprove each finding and to default to refuted when uncertain, and record every killed finding with the citation that disproved it. Do not fan out one refuter per finding: wos/context-budget.md measures 400k to 1.3M tokens per 10-agent batch, and a real run carried 35 queued fixes. Agreement between raise passes reading the same file the same way is correlated, not independent, so N agreeing passes raise confidence in what was noticed, never in whether it is true. A verification stage that only ever confirms is not verifying.

Required output:

1. Overall assessment
2. Must-fix issues
3. Should-fix issues
4. Test gaps
5. Over-engineering gate: flag single-caller abstractions, speculative config or flags nobody asked for, dead code left behind, a construction far larger than the need, and unrequested generality. Ground each flag in "no current caller" or "not in DECISIONS.md" so it stays evidence-based. Advisory and subject to the no-op rule, not a forced finding.
6. Final verdict
7. Exact TASK_STATE.md update block, or explicit TASK_STATE: NO_CHANGE
8. Recommended next command
9. Recommended editor mode
10. Why this is the correct next step
11. What should explicitly not be done yet

Review prompt scaffold (optional)

<!-- shared:xml-review-scaffold -->

When the review directives in this command are ambiguous, parse them in three labeled parts: Instructions (what to do), Context (background, not a rule), and Constraints (hard limits that override the rest). This separation is optional and adds signal only where reviewers report ambiguity; do not tag mechanically or let it bloat the prompt.

Claim grounding (active epistemic humility)

<!-- shared:claim-grounding -->

Claim grounding (active epistemic humility). This block governs what you may assert and how you record it. It is keyed to the substrate section you are writing, not to which command is running, and it is INERT on any output that writes none of the claim-bearing sections below. Full contract and rationale: wos/active-epistemic-humility.md.

1. When this applies. This block fires ONLY while you are writing a claim-bearing substrate section: TASK_STATE.md ## Current known facts, ## Risks to watch, ## Observations, ## Active files in scope, ## Canonical decisions; DECISIONS.md ## Locked decisions; IMPLEMENTATION_PLAN.md ## Current gaps, ## Risks and mitigations; IMPACT_ANALYSIS.md; EXTERNAL_RESEARCH.md; REFERENCES.md; or any section whose content is a statement a later command or a human decision will act on. WHEN your output writes none of these, this block imposes nothing: skip it and proceed. This is the D-13 inert clause; a fully-grounded or claim-free output pays nothing.

2. The unit is the load-bearing claim. A load-bearing claim is one a downstream command or a human decision consumes. A passing aside is not load-bearing; a statement someone will act on is. Apply the rest of this block per load-bearing claim, not per sentence.

3. Ground it or abstain. Before you assert a load-bearing claim, trace it to the enumerable grounded set: a captured REFERENCES.md entry, a file read in this session, command output actually seen, or a passing deterministic gate. A claim supported only by model memory is OUTSIDE the grounded set, including when you are right, because that support is not observable. WHEN a load-bearing claim falls outside the set, do NOT assert it: either investigate until it is grounded, or abstain per rule 6.

4. Status records provenance, never confidence. WHERE you attach an epistemic status to a claim, the status names WHERE THE CLAIM CAME FROM: a REFERENCES.md entry title, a file path plus line, or the gate output it came from. It SHALL NOT express a degree of certainty. Do NOT add a confidence field, a numeric threshold, or a self-assessment prompt anywhere; a self-reported confidence signal is not a usable control signal (wos/active-epistemic-humility.md Part 1.3). A status whose referent slot is empty is read as UNKNOWN, not as a weak yes.

5. Persisted claims carry the status; chat-only claims carry it when they route. Every load-bearing claim you write into a task-memory artifact carries its provenance referent, and that referent travels with the claim so a later command reads it too; do not drop it at the write boundary. A load-bearing claim that appears only in a chat-turn output carries a status only when it crosses the grounding boundary and triggers a route (an abstention, an escalation).

6. Abstain as a routed continuation, never a bare refusal. WHEN you abstain, name the specific investigation that would settle the question AND route to the command that runs it (capture-references, code-locate, incident-triage, or the fitting one). A withholding that stalls the work is invalid output. Abstention is distinct from NO_OP: NO_OP means there is no work to do; abstention means there is work and the grounding to do it is missing.

7. An unfired gate is not evidence. The absence of a fired check does not mean grounding existed. Do not read silence here as a pass.

Standard output layout (required)

<!-- shared:standard-output-layout -->

Produce the command output using this structure (English only):

Artifact changes

<!-- shared:artifact-changes-default -->

Follow ## Global output contract in WORKFLOW_OPERATING_SYSTEM.md for APPLIED / PROPOSED / SKIP rules.

Command transcript

<!-- shared:command-transcript-standard -->

Brief audit trail (max 4 lines; max 3 in no-op runs with NO_OP_TRACE).

Handoff

<!-- shared:handoff-body -->

Use the adaptive ending format from WORKFLOW_OPERATING_SYSTEM.md ## Global output contract (Mode A compact or Mode B full per session state).

Deliverable reconcile (closure gate, per ADR-0056)

<!-- shared:deliverable-reconcile -->

Deliverable reconcile (per ADR-0056). Reconcile the task's ## Requested deliverables ledger in TASK_STATE.md against the delivered work. The gate is lifecycle-aware: it hard-fails only when the run is finalizing the whole task, and reports without failing at a mid-task checkpoint.

1. Locate the ledger. Read ## Requested deliverables in TASK_STATE.md. WHEN the section is absent (a legacy task that predates the ledger), OR its only row is the - none named sentinel (a brief that named no concrete deliverable), this gate is a no-op: skip it and proceed.

2. Classify the context. A finalization run is task-close, or review-hard run as the pre-PR final pass. A checkpoint run is where-we-at or slice-closure (and any review-hard run that is not the pre-PR final). At a checkpoint a row still tagged in-scope that is not yet done is normal remaining work, not a defect.

3. Define reconciled vs silent omission. A row is reconciled when it is done (in the delivered work) or de-scoped:<reason> with that reason recorded in DECISIONS.md. A deliverable named in the brief that has NO ledger row at all, or a row that was dropped without a recorded de-scope, is a silent omission. To detect the no-row case you MUST cross-check the ledger against the brief: read the task's README.md (which task-init seeds from the brief) and the original request when it is in conversation context, and confirm every deliverable named there has a ## Requested deliverables row. A named deliverable with no row means the ledger was seeded incompletely at task-init, and it is a silent omission. WHEN no brief artifact is available to cross-check, reconcile the rows that exist and state in the output that ledger-vs-brief completeness could not be re-verified (do not claim it was).

4. Apply the gate by context.

   • WHEN finalizing: IF any row is unreconciled (still in-scope, or a silent omission per step 3), THEN this command's output is invalid. Name each unreconciled deliverable, state whether it should be delivered or de-scoped, and route to decision-interview (record a de-scope) or implementation-plan (plan the missing work).
   • WHILE at a checkpoint: report each not-yet-done in-scope row as remaining work and do NOT invalidate output on that basis. A silent omission (step 3) is NOT normal progress: name the missing deliverable, record it in the TASK_STATE.md checkpoint output as a must-address finding, and route it to decision-interview (to record a de-scope) or implementation-plan (to seed and plan the missing deliverable), the same repair routing as the finalization branch. At a checkpoint neither case invalidates the whole output: an in-scope-not-yet-done row is reported as remaining work, and a silent omission is named and routed as a must-address finding (never a bare one-line mention). Output invalidation for an unreconciled row happens only in the finalization branch.

A de-scope is allowed; silence is not. This generalizes the repo-level "reject silent omission of any repo in ## Repositories" completeness check from repositories to user-named deliverables. The ledger is seeded at task-init and pointer-linked from SOURCE_OF_TRUTH.md.

References status (finalization, X2)

<!-- shared:references-reconcile -->

References reconcile (X2, 2026-07-18). Reconcile the references a task cited (its REFERENCES.md deliverable, an EXTERNAL_RESEARCH.md, or the project-level references it grounded in) against what the task actually shipped, enforcing "cite only what you used." Lifecycle-aware: it reports at a mid-task checkpoint and hard-fails only when finalizing the whole task.

1. Gate on presence. This sub-check fires only WHEN the task produced or cited references: a REFERENCES.md or EXTERNAL_RESEARCH.md in the task folder, or a Grounded in: citation in the shipped work. WHEN none is present, it is a no-op: skip and proceed.

2. Classify the context. A finalization run is task-close (or review-hard as the pre-PR final pass). A checkpoint run is slice-closure or where-we-at. At a checkpoint a cited reference not yet reflected is normal in-progress work, not a defect.

3. Reconcile cited vs reflected. For each reference the task cited, confirm the shipped work materially reflects it (a real layout, behavior, or decision traceable to that reference), not merely a name-drop. A reference cited with no material trace in the shipped work is a cited-but-unused reference: this is the failure the brief names ("if the final result does not reflect the references you cited, the REFERENCES.md is wrong").

4. Apply the gate by context.

   • WHEN finalizing: IF any cited reference is unused (no material trace) THEN name it and require either removing the citation or pointing to where it is reflected, and route to implement-slice-complement (fix the citation) before closing.
   • WHILE at a checkpoint: report each cited-but-unused reference as a must-address finding (name it, route to implement-slice-complement), and do NOT invalidate the whole output on that basis.

"Cite only what you used" is the invariant. This is the produce-side gate for the capture-references and external-research artifacts, the design-and-research analog of the deliverable-reconcile completeness check.

Definition of done (command output)

• Issues are ranked must/should/optional with concrete references to code/tests.
• No invented problems; if solid, say so clearly.
• TASK_STATE.md updates are PROPOSED unless persisting in Agent mode.
• A runtime-debug payload in the invocation args was routed to incident-triage (or split per the triage rule) BEFORE any review step; absorbing one into a review is invalid output.
• The explicit "Runtime-debug payload: yes/no" line is present as the first line of output, and if "yes", the response routed to incident-triage instead of continuing the review.
• Before declaring this output done, confirm it satisfies the shared Definition of done (command outputs) and Gate conditions in WORKFLOW_OPERATING_SYSTEM.md.

Quality bar: Maximize signal. Prioritize correctness, safety, and maintainability over stylistic commentary.

<!-- cache-breakpoint -->