Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
---
title: A docstring that enumerates accepted forms is a test table, not prose
date: 2026-09-08
last_updated: 2026-09-08
verified: 2026-09-08
category: best-practices
module: scripts/check-mutation-guards.ts
problem_type: best_practice
component: tooling
severity: high
applies_when:
- a matcher, classifier, or parser documents the syntactic forms it accepts
- the documented enumeration was written from a specification rather than from the pattern
- review catches a case the implementation misses and the fix lands without a regression test
tags:
- fail-open
- classifier
- counterexample
- regression-tests
- grammar
---

# A docstring that enumerates accepted forms is a test table, not prose

## Context

When a matcher's documentation lists the forms it accepts, that list is a specification the code is claiming to satisfy. Nothing checks the claim. The enumeration and the pattern drift independently, and the drift is invisible because the doc reads as authoritative and the tests were usually written from the same mental model as the pattern.

Found in a barrel-detection matcher whose docstring enumerated re-export forms the regex did not all cover. The consequence is not a crash — the classifier silently returns the wrong answer for that form, and whatever it gates fails open.

## Guidance

### Drive every enumerated form through the actual matcher

Take the list from the docstring, feed each entry to the real implementation, and assert the result. Record it as a truth table so the correspondence is visible rather than assumed. A form in the docs with no row in the table is an untested claim.

Divergence fails in the direction the guard exists to prevent: a form the docs say is recognised, and the matcher does not, is accepted when it should be caught.

### Keep the counterexample as an executable artifact

When review catches a missed case, fixing the pattern is half the work. Add a test that reconstructs the failing input and asserts the pre-fix logic misses it — proving the test discriminates, not merely that the current code passes. Without that, the catch survives only as a comment in a closed review thread, and the next refactor can reintroduce the gap against a green suite.

The generalisation: prove each rule discriminates by deleting it and watching a test go red. A rule whose removal breaks nothing is not being enforced by the suite.

## Why This Matters

Documentation drift is normally a readability problem. In a classifier it is a correctness problem, because the enumeration is load-bearing: reviewers reason from it, and the next person to extend the matcher treats it as the contract. A docstring that overstates what the pattern matches is a latent fail-open with a comment vouching for it.

## When to Apply

- Writing or reviewing a regex, parser, or classifier whose documentation lists accepted inputs
- Any review round that produces a "it also needs to handle X" finding
- Two call sites needing the same extraction with different tolerance

## Examples

Enumeration as a table rather than prose:

```ts
// Every form the docstring claims, driven through the real matcher. The matcher takes
// a path and a source reader, so each row is a fixture file rather than a bare line.
it.each([
['export * from "./x"', true],
['export type * from "./x"', true], // the form the pattern originally missed
['export {a} from "./x"', false],
])('classifies %s as barrel=%s', (source, expected) => {
expect(isPureReexportBarrel('fixture.ts', () => source)).toBe(expected)
})
```

The live table is in `scripts/mutation-guards-config.test.ts`.

## Related

- [`a-mutation-score-can-measure-nothing`](a-mutation-score-can-measure-nothing-2026-09-08.md) — the other way a green signal can mean nothing was checked
- [`equivalence-refactors-need-differential-proofs-past-the-bound`](equivalence-refactors-need-differential-proofs-past-the-bound-2026-09-06.md) — bounded verification that reads as exhaustive
- [`mutation-coverage-is-silent-about-unwritten-branches`](../security-issues/mutation-coverage-is-silent-about-unwritten-branches-2026-09-05.md) — reading the grammar before trusting the coverage
- Merge commit `69a99af`
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
---
title: A mutation score can measure nothing — check reachability before writing tests
date: 2026-09-08
last_updated: 2026-09-08
verified: 2026-09-08
category: best-practices
module: scripts/check-mutation-guards.ts
problem_type: best_practice
component: testing_framework
severity: high
applies_when:
- a module reports 0 killed mutants and the instinct is to write more tests
- a module's decision table, regex array, or allowlist is a top-level `const`
- removing an entry from Stryker's `testFiles` while other modules remain in `mutate`
- a module's only tests import it through a package's `dist` output or published specifier
tags:
- mutation-testing
- stryker
- instrumentation
- static-mutants
- test-topology
- false-confidence
---

# A mutation score can measure nothing — check reachability before writing tests

## Context

A mutation score is a claim about which mutants your tests killed. It is silent about mutants the harness never gave any test a chance to kill. Two mechanisms produce that silence, and both look like ordinary coverage from the outside — the module is listed in `mutate`, tests exist and pass, and the report prints a number.


## Guidance

### A top-level `const` is evaluated once per worker, so its mutants are static

Stryker imports a module once per worker process, so a module-level `const` initializer runs once and never again. Data computed there is shared across every test in that worker, and a mutant inside it is fixed for the worker's lifetime — no test can observe a difference, so none can kill it.

`scripts/check-wiki-authority.ts` held its identity set and guarded-path regex array as top-level `const`s: the whole decision table was frozen before any test ran, and the baseline was 0 killed. Converting both to functions took it to 146 killed **with no new tests written**. The assertions had always been adequate; the mutants had never been reachable.

Read a 0-killed baseline as an instrumentation problem first. Writing tests against a frozen constant produces more passing tests and the same zero.

Before converting, confirm the move is semantics-preserving. It was here, but each of these is a real way it could fail:

- **Allocation state** — a fresh `Set` or array per call is neutral only if no caller mutates it after construction.
- **Regex `lastIndex`** — these patterns carry no `g` or `y` flag, so there is no cross-call state. Add either flag and per-call reallocation becomes observable.
- **Encoding defaults** — for text, `readFile(path)` then `.toString()` equals `readFile(path, 'utf8')`: `Buffer.prototype.toString()` defaults to utf8 and neither form strips a BOM. Not a general identity — for binary or non-utf8 data, or where the buffer feeds a non-string consumer, the two differ.
- **String joins** — if the constant feeds a formatted message, the join is part of an output contract.

### `mutate` and `testFiles` are independent levers

A test that imports a module through its compiled `dist` output or its published package specifier exercises the built artifact. Stryker instruments the source tree, so that test contributes zero kills to the source module no matter how thoroughly it exercises the behavior.

The consequence is that a module can sit in `mutate` with tests that read like coverage and produce no signal at all. It also makes `testFiles` edits quietly dangerous: removing one entry can orphan a module elsewhere in `mutate` whose only source-path exerciser lived in that file.

Before removing anything from `testFiles`, enumerate every module still in `mutate` and confirm each retains at least one test reaching it through a **source-tree** import. Record which test file is each module's live exerciser, so a later demotion cannot orphan one silently.

Scope `mutate` by materiality — does a mutant here threaten the guarantee the module exists to provide — rather than by module shape.

## Why This Matters

Both mechanisms fail in the reassuring direction. A clean or improving score is read as evidence the guard is tested, when it can equally mean the guard was never instrumented. For modules that exist to enforce a privacy or authority boundary, that is the worst possible failure mode for a metric: it is loudest exactly when it is least informative.

The second mechanism is worse than the first, because a static mutant reports 0 and draws attention, while a `dist`-imported test can leave a module reporting a plausible partial score assembled entirely from other tests.

## When to Apply

- A mutation baseline is 0, or implausibly low for a module with real tests
- Any module-level `const` holding data the guard's decisions depend on
- Editing `mutate` or `testFiles` in `stryker.config.json`
- A package with a committed `dist/` whose tests could import either side

## Examples

The conversion that unfroze the decision table:

```ts
// Before: evaluated once per Stryker worker; every mutant inside is static.
const GUARDED_PATTERNS = [/^knowledge\/wiki\/[^/]+\/.+\.md$/, /* … */]

// After: evaluated per call, so each mutant is observable by a test.
function guardedPatterns(): readonly RegExp[] {
return [/^knowledge\/wiki\/[^/]+\/.+\.md$/, /* … */]
}
```

The import that looks like coverage and isn't:

```ts
// Contributes zero mutant kills to the source module — exercises the built artifact.
import {checkPrivateLeak} from '@fro-bot/wiki-write-core'

// Instrumented, and therefore counted.
import {checkPrivateLeak} from './private-leak.ts'
```

## Related

- [`enumerate-mutator-variants-before-a-stryker-directive`](enumerate-mutator-variants-before-a-stryker-directive-2026-09-05.md) — the adjacent trap: a mutant that *is* reachable, silenced by an over-broad directive
- [`mutation-coverage-is-silent-about-unwritten-branches`](../security-issues/mutation-coverage-is-silent-about-unwritten-branches-2026-09-05.md) — the third silence: a branch the code never wrote cannot be mutated at all
- [`size-subprocess-buffers-selectively-at-call-sites`](size-subprocess-buffers-selectively-at-call-sites-2026-08-31.md) — states the static-`const` mechanism in a code comment; the rule lives here
- Merge commits `3bd59db` and `498f33d`
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
---
title: A policy scanner must parse tokens, not prose — and the tool may normalise away what you are checking for
date: 2026-09-08
last_updated: 2026-09-08
verified: 2026-09-08
category: best-practices
module: scripts/check-mutation-guards.ts
problem_type: best_practice
component: tooling
severity: high
applies_when:
- writing a check that enforces policy over directives, annotations, or pragmas in source
- deriving a verdict from a tool's report rather than from its exit code
- a check searches free-form text that also contains author-written prose
- a report is keyed by file and the set of files is itself configurable
tags:
- fail-closed
- policy-enforcement
- structured-output
- stryker
- closed-vocabulary
---

# A policy scanner must parse tokens, not prose — and the tool may normalise away what you are checking for

## Context

The mutation guard enforces a policy on `Stryker disable` directives: each must be `next-line`-scoped, name a single mutator, and carry a non-empty reason. Two layers were written to enforce it — a check over the JSON report, and a textual scan of the source.

Only one of them can enforce the reason rule, and it is not the obvious one. The report-level check is wired correctly — `check-mutation-guards.ts` preserves a missing `statusReason` as `undefined` and `isEmptyReason` treats that as a violation — but it can only fire if a reasonless directive reaches the report as an absent or blank reason. Whether it ever does is upstream behaviour, and this repo has evidence pointing the other way: a code comment recording a live Stryker run notes that framework-ignored mutants arrive with a **non-empty** `statusReason` supplied by the tool.

**What is verified, precisely:** the check is implemented and would fire on an absent or blank reason. Whether a directive written without a reason can produce that state has not been traced into upstream source here. If it cannot, the textual scanner is the sole enforcement of the reason rule and the report-level check is decorative — which is the state worth knowing about, and exactly what nothing in CI would tell you.

## Guidance

### Verify where enforcement actually lives before trusting a check

A check that cannot fail is worse than a missing check, because it reads as coverage. When two layers enforce the same policy, establish which one is load-bearing by making each fail on purpose. A layer that stays green against input it should reject is not redundancy.

### Match against parsed tokens, never substrings of free-form text

A directive line contains both structure and an author-written reason. `remainder.includes('next-line')` matches a directive that is correctly scoped and also matches a reason that happens to contain the phrase — so the scope check passes on text that has nothing to do with scope.

Parse the directive into its parts and match against those. While you are there, the same grammar carries three cases a substring approach misses entirely:

- block-comment forms alongside line comments
- multiple directives on one line
- directives whose reason spans a line break

### Cross-check report keys against the configured scope

A report can only describe files that were instrumented. If a module is in `mutate` but absent from the report, a per-file loop over report keys iterates zero times for it and reports clean. Intersect the report's keys with the configured `mutate` list and fail closed on any module that produced no entry — silence about a module is not evidence about it.

### Derive verdicts from structured output, fail closed on malformed entries

An exit code compresses everything the tool knows into one bit, and tools are inconsistent about which failures set it. Read the structured report and classify from it. Treat a malformed or unparseable entry as a failure rather than skipping it, so a shape change upstream cannot quietly reduce what the gate checks.

### Prove each rule discriminates

Delete the rule and confirm a test goes red. A policy rule that no test pins is a rule the next refactor can remove without resistance.

## Why This Matters

Every failure here is silent and in the reassuring direction. A vacuous check reports pass. An uninstrumented module reports clean. A substring match reports correctly-scoped. The gate keeps returning the answer everyone wants while enforcing progressively less, and nothing in CI distinguishes that from genuine compliance.

## When to Apply

- Writing any check that reads a tool's report to enforce a policy
- Enforcing rules over comment-embedded directives in any language
- A gate whose scope is configurable, where "no findings" and "not examined" are different states with the same output

## Examples

Scope matched against structure rather than text:

```ts
// Passes on a reason that merely mentions the phrase.
if (remainder.includes('next-line')) { /* treated as correctly scoped */ }

// Scope is a parsed token; the reason is a separate field.
const {scope, mutators, reason} = parseDirective(line)
if (scope !== 'next-line') { report(...) }
```

Silence distinguished from cleanliness:

```ts
// A module in `mutate` with no report entry must fail, not pass by omission.
const missing = configuredMutateTargets.filter(target => !(target in report.files))
if (missing.length > 0) { failClosed(missing) }
```

## Related

- [`enumerate-mutator-variants-before-a-stryker-directive`](enumerate-mutator-variants-before-a-stryker-directive-2026-09-05.md) — the policy this scanner enforces, from the directive author's side
- [`a-mutation-score-can-measure-nothing`](a-mutation-score-can-measure-nothing-2026-09-08.md) — the same silence one layer down, in the instrumentation
- [`status-vocabulary-must-cover-every-report-surface`](status-vocabulary-must-cover-every-report-surface-2026-08-31.md) — closed vocabularies across reporting surfaces
- Merge commit `796ddf2`
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,7 @@ the mutation gate exists to catch.
- `docs/plans/2026-09-04-001-feat-counterexample-proven-guards-plan.md`, Unit 5A-1 and 5B-3 Result blocks
- `docs/solutions/best-practices/equivalence-refactors-need-differential-proofs-past-the-bound-2026-09-06.md`
— when the fix is a refactor rather than a directive, the proof it needs
- `docs/solutions/best-practices/a-policy-scanner-must-parse-tokens-not-prose-2026-09-08.md` — the same policy
from the enforcing side: how `check-mutation-guards.ts` must read these directives
- `docs/solutions/best-practices/a-mutation-score-can-measure-nothing-2026-09-08.md` — the adjacent trap,
where a mutant is never reachable rather than silenced
Loading
Loading