review-rigor: R12 — verify a fix against the API's real failure behavior

docs/core/review-rigor.md

@@ -138,6 +138,7 @@ The diff is never the whole system. Most expensive misses live in the *relations
 - **R3 — Claim-vs-diff reconciliation.** For "Closes #N", "no behavior change", or an enumerated list of deliverables: verify each claim is actually present in the diff, and that nothing beyond the declared scope changed. Sharper case: a claimed *guarantee* delivered by a **weaker mechanism** than stated, with no test exercising it, is a claim-vs-impl gap — e.g. the body claims "corrupt/truncated input is rejected" but the code only magic-byte-sniffs and then detect-and-corrects, never rejecting. Require the stronger mechanism, or drop the claim. **A claim the change authored is always in review scope, even when the code that would fulfill it is not** — the scope-discipline out-of-scope carve-out (see *Scope discipline*) governs the untouched *code*, never the *claim* this change wrote in its body / commit / branch. So when a claim overstates what the diff delivers and the code that would deliver the rest is pre-existing or out of scope to fix, the deterministic verdict is `blocker` with the **cheap in-scope remedy: narrow the claim to what the diff actually delivers** (a one-line body/commit edit the author is right here to make) — never an `approve` that lets the overbroad claim stand as a "non-blocker", and never a demand to fix the out-of-scope code. Two reviewers splitting here — one blocking the claim, one waving it through as out-of-scope — is the canonical verdict-variance this rule removes: the claim's fate is decided by R3 identically for both, not by whether the referenced code is in scope. (e.g. a body claiming "makes *every* X-gated dial resilient" while several pre-existing dials in untouched services still read the raw env → narrow to "the three shared gates + Y", or route them; a "Closes #N" claiming full resolution while half the acceptance lives in an untouched module → narrow to "Partially addresses #N", or deliver it.)
 - **R4 — Failure-path honesty.** Error branches must not overwrite a failure with success, swallow the error into `_`, or fall through to a permissive default (secret/auth paths must fail closed). Whole-object writes built from a partial form must not drop sibling fields. Any change feeding a dashboard, tally, or aggregate must quantify the number it moves. Detection cue: a remote or side-effecting mutation reported as `applied` on the strength of the mutating command's exit status alone, without reading back the observed effect — idempotent-tool exit codes (a stream-editor `c\` replace, a `grep -q` chain, an HTTP 200-on-noop) return success on a no-op, masking a mutation that never landed; confirm the effect, not the exit code.
 - **R5 — Test drives the defect path.** For every blocker, ask: would the suite have caught this? Require a test on the *actual* failure / retry / concurrent / boundary path — not an adjacent happy-path test that merely touches the same file.
+- **R12 — Verify the fix against the API's real behavior, and the test against the real failure.** When a change's correctness depends on how an **external/framework/library API behaves** — especially *whether it reports a failure* — confirm against that API's **actual behavior or source**, never its name, signature, or your model of it. A method that *looks* like it surfaces an error may buffer and discard it: a write helper whose `write()`/`store()` returns success and whose `get_error()` reads OK **after a flush that silently dropped the error** did not persist the bytes, so a fix that gates on that return still ships the corruption it meant to stop. The trap sharpens when the change carries its **own test seam**: a debug hook the author wrote to *force* the failure tests the author's **model** of the failure, so it passes whether or not the real API behaves that way — a green test over a wrong assumption. Require the test to drive the **real failure mode** (a genuine resource limit / full disk / read-only mount / truncated write; here `RLIMIT_FSIZE=0`), or to **verify the achieved effect directly** (read the closed file back and compare) rather than trust the API's error signal. `pass` = the API's real behavior is cited (source or a real-failure test) and the fix holds against it · `blocker` = the fix relies on an error signal the API does not actually raise, or its only test is a self-authored seam modeling the assumed failure · `n/a` = correctness does not turn on an external API's failure reporting. Detection cues: a fix that checks a return/`get_error()`/`errno` to detect a write/flush/close/network failure; a test that arms the failure through a `debug_force_*` / mock the change itself introduced, with no real-resource or read-back check.
 - **R6 — Validator ordering.** Where validators or filters are evaluated first-match-wins, check that an earlier entry doesn't shadow a later one (the later rule becomes unreachable).
 - **R7 — Concurrency / security primitives.** Watch for: re-entrant lock self-deadlock; a lock released across a remote call and then a stale write-back; forgeable or default keys; client-supplied privilege (trusting a field the client controls); a missing signing-method assertion on token verification. **Client-supplied privilege has a decisive cue — sibling divergence:** when a new write path sets an authorization-carrying field (level, role, permission, group/ownership, tenant) that a sibling creation/mutation path for the same entity forces, validates, or derives server-side, the new path must apply the same discipline. A path that copies such a field from client input verbatim — even behind an admin/permission gate — widens the authority surface versus its siblings and is a blocker, not a normal CRUD field. Detection cue: two paths setting the same privilege field with different server-owned discipline (one forces/validates, the newer one honors the client value); e.g. a bulk/import path honoring a role or level that the single-create path forces.
 - **R8 — Untrusted/active content on upload *and* serve.** If a change accepts or stores a user-supplied content type that can carry active/executable content (`image/svg+xml`, `text/html`, …), trace the **serve/render** path and confirm execution is neutralized — CSP, `Content-Disposition: attachment` (non-inline), or sanitization. Accept-without-neutralize is stored-XSS / content-injection. This check fires on the **consume side**, not only the accept side. An automated FIX that newly enables such a type (e.g. adding SVG detection to an allowlist or sniffer) can itself open this surface — re-run R8 after any allowlist/detector change. Detection cue: an allowlist or validator admitting svg/html with no non-executable serve guard.