From a993b90ee1e119bb545fb2edc231b3970990d031 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 19:30:34 +0200 Subject: [PATCH 1/9] docs(796): verification code is code under test, and the proof obligation splits by where the checker runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #793 added one small operator-run check and five cold-review rounds found five false greens in the verification code itself, each introduced by the fix for the previous one. All five sat outside `docs/guard-inventory.md`'s population and always would have: a `perl` wrapper inside an `if` is not a file, and the script holding it is invoked by no workflow or hook. Records `testing.verification-code-needs-its-own-proof`: the obligation follows the VERDICT rather than the file, and how it is discharged depends on where the construct runs. A checker CI can execute keeps the existing contract (an inventory row, and at MUTATION a declared clause re-applied every run). A checker CI cannot execute — operator-run, needing gitignored config or a live server — gets neither, and ships a witnessed negative control recorded in its PR instead. Extending the manifest to the second class was considered and rejected because it would reproduce the defect it was meant to close: `scripts/mcp_smoke.py` needs `.mcp.json` (gitignored) and a cold-built language server, so a row for it would be a declared mutation that never executes — the exact false green `testing.mutation-claims-are-executed` exists to abolish. The residual (a recorded control decays; nothing re-runs it) is stated in the record rather than papered over. Also folds in the stop-and-subtract trigger (two consecutive rounds of fix-creates-the-next-defect means delete the layer, not guard it) and the macOS timeout recipe, whose wrong form was the recommended one. Every runtime claim in the record was executed rather than read, per its own rule: `perl -e 'alarm shift; exec @ARGV' 5 /nope` exits 0 (the false green, reproduced) while the `or die` form exits 2; `[ -x /bin ]` is true; neither `timeout` nor `gtimeout` is present; `.mcp.json` is ignored at `.gitignore:12`. Refs: #796 Co-Authored-By: Claude Opus 5 (1M context) --- docs/decisions/README.md | 1 + .../verification-code-needs-its-own-proof.md | 72 +++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 docs/decisions/records/testing/verification-code-needs-its-own-proof.md diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 13e3f327e..579072356 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -218,6 +218,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `testing.playwright-mcp-download-and-recovery` | In Playwright-MCP E2E, fetch file-download endpoints with curl — never a browser tab or `window.open` — and if browser tools stall repeatedly, `pkill -f ms-playwright-mcp` and drive a fresh session. | 2026-07-21 | [link](records/testing/playwright-mcp-download-and-recovery.md) | | `testing.scripted-playout-golden-deferred` | The `PlayoutBuildGoldenTests` in-memory golden net covers Sequential (YAML) as of #381. Scripted's *end-to-end pipeline* is excluded — `ScriptedPlayoutBuilder` runs a user-authored external program that drives the engine over HTTP loopback, which the in-memory harness can't pin — so that full-pipeline (integration) harness is deferred to #563. But the scheduling *behavior* those scripts drive lives entirely in the in-process `SchedulingEngine` (the `ScriptedScheduleController` is a 1:1 pass-through to it), which IS directly unit/golden-testable; the earlier "Scripted is un-golden-able by construction" framing overstated the constraint by conflating transport with engine. #395 extracts that shared switch to `ContentEnumeratorBuilder` and adds a direct regression net (`ContentEnumeratorBuilderTests`) over it. | 2026-07-22 | [link](records/testing/scripted-playout-golden-deferred.md) | | `testing.troubleshoot-path-cannot-test-branding` | Verify logo/watermark/bug changes through a real channel playout — a green troubleshoot run proves nothing about branding. | 2026-07-21 | [link](records/testing/troubleshoot-path-cannot-test-branding.md) | +| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files that happen to sit in the guard population or are named like guards. A verification construct is not trusted because it is small, because it is new, or because it is the thing doing the checking; every one of the five defects measured below was in code nobody had counted as a guard. HOW THE OBLIGATION IS DISCHARGED DEPENDS ON WHERE THE CONSTRUCT RUNS, and that split is this record's decision rather than a restatement of #775. (1) A checker CI CAN execute keeps the existing contract unchanged: a row in `docs/guard-inventory.md`, and at `MUTATION` a declared clause in `scripts/tests/mutation_manifest.py` re-applied every run. (2) A checker CI CANNOT execute — operator-run, requiring gitignored config or a live server; `scripts/check-local-lsp.sh` and `scripts/mcp_smoke.py` are today's examples, both already outside the guard population and classified `N/A` in `docs/remote-state-inventory.md` — gets NEITHER a guard-inventory row NOR a manifest entry. It ships a WITNESSED NEGATIVE CONTROL: the failure path is EXECUTED once, the false-green form is reproduced, and both are recorded in the PR that introduces or changes the construct. THE REASON FOR REFUSING THE EXTENSION IS THE POINT — a manifest row for a checker the harness cannot run would be a declared mutation that never executes, which is exactly the false green `testing.mutation-claims-are-executed` exists to abolish, so extending the manifest would reproduce the defect the extension was meant to close. STOP-AND-SUBTRACT: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, stop adding guards to the construct and DELETE the layer generating them — this is the general form of the withdraw-after-N-rounds threshold the string-predicate rule already carries. READING THE CONSTRUCT IS NOT EXECUTING IT: for shell and process-level constructs the failure path must actually be run, because shellcheck caught 0 of the 5 defects here and every one of them is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | ## Review due diff --git a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md new file mode 100644 index 000000000..3254f182b --- /dev/null +++ b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md @@ -0,0 +1,72 @@ +--- +key: testing.verification-code-needs-its-own-proof +title: '2026-08-28 — verification code is code under test: a harness, wrapper or checker ships a witnessed failure path, and where CI cannot run it that proof is a recorded negative control rather than a manifest row (#796)' +status: active +since: '2026-08-28' +supersedes: none +superseded-by: none +rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files that happen to sit in the guard population or are named like guards. A verification construct is not trusted because it is small, because it is new, or because it is the thing doing the checking; every one of the five defects measured below was in code nobody had counted as a guard. HOW THE OBLIGATION IS DISCHARGED DEPENDS ON WHERE THE CONSTRUCT RUNS, and that split is this record''s decision rather than a restatement of #775. (1) A checker CI CAN execute keeps the existing contract unchanged: a row in `docs/guard-inventory.md`, and at `MUTATION` a declared clause in `scripts/tests/mutation_manifest.py` re-applied every run. (2) A checker CI CANNOT execute — operator-run, requiring gitignored config or a live server; `scripts/check-local-lsp.sh` and `scripts/mcp_smoke.py` are today''s examples, both already outside the guard population and classified `N/A` in `docs/remote-state-inventory.md` — gets NEITHER a guard-inventory row NOR a manifest entry. It ships a WITNESSED NEGATIVE CONTROL: the failure path is EXECUTED once, the false-green form is reproduced, and both are recorded in the PR that introduces or changes the construct. THE REASON FOR REFUSING THE EXTENSION IS THE POINT — a manifest row for a checker the harness cannot run would be a declared mutation that never executes, which is exactly the false green `testing.mutation-claims-are-executed` exists to abolish, so extending the manifest would reproduce the defect the extension was meant to close. STOP-AND-SUBTRACT: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, stop adding guards to the construct and DELETE the layer generating them — this is the general form of the withdraw-after-N-rounds threshold the string-predicate rule already carries. READING THE CONSTRUCT IS NOT EXECUTING IT: for shell and process-level constructs the failure path must actually be run, because shellcheck caught 0 of the 5 defects here and every one of them is indistinguishable from correct by reading.' +signals: 'the verification harness is itself unproven code · a false green in the checker rather than the checked · `[ -x "$cmd" ]` is true for a DIRECTORY · perl alarm/exec exits 0 without `or die` · macOS has no `timeout` and no `gtimeout` · smoke test accepts an impostor server · each fix created the next round''s defect · stop and subtract rather than add a layer · an operator-run checker gets NO guard-inventory row and NO manifest entry · a declared mutation that never executes is a false green · witnessed negative control recorded in the PR · shellcheck caught 0 of 5 · paths: `scripts/check-local-lsp.sh`, `scripts/mcp_smoke.py`, `scripts/tests/mutation_manifest.py`, `docs/guard-inventory.md`, `docs/remote-state-inventory.md` · issues: #796, #793, #790, #785, #777, #776' +mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-14: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; with `or die` it dies loudly). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and a `gtimeout` that is absent exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps that travel with it: `[ -x "$cmd" ]` does not establish a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. The single in-repo call site carrying all three is `scripts/check-local-lsp.sh`.' +--- + +**The measurement.** #793 added one small operator-run check — a smoke test that starts the +`csharp-lsp` MCP server and lists its tools. Five independent cold-review rounds found five false +greens *in the verification code itself*, and each was introduced by the fix for the previous one: + +| # | The check | Why it reported success without checking | +|---|---|---| +| 1 | `[ -x "$cmd" ]` | a **directory** satisfies `-x`; `[ -x /bin ]` is true | +| 2 | smoke test accepts any id-matching reply | an impostor server, and `{"id":1}`, both passed | +| 3 | `perl -e 'alarm shift; exec @ARGV'` wrapper | perl **exits 0 when `exec` fails** → prints PASS having run nothing | +| 4 | message/line caps added to bound #3's replacement | an over-long line had its **suffix re-parsed** as a fresh message | +| 5 | pending-id registered before send | a server could **pre-answer** an id it could predict | + +None was hypothetical: each was reproduced with a control before being fixed. + +**Why this is a separate rule and not an application of #775.** `testing.guard-ships-with-mutation- +proof` is scoped by a population — the guard files `docs/guard-inventory.md` derives and grades. All +five defects above sat outside that population and always would have: a `perl` wrapper inside an +`if` is not a file, and the script containing it is invoked by no workflow or hook. The scoping is +not a bug in the inventory; it is what keeps the inventory from becoming a chore that gets +rubber-stamped. What was missing is the claim that the obligation follows the *verdict* rather than +the *file*, which is what this record adds. + +**Why the manifest is not extended, stated as the decision it is.** The obvious move — bring +operator-run checkers into `mutation_manifest.py` — was considered and rejected. The harness applies +every declared clause to an isolated copy of the repository on each run and requires the row's named +test to go red. `scripts/mcp_smoke.py` cannot participate: driving it needs `.mcp.json`, which is +gitignored, and a cold-built language server that takes 30–60s to come up. A row for it would +therefore be a declared mutation that never executes — a `MUTATION` grade that is *only* a claim, +which is the precise shape `testing.mutation-claims-are-executed` was written to abolish. Extending +the manifest would have reproduced, inside the fix, the defect the fix was for. That pattern has now +happened often enough here to be worth naming rather than rediscovering. + +The residual, stated rather than papered over: a witnessed negative control recorded in a PR is +weaker than a mutation re-run every suite. It decays — nothing re-executes it, and a later edit can +silently undo what it proved. It is chosen because the alternative is not a stronger check but a +green one that runs nowhere, and a check that runs nowhere has been this repo's most expensive +recurring failure (#631, #719, #751). Where a checker later becomes CI-executable, it moves to +branch (1) and acquires a real row. + +**Stop-and-subtract, and the evidence for the threshold.** Rounds 3→4 above are the clearest case: +round 3 replaced the broken wrapper, round 4 found that the caps round 3 added re-parsed an +over-long line's suffix, and the fix was **deleting those caps** — three defects with one cause. +Round 5 likewise replaced lock-based ordering with unguessable random ids: strictly less code, and +it removed the race rather than policing it. Two consecutive rounds of fix-creates-the-next-defect +is the signal that the layer, not the clause, is wrong. Compare `fix-the-boundary-not-the-site`; +the string-predicate rule already carries a withdraw threshold, and this is its general form. + +**Read-the-construct fails; execute-it works.** Defect 3 is indistinguishable from correct by +reading, and shellcheck flagged none of the five. #776 reached the same conclusion the same week +from a different subsystem — `exec … 2>/dev/null` silences the shell, and a trap's `exit "$?"` +invents a status — with no shared code or reviewers between them. Two independent measurements of +one claim: for shell and process-level constructs, static reading and linting do not substitute for +running the failure path. + +**Applied to itself.** The `perl … exec` idiom that produced defect 3 came from written project +memory that recommended the form without `or die`. Correct advice, written down, was still a single +point of failure — the recipe had never had its own failure path executed. That is why the corrected +idiom lives in this record's `mechanics` rather than only at the call site: the call site is right +today, and was the only in-repo copy at the time of writing, but a buried-in-one-script recipe is +not discoverable by the next session that needs it. -- 2.47.3 From de8051dae675e7bbbef767340a8e9da7ac659b96 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 19:54:58 +0200 Subject: [PATCH 2/9] fix(796): ship the proof the record was claiming an exemption from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cold review BLOCKED the first commit on two findings, both correct, and both refuted by execution rather than argument. 1. The central premise was false. The record said `scripts/mcp_smoke.py` "cannot participate" in the mutation harness because driving it needs the gitignored `.mcp.json` and a cold-built language server. It takes its config path and server name as POSITIONAL ARGUMENTS (`mcp_smoke.py:78`), so a test can hand it a synthetic config pointing at a stub responder. The record failed its own headline rule — that claim was read, not executed. 2. The split was drawn on the wrong axis. "CI cannot execute it" also describes every `.claude/hooks/*.sh` and `.husky/pre-push`, all of which carry MUTATION rows because a pytest proof drives them in a sandbox. The discriminator is whether a proof test can drive the construct HERMETICALLY. So instead of restating the exemption, this ships the proof: - `scripts/tests/test_mcp_smoke.py` — a stub JSON-RPC responder and five cases pinning the defects the checker has already had (impostor identity, a body carrying only an id, a missing expected tool, a pre-answered request), plus an anti-vacuity positive control. Hermetic: no `.mcp.json`, no language server, 7.9s. - A declared clause in `mutation_manifest.py` targeting `mcp_smoke.py`'s unguessable request id, using the `guard=test / target=script` shape that already exists for `mutation_harness_lib.py` — no population derivation changes. Witnessed red, not asserted: replacing `id_init = secrets.randbelow(...)` with `id_init = 1` makes the pre-answer ACCEPTED at `initialize`, so the run dies one stage later at `tools/list` (rc 9 -> 10) and the proof reddens with its declared diagnostic. Only that one test reddened; the mutation is clause-scoped. `mcp_smoke.py` still gets no inventory row — a row for it is rejected as a phantom, measured — so the row goes to the test file, which joins the population automatically. Also corrected: the "acquires a real row" migration the mechanism would refuse (HIGH 3); "shellcheck caught 0 of 5", re-measured as neither of the TWO shell defects, the other three being Python and outside its reach (MEDIUM 4); a threshold misattributed to `release.verdict-vocabulary-shared`, which carries a categorical ban rather than a count (MEDIUM 5); an unverifiable "no shared reviewers" claim (MEDIUM 6); a dangling cross-reference to a private memory filename (LOW 7); and the scope-limit item that actually excludes defects 2/4/5 (LOW 8). Docs updated in the same PR per CLAUDE.md: `docs/README.md`'s task-signal map now routes to four guard rules rather than three, and `docs/guard-inventory.md`'s scope limit item 6 records that being outside the population is not an exemption. Refs: #796 Co-Authored-By: Claude Opus 5 (1M context) --- docs/README.md | 2 +- docs/decisions/README.md | 2 +- .../verification-code-needs-its-own-proof.md | 102 ++++++------ docs/guard-inventory.md | 10 +- scripts/tests/mutation_manifest.py | 19 +++ scripts/tests/test_mcp_smoke.py | 149 ++++++++++++++++++ 6 files changed, 233 insertions(+), 51 deletions(-) create mode 100644 scripts/tests/test_mcp_smoke.py diff --git a/docs/README.md b/docs/README.md index 154afc50e..0a0407a62 100644 --- a/docs/README.md +++ b/docs/README.md @@ -27,7 +27,7 @@ doc below, or that changes which sections a task signal points to.** | Concurrency / optimistic-locking work | `docs/api-conventions.md` §7a/b/c + `docs/decisions/optimistic-concurrency.md` | | Auth / security-surface work | `docs/decisions/api-auth-security.md` | | CI / release pipeline work | `docs/ci-cd.md` + `docs/decisions/release-ci-governance.md` | -| Proposing a new guard / CI check / regression test convention | `docs/defect-shapes-773.md` §4 (detector menu + the classes where no detector is plausible), then the three rules every guard must satisfy: `docs/decisions/records/testing/guard-derives-population-from-source.md`, `…/guard-ships-with-mutation-proof.md` and `…/mutation-claims-are-executed.md` (a `MUTATION` grade carries a DECLARED clause mutation that is re-run every suite) | +| Proposing a new guard / CI check / regression test convention | `docs/defect-shapes-773.md` §4 (detector menu + the classes where no detector is plausible), then the four rules every guard must satisfy: `docs/decisions/records/testing/guard-derives-population-from-source.md`, `…/guard-ships-with-mutation-proof.md`, `…/mutation-claims-are-executed.md` (a `MUTATION` grade carries a DECLARED clause mutation that is re-run every suite) and `…/verification-code-needs-its-own-proof.md` (the obligation follows the VERDICT, so it binds the harness, wrapper or checker doing the checking — not only the files in the guard population) | | Adding or bounding a consequential numeric config field (an FFmpeg profile tunable, a pipeline knob) | `docs/api-conventions.md` §3d — reject out of range with a 422 naming the bound and its consequence, never accept-then-rewrite; validate against the constants the renderer reads, keep the render-time clamp for pre-existing rows, and let an UNCHANGED legacy value through on update. Then `api.ffmpeg-profile-numeric-bounds` | | Testing a surface gated by config / an env var / a credential | `docs/decisions/records/testing/deny-path-at-production-config-value.md` — cover the setting absent, at its production value, and each opt-out, and assert the DENY branch | | Touching a full-replace write path or a hand-built request object | `docs/decisions/records/testing/full-replace-asserts-field-list.md` — derive the field list from the DTO and assert set equality; reconcile by id where child state exists. In the SPA the same rule is enforced by the type system: `docs/spa-conventions.md` §4b — build the body as `Complete`, annotating BOTH the wrapper parameter and every construction site | diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 579072356..4ecf59cdb 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -218,7 +218,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `testing.playwright-mcp-download-and-recovery` | In Playwright-MCP E2E, fetch file-download endpoints with curl — never a browser tab or `window.open` — and if browser tools stall repeatedly, `pkill -f ms-playwright-mcp` and drive a fresh session. | 2026-07-21 | [link](records/testing/playwright-mcp-download-and-recovery.md) | | `testing.scripted-playout-golden-deferred` | The `PlayoutBuildGoldenTests` in-memory golden net covers Sequential (YAML) as of #381. Scripted's *end-to-end pipeline* is excluded — `ScriptedPlayoutBuilder` runs a user-authored external program that drives the engine over HTTP loopback, which the in-memory harness can't pin — so that full-pipeline (integration) harness is deferred to #563. But the scheduling *behavior* those scripts drive lives entirely in the in-process `SchedulingEngine` (the `ScriptedScheduleController` is a 1:1 pass-through to it), which IS directly unit/golden-testable; the earlier "Scripted is un-golden-able by construction" framing overstated the constraint by conflating transport with engine. #395 extracts that shared switch to `ContentEnumeratorBuilder` and adds a direct regression net (`ContentEnumeratorBuilderTests`) over it. | 2026-07-22 | [link](records/testing/scripted-playout-golden-deferred.md) | | `testing.troubleshoot-path-cannot-test-branding` | Verify logo/watermark/bug changes through a real channel playout — a green troubleshoot run proves nothing about branding. | 2026-07-21 | [link](records/testing/troubleshoot-path-cannot-test-branding.md) | -| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files that happen to sit in the guard population or are named like guards. A verification construct is not trusted because it is small, because it is new, or because it is the thing doing the checking; every one of the five defects measured below was in code nobody had counted as a guard. HOW THE OBLIGATION IS DISCHARGED DEPENDS ON WHERE THE CONSTRUCT RUNS, and that split is this record's decision rather than a restatement of #775. (1) A checker CI CAN execute keeps the existing contract unchanged: a row in `docs/guard-inventory.md`, and at `MUTATION` a declared clause in `scripts/tests/mutation_manifest.py` re-applied every run. (2) A checker CI CANNOT execute — operator-run, requiring gitignored config or a live server; `scripts/check-local-lsp.sh` and `scripts/mcp_smoke.py` are today's examples, both already outside the guard population and classified `N/A` in `docs/remote-state-inventory.md` — gets NEITHER a guard-inventory row NOR a manifest entry. It ships a WITNESSED NEGATIVE CONTROL: the failure path is EXECUTED once, the false-green form is reproduced, and both are recorded in the PR that introduces or changes the construct. THE REASON FOR REFUSING THE EXTENSION IS THE POINT — a manifest row for a checker the harness cannot run would be a declared mutation that never executes, which is exactly the false green `testing.mutation-claims-are-executed` exists to abolish, so extending the manifest would reproduce the defect the extension was meant to close. STOP-AND-SUBTRACT: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, stop adding guards to the construct and DELETE the layer generating them — this is the general form of the withdraw-after-N-rounds threshold the string-predicate rule already carries. READING THE CONSTRUCT IS NOT EXECUTING IT: for shell and process-level constructs the failure path must actually be run, because shellcheck caught 0 of the 5 defects here and every one of them is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | +| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: every `.claude/hooks/*.sh` and `.husky/pre-push` is operator-run and CI never fires them AS HOOKS, yet all carry `MUTATION` rows, because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those already-proven guards into the exempt branch. "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives every protocol branch it has — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | ## Review due diff --git a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md index 3254f182b..76b4ff05f 100644 --- a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md +++ b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md @@ -1,18 +1,19 @@ --- key: testing.verification-code-needs-its-own-proof -title: '2026-08-28 — verification code is code under test: a harness, wrapper or checker ships a witnessed failure path, and where CI cannot run it that proof is a recorded negative control rather than a manifest row (#796)' +title: '2026-08-28 — verification code is code under test: the obligation follows the VERDICT, and the exempt branch is what no proof test can drive hermetically (#796)' status: active since: '2026-08-28' supersedes: none superseded-by: none -rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files that happen to sit in the guard population or are named like guards. A verification construct is not trusted because it is small, because it is new, or because it is the thing doing the checking; every one of the five defects measured below was in code nobody had counted as a guard. HOW THE OBLIGATION IS DISCHARGED DEPENDS ON WHERE THE CONSTRUCT RUNS, and that split is this record''s decision rather than a restatement of #775. (1) A checker CI CAN execute keeps the existing contract unchanged: a row in `docs/guard-inventory.md`, and at `MUTATION` a declared clause in `scripts/tests/mutation_manifest.py` re-applied every run. (2) A checker CI CANNOT execute — operator-run, requiring gitignored config or a live server; `scripts/check-local-lsp.sh` and `scripts/mcp_smoke.py` are today''s examples, both already outside the guard population and classified `N/A` in `docs/remote-state-inventory.md` — gets NEITHER a guard-inventory row NOR a manifest entry. It ships a WITNESSED NEGATIVE CONTROL: the failure path is EXECUTED once, the false-green form is reproduced, and both are recorded in the PR that introduces or changes the construct. THE REASON FOR REFUSING THE EXTENSION IS THE POINT — a manifest row for a checker the harness cannot run would be a declared mutation that never executes, which is exactly the false green `testing.mutation-claims-are-executed` exists to abolish, so extending the manifest would reproduce the defect the extension was meant to close. STOP-AND-SUBTRACT: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, stop adding guards to the construct and DELETE the layer generating them — this is the general form of the withdraw-after-N-rounds threshold the string-predicate rule already carries. READING THE CONSTRUCT IS NOT EXECUTING IT: for shell and process-level constructs the failure path must actually be run, because shellcheck caught 0 of the 5 defects here and every one of them is indistinguishable from correct by reading.' -signals: 'the verification harness is itself unproven code · a false green in the checker rather than the checked · `[ -x "$cmd" ]` is true for a DIRECTORY · perl alarm/exec exits 0 without `or die` · macOS has no `timeout` and no `gtimeout` · smoke test accepts an impostor server · each fix created the next round''s defect · stop and subtract rather than add a layer · an operator-run checker gets NO guard-inventory row and NO manifest entry · a declared mutation that never executes is a false green · witnessed negative control recorded in the PR · shellcheck caught 0 of 5 · paths: `scripts/check-local-lsp.sh`, `scripts/mcp_smoke.py`, `scripts/tests/mutation_manifest.py`, `docs/guard-inventory.md`, `docs/remote-state-inventory.md` · issues: #796, #793, #790, #785, #777, #776' -mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-14: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; with `or die` it dies loudly). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and a `gtimeout` that is absent exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps that travel with it: `[ -x "$cmd" ]` does not establish a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. The single in-repo call site carrying all three is `scripts/check-local-lsp.sh`.' +rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: every `.claude/hooks/*.sh` and `.husky/pre-push` is operator-run and CI never fires them AS HOOKS, yet all carry `MUTATION` rows, because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those already-proven guards into the exempt branch. "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives every protocol branch it has — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' +signals: 'the verification harness is itself unproven code · a false green in the checker rather than the checked · `[ -x "$cmd" ]` is true for a DIRECTORY · perl alarm/exec exits 0 without `or die` · macOS has no `timeout` and no `gtimeout` · smoke test accepts an impostor server · a server that pre-answers a guessable id · each fix created the next round''s defect · stop and subtract rather than add a layer · hermetically drivable is the test, not does-CI-run-it · a checker is not exempt because its CALLER cannot run in CI · the checker needs no inventory row — name the proof test as `guard` and the checker as `target` · a transitively-reached script is rejected as a PHANTOM · witnessed negative control for what cannot be driven · paths: `scripts/mcp_smoke.py`, `scripts/tests/test_mcp_smoke.py`, `scripts/check-local-lsp.sh`, `scripts/tests/mutation_manifest.py`, `docs/guard-inventory.md` · issues: #796, #793, #790, #785, #777, #776' +mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-28: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; the `or die` form exits 2 and says why). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and an absent `gtimeout` exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps travel with it: `[ -x "$cmd" ]` does not establish that a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. The single in-repo call site carrying all three is `scripts/check-local-lsp.sh`.' --- **The measurement.** #793 added one small operator-run check — a smoke test that starts the `csharp-lsp` MCP server and lists its tools. Five independent cold-review rounds found five false -greens *in the verification code itself*, and each was introduced by the fix for the previous one: +greens *in the verification code itself*, each introduced by the fix for the previous one and each +reproduced with a control before being fixed: | # | The check | Why it reported success without checking | |---|---|---| @@ -22,51 +23,58 @@ greens *in the verification code itself*, and each was introduced by the fix for | 4 | message/line caps added to bound #3's replacement | an over-long line had its **suffix re-parsed** as a fresh message | | 5 | pending-id registered before send | a server could **pre-answer** an id it could predict | -None was hypothetical: each was reproduced with a control before being fixed. - **Why this is a separate rule and not an application of #775.** `testing.guard-ships-with-mutation- -proof` is scoped by a population — the guard files `docs/guard-inventory.md` derives and grades. All -five defects above sat outside that population and always would have: a `perl` wrapper inside an -`if` is not a file, and the script containing it is invoked by no workflow or hook. The scoping is -not a bug in the inventory; it is what keeps the inventory from becoming a chore that gets -rubber-stamped. What was missing is the claim that the obligation follows the *verdict* rather than -the *file*, which is what this record adds. +proof` is scoped by a population — the guard files `docs/guard-inventory.md` derives and grades. +Defect 1 and 3 are a `perl` wrapper inside an `if`, which is not a file at all; 2, 4 and 5 live in +`scripts/mcp_smoke.py`, which *is* a file but is reached only through another script and so falls +under scope limit item 6. That scoping is what keeps the inventory from becoming a chore that gets +rubber-stamped; what was missing is the claim that the obligation follows the *verdict*, not the *file*. -**Why the manifest is not extended, stated as the decision it is.** The obvious move — bring -operator-run checkers into `mutation_manifest.py` — was considered and rejected. The harness applies -every declared clause to an isolated copy of the repository on each run and requires the row's named -test to go red. `scripts/mcp_smoke.py` cannot participate: driving it needs `.mcp.json`, which is -gitignored, and a cold-built language server that takes 30–60s to come up. A row for it would -therefore be a declared mutation that never executes — a `MUTATION` grade that is *only* a claim, -which is the precise shape `testing.mutation-claims-are-executed` was written to abolish. Extending -the manifest would have reproduced, inside the fix, the defect the fix was for. That pattern has now -happened often enough here to be worth naming rather than rediscovering. +**The discriminator, stated precisely because the obvious one is wrong.** It is not "does CI run +this": CI never fires `.claude/hooks/*.sh` or `.husky/pre-push` as hooks either, and all of them +carry `MUTATION` rows, because a pytest proof drives them in a sandbox. The question is whether a +proof test can drive the construct *hermetically*, and "it needs a live server" usually fails that +test rather than passing it. `mcp_smoke.py` takes its config path and server name as positional +arguments, so `scripts/tests/test_mcp_smoke.py` hands it a synthetic config pointing at a stub +responder and drives every protocol branch — identity mismatch, a body carrying only an id, a +missing tool, a pre-answered request — in under eight seconds. `check-local-lsp.sh` genuinely +cannot be driven that way: it asserts a global `csharp-ls` on PATH, a `host/fxr` under +`DOTNET_ROOT`, and a root `node_modules`, which is developer-machine state no runner has. That, and +not "it is operator-run", is what puts it in the exempt branch — and the part of the construct +carrying the defect history sits on the provable side of the seam. -The residual, stated rather than papered over: a witnessed negative control recorded in a PR is -weaker than a mutation re-run every suite. It decays — nothing re-executes it, and a later edit can -silently undo what it proved. It is chosen because the alternative is not a stronger check but a -green one that runs nowhere, and a check that runs nowhere has been this repo's most expensive -recurring failure (#631, #719, #751). Where a checker later becomes CI-executable, it moves to -branch (1) and acquires a real row. +**Where the proof lives, and why the checker needs no row.** `docs/guard-inventory.md`'s population +is derived from workflow and hook call sites, so a row for `mcp_smoke.py` is rejected outright — +measured, not assumed: -**Stop-and-subtract, and the evidence for the threshold.** Rounds 3→4 above are the clearest case: -round 3 replaced the broken wrapper, round 4 found that the caps round 3 added re-parsed an -over-long line's suffix, and the fix was **deleting those caps** — three defects with one cause. -Round 5 likewise replaced lock-based ordering with unguessable random ids: strictly less code, and -it removed the race rather than policing it. Two consecutive rounds of fix-creates-the-next-defect -is the signal that the layer, not the clause, is wrong. Compare `fix-the-boundary-not-the-site`; -the string-predicate rule already carries a withdraw threshold, and this is its general form. + AssertionError: guard-inventory.md lists ['scripts/mcp_smoke.py'], which no longer exist or are + no longer invoked by any workflow or hook. A row for a guard that does not run reads as coverage + and is not. + +The row is therefore never the route. The proof goes in `scripts/tests/test_*.py`, which joins the +population automatically and carries the row, and the manifest entry names that test as `guard` with +the checker as `target` — the shape `test_mutation_harness.py` already uses for +`mutation_harness_lib.py`, itself outside the population under scope limit item 7. No population +derivation changes; the declared clause is re-applied every run like any other. + +**The residual, for the exempt branch only.** A witnessed negative control recorded in a PR is +weaker than a mutation re-run every suite: nothing re-executes it, and a later edit can silently +undo what it proved. The alternative there is not a stronger check but a green one that runs +nowhere — this repo's most expensive recurring failure (#631, #719, #751). + +**Stop-and-subtract.** Rounds 3→4 are the clearest case: round 3 replaced the broken wrapper, round +4 found that the caps round 3 added re-parsed an over-long line's suffix, and the fix was **deleting +those caps** — three defects with one cause. Round 5 likewise replaced lock-based ordering with +unguessable random ids: strictly less code, and it removed the race rather than policing it. Two +consecutive rounds of fix-creates-the-next-defect is adopted here as the threshold; the six-round +text-comparison case behind `release.verdict-vocabulary-shared` is precedent for the shape, not its +source, and it concluded in a categorical ban rather than a count. **Read-the-construct fails; execute-it works.** Defect 3 is indistinguishable from correct by -reading, and shellcheck flagged none of the five. #776 reached the same conclusion the same week -from a different subsystem — `exec … 2>/dev/null` silences the shell, and a trap's `exit "$?"` -invents a status — with no shared code or reviewers between them. Two independent measurements of -one claim: for shell and process-level constructs, static reading and linting do not substitute for -running the failure path. - -**Applied to itself.** The `perl … exec` idiom that produced defect 3 came from written project -memory that recommended the form without `or die`. Correct advice, written down, was still a single -point of failure — the recipe had never had its own failure path executed. That is why the corrected -idiom lives in this record's `mechanics` rather than only at the call site: the call site is right -today, and was the only in-repo copy at the time of writing, but a buried-in-one-script recipe is -not discoverable by the next session that needs it. +reading. ShellCheck 0.11.0 over reconstructed forms of the two shell defects returns only style nits +(SC2292, SC2250) and an unused-variable warning — nothing about either; the remaining three are +Python and outside its reach, so the honest figure is two of two, not zero of five. #776 reached the +same conclusion the same week from a different subsystem, with no shared code: `exec … 2>/dev/null` +silences the shell, and a trap's `exit "$?"` invents a status. The idiom behind defect 3 came from +written project memory recommending the form without `or die` — correct advice, written down, was +still a single point of failure, because the recipe had never had its own failure path executed. diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index b520e9621..909d28549 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -69,7 +69,12 @@ inventory shipped with were sitting in the gap: 5. **Non-`.sh` hooks** — the hook pattern is `*.sh` only. (`.yaml` workflows are no longer in this gap: the caller scan matches `*.yml` and `*.yaml` since #806, because Gitea accepts both.) 6. **Transitive calls** — a script invoked only by another script, rather than by a workflow or - hook, is not discovered. + hook, is not discovered. Being outside this population is NOT an exemption from proof: + `testing.verification-code-needs-its-own-proof` (#796) puts such a script in the ordinary branch + whenever a proof test can drive it hermetically. A row here is not the route — the population is + derived from the call sites, so one is rejected as a phantom — but the manifest can still name + the checker, as `target`, with its proof test as `guard`. `scripts/mcp_smoke.py` is the worked + example: no row of its own, proven by `scripts/tests/test_mcp_smoke.py`, which carries the row. 7. **Non-`test_` modules under `scripts/tests/`** — the pattern is `test_*.py`, so `conftest.py`, `mutation_harness_lib.py`, `mutation_manifest.py` and `tracked_files.py` are outside the population and hold no rows. They are not guards (they assert nothing on their own), but the @@ -211,6 +216,7 @@ recorded as unexamined rather than as cleared. | `scripts/tests/test_guard_populations_derive_from_git.py` | the `script-tests` job, on a guard whose file population admits a file git does not track | GUARD | MUTATION | `test_guard_populations_derive_from_git.py::test_no_derivation_admits_an_untracked_file` | | `scripts/tests/test_hook_fire_log.py` | the `script-tests` job, on a hook that stops reporting that it fired, or whose reporting changes what the harness sees | GUARD | MUTATION | `test_hook_fire_log.py::test_a_hook_that_LOSES_its_instrumentation_is_DETECTED` | | `scripts/tests/test_jq_preflight.py` | the `script-tests` job | PROOF | NONE | — | +| `scripts/tests/test_mcp_smoke.py` | the `script-tests` job, when the MCP smoke checker accepts a server it should refuse — an impostor identity, a reply carrying only an id, a missing expected tool, or an answer pre-answered before the request | GUARD | MUTATION | `test_mcp_smoke.py::test_MUTATION_a_PRE_ANSWERED_id_is_refused_because_the_request_ids_are_UNGUESSABLE` | | `scripts/tests/test_merge_consent_base_change.py` | the `script-tests` job | PROOF | NONE | — | | `scripts/tests/test_merge_consent_exemption.py` | the `script-tests` job | PROOF | NONE | — | | `scripts/tests/test_merge_consent_required_check.py` | the `script-tests` job | PROOF | NONE | — | @@ -247,7 +253,7 @@ clause-provable and the entry is regraded. ## What the numbers say -42 guards, 6 tooling scripts, 21 proof files. **20 guards carry a mutation proof; 6 are +43 guards, 6 tooling scripts, 21 proof files. **21 guards carry a mutation proof; 6 are behaviour-only; 16 have none.** These figures are asserted against the table by `test_the_summary_counts_match_the_table` — they were wrong in the first draft (28/4/6/3/19 against a table holding 27/5/6/3/18), because a hand-maintained summary of a table is a second copy of it, diff --git a/scripts/tests/mutation_manifest.py b/scripts/tests/mutation_manifest.py index fe2f8b981..caeb94ee2 100644 --- a/scripts/tests/mutation_manifest.py +++ b/scripts/tests/mutation_manifest.py @@ -310,6 +310,25 @@ MUTATIONS: tuple[Mutation, ...] = ( "one checkout, so the clause is unambiguous there; the trailing comment line is part of it " "only to make the match unique within the file.", ), + Mutation( + guard="scripts/tests/test_mcp_smoke.py", + target="scripts/mcp_smoke.py", + clause=" id_init = secrets.randbelow(2**31 - 1000) + 1000", + replacement=" id_init = 1", + proof="test_mcp_smoke.py::test_MUTATION_a_PRE_ANSWERED_id_is_refused_because_the_request_ids_are_UNGUESSABLE", + granularity=CLAUSE, + expect="a pre-answered id was ACCEPTED", + why="The target is not the guard for the same structural reason as the harness entry above: " + "`scripts/mcp_smoke.py` is a checker reached only transitively, through " + "`scripts/check-local-lsp.sh`, so it holds no inventory row of its own and the clause has to " + "be disarmed in it directly (`testing.verification-code-needs-its-own-proof`). The clause is " + "the unguessable request id, which is the ONLY thing refusing a server that answers before " + "it is asked — a lock cannot reject a frame emitted before the request, which is why #793 " + "round 5 replaced the lock rather than tightening it. Disarmed, the stub's pre-answer is " + "accepted at `initialize` and the run dies one stage later at `tools/list`, so the proof " + "asserts the STAGE (rc 9 and the initialize diagnostic) rather than mere failure: the mutant " + "still fails, and a test checking only that would stay green over a real vulnerability.", + ), ) diff --git a/scripts/tests/test_mcp_smoke.py b/scripts/tests/test_mcp_smoke.py new file mode 100644 index 000000000..c67b53667 --- /dev/null +++ b/scripts/tests/test_mcp_smoke.py @@ -0,0 +1,149 @@ +"""`scripts/mcp_smoke.py` answers its own question correctly (ersatztv#796). + +WHY THIS FILE EXISTS AND WHY IT IS NOT A ROW FOR `mcp_smoke.py`. That script is the checker that +carried three of #793's five false greens: it accepted any reply carrying the right id, its +message/line caps re-parsed an over-long line's suffix as a fresh message, and it registered a +pending id before sending so a server could pre-answer a predictable one. Each was reproduced with a +control at the time and then fixed, and NONE of those controls survived as a test — so nothing +re-ran them and a later edit could silently undo what they proved. `testing.verification-code-needs- +its-own-proof` is the rule that says a checker owes an executed proof; this is that proof. + +`mcp_smoke.py` itself gets no `docs/guard-inventory.md` row: the inventory's population is derived +from workflow and hook call sites and this script is reached only transitively, through +`scripts/check-local-lsp.sh` (scope limit item 6), so a row for it is rejected as a phantom. THIS +file joins the population as an ordinary `scripts/tests/test_*.py` and carries the row, and the +manifest entry names it as `guard` with `scripts/mcp_smoke.py` as `target` — the same shape +`test_mutation_harness.py` uses for `mutation_harness_lib.py`. + +HERMETIC BY CONSTRUCTION, which is the point that makes the proof possible at all. `mcp_smoke.py` +takes its config path and server name as POSITIONAL ARGUMENTS, so nothing here touches the +gitignored `.mcp.json` or starts a language server: every case below drives it against a synthetic +config in a tmpdir pointing at the stub in `_STUB`. "It needs a real server" exempts the +INTEGRATION — `check-local-lsp.sh` driving the actual csharp-lsp — never this protocol logic. + +NOT TESTED, DELIBERATELY: the unbounded raw line buffer. `mcp_smoke.py` states that as an ACCEPTED +LIMIT, because the caps that would bound it were themselves defect 4. A test pushing toward +reintroducing them would drive the file back into the failure it already paid for. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +SMOKE = REPO_ROOT / "scripts" / "mcp_smoke.py" + +# The stub speaks just enough JSON-RPC to exercise each branch. `preanswer` is the security case: +# it emits replies for the LOW, GUESSABLE ids a predictable client would use, BEFORE reading +# anything, and then goes silent — so it can only be accepted by a client whose ids can be guessed. +_STUB = r''' +import json, sys, time + +mode = sys.argv[1] + +def emit(obj): + sys.stdout.write(json.dumps(obj) + "\n") + sys.stdout.flush() + +def init_result(name="stub-mcp"): + return {"serverInfo": {"name": name, "version": "0.1"}, "capabilities": {}} + +def tools_result(names): + return {"tools": [{"name": n} for n in names]} + +if mode == "preanswer": + emit({"jsonrpc": "2.0", "id": 1, "result": init_result()}) + emit({"jsonrpc": "2.0", "id": 2, "result": tools_result(["alpha", "beta"])}) + time.sleep(30) + sys.exit(0) + +for line in sys.stdin: + line = line.strip() + if not line: + continue + try: + msg = json.loads(line) + except ValueError: + continue + mid, method = msg.get("id"), msg.get("method") + if method == "initialize": + if mode == "impostor": + emit({"jsonrpc": "2.0", "id": mid, "result": init_result("impostor-mcp")}) + elif mode == "no_result": + emit({"jsonrpc": "2.0", "id": mid}) + else: + emit({"jsonrpc": "2.0", "id": mid, "result": init_result()}) + elif method == "tools/list": + names = ["alpha"] if mode == "missing_tool" else ["alpha", "beta"] + emit({"jsonrpc": "2.0", "id": mid, "result": tools_result(names)}) +''' + + +def _run(tmp_path, mode: str, *extra: str, timeout: int = 5): + """Drive `mcp_smoke.py` against the stub in `mode`. Returns (returncode, combined output).""" + stub = tmp_path / "stub_server.py" + stub.write_text(_STUB) + cfg = tmp_path / "synthetic.mcp.json" + cfg.write_text( + json.dumps({"mcpServers": {"stub": {"command": sys.executable, "args": [str(stub), mode]}}}) + ) + proc = subprocess.run( + [sys.executable, str(SMOKE), str(cfg), "stub", str(timeout), *extra], + capture_output=True, + text=True, + timeout=timeout + 40, + ) + return proc.returncode, proc.stdout + proc.stderr + + +# ------------------------------------------------------------------------------------------------ +# ANTI-VACUITY FIRST. Every case below asserts a FAILURE, and a harness that cannot make the checker +# succeed would report all of them green while proving nothing about the checker's rejections. +# ------------------------------------------------------------------------------------------------ + + +def test_the_harness_can_make_the_checker_PASS(tmp_path): + rc, out = _run(tmp_path, "good", "--expect-server", "stub-mcp", "--expect-tool", "alpha") + assert rc == 0, f"the positive control did not pass, so every rejection below proves nothing: {out}" + assert "OK:" in out and "stub-mcp" in out, out + + +def test_an_IMPOSTOR_server_is_rejected_on_identity(tmp_path): + """#793 defect 2: 'it answered' is not 'the configured server answered'.""" + rc, out = _run(tmp_path, "impostor", "--expect-server", "stub-mcp") + assert rc == 13, f"an impostor server was accepted (rc={rc}): {out}" + assert "wrong server" in out, out + + +def test_a_reply_carrying_only_an_ID_is_rejected_as_malformed(tmp_path): + """#793 defect 2, the other half: a bare `{\"id\": N}` is a reply, not an answer.""" + rc, out = _run(tmp_path, "no_result") + assert rc == 12, f"a body with no 'result' was accepted (rc={rc}): {out}" + assert "no 'result' object" in out, out + + +def test_a_MISSING_expected_tool_is_reported(tmp_path): + rc, out = _run(tmp_path, "missing_tool", "--expect-tool", "alpha", "--expect-tool", "beta") + assert rc == 14, f"a server missing an expected tool was accepted (rc={rc}): {out}" + assert "missing expected tool" in out, out + + +def test_MUTATION_a_PRE_ANSWERED_id_is_refused_because_the_request_ids_are_UNGUESSABLE(tmp_path): + """#793 defect 5, and the declared clause mutation for this guard. + + The stub answers ids 1 and 2 before reading anything, then goes silent. With random request ids + the pre-answer cannot match, so `initialize` times out and the run is refused at THAT stage. + Replace `id_init = secrets.randbelow(...)` with `id_init = 1` and the pre-answer is accepted: + the run gets past identity and dies later, at tools/list — a different stage and a different + diagnostic, which is what turns this assertion red. Asserting the STAGE rather than merely + 'it failed' is what makes the mutation visible; the mutant still fails, just not here. + """ + rc, out = _run(tmp_path, "preanswer", "--expect-server", "stub-mcp") + assert rc == 9 and "no 'initialize' response" in out, ( + f"a pre-answered id was ACCEPTED: the checker got past `initialize` against a server that " + f"replied before it was asked (rc={rc}). Unguessable request ids are the only thing closing " + f"that race — a lock cannot reject a frame emitted before the request.\n{out}" + ) -- 2.47.3 From 338e04dcf1b46ce5ad95251f0e0bc59204905e78 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 20:04:38 +0200 Subject: [PATCH 3/9] fix(796): the hooks claim said "all", the table says four MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-caught before re-review landed, and it is the same shape as the finding that blocked round 1: a universal asserted rather than measured. The record argued that "can CI execute it" is the wrong discriminator because every `.claude/hooks/*.sh` and `.husky/pre-push` is operator-run yet "all carry MUTATION rows". Reading the table: 3 of 13 hooks do (`posttooluse-worktree-marker`, `pretooluse-bom-guard`, `pretooluse-worktree-guard`), plus `.husky/pre-push` — four in total. Most of the rest are graded NONE. The argument is unaffected: four guards that CI never fires as hooks are nonetheless proven by a pytest proof driving them in a sandbox, which is all that is needed to refute the does-CI-run-it discriminator. Only the quantifier was wrong, and the correction now names the four and says explicitly that the NONE majority is unproven debt rather than evidence for the other reading. Refs: #796 Co-Authored-By: Claude Opus 5 (1M context) --- docs/decisions/README.md | 2 +- .../testing/verification-code-needs-its-own-proof.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 4ecf59cdb..56bece834 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -218,7 +218,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `testing.playwright-mcp-download-and-recovery` | In Playwright-MCP E2E, fetch file-download endpoints with curl — never a browser tab or `window.open` — and if browser tools stall repeatedly, `pkill -f ms-playwright-mcp` and drive a fresh session. | 2026-07-21 | [link](records/testing/playwright-mcp-download-and-recovery.md) | | `testing.scripted-playout-golden-deferred` | The `PlayoutBuildGoldenTests` in-memory golden net covers Sequential (YAML) as of #381. Scripted's *end-to-end pipeline* is excluded — `ScriptedPlayoutBuilder` runs a user-authored external program that drives the engine over HTTP loopback, which the in-memory harness can't pin — so that full-pipeline (integration) harness is deferred to #563. But the scheduling *behavior* those scripts drive lives entirely in the in-process `SchedulingEngine` (the `ScriptedScheduleController` is a 1:1 pass-through to it), which IS directly unit/golden-testable; the earlier "Scripted is un-golden-able by construction" framing overstated the constraint by conflating transport with engine. #395 extracts that shared switch to `ContentEnumeratorBuilder` and adds a direct regression net (`ContentEnumeratorBuilderTests`) over it. | 2026-07-22 | [link](records/testing/scripted-playout-golden-deferred.md) | | `testing.troubleshoot-path-cannot-test-branding` | Verify logo/watermark/bug changes through a real channel playout — a green troubleshoot run proves nothing about branding. | 2026-07-21 | [link](records/testing/troubleshoot-path-cannot-test-branding.md) | -| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: every `.claude/hooks/*.sh` and `.husky/pre-push` is operator-run and CI never fires them AS HOOKS, yet all carry `MUTATION` rows, because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those already-proven guards into the exempt branch. "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives every protocol branch it has — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | +| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives every protocol branch it has — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | ## Review due diff --git a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md index 76b4ff05f..fd85679e6 100644 --- a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md +++ b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md @@ -5,7 +5,7 @@ status: active since: '2026-08-28' supersedes: none superseded-by: none -rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: every `.claude/hooks/*.sh` and `.husky/pre-push` is operator-run and CI never fires them AS HOOKS, yet all carry `MUTATION` rows, because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those already-proven guards into the exempt branch. "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives every protocol branch it has — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' +rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives every protocol branch it has — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' signals: 'the verification harness is itself unproven code · a false green in the checker rather than the checked · `[ -x "$cmd" ]` is true for a DIRECTORY · perl alarm/exec exits 0 without `or die` · macOS has no `timeout` and no `gtimeout` · smoke test accepts an impostor server · a server that pre-answers a guessable id · each fix created the next round''s defect · stop and subtract rather than add a layer · hermetically drivable is the test, not does-CI-run-it · a checker is not exempt because its CALLER cannot run in CI · the checker needs no inventory row — name the proof test as `guard` and the checker as `target` · a transitively-reached script is rejected as a PHANTOM · witnessed negative control for what cannot be driven · paths: `scripts/mcp_smoke.py`, `scripts/tests/test_mcp_smoke.py`, `scripts/check-local-lsp.sh`, `scripts/tests/mutation_manifest.py`, `docs/guard-inventory.md` · issues: #796, #793, #790, #785, #777, #776' mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-28: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; the `or die` form exits 2 and says why). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and an absent `gtimeout` exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps travel with it: `[ -x "$cmd" ]` does not establish that a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. The single in-repo call site carrying all three is `scripts/check-local-lsp.sh`.' --- @@ -31,8 +31,8 @@ under scope limit item 6. That scoping is what keeps the inventory from becoming rubber-stamped; what was missing is the claim that the obligation follows the *verdict*, not the *file*. **The discriminator, stated precisely because the obvious one is wrong.** It is not "does CI run -this": CI never fires `.claude/hooks/*.sh` or `.husky/pre-push` as hooks either, and all of them -carry `MUTATION` rows, because a pytest proof drives them in a sandbox. The question is whether a +this": CI never fires `.claude/hooks/*.sh` or `.husky/pre-push` as hooks either, yet four of them +are graded `MUTATION`, because a pytest proof drives them in a sandbox. The question is whether a proof test can drive the construct *hermetically*, and "it needs a live server" usually fails that test rather than passing it. `mcp_smoke.py` takes its config path and server name as positional arguments, so `scripts/tests/test_mcp_smoke.py` hands it a synthetic config pointing at a stub -- 2.47.3 From cd9f24a67c1d854b1935f87f00c98243168a96ee Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 20:32:17 +0200 Subject: [PATCH 4/9] =?UTF-8?q?fix(796):=20round=202=20=E2=80=94=20the=20r?= =?UTF-8?q?uff=20red,=20and=20the=20twin=20that=20turned=20out=20not=20to?= =?UTF-8?q?=20be=20one?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cold re-review round 2. One finding was a genuine CI red I should have caught locally; the rest are overclaims in prose. One of its findings dissolved under measurement into something more useful. **The red.** `ruff format --check` failed on `scripts/tests/test_mcp_smoke.py` — the only file of the 47 tracked Python files that did. I never ran ruff on a file I wrote, which is a standing rule and not a subtle one. Formatted; the whole population is clean and `ruff check` passes. **The twin that was not a twin.** The review flagged `id_tools` as an unproven sibling clause: mutating it leaves every case green, while the inventory advertised the guard as catching "an answer pre-answered before the request". Measured on isolated copies, three ways: - `id_tools = 2` alone -> all green - reader retention clause alone -> all green - BOTH -> rc 0, the false green So the tools stage is held by TWO mechanisms that mask each other — the reader keeps only the reply matching the id in flight, so a `tools/list` frame emitted while `initialize` is pending is dropped whatever its id. That is #685's shape, benign here but worth naming. `id_init` is the one place a pre-answer is SINGLY exploitable, which is why it stays the declared clause. The tools case is kept as behavioural coverage with a docstring that states the measurement instead of the mechanism I had assumed — the first version of it narrated a vulnerability that does not exist, which is the defect this issue is about, one level in. **The absence-assertion.** The review demonstrated that gutting the stub so it emits NOTHING still satisfied the pre-answer test, because a silent server also yields rc 9. The stub now drops a marker file once it has emitted, and the test asserts it — so the case cannot pass without having posed the attack it narrates. **Anti-vacuity is now a dependency, not a convention.** The positive control moved into a module fixture the refusal tests depend on; a node-id selection or `-k` can no longer skip it. Prose corrections: coverage stated as six named refusal paths rather than "every protocol branch" (the config/command/spawn stages are not driven); "the defect history sits on the provable side" conceded — two of the five defects were in `check-local-lsp.sh`, the file this record puts in the exempt branch; the wall-clock timing dropped for the invariant; and the `mechanics` claim that one call site carries all three traps narrowed to the two it actually carries. The `GUARD` row citing itself is now recorded as a deliberate vocabulary stretch rather than left to look routine. Verification: 1104 passed, 2 skipped; ruff format+check clean over all 47 tracked Python files; decisions-validate OK. Refs: #796 Co-Authored-By: Claude Opus 5 (1M context) --- docs/decisions/README.md | 2 +- .../verification-code-needs-its-own-proof.md | 15 +- docs/guard-inventory.md | 7 +- scripts/tests/mutation_manifest.py | 10 +- scripts/tests/test_mcp_smoke.py | 150 +++++++++++++----- 5 files changed, 137 insertions(+), 47 deletions(-) diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 56bece834..746646cc3 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -218,7 +218,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `testing.playwright-mcp-download-and-recovery` | In Playwright-MCP E2E, fetch file-download endpoints with curl — never a browser tab or `window.open` — and if browser tools stall repeatedly, `pkill -f ms-playwright-mcp` and drive a fresh session. | 2026-07-21 | [link](records/testing/playwright-mcp-download-and-recovery.md) | | `testing.scripted-playout-golden-deferred` | The `PlayoutBuildGoldenTests` in-memory golden net covers Sequential (YAML) as of #381. Scripted's *end-to-end pipeline* is excluded — `ScriptedPlayoutBuilder` runs a user-authored external program that drives the engine over HTTP loopback, which the in-memory harness can't pin — so that full-pipeline (integration) harness is deferred to #563. But the scheduling *behavior* those scripts drive lives entirely in the in-process `SchedulingEngine` (the `ScriptedScheduleController` is a 1:1 pass-through to it), which IS directly unit/golden-testable; the earlier "Scripted is un-golden-able by construction" framing overstated the constraint by conflating transport with engine. #395 extracts that shared switch to `ContentEnumeratorBuilder` and adds a direct regression net (`ContentEnumeratorBuilderTests`) over it. | 2026-07-22 | [link](records/testing/scripted-playout-golden-deferred.md) | | `testing.troubleshoot-path-cannot-test-branding` | Verify logo/watermark/bug changes through a real channel playout — a green troubleshoot run proves nothing about branding. | 2026-07-21 | [link](records/testing/troubleshoot-path-cannot-test-branding.md) | -| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives every protocol branch it has — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | +| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives SIX of its refusal paths — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. That is a subset, not every branch: the config, command-resolution and spawn stages are not driven, and the test file lists what it covers rather than implying the rest. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | ## Review due diff --git a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md index fd85679e6..732519c6e 100644 --- a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md +++ b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md @@ -5,9 +5,9 @@ status: active since: '2026-08-28' supersedes: none superseded-by: none -rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives every protocol branch it has — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' +rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives SIX of its refusal paths — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. That is a subset, not every branch: the config, command-resolution and spawn stages are not driven, and the test file lists what it covers rather than implying the rest. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' signals: 'the verification harness is itself unproven code · a false green in the checker rather than the checked · `[ -x "$cmd" ]` is true for a DIRECTORY · perl alarm/exec exits 0 without `or die` · macOS has no `timeout` and no `gtimeout` · smoke test accepts an impostor server · a server that pre-answers a guessable id · each fix created the next round''s defect · stop and subtract rather than add a layer · hermetically drivable is the test, not does-CI-run-it · a checker is not exempt because its CALLER cannot run in CI · the checker needs no inventory row — name the proof test as `guard` and the checker as `target` · a transitively-reached script is rejected as a PHANTOM · witnessed negative control for what cannot be driven · paths: `scripts/mcp_smoke.py`, `scripts/tests/test_mcp_smoke.py`, `scripts/check-local-lsp.sh`, `scripts/tests/mutation_manifest.py`, `docs/guard-inventory.md` · issues: #796, #793, #790, #785, #777, #776' -mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-28: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; the `or die` form exits 2 and says why). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and an absent `gtimeout` exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps travel with it: `[ -x "$cmd" ]` does not establish that a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. The single in-repo call site carrying all three is `scripts/check-local-lsp.sh`.' +mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-28: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; the `or die` form exits 2 and says why). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and an absent `gtimeout` exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps travel with it: `[ -x "$cmd" ]` does not establish that a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. `scripts/check-local-lsp.sh` is the single in-repo site carrying the perl recipe and the check-the-exit-code discipline; the `-x` trap appears there only in a comment recording what it deliberately does not do.' --- **The measurement.** #793 added one small operator-run check — a smoke test that starts the @@ -36,12 +36,15 @@ are graded `MUTATION`, because a pytest proof drives them in a sandbox. The ques proof test can drive the construct *hermetically*, and "it needs a live server" usually fails that test rather than passing it. `mcp_smoke.py` takes its config path and server name as positional arguments, so `scripts/tests/test_mcp_smoke.py` hands it a synthetic config pointing at a stub -responder and drives every protocol branch — identity mismatch, a body carrying only an id, a -missing tool, a pre-answered request — in under eight seconds. `check-local-lsp.sh` genuinely +responder and drives six of its refusal paths — identity mismatch, a body carrying only an id, a +missing tool, a pre-answered `initialize`, a pre-answered `tools/list` — with no server to start. +That is a subset and the file says so. `check-local-lsp.sh` genuinely cannot be driven that way: it asserts a global `csharp-ls` on PATH, a `host/fxr` under `DOTNET_ROOT`, and a root `node_modules`, which is developer-machine state no runner has. That, and -not "it is operator-run", is what puts it in the exempt branch — and the part of the construct -carrying the defect history sits on the provable side of the seam. +not "it is operator-run", is what puts it in the exempt branch. Note what that concedes: two of +the five defects below (the `-x` test and the `perl` wrapper) were in `check-local-lsp.sh` itself, +so the exempt branch is not defect-free — it is the part where a recorded control is the best +available proof. Three of the five were in `mcp_smoke.py`, and those are the ones now pinned. **Where the proof lives, and why the checker needs no row.** `docs/guard-inventory.md`'s population is derived from workflow and hook call sites, so a row for `mcp_smoke.py` is rejected outright — diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index 909d28549..52598e3ea 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -75,6 +75,11 @@ inventory shipped with were sitting in the gap: derived from the call sites, so one is rejected as a phantom — but the manifest can still name the checker, as `target`, with its proof test as `guard`. `scripts/mcp_smoke.py` is the worked example: no row of its own, proven by `scripts/tests/test_mcp_smoke.py`, which carries the row. + That row is `GUARD` citing itself, which stretches the vocabulary and is recorded rather than + waved through: the self-citation carve-out in `test_every_proof_ref_points_at_a_row_marked_PROOF` + is worded for a checker guarding a repo invariant, whereas this one guards another script. It is + admitted because that script holds no row to cite — the same shape as item 7's note that a row + for a library would need a `Kind` the vocabulary does not have. 7. **Non-`test_` modules under `scripts/tests/`** — the pattern is `test_*.py`, so `conftest.py`, `mutation_harness_lib.py`, `mutation_manifest.py` and `tracked_files.py` are outside the population and hold no rows. They are not guards (they assert nothing on their own), but the @@ -216,7 +221,7 @@ recorded as unexamined rather than as cleared. | `scripts/tests/test_guard_populations_derive_from_git.py` | the `script-tests` job, on a guard whose file population admits a file git does not track | GUARD | MUTATION | `test_guard_populations_derive_from_git.py::test_no_derivation_admits_an_untracked_file` | | `scripts/tests/test_hook_fire_log.py` | the `script-tests` job, on a hook that stops reporting that it fired, or whose reporting changes what the harness sees | GUARD | MUTATION | `test_hook_fire_log.py::test_a_hook_that_LOSES_its_instrumentation_is_DETECTED` | | `scripts/tests/test_jq_preflight.py` | the `script-tests` job | PROOF | NONE | — | -| `scripts/tests/test_mcp_smoke.py` | the `script-tests` job, when the MCP smoke checker accepts a server it should refuse — an impostor identity, a reply carrying only an id, a missing expected tool, or an answer pre-answered before the request | GUARD | MUTATION | `test_mcp_smoke.py::test_MUTATION_a_PRE_ANSWERED_id_is_refused_because_the_request_ids_are_UNGUESSABLE` | +| `scripts/tests/test_mcp_smoke.py` | the `script-tests` job, when the MCP smoke checker accepts a server it should refuse — an impostor identity, a reply carrying only an id, a missing expected tool, or an `initialize` pre-answered before the request (the `tools/list` twin is held by two mechanisms jointly and is covered behaviourally, not by the declared clause) | GUARD | MUTATION | `test_mcp_smoke.py::test_MUTATION_a_PRE_ANSWERED_id_is_refused_because_the_request_ids_are_UNGUESSABLE` | | `scripts/tests/test_merge_consent_base_change.py` | the `script-tests` job | PROOF | NONE | — | | `scripts/tests/test_merge_consent_exemption.py` | the `script-tests` job | PROOF | NONE | — | | `scripts/tests/test_merge_consent_required_check.py` | the `script-tests` job | PROOF | NONE | — | diff --git a/scripts/tests/mutation_manifest.py b/scripts/tests/mutation_manifest.py index caeb94ee2..6a2fb4f34 100644 --- a/scripts/tests/mutation_manifest.py +++ b/scripts/tests/mutation_manifest.py @@ -327,7 +327,15 @@ MUTATIONS: tuple[Mutation, ...] = ( "round 5 replaced the lock rather than tightening it. Disarmed, the stub's pre-answer is " "accepted at `initialize` and the run dies one stage later at `tools/list`, so the proof " "asserts the STAGE (rc 9 and the initialize diagnostic) rather than mere failure: the mutant " - "still fails, and a test checking only that would stay green over a real vulnerability.", + "still fails, and a test checking only that would stay green over a real vulnerability. THE " + "`id_tools` TWIN IS DELIBERATELY NOT THIS CLAUSE, measured rather than assumed: replacing " + "`id_tools` alone leaves every case green, because the reader keeps only the reply matching " + "the id in flight, so a `tools/list` frame emitted while `initialize` is pending is dropped " + "whatever its id. Disarming that retention clause alone is green too. Only BOTH together " + "produce the false green (rc 0), so the tools stage is held by two mechanisms that mask each " + "other and neither is singly detectable — #685's shape. `id_init` is the one place a " + "pre-answer IS singly exploitable, which is why it is the declared clause; the tools stage is " + "covered behaviourally in the same file.", ), ) diff --git a/scripts/tests/test_mcp_smoke.py b/scripts/tests/test_mcp_smoke.py index c67b53667..e84abe466 100644 --- a/scripts/tests/test_mcp_smoke.py +++ b/scripts/tests/test_mcp_smoke.py @@ -15,11 +15,17 @@ file joins the population as an ordinary `scripts/tests/test_*.py` and carries t manifest entry names it as `guard` with `scripts/mcp_smoke.py` as `target` — the same shape `test_mutation_harness.py` uses for `mutation_harness_lib.py`. -HERMETIC BY CONSTRUCTION, which is the point that makes the proof possible at all. `mcp_smoke.py` -takes its config path and server name as POSITIONAL ARGUMENTS, so nothing here touches the -gitignored `.mcp.json` or starts a language server: every case below drives it against a synthetic -config in a tmpdir pointing at the stub in `_STUB`. "It needs a real server" exempts the -INTEGRATION — `check-local-lsp.sh` driving the actual csharp-lsp — never this protocol logic. +HERMETIC BY CONSTRUCTION, which is what makes the proof possible at all. `mcp_smoke.py` takes its +config path and server name as POSITIONAL ARGUMENTS, so nothing here touches the gitignored +`.mcp.json` or starts a language server: every case below drives it against a synthetic config in a +tmpdir pointing at the stub in `_STUB`. "It needs a real server" exempts the INTEGRATION — +`check-local-lsp.sh` driving the actual csharp-lsp — never this protocol logic. + +WHAT IS COVERED, stated as a list rather than as "every branch", because the honest figure is a +subset: the success path, and the refusals for a pre-answered `initialize` (9), a pre-answered +`tools/list` (10), an `initialize` reply with no `result` (12), a wrong `serverInfo.name` (13) and a +missing expected tool (14). `mcp_smoke.py` has other `fail()` sites — the config, command-resolution +and spawn stages, and several further malformed shapes — and they are NOT driven here. NOT TESTED, DELIBERATELY: the unbounded raw line buffer. `mcp_smoke.py` states that as an ACCEPTED LIMIT, because the caps that would bound it were themselves defect 4. A test pushing toward @@ -33,30 +39,39 @@ import subprocess import sys from pathlib import Path +import pytest + REPO_ROOT = Path(__file__).resolve().parents[2] SMOKE = REPO_ROOT / "scripts" / "mcp_smoke.py" -# The stub speaks just enough JSON-RPC to exercise each branch. `preanswer` is the security case: -# it emits replies for the LOW, GUESSABLE ids a predictable client would use, BEFORE reading -# anything, and then goes silent — so it can only be accepted by a client whose ids can be guessed. -_STUB = r''' +# The stub speaks just enough JSON-RPC to exercise each case. The two `preanswer` arms are the +# security cases: they emit replies for the LOW, GUESSABLE ids a predictable client would use, +# BEFORE the corresponding request arrives, and then go silent — so they can only be accepted by a +# client whose ids can be guessed. Each drops a MARKER file once it has emitted, because otherwise +# a stub that silently emitted NOTHING would satisfy the same assertion: "no answer" and "the +# pre-answer was refused" are the same observation from outside, and only the marker separates them. +_STUB = r""" import json, sys, time -mode = sys.argv[1] +mode, marker = sys.argv[1], sys.argv[2] def emit(obj): sys.stdout.write(json.dumps(obj) + "\n") sys.stdout.flush() +def mark(): + with open(marker, "w") as fh: + fh.write("emitted\n") + def init_result(name="stub-mcp"): return {"serverInfo": {"name": name, "version": "0.1"}, "capabilities": {}} def tools_result(names): return {"tools": [{"name": n} for n in names]} -if mode == "preanswer": +if mode == "preanswer_init": emit({"jsonrpc": "2.0", "id": 1, "result": init_result()}) - emit({"jsonrpc": "2.0", "id": 2, "result": tools_result(["alpha", "beta"])}) + mark() time.sleep(30) sys.exit(0) @@ -76,19 +91,26 @@ for line in sys.stdin: emit({"jsonrpc": "2.0", "id": mid}) else: emit({"jsonrpc": "2.0", "id": mid, "result": init_result()}) + if mode == "preanswer_tools": + # answer initialize honestly, then pre-answer the tools/list id we hope to guess + emit({"jsonrpc": "2.0", "id": 2, "result": tools_result(["alpha", "beta"])}) + mark() elif method == "tools/list": + if mode == "preanswer_tools": + continue # already "answered" it above, at a guessed id names = ["alpha"] if mode == "missing_tool" else ["alpha", "beta"] emit({"jsonrpc": "2.0", "id": mid, "result": tools_result(names)}) -''' +""" -def _run(tmp_path, mode: str, *extra: str, timeout: int = 5): - """Drive `mcp_smoke.py` against the stub in `mode`. Returns (returncode, combined output).""" +def _run(tmp_path: Path, mode: str, *extra: str, timeout: int = 5): + """Drive `mcp_smoke.py` against the stub in `mode`. Returns (rc, output, marker_path).""" stub = tmp_path / "stub_server.py" stub.write_text(_STUB) + marker = tmp_path / "emitted.marker" cfg = tmp_path / "synthetic.mcp.json" cfg.write_text( - json.dumps({"mcpServers": {"stub": {"command": sys.executable, "args": [str(stub), mode]}}}) + json.dumps({"mcpServers": {"stub": {"command": sys.executable, "args": [str(stub), mode, str(marker)]}}}) ) proc = subprocess.run( [sys.executable, str(SMOKE), str(cfg), "stub", str(timeout), *extra], @@ -96,54 +118,106 @@ def _run(tmp_path, mode: str, *extra: str, timeout: int = 5): text=True, timeout=timeout + 40, ) - return proc.returncode, proc.stdout + proc.stderr + return proc.returncode, proc.stdout + proc.stderr, marker # ------------------------------------------------------------------------------------------------ -# ANTI-VACUITY FIRST. Every case below asserts a FAILURE, and a harness that cannot make the checker -# succeed would report all of them green while proving nothing about the checker's rejections. +# ANTI-VACUITY AS A DEPENDENCY, NOT A CONVENTION. Every case below asserts a REFUSAL, and a harness +# that could not make the checker SUCCEED would report all of them green while proving nothing. A +# sibling test placed first does not establish that: `-k`, a node-id selection, or the mutation +# harness running one test by id all skip it. A fixture the rejection tests DEPEND on cannot be +# skipped that way. # ------------------------------------------------------------------------------------------------ -def test_the_harness_can_make_the_checker_PASS(tmp_path): - rc, out = _run(tmp_path, "good", "--expect-server", "stub-mcp", "--expect-tool", "alpha") - assert rc == 0, f"the positive control did not pass, so every rejection below proves nothing: {out}" - assert "OK:" in out and "stub-mcp" in out, out +@pytest.fixture(scope="module") +def harness_is_live(tmp_path_factory) -> None: + rc, out, _ = _run( + tmp_path_factory.mktemp("positive_control"), + "good", + "--expect-server", + "stub-mcp", + "--expect-tool", + "alpha", + ) + assert rc == 0 and "OK:" in out and "stub-mcp" in out, ( + f"the positive control did not pass, so every refusal asserted in this file proves nothing " + f"— the stub or the config shape is broken, not the checker (rc={rc}):\n{out}" + ) -def test_an_IMPOSTOR_server_is_rejected_on_identity(tmp_path): +def test_the_harness_can_make_the_checker_PASS(harness_is_live) -> None: + """The positive control, surfaced as a test so it is visible in the report as well as depended on.""" + + +def test_an_IMPOSTOR_server_is_rejected_on_identity(harness_is_live, tmp_path) -> None: """#793 defect 2: 'it answered' is not 'the configured server answered'.""" - rc, out = _run(tmp_path, "impostor", "--expect-server", "stub-mcp") + rc, out, _ = _run(tmp_path, "impostor", "--expect-server", "stub-mcp") assert rc == 13, f"an impostor server was accepted (rc={rc}): {out}" assert "wrong server" in out, out -def test_a_reply_carrying_only_an_ID_is_rejected_as_malformed(tmp_path): - """#793 defect 2, the other half: a bare `{\"id\": N}` is a reply, not an answer.""" - rc, out = _run(tmp_path, "no_result") +def test_a_reply_carrying_only_an_ID_is_rejected_as_malformed(harness_is_live, tmp_path) -> None: + """#793 defect 2, the other half: a bare `{"id": N}` is a reply, not an answer.""" + rc, out, _ = _run(tmp_path, "no_result") assert rc == 12, f"a body with no 'result' was accepted (rc={rc}): {out}" assert "no 'result' object" in out, out -def test_a_MISSING_expected_tool_is_reported(tmp_path): - rc, out = _run(tmp_path, "missing_tool", "--expect-tool", "alpha", "--expect-tool", "beta") +def test_a_MISSING_expected_tool_is_reported(harness_is_live, tmp_path) -> None: + rc, out, _ = _run(tmp_path, "missing_tool", "--expect-tool", "alpha", "--expect-tool", "beta") assert rc == 14, f"a server missing an expected tool was accepted (rc={rc}): {out}" assert "missing expected tool" in out, out -def test_MUTATION_a_PRE_ANSWERED_id_is_refused_because_the_request_ids_are_UNGUESSABLE(tmp_path): +def test_MUTATION_a_PRE_ANSWERED_id_is_refused_because_the_request_ids_are_UNGUESSABLE( + harness_is_live, tmp_path +) -> None: """#793 defect 5, and the declared clause mutation for this guard. - The stub answers ids 1 and 2 before reading anything, then goes silent. With random request ids - the pre-answer cannot match, so `initialize` times out and the run is refused at THAT stage. - Replace `id_init = secrets.randbelow(...)` with `id_init = 1` and the pre-answer is accepted: - the run gets past identity and dies later, at tools/list — a different stage and a different - diagnostic, which is what turns this assertion red. Asserting the STAGE rather than merely - 'it failed' is what makes the mutation visible; the mutant still fails, just not here. + The stub answers id 1 before reading anything, then goes silent. With random request ids the + pre-answer cannot match, so `initialize` times out and the run is refused at THAT stage. Replace + `id_init = secrets.randbelow(...)` with `id_init = 1` and the pre-answer is accepted: the run + gets past identity and dies one stage later, at `tools/list`. Asserting the STAGE rather than + merely 'it failed' is what makes the mutation visible — the mutant still fails, just not here. + + The marker assertion is not decoration. Without it this test is satisfied by a stub that emits + NOTHING AT ALL, since a silent server also produces rc 9 — an absence standing in for the + refusal the test claims to observe. """ - rc, out = _run(tmp_path, "preanswer", "--expect-server", "stub-mcp") + rc, out, marker = _run(tmp_path, "preanswer_init", "--expect-server", "stub-mcp") + assert marker.exists(), ( + "the stub never emitted its pre-answer, so this case did not pose the attack it narrates: " + "rc 9 below would only mean the server said nothing." + ) assert rc == 9 and "no 'initialize' response" in out, ( f"a pre-answered id was ACCEPTED: the checker got past `initialize` against a server that " f"replied before it was asked (rc={rc}). Unguessable request ids are the only thing closing " f"that race — a lock cannot reject a frame emitted before the request.\n{out}" ) + + +def test_a_PRE_ANSWERED_tools_list_is_refused_by_TWO_mechanisms_TOGETHER(harness_is_live, tmp_path) -> None: + """The `tools/list` twin of the case above — and the measurement that explains why `id_tools` is + NOT this guard's declared clause. + + `mcp_smoke.py` draws two unguessable ids, and only `id_init` is singly load-bearing. Measured on + an isolated copy: replacing `id_tools` alone leaves every case here green, because the reader + retains only the reply matching the id currently in flight (`if rid != pending ... continue`), so + a `tools/list` frame emitted while `initialize` is still pending is dropped whatever its id. + Disarming that retention clause alone is likewise green, because the real `id_tools` is still + unguessable. Disarm BOTH and this case goes red at rc 0 — the checker accepts a tool list at an + id it never requested. + + So the tools stage is held by two mechanisms that mask each other (#685's shape, benign here but + worth naming): neither is detectable alone, which is exactly why the declared clause is the + `initialize` id, where the pre-answer IS singly exploitable. This case is behavioural coverage + for the pair, not a second mutation proof. + """ + rc, out, marker = _run(tmp_path, "preanswer_tools", "--expect-server", "stub-mcp", "--expect-tool", "alpha") + assert marker.exists(), "the stub never emitted its pre-answered tools/list, so this case posed nothing" + assert rc == 10 and "no 'tools/list' response" in out, ( + f"a pre-answered tools/list was ACCEPTED: the checker took a tool list at an id it never " + f"requested (rc={rc}). That needs BOTH the pending-registration and an unguessable " + f"`id_tools`; this went green with one of them gone.\n{out}" + ) -- 2.47.3 From ea0a9fd2b4e1e7c5b21b218513eb9f192edb62db Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 20:37:18 +0200 Subject: [PATCH 5/9] docs(796): three prose imprecisions, self-caught by re-auditing every claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not review findings — a systematic pass over every factual claim the record makes, checking each against the file it describes rather than against my memory of it. 1. The record said defects 1 and 3 were "a `perl` wrapper inside an `if`, which is not a file at all". Defect 1 was a `[ -x "$cmd" ]` test, not the wrapper, and both live in `scripts/check-local-lsp.sh` — which IS a file. The point survives and is now stated correctly: that file is outside the population because no workflow or hook invokes it, and `mcp_smoke.py` because it is reached only through it (scope limit item 6). 2. "drives SIX of its refusal paths" — the file has six test functions, of which one is the positive control. It drives FIVE refusals plus the success path. A count taken from the wrong denominator, which is the coverage-overclaim shape round 2 had just corrected elsewhere in the same paragraph. 3. `docs/README.md` called this "the four rules every guard must satisfy". It is not a fourth guard rule — its whole content is that the obligation extends BEYOND guards, to the verification code doing the checking. Restored to three guard rules plus this one as an extension, which is also what makes the task-signal row useful to someone whose file holds no inventory row. Refs: #796 Co-Authored-By: Claude Opus 5 (1M context) --- docs/README.md | 2 +- docs/decisions/README.md | 2 +- .../testing/verification-code-needs-its-own-proof.md | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/README.md b/docs/README.md index 0a0407a62..9e9e96a4e 100644 --- a/docs/README.md +++ b/docs/README.md @@ -27,7 +27,7 @@ doc below, or that changes which sections a task signal points to.** | Concurrency / optimistic-locking work | `docs/api-conventions.md` §7a/b/c + `docs/decisions/optimistic-concurrency.md` | | Auth / security-surface work | `docs/decisions/api-auth-security.md` | | CI / release pipeline work | `docs/ci-cd.md` + `docs/decisions/release-ci-governance.md` | -| Proposing a new guard / CI check / regression test convention | `docs/defect-shapes-773.md` §4 (detector menu + the classes where no detector is plausible), then the four rules every guard must satisfy: `docs/decisions/records/testing/guard-derives-population-from-source.md`, `…/guard-ships-with-mutation-proof.md`, `…/mutation-claims-are-executed.md` (a `MUTATION` grade carries a DECLARED clause mutation that is re-run every suite) and `…/verification-code-needs-its-own-proof.md` (the obligation follows the VERDICT, so it binds the harness, wrapper or checker doing the checking — not only the files in the guard population) | +| Proposing a new guard / CI check / regression test convention | `docs/defect-shapes-773.md` §4 (detector menu + the classes where no detector is plausible), then the three rules every guard must satisfy: `docs/decisions/records/testing/guard-derives-population-from-source.md`, `…/guard-ships-with-mutation-proof.md` and `…/mutation-claims-are-executed.md` (a `MUTATION` grade carries a DECLARED clause mutation that is re-run every suite) — plus `…/verification-code-needs-its-own-proof.md`, which extends the same obligation BEYOND guards to the harness, wrapper or checker doing the checking, and says where its proof lives when the checker holds no row | | Adding or bounding a consequential numeric config field (an FFmpeg profile tunable, a pipeline knob) | `docs/api-conventions.md` §3d — reject out of range with a 422 naming the bound and its consequence, never accept-then-rewrite; validate against the constants the renderer reads, keep the render-time clamp for pre-existing rows, and let an UNCHANGED legacy value through on update. Then `api.ffmpeg-profile-numeric-bounds` | | Testing a surface gated by config / an env var / a credential | `docs/decisions/records/testing/deny-path-at-production-config-value.md` — cover the setting absent, at its production value, and each opt-out, and assert the DENY branch | | Touching a full-replace write path or a hand-built request object | `docs/decisions/records/testing/full-replace-asserts-field-list.md` — derive the field list from the DTO and assert set equality; reconcile by id where child state exists. In the SPA the same rule is enforced by the type system: `docs/spa-conventions.md` §4b — build the body as `Complete`, annotating BOTH the wrapper parameter and every construction site | diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 746646cc3..57fb3af51 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -218,7 +218,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `testing.playwright-mcp-download-and-recovery` | In Playwright-MCP E2E, fetch file-download endpoints with curl — never a browser tab or `window.open` — and if browser tools stall repeatedly, `pkill -f ms-playwright-mcp` and drive a fresh session. | 2026-07-21 | [link](records/testing/playwright-mcp-download-and-recovery.md) | | `testing.scripted-playout-golden-deferred` | The `PlayoutBuildGoldenTests` in-memory golden net covers Sequential (YAML) as of #381. Scripted's *end-to-end pipeline* is excluded — `ScriptedPlayoutBuilder` runs a user-authored external program that drives the engine over HTTP loopback, which the in-memory harness can't pin — so that full-pipeline (integration) harness is deferred to #563. But the scheduling *behavior* those scripts drive lives entirely in the in-process `SchedulingEngine` (the `ScriptedScheduleController` is a 1:1 pass-through to it), which IS directly unit/golden-testable; the earlier "Scripted is un-golden-able by construction" framing overstated the constraint by conflating transport with engine. #395 extracts that shared switch to `ContentEnumeratorBuilder` and adds a direct regression net (`ContentEnumeratorBuilderTests`) over it. | 2026-07-22 | [link](records/testing/scripted-playout-golden-deferred.md) | | `testing.troubleshoot-path-cannot-test-branding` | Verify logo/watermark/bug changes through a real channel playout — a green troubleshoot run proves nothing about branding. | 2026-07-21 | [link](records/testing/troubleshoot-path-cannot-test-branding.md) | -| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives SIX of its refusal paths — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. That is a subset, not every branch: the config, command-resolution and spawn stages are not driven, and the test file lists what it covers rather than implying the rest. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | +| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives FIVE of its refusal paths plus the success path — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. That is a subset, not every branch: the config, command-resolution and spawn stages are not driven, and the test file lists what it covers rather than implying the rest. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | ## Review due diff --git a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md index 732519c6e..cc67cf60e 100644 --- a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md +++ b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md @@ -5,7 +5,7 @@ status: active since: '2026-08-28' supersedes: none superseded-by: none -rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives SIX of its refusal paths — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. That is a subset, not every branch: the config, command-resolution and spawn stages are not driven, and the test file lists what it covers rather than implying the rest. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' +rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives FIVE of its refusal paths plus the success path — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. That is a subset, not every branch: the config, command-resolution and spawn stages are not driven, and the test file lists what it covers rather than implying the rest. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' signals: 'the verification harness is itself unproven code · a false green in the checker rather than the checked · `[ -x "$cmd" ]` is true for a DIRECTORY · perl alarm/exec exits 0 without `or die` · macOS has no `timeout` and no `gtimeout` · smoke test accepts an impostor server · a server that pre-answers a guessable id · each fix created the next round''s defect · stop and subtract rather than add a layer · hermetically drivable is the test, not does-CI-run-it · a checker is not exempt because its CALLER cannot run in CI · the checker needs no inventory row — name the proof test as `guard` and the checker as `target` · a transitively-reached script is rejected as a PHANTOM · witnessed negative control for what cannot be driven · paths: `scripts/mcp_smoke.py`, `scripts/tests/test_mcp_smoke.py`, `scripts/check-local-lsp.sh`, `scripts/tests/mutation_manifest.py`, `docs/guard-inventory.md` · issues: #796, #793, #790, #785, #777, #776' mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-28: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; the `or die` form exits 2 and says why). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and an absent `gtimeout` exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps travel with it: `[ -x "$cmd" ]` does not establish that a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. `scripts/check-local-lsp.sh` is the single in-repo site carrying the perl recipe and the check-the-exit-code discipline; the `-x` trap appears there only in a comment recording what it deliberately does not do.' --- @@ -25,9 +25,9 @@ reproduced with a control before being fixed: **Why this is a separate rule and not an application of #775.** `testing.guard-ships-with-mutation- proof` is scoped by a population — the guard files `docs/guard-inventory.md` derives and grades. -Defect 1 and 3 are a `perl` wrapper inside an `if`, which is not a file at all; 2, 4 and 5 live in -`scripts/mcp_smoke.py`, which *is* a file but is reached only through another script and so falls -under scope limit item 6. That scoping is what keeps the inventory from becoming a chore that gets +Defects 1 and 3 are a `[ -x ]` test and a `perl` wrapper inside `scripts/check-local-lsp.sh`, which +no workflow or hook invokes; 2, 4 and 5 live in `scripts/mcp_smoke.py`, reached only through that +script and so excluded by scope limit item 6. Neither file is in the population. That scoping is what keeps the inventory from becoming a chore that gets rubber-stamped; what was missing is the claim that the obligation follows the *verdict*, not the *file*. **The discriminator, stated precisely because the obvious one is wrong.** It is not "does CI run @@ -36,7 +36,7 @@ are graded `MUTATION`, because a pytest proof drives them in a sandbox. The ques proof test can drive the construct *hermetically*, and "it needs a live server" usually fails that test rather than passing it. `mcp_smoke.py` takes its config path and server name as positional arguments, so `scripts/tests/test_mcp_smoke.py` hands it a synthetic config pointing at a stub -responder and drives six of its refusal paths — identity mismatch, a body carrying only an id, a +responder and drives five of its refusal paths plus the success path — identity mismatch, a body carrying only an id, a missing tool, a pre-answered `initialize`, a pre-answered `tools/list` — with no server to start. That is a subset and the file says so. `check-local-lsp.sh` genuinely cannot be driven that way: it asserts a global `csharp-ls` on PATH, a `host/fxr` under -- 2.47.3 From 42bb3f146e2131b2a30c89ce1a8d2fdc4d23d15d Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 21:03:12 +0200 Subject: [PATCH 6/9] =?UTF-8?q?docs(796):=20round=203=20=E2=80=94=20subtra?= =?UTF-8?q?ct=20the=20counts=20instead=20of=20correcting=20them=20again?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3 fired this record's own stop-and-subtract trigger, and the useful part of the review was identifying WHICH layer to delete. Round 2's findings lived in the new test file; every round-3 finding lives in record prose, and each is a hand-maintained QUANTITY or NOVELTY claim that round 2 introduced while correcting a different overclaim. Both counts round 2 wrote were off by one, in the same direction. The test file itself survived six independent attacks this round with zero findings. So the layer being deleted is the counts, not the proof. - The coverage count is GONE rather than corrected a third time. The test file's docstring already enumerates what it drives and names what it does not, so the record points at that one list instead of keeping a number in step with it. This is `state-the-invariant-not-the-measurement`, owed to a record whose subject is unmeasured verification claims. Corroborating: the one place this was stated by ENUMERATION rather than by count — the inventory's `Blocks` column — was correct throughout. - "Three of the five were in `mcp_smoke.py`, and those are the ones now pinned" was false for defect 4. Reintroducing its shape (a line-cap chunker whose overflow suffix re-parses as a fresh message) leaves the suite green, by design: the file records that territory as deliberately uncovered, because the caps that would bound it WERE the defect. Now says defects 2 and 5 are pinned and why 4 is not. - The inventory's "stretches the vocabulary" overstated novelty. Measured: 10 pre-existing rows self-cite, and `test_mutation_harness.py` is already a self-citing GUARD whose declared `target` is another script. This is the second row of that shape, not a new one. Two further round-3 findings were already fixed in ea0a9fd2b before the review landed (the SIX/FIVE count and the defect-1-is-not-a-perl-wrapper claim); its independent instrumentation of `fail()`'s call sites confirms those fixes land on the right five sites. Also caught by the decisions guard on the way through: a bare apostrophe inside the single-quoted `rule:` scalar, which PyYAML rejects. Doubled. Verification: 1104 passed, 2 skipped; ruff format+check clean over all 47 tracked Python files; decisions-validate OK. Refs: #796 Co-Authored-By: Claude Opus 5 (1M context) --- docs/decisions/README.md | 2 +- .../testing/verification-code-needs-its-own-proof.md | 11 ++++++----- docs/guard-inventory.md | 10 +++++----- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 57fb3af51..47322a126 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -218,7 +218,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `testing.playwright-mcp-download-and-recovery` | In Playwright-MCP E2E, fetch file-download endpoints with curl — never a browser tab or `window.open` — and if browser tools stall repeatedly, `pkill -f ms-playwright-mcp` and drive a fresh session. | 2026-07-21 | [link](records/testing/playwright-mcp-download-and-recovery.md) | | `testing.scripted-playout-golden-deferred` | The `PlayoutBuildGoldenTests` in-memory golden net covers Sequential (YAML) as of #381. Scripted's *end-to-end pipeline* is excluded — `ScriptedPlayoutBuilder` runs a user-authored external program that drives the engine over HTTP loopback, which the in-memory harness can't pin — so that full-pipeline (integration) harness is deferred to #563. But the scheduling *behavior* those scripts drive lives entirely in the in-process `SchedulingEngine` (the `ScriptedScheduleController` is a 1:1 pass-through to it), which IS directly unit/golden-testable; the earlier "Scripted is un-golden-able by construction" framing overstated the constraint by conflating transport with engine. #395 extracts that shared switch to `ContentEnumeratorBuilder` and adds a direct regression net (`ContentEnumeratorBuilderTests`) over it. | 2026-07-22 | [link](records/testing/scripted-playout-golden-deferred.md) | | `testing.troubleshoot-path-cannot-test-branding` | Verify logo/watermark/bug changes through a real channel playout — a green troubleshoot run proves nothing about branding. | 2026-07-21 | [link](records/testing/troubleshoot-path-cannot-test-branding.md) | -| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives FIVE of its refusal paths plus the success path — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. That is a subset, not every branch: the config, command-resolution and spawn stages are not driven, and the test file lists what it covers rather than implying the rest. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | +| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives it with no gitignored `.mcp.json` and no language server, which is how it is proven in `scripts/tests/test_mcp_smoke.py`. That coverage is a SUBSET and no count of it is kept here: the file's own docstring enumerates the cases it drives and names what it does not, so there is one list rather than a number to keep in step with it. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | ## Review due diff --git a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md index cc67cf60e..85a6fc962 100644 --- a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md +++ b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md @@ -5,7 +5,7 @@ status: active since: '2026-08-28' supersedes: none superseded-by: none -rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives FIVE of its refusal paths plus the success path — no gitignored `.mcp.json`, no language server — and it is proven that way in `scripts/tests/test_mcp_smoke.py`. That is a subset, not every branch: the config, command-resolution and spawn stages are not driven, and the test file lists what it covers rather than implying the rest. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' +rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives it with no gitignored `.mcp.json` and no language server, which is how it is proven in `scripts/tests/test_mcp_smoke.py`. That coverage is a SUBSET and no count of it is kept here: the file''s own docstring enumerates the cases it drives and names what it does not, so there is one list rather than a number to keep in step with it. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' signals: 'the verification harness is itself unproven code · a false green in the checker rather than the checked · `[ -x "$cmd" ]` is true for a DIRECTORY · perl alarm/exec exits 0 without `or die` · macOS has no `timeout` and no `gtimeout` · smoke test accepts an impostor server · a server that pre-answers a guessable id · each fix created the next round''s defect · stop and subtract rather than add a layer · hermetically drivable is the test, not does-CI-run-it · a checker is not exempt because its CALLER cannot run in CI · the checker needs no inventory row — name the proof test as `guard` and the checker as `target` · a transitively-reached script is rejected as a PHANTOM · witnessed negative control for what cannot be driven · paths: `scripts/mcp_smoke.py`, `scripts/tests/test_mcp_smoke.py`, `scripts/check-local-lsp.sh`, `scripts/tests/mutation_manifest.py`, `docs/guard-inventory.md` · issues: #796, #793, #790, #785, #777, #776' mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-28: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; the `or die` form exits 2 and says why). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and an absent `gtimeout` exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps travel with it: `[ -x "$cmd" ]` does not establish that a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. `scripts/check-local-lsp.sh` is the single in-repo site carrying the perl recipe and the check-the-exit-code discipline; the `-x` trap appears there only in a comment recording what it deliberately does not do.' --- @@ -36,15 +36,16 @@ are graded `MUTATION`, because a pytest proof drives them in a sandbox. The ques proof test can drive the construct *hermetically*, and "it needs a live server" usually fails that test rather than passing it. `mcp_smoke.py` takes its config path and server name as positional arguments, so `scripts/tests/test_mcp_smoke.py` hands it a synthetic config pointing at a stub -responder and drives five of its refusal paths plus the success path — identity mismatch, a body carrying only an id, a -missing tool, a pre-answered `initialize`, a pre-answered `tools/list` — with no server to start. -That is a subset and the file says so. `check-local-lsp.sh` genuinely +responder and drives it with no server to start. What it covers is a subset, enumerated in that +file's docstring rather than counted here. `check-local-lsp.sh` genuinely cannot be driven that way: it asserts a global `csharp-ls` on PATH, a `host/fxr` under `DOTNET_ROOT`, and a root `node_modules`, which is developer-machine state no runner has. That, and not "it is operator-run", is what puts it in the exempt branch. Note what that concedes: two of the five defects below (the `-x` test and the `perl` wrapper) were in `check-local-lsp.sh` itself, so the exempt branch is not defect-free — it is the part where a recorded control is the best -available proof. Three of the five were in `mcp_smoke.py`, and those are the ones now pinned. +available proof. Of the three in `mcp_smoke.py`, defects 2 and 5 are pinned by tests; defect 4's +territory is deliberately left uncovered, because the message caps that would bound it WERE that +defect, and a test pressing on it would argue for reintroducing them. **Where the proof lives, and why the checker needs no row.** `docs/guard-inventory.md`'s population is derived from workflow and hook call sites, so a row for `mcp_smoke.py` is rejected outright — diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index 52598e3ea..1e7821e5d 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -75,11 +75,11 @@ inventory shipped with were sitting in the gap: derived from the call sites, so one is rejected as a phantom — but the manifest can still name the checker, as `target`, with its proof test as `guard`. `scripts/mcp_smoke.py` is the worked example: no row of its own, proven by `scripts/tests/test_mcp_smoke.py`, which carries the row. - That row is `GUARD` citing itself, which stretches the vocabulary and is recorded rather than - waved through: the self-citation carve-out in `test_every_proof_ref_points_at_a_row_marked_PROOF` - is worded for a checker guarding a repo invariant, whereas this one guards another script. It is - admitted because that script holds no row to cite — the same shape as item 7's note that a row - for a library would need a `Kind` the vocabulary does not have. + That row is `GUARD` citing itself, which is the SECOND row of that shape rather than a new one: + `test_mutation_harness.py` already self-cites while its declared `target` is another script + (`mutation_harness_lib.py`, outside this population under item 7). Worth noting only because the + self-citation carve-out in `test_every_proof_ref_points_at_a_row_marked_PROOF` is worded for a + checker guarding a repo invariant, and both of these guard a script instead. 7. **Non-`test_` modules under `scripts/tests/`** — the pattern is `test_*.py`, so `conftest.py`, `mutation_harness_lib.py`, `mutation_manifest.py` and `tracked_files.py` are outside the population and hold no rows. They are not guards (they assert nothing on their own), but the -- 2.47.3 From 13001ee2dde54f80799f1c62841edde1477ad56b Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 21:13:12 +0200 Subject: [PATCH 7/9] docs(796): sweep every remaining quantity and novelty claim, and delete the ones that rot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-run sweep of the whole change for hand-maintained counts, universals and novelty claims — the class round 3 identified as this change's defect generator. It found four more, including one introduced by round 3's own fix. - **"the SECOND row of that shape"** (guard-inventory item 6) was FALSE. Round 3 correctly said my "vocabulary stretch" framing overstated novelty; my correction overstated precision in the other direction. Measured by joining the inventory's self-citing rows against the manifest: EIGHT rows are a self-citing GUARD whose declared `target` is a different file, and `test_review_verdict_vocabulary.py` -> `scripts/check-review-verdict.sh` is a closer analogue than the one I named. The ordinal is deleted rather than corrected — two examples, no count. - **"the single in-repo site carrying the perl recipe AND the check-the-exit-code discipline"** asserted a uniqueness that cannot be checked; plenty of scripts check exit codes. Narrowed to the half that is verifiable and verified: the perl recipe appears in exactly one place. - **"Every case below asserts a REFUSAL"** was false of the block it introduces — the positive control sits below that comment and asserts a pass. - **"the ONLY thing refusing a server that answers before it is asked"** was unqualified in both the test message and the manifest `why`, while round 2 had measured that the `tools/list` stage is held by two mechanisms. Both now say AT THE `initialize` STAGE, and say why the pending-registration cannot help there: that id is already in flight when the pre-answer arrives. Claims swept and confirmed TRUE rather than changed: "every one of the five defects was in code nobody had counted as a guard"; "FOUR of them carry MUTATION rows"; "NEITHER of the two shell defects"; "NONE of those controls survived as a test" (nothing but this file and the manifest references `mcp_smoke.py`); "replacing `id_tools` alone leaves every case here green". Verification: 1104 passed, 2 skipped; ruff format+check clean over all 47 tracked Python files; decisions-validate OK. Refs: #796 Co-Authored-By: Claude Opus 5 (1M context) --- .../testing/verification-code-needs-its-own-proof.md | 2 +- docs/guard-inventory.md | 11 ++++++----- scripts/tests/mutation_manifest.py | 7 ++++--- scripts/tests/test_mcp_smoke.py | 10 ++++++---- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md index 85a6fc962..c38739498 100644 --- a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md +++ b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md @@ -7,7 +7,7 @@ supersedes: none superseded-by: none rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives it with no gitignored `.mcp.json` and no language server, which is how it is proven in `scripts/tests/test_mcp_smoke.py`. That coverage is a SUBSET and no count of it is kept here: the file''s own docstring enumerates the cases it drives and names what it does not, so there is one list rather than a number to keep in step with it. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' signals: 'the verification harness is itself unproven code · a false green in the checker rather than the checked · `[ -x "$cmd" ]` is true for a DIRECTORY · perl alarm/exec exits 0 without `or die` · macOS has no `timeout` and no `gtimeout` · smoke test accepts an impostor server · a server that pre-answers a guessable id · each fix created the next round''s defect · stop and subtract rather than add a layer · hermetically drivable is the test, not does-CI-run-it · a checker is not exempt because its CALLER cannot run in CI · the checker needs no inventory row — name the proof test as `guard` and the checker as `target` · a transitively-reached script is rejected as a PHANTOM · witnessed negative control for what cannot be driven · paths: `scripts/mcp_smoke.py`, `scripts/tests/test_mcp_smoke.py`, `scripts/check-local-lsp.sh`, `scripts/tests/mutation_manifest.py`, `docs/guard-inventory.md` · issues: #796, #793, #790, #785, #777, #776' -mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-28: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; the `or die` form exits 2 and says why). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and an absent `gtimeout` exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps travel with it: `[ -x "$cmd" ]` does not establish that a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. `scripts/check-local-lsp.sh` is the single in-repo site carrying the perl recipe and the check-the-exit-code discipline; the `-x` trap appears there only in a comment recording what it deliberately does not do.' +mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-28: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; the `or die` form exits 2 and says why). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and an absent `gtimeout` exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps travel with it: `[ -x "$cmd" ]` does not establish that a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. The perl recipe appears in exactly one place in-repo, `scripts/check-local-lsp.sh`; the `-x` trap appears there only in a comment recording what it deliberately does not do.' --- **The measurement.** #793 added one small operator-run check — a smoke test that starts the diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index 1e7821e5d..641a3d456 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -75,11 +75,12 @@ inventory shipped with were sitting in the gap: derived from the call sites, so one is rejected as a phantom — but the manifest can still name the checker, as `target`, with its proof test as `guard`. `scripts/mcp_smoke.py` is the worked example: no row of its own, proven by `scripts/tests/test_mcp_smoke.py`, which carries the row. - That row is `GUARD` citing itself, which is the SECOND row of that shape rather than a new one: - `test_mutation_harness.py` already self-cites while its declared `target` is another script - (`mutation_harness_lib.py`, outside this population under item 7). Worth noting only because the - self-citation carve-out in `test_every_proof_ref_points_at_a_row_marked_PROOF` is worded for a - checker guarding a repo invariant, and both of these guard a script instead. + That row is `GUARD` citing itself while its declared `target` is a different file — an ESTABLISHED + shape here, not a new one, and deliberately not counted: `test_review_verdict_vocabulary.py` + targets `scripts/check-review-verdict.sh` and `test_mutation_harness.py` targets + `mutation_harness_lib.py`. Worth noting only because the self-citation carve-out in + `test_every_proof_ref_points_at_a_row_marked_PROOF` is worded for a checker guarding a repo + invariant, whereas these guard a script. 7. **Non-`test_` modules under `scripts/tests/`** — the pattern is `test_*.py`, so `conftest.py`, `mutation_harness_lib.py`, `mutation_manifest.py` and `tracked_files.py` are outside the population and hold no rows. They are not guards (they assert nothing on their own), but the diff --git a/scripts/tests/mutation_manifest.py b/scripts/tests/mutation_manifest.py index 6a2fb4f34..9d09040a4 100644 --- a/scripts/tests/mutation_manifest.py +++ b/scripts/tests/mutation_manifest.py @@ -322,9 +322,10 @@ MUTATIONS: tuple[Mutation, ...] = ( "`scripts/mcp_smoke.py` is a checker reached only transitively, through " "`scripts/check-local-lsp.sh`, so it holds no inventory row of its own and the clause has to " "be disarmed in it directly (`testing.verification-code-needs-its-own-proof`). The clause is " - "the unguessable request id, which is the ONLY thing refusing a server that answers before " - "it is asked — a lock cannot reject a frame emitted before the request, which is why #793 " - "round 5 replaced the lock rather than tightening it. Disarmed, the stub's pre-answer is " + "the unguessable request id, which AT THE `initialize` STAGE is the only thing refusing a " + "server that answers before it is asked: the pending-registration cannot help there, because " + "that id is already in flight when the pre-answer arrives, which is why #793 round 5 replaced " + "the lock rather than tightening it. Disarmed, the stub's pre-answer is " "accepted at `initialize` and the run dies one stage later at `tools/list`, so the proof " "asserts the STAGE (rc 9 and the initialize diagnostic) rather than mere failure: the mutant " "still fails, and a test checking only that would stay green over a real vulnerability. THE " diff --git a/scripts/tests/test_mcp_smoke.py b/scripts/tests/test_mcp_smoke.py index e84abe466..a45c8416e 100644 --- a/scripts/tests/test_mcp_smoke.py +++ b/scripts/tests/test_mcp_smoke.py @@ -122,8 +122,9 @@ def _run(tmp_path: Path, mode: str, *extra: str, timeout: int = 5): # ------------------------------------------------------------------------------------------------ -# ANTI-VACUITY AS A DEPENDENCY, NOT A CONVENTION. Every case below asserts a REFUSAL, and a harness -# that could not make the checker SUCCEED would report all of them green while proving nothing. A +# ANTI-VACUITY AS A DEPENDENCY, NOT A CONVENTION. Every case below except the positive control itself +# asserts a REFUSAL, and a harness that could not make the checker SUCCEED would report all of those +# green while proving nothing. A # sibling test placed first does not establish that: `-k`, a node-id selection, or the mutation # harness running one test by id all skip it. A fixture the rejection tests DEPEND on cannot be # skipped that way. @@ -192,8 +193,9 @@ def test_MUTATION_a_PRE_ANSWERED_id_is_refused_because_the_request_ids_are_UNGUE ) assert rc == 9 and "no 'initialize' response" in out, ( f"a pre-answered id was ACCEPTED: the checker got past `initialize` against a server that " - f"replied before it was asked (rc={rc}). Unguessable request ids are the only thing closing " - f"that race — a lock cannot reject a frame emitted before the request.\n{out}" + f"replied before it was asked (rc={rc}). At THIS stage an unguessable id is the only thing " + f"closing that race — the pending-registration cannot help, because the id is already in " + f"flight when the pre-answer arrives.\n{out}" ) -- 2.47.3 From 23624af50bac89a3c1e95143c645ce0cac7e606c Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 21:22:18 +0200 Subject: [PATCH 8/9] =?UTF-8?q?docs(796):=20round=204=20=E2=80=94=20replac?= =?UTF-8?q?e=20uniqueness=20claims=20with=20executing-site=20claims?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 4's sweep found that my round-3 fix for a false count was itself false, and that the pending replacement for a second one was false in a new way. Both are the same class this change keeps regenerating: a hand-maintained quantity or uniqueness claim about prose. - "The perl recipe appears in exactly one place in-repo" — false. `git grep 'alarm shift'` returns three hits in two files, and two of them are inside this record (the `mechanics:` line and defect-table row 3). Replaced with the claim that is both true and greppable: the only place that RUNS it is `check-local-lsp.sh:107`. An executing-site claim survives the record quoting itself; a repo-wide uniqueness claim does not. - "three defects with one cause" is not supported by the record's own table, which attributes one numbered defect to the caps. It came from #796's body, so it is not invented, but no reader can check it from here. Replaced with the invariant it was standing in for: the layer was the cause, not any clause inside it. - "each introduced by the fix for the previous one" — defect 1 was in the initial version of `check-local-lsp.sh`, so it had no predecessor. Now "each after the first". - `docs/README.md` said "the three rules every guard must satisfy" immediately before a cell listing four links. True today and rots on the next rule; the count is dropped. - The test file's list of undriven `fail()` sites read as exhaustive while omitting the usage, `--project`, initialize-returned-an-error and zero-tools stages. Now says "among them" and names those too. - `mutation_harness_lib.py` is a module, not a script; the inventory now says "another file". The round-3 finding that prompted this (the false "SECOND row of that shape") was already committed as fixed in 13001ee2d before this review landed — round 4 independently re-derived the population and confirms that correction, and that both precedents it names are real. Verification: 1104 passed, 2 skipped; ruff format+check clean over all 47 tracked Python files; decisions-validate OK; catalog up to date. Refs: #796 Co-Authored-By: Claude Opus 5 (1M context) --- docs/README.md | 2 +- .../testing/verification-code-needs-its-own-proof.md | 7 ++++--- docs/guard-inventory.md | 2 +- scripts/tests/test_mcp_smoke.py | 5 +++-- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/README.md b/docs/README.md index 9e9e96a4e..6638ccb3c 100644 --- a/docs/README.md +++ b/docs/README.md @@ -27,7 +27,7 @@ doc below, or that changes which sections a task signal points to.** | Concurrency / optimistic-locking work | `docs/api-conventions.md` §7a/b/c + `docs/decisions/optimistic-concurrency.md` | | Auth / security-surface work | `docs/decisions/api-auth-security.md` | | CI / release pipeline work | `docs/ci-cd.md` + `docs/decisions/release-ci-governance.md` | -| Proposing a new guard / CI check / regression test convention | `docs/defect-shapes-773.md` §4 (detector menu + the classes where no detector is plausible), then the three rules every guard must satisfy: `docs/decisions/records/testing/guard-derives-population-from-source.md`, `…/guard-ships-with-mutation-proof.md` and `…/mutation-claims-are-executed.md` (a `MUTATION` grade carries a DECLARED clause mutation that is re-run every suite) — plus `…/verification-code-needs-its-own-proof.md`, which extends the same obligation BEYOND guards to the harness, wrapper or checker doing the checking, and says where its proof lives when the checker holds no row | +| Proposing a new guard / CI check / regression test convention | `docs/defect-shapes-773.md` §4 (detector menu + the classes where no detector is plausible), then the rules every guard must satisfy: `docs/decisions/records/testing/guard-derives-population-from-source.md`, `…/guard-ships-with-mutation-proof.md` and `…/mutation-claims-are-executed.md` (a `MUTATION` grade carries a DECLARED clause mutation that is re-run every suite) — plus `…/verification-code-needs-its-own-proof.md`, which extends the same obligation BEYOND guards to the harness, wrapper or checker doing the checking, and says where its proof lives when the checker holds no row | | Adding or bounding a consequential numeric config field (an FFmpeg profile tunable, a pipeline knob) | `docs/api-conventions.md` §3d — reject out of range with a 422 naming the bound and its consequence, never accept-then-rewrite; validate against the constants the renderer reads, keep the render-time clamp for pre-existing rows, and let an UNCHANGED legacy value through on update. Then `api.ffmpeg-profile-numeric-bounds` | | Testing a surface gated by config / an env var / a credential | `docs/decisions/records/testing/deny-path-at-production-config-value.md` — cover the setting absent, at its production value, and each opt-out, and assert the DENY branch | | Touching a full-replace write path or a hand-built request object | `docs/decisions/records/testing/full-replace-asserts-field-list.md` — derive the field list from the DTO and assert set equality; reconcile by id where child state exists. In the SPA the same rule is enforced by the type system: `docs/spa-conventions.md` §4b — build the body as `Complete`, annotating BOTH the wrapper parameter and every construction site | diff --git a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md index c38739498..dd527a13e 100644 --- a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md +++ b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md @@ -7,12 +7,12 @@ supersedes: none superseded-by: none rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives it with no gitignored `.mcp.json` and no language server, which is how it is proven in `scripts/tests/test_mcp_smoke.py`. That coverage is a SUBSET and no count of it is kept here: the file''s own docstring enumerates the cases it drives and names what it does not, so there is one list rather than a number to keep in step with it. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' signals: 'the verification harness is itself unproven code · a false green in the checker rather than the checked · `[ -x "$cmd" ]` is true for a DIRECTORY · perl alarm/exec exits 0 without `or die` · macOS has no `timeout` and no `gtimeout` · smoke test accepts an impostor server · a server that pre-answers a guessable id · each fix created the next round''s defect · stop and subtract rather than add a layer · hermetically drivable is the test, not does-CI-run-it · a checker is not exempt because its CALLER cannot run in CI · the checker needs no inventory row — name the proof test as `guard` and the checker as `target` · a transitively-reached script is rejected as a PHANTOM · witnessed negative control for what cannot be driven · paths: `scripts/mcp_smoke.py`, `scripts/tests/test_mcp_smoke.py`, `scripts/check-local-lsp.sh`, `scripts/tests/mutation_manifest.py`, `docs/guard-inventory.md` · issues: #796, #793, #790, #785, #777, #776' -mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-28: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; the `or die` form exits 2 and says why). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and an absent `gtimeout` exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps travel with it: `[ -x "$cmd" ]` does not establish that a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. The perl recipe appears in exactly one place in-repo, `scripts/check-local-lsp.sh`; the `-x` trap appears there only in a comment recording what it deliberately does not do.' +mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-28: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; the `or die` form exits 2 and says why). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and an absent `gtimeout` exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps travel with it: `[ -x "$cmd" ]` does not establish that a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. The only place that RUNS this recipe is `scripts/check-local-lsp.sh:107` (the other in-repo occurrences of the idiom are in this record); the `-x` trap appears there only in a comment recording what it deliberately does not do.' --- **The measurement.** #793 added one small operator-run check — a smoke test that starts the `csharp-lsp` MCP server and lists its tools. Five independent cold-review rounds found five false -greens *in the verification code itself*, each introduced by the fix for the previous one and each +greens *in the verification code itself*, each after the first introduced by the fix for the one before, and each reproduced with a control before being fixed: | # | The check | Why it reported success without checking | @@ -68,7 +68,8 @@ nowhere — this repo's most expensive recurring failure (#631, #719, #751). **Stop-and-subtract.** Rounds 3→4 are the clearest case: round 3 replaced the broken wrapper, round 4 found that the caps round 3 added re-parsed an over-long line's suffix, and the fix was **deleting -those caps** — three defects with one cause. Round 5 likewise replaced lock-based ordering with +those caps** rather than guarding them — the layer was the cause, not any clause inside it. Round 5 +likewise replaced lock-based ordering with unguessable random ids: strictly less code, and it removed the race rather than policing it. Two consecutive rounds of fix-creates-the-next-defect is adopted here as the threshold; the six-round text-comparison case behind `release.verdict-vocabulary-shared` is precedent for the shape, not its diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index 641a3d456..1e1727d2f 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -80,7 +80,7 @@ inventory shipped with were sitting in the gap: targets `scripts/check-review-verdict.sh` and `test_mutation_harness.py` targets `mutation_harness_lib.py`. Worth noting only because the self-citation carve-out in `test_every_proof_ref_points_at_a_row_marked_PROOF` is worded for a checker guarding a repo - invariant, whereas these guard a script. + invariant, whereas these guard another file. 7. **Non-`test_` modules under `scripts/tests/`** — the pattern is `test_*.py`, so `conftest.py`, `mutation_harness_lib.py`, `mutation_manifest.py` and `tracked_files.py` are outside the population and hold no rows. They are not guards (they assert nothing on their own), but the diff --git a/scripts/tests/test_mcp_smoke.py b/scripts/tests/test_mcp_smoke.py index a45c8416e..fb1cad340 100644 --- a/scripts/tests/test_mcp_smoke.py +++ b/scripts/tests/test_mcp_smoke.py @@ -24,8 +24,9 @@ tmpdir pointing at the stub in `_STUB`. "It needs a real server" exempts the INT WHAT IS COVERED, stated as a list rather than as "every branch", because the honest figure is a subset: the success path, and the refusals for a pre-answered `initialize` (9), a pre-answered `tools/list` (10), an `initialize` reply with no `result` (12), a wrong `serverInfo.name` (13) and a -missing expected tool (14). `mcp_smoke.py` has other `fail()` sites — the config, command-resolution -and spawn stages, and several further malformed shapes — and they are NOT driven here. +missing expected tool (14). `mcp_smoke.py` has many other `fail()` sites — among them the usage, +config, command-resolution, `--project` and spawn stages, an `initialize` that returns an error, +zero well-formed tools, and further malformed shapes — and none of those is driven here. NOT TESTED, DELIBERATELY: the unbounded raw line buffer. `mcp_smoke.py` states that as an ACCEPTED LIMIT, because the caps that would bound it were themselves defect 4. A test pushing toward -- 2.47.3 From 1080e0bba43d114add78717f4cad1cecd398a927 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 21:42:35 +0200 Subject: [PATCH 9/9] docs(796): remove the last hand-maintained numbers, on a MERGEABLE verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 5 returned MERGEABLE and found nothing false. These are its durability findings — statements that are true today and that an unrelated future edit would quietly falsify. Taking them because each is one line and each removes a number that nothing checks; the class has cost four rounds already. - "yet FOUR of them carry MUTATION rows — " was the most exposed claim in the change: the same sentence calls the remaining hooks "unproven debt", so it invites the very upgrade that falsifies its own count, in three places (the `rule:`, the body, the mirrored catalog row), with nothing checking it. The argument needs at-least-one, not exactly-four. Now "several … among them". - "The only place that RUNS this recipe is `check-local-lsp.sh:107`" was true, but the record exists to publish that recipe for reuse — the first script to adopt it falsifies the claim, and that author has no reason to open this record. The line number rots on any edit above it. Both dropped. - "scope limit item 6"/"item 7" are positional references into another file's numbered list, bound by nothing: appending is safe, inserting silently repoints them. Now named as well as numbered. - "so there is one list rather than a number" — the inventory row's "what it blocks" cell is a second enumeration, mandated by the row format. "a list". - The defect table sits ABOVE the paragraph that said "the five defects below". - `scripts/mcp_smoke.py`'s ACCEPTED LIMIT note said the caps "caused three defects in an earlier round", which contradicts the record that is now the canonical account and counts them as one of five. Pre-existing, but this PR is what makes it a contradiction, so it is fixed here rather than left for a reader to reconcile. Docstring only — the declared mutation clause is untouched and still unique in the file. Verification: 1104 passed, 2 skipped; ruff format+check clean over all 47 tracked Python files; decisions-validate OK; catalog up to date. Refs: #796 Co-Authored-By: Claude Opus 5 (1M context) --- docs/decisions/README.md | 2 +- .../testing/verification-code-needs-its-own-proof.md | 10 +++++----- scripts/mcp_smoke.py | 2 +- scripts/tests/test_mcp_smoke.py | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 47322a126..38079c8eb 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -218,7 +218,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `testing.playwright-mcp-download-and-recovery` | In Playwright-MCP E2E, fetch file-download endpoints with curl — never a browser tab or `window.open` — and if browser tools stall repeatedly, `pkill -f ms-playwright-mcp` and drive a fresh session. | 2026-07-21 | [link](records/testing/playwright-mcp-download-and-recovery.md) | | `testing.scripted-playout-golden-deferred` | The `PlayoutBuildGoldenTests` in-memory golden net covers Sequential (YAML) as of #381. Scripted's *end-to-end pipeline* is excluded — `ScriptedPlayoutBuilder` runs a user-authored external program that drives the engine over HTTP loopback, which the in-memory harness can't pin — so that full-pipeline (integration) harness is deferred to #563. But the scheduling *behavior* those scripts drive lives entirely in the in-process `SchedulingEngine` (the `ScriptedScheduleController` is a 1:1 pass-through to it), which IS directly unit/golden-testable; the earlier "Scripted is un-golden-able by construction" framing overstated the constraint by conflating transport with engine. #395 extracts that shared switch to `ContentEnumeratorBuilder` and adds a direct regression net (`ContentEnumeratorBuilderTests`) over it. | 2026-07-22 | [link](records/testing/scripted-playout-golden-deferred.md) | | `testing.troubleshoot-path-cannot-test-branding` | Verify logo/watermark/bug changes through a real channel playout — a green troubleshoot run proves nothing about branding. | 2026-07-21 | [link](records/testing/troubleshoot-path-cannot-test-branding.md) | -| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives it with no gitignored `.mcp.json` and no language server, which is how it is proven in `scripts/tests/test_mcp_smoke.py`. That coverage is a SUBSET and no count of it is kept here: the file's own docstring enumerates the cases it drives and names what it does not, so there is one list rather than a number to keep in step with it. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | +| `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet several of them carry `MUTATION` rows — among them `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives it with no gitignored `.mcp.json` and no language server, which is how it is proven in `scripts/tests/test_mcp_smoke.py`. That coverage is a SUBSET and no count of it is kept here: the file's own docstring enumerates the cases it drives and names what it does not, so there is a list rather than a number to keep in step with it. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | ## Review due diff --git a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md index dd527a13e..aaf24d761 100644 --- a/docs/decisions/records/testing/verification-code-needs-its-own-proof.md +++ b/docs/decisions/records/testing/verification-code-needs-its-own-proof.md @@ -5,9 +5,9 @@ status: active since: '2026-08-28' supersedes: none superseded-by: none -rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet FOUR of them carry `MUTATION` rows — `posttooluse-worktree-marker.sh`, `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those four already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives it with no gitignored `.mcp.json` and no language server, which is how it is proven in `scripts/tests/test_mcp_smoke.py`. That coverage is a SUBSET and no count of it is kept here: the file''s own docstring enumerates the cases it drives and names what it does not, so there is one list rather than a number to keep in step with it. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' +rule: 'VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet several of them carry `MUTATION` rows — among them `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives it with no gitignored `.mcp.json` and no language server, which is how it is proven in `scripts/tests/test_mcp_smoke.py`. That coverage is a SUBSET and no count of it is kept here: the file''s own docstring enumerates the cases it drives and names what it does not, so there is a list rather than a number to keep in step with it. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record''s own threshold: when a review round''s finding was CREATED by the previous round''s fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading.' signals: 'the verification harness is itself unproven code · a false green in the checker rather than the checked · `[ -x "$cmd" ]` is true for a DIRECTORY · perl alarm/exec exits 0 without `or die` · macOS has no `timeout` and no `gtimeout` · smoke test accepts an impostor server · a server that pre-answers a guessable id · each fix created the next round''s defect · stop and subtract rather than add a layer · hermetically drivable is the test, not does-CI-run-it · a checker is not exempt because its CALLER cannot run in CI · the checker needs no inventory row — name the proof test as `guard` and the checker as `target` · a transitively-reached script is rejected as a PHANTOM · witnessed negative control for what cannot be driven · paths: `scripts/mcp_smoke.py`, `scripts/tests/test_mcp_smoke.py`, `scripts/check-local-lsp.sh`, `scripts/tests/mutation_manifest.py`, `docs/guard-inventory.md` · issues: #796, #793, #790, #785, #777, #776' -mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-28: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; the `or die` form exits 2 and says why). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and an absent `gtimeout` exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps travel with it: `[ -x "$cmd" ]` does not establish that a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. The only place that RUNS this recipe is `scripts/check-local-lsp.sh:107` (the other in-repo occurrences of the idiom are in this record); the `-x` trap appears there only in a comment recording what it deliberately does not do.' +mechanics: 'THE macOS TIMEOUT RECIPE, recorded here because the WRONG form was the recommended one and a session that followed it verbatim produced a false green (#793): `perl -e ''alarm shift; exec @ARGV or die "exec failed: $!\n"'' `. The `or die` is load-bearing — without it perl exits 0 when `exec` fails, so a missing or misspelt binary reports SUCCESS having run nothing (measured 2026-08-28: `perl -e ''alarm shift; exec @ARGV'' 5 /nope` exits 0; the `or die` form exits 2 and says why). macOS has neither `timeout` (GNU coreutils) nor `gtimeout` on this fleet, and an absent `gtimeout` exits 127 with empty output, which reads as a clean pass to any caller grepping stdout for a failure string. Two traps travel with it: `[ -x "$cmd" ]` does not establish that a command is runnable, because a directory satisfies `-x` (`[ -x /bin ]` is true); and whatever wrapper is used, its EXIT CODE must be checked rather than its stdout. The place that RUNS this recipe is `scripts/check-local-lsp.sh`; the other in-repo occurrences of the idiom are in this record. The `-x` trap appears there only in a comment recording what it deliberately does not do.' --- **The measurement.** #793 added one small operator-run check — a smoke test that starts the @@ -27,11 +27,11 @@ reproduced with a control before being fixed: proof` is scoped by a population — the guard files `docs/guard-inventory.md` derives and grades. Defects 1 and 3 are a `[ -x ]` test and a `perl` wrapper inside `scripts/check-local-lsp.sh`, which no workflow or hook invokes; 2, 4 and 5 live in `scripts/mcp_smoke.py`, reached only through that -script and so excluded by scope limit item 6. Neither file is in the population. That scoping is what keeps the inventory from becoming a chore that gets +script and so excluded by the transitive-calls scope limit (item 6). Neither file is in the population. That scoping is what keeps the inventory from becoming a chore that gets rubber-stamped; what was missing is the claim that the obligation follows the *verdict*, not the *file*. **The discriminator, stated precisely because the obvious one is wrong.** It is not "does CI run -this": CI never fires `.claude/hooks/*.sh` or `.husky/pre-push` as hooks either, yet four of them +this": CI never fires `.claude/hooks/*.sh` or `.husky/pre-push` as hooks either, yet several of them are graded `MUTATION`, because a pytest proof drives them in a sandbox. The question is whether a proof test can drive the construct *hermetically*, and "it needs a live server" usually fails that test rather than passing it. `mcp_smoke.py` takes its config path and server name as positional @@ -41,7 +41,7 @@ file's docstring rather than counted here. `check-local-lsp.sh` genuinely cannot be driven that way: it asserts a global `csharp-ls` on PATH, a `host/fxr` under `DOTNET_ROOT`, and a root `node_modules`, which is developer-machine state no runner has. That, and not "it is operator-run", is what puts it in the exempt branch. Note what that concedes: two of -the five defects below (the `-x` test and the `perl` wrapper) were in `check-local-lsp.sh` itself, +the five defects above (the `-x` test and the `perl` wrapper) were in `check-local-lsp.sh` itself, so the exempt branch is not defect-free — it is the part where a recorded control is the best available proof. Of the three in `mcp_smoke.py`, defects 2 and 5 are pinned by tests; defect 4's territory is deliberately left uncovered, because the message caps that would bound it WERE that diff --git a/scripts/mcp_smoke.py b/scripts/mcp_smoke.py index 4099a20c5..6c4913be8 100644 --- a/scripts/mcp_smoke.py +++ b/scripts/mcp_smoke.py @@ -22,7 +22,7 @@ deliberately share a stage code, so read the message, not the number. ACCEPTED LIMIT: a server that writes a gigabyte with no newline can still exhaust memory before the timeout fires. Guarding that needs the very frame-capping reader -whose caps caused three defects in an earlier round, and the input here is our OWN +whose caps were themselves a defect in an earlier round, and the input here is our OWN configured server on a developer machine — not an adversary. Stated rather than silently unhandled. diff --git a/scripts/tests/test_mcp_smoke.py b/scripts/tests/test_mcp_smoke.py index fb1cad340..dcb33f620 100644 --- a/scripts/tests/test_mcp_smoke.py +++ b/scripts/tests/test_mcp_smoke.py @@ -10,7 +10,7 @@ its-own-proof` is the rule that says a checker owes an executed proof; this is t `mcp_smoke.py` itself gets no `docs/guard-inventory.md` row: the inventory's population is derived from workflow and hook call sites and this script is reached only transitively, through -`scripts/check-local-lsp.sh` (scope limit item 6), so a row for it is rejected as a phantom. THIS +`scripts/check-local-lsp.sh` (the transitive-calls scope limit, item 6), so a row for it is rejected as a phantom. THIS file joins the population as an ordinary `scripts/tests/test_*.py` and carries the row, and the manifest entry names it as `guard` with `scripts/mcp_smoke.py` as `target` — the same shape `test_mutation_harness.py` uses for `mutation_harness_lib.py`. -- 2.47.3