Skip to content

docs: Mutation-Gated Testing — every new production behavior needs a demonstrable killer #676

Description

@Hero-Gamer

⚠️ Caveat: This is brainstorming only. The intent is to make the PR clean from a mutants perspective on the first commit — whether by a human or AI — by killing mutants before PR submission, rather than picking them up retroactively from the CI mutant job and reworking after the fact.


Problem

We have had a class of cleanup PRs where the first implementation was green but a reviewer later found that meaningful behavior was not actually tested and a mutant survived:

  • store leftover files that could be deleted without tests failing
  • P2P ranking / peer selection changes not asserted
  • IBD drain / timeout behavior not observable
  • consensus / RPC rejection returning wrong variant but is_err() passed

Examples: #593, #670, #322, #390, #425, #533, #625, #668, #671

The goal is not 100% mutation score. The goal is that no new production behavior can land without a test that demonstrably fails when that behavior is removed or inverted.

This proposal was iterated and battle-tested against 60 random PRs across store, query, consensus, net, IBD, mempool, rpc, electrum, esplora, cli and docs to make sure it is practical for both human and AI contributors.

Proposal

Core rule:

Every new production behavior must have a killer that demonstrably fails when that behavior is removed or inverted. Name alone is not proof.

Killer = test / journey / check + exact observable assertion.

What counts as behavior:

  • New rejection / acceptance decision, error code, RPC field
  • New file, cleanup path, cap, format, crash recovery
  • New ranking, selection, timeout, prefill, persistence
  • New concurrency, cancellation, ordering guarantee

Refactor / move with no behavior change needs no new killer, but must show 0 applicable survivors.

Tiers

Tier0 Primitives / unsafe / serialization / SIMD / arithmetic

  • Independent reference / oracle, not copy of optimized impl
  • Boundary + invalid encoding tests
  • cargo mutants -p <crate> -f <file> --in-diff /tmp/pr.diff -j2 -- --skip core -q -> 0 unexplained

Tier1 Observable decisions (consensus, RPC, mempool, CLI/config)

  • Assert exact variant / code, not is_err() or contains("invalid")
  • Exercise real accept / reject path
  • May reuse existing journey if you can demonstrate it fails without the behavior

Tier2 Stateful P2P / IBD / overlay / Electrum / Esplora + Time + Concurrency

  • Assert intermediate state: selected_peer, ranking decision, take_window, last_write, counter, vector content, reopened state — not just final tip height
  • Time: injected mock clock is default. Env-var shortening only with linked issue + reviewer exception
  • Concurrency / async / cancellation: cargo-mutants is advisory only. Needs property test for ordering / cancellation
  • Survivor allowed only if tool attributes kill to named journey OR you provide reproducible targeted command. Label alone is insufficient

Tier3 Store / Query / File / Crash / Schema

  • Lifecycle: precondition exists + operation + postcondition gone/correct + semantic result. Existence-after alone is insufficient
  • Crash: interrupt at point -> reopen -> assert documented recovery
  • Cap: assert bounded work stops at cap
  • Schema: test every compatibility direction promised by SCHEMA.md / COMPAT.md + migration
  • Cross-path: if invariant spans sendraw / submitpackage / RPC / Electrum / Esplora / CLI, killer must cover each path or justify canonical impl

Tier4 Tooling / CI / lint / release / build / deps

  • No mutants gate. Use fixture killer: invalid fixture -> lint fails, etc.
  • Deps: classify by type — consensus/protocol -> functional/compat tests, runtime/allocator -> relevant integration, dev-only -> build

Tier5 Docs-only

  • Exempt if diff only *.md and no crates/ change. COMPAT.md must be updated when shipped behavior changes

First-commit gate

git diff origin/master.. --unified=0 > /tmp/pr.diff
cargo test -p <crate> --lib -- --quiet
cargo mutants -p <crate> --in-diff /tmp/pr.diff -j2 -- --skip core -q  # 0 unexplained

Survivor classification in PR description with reviewer checkbox:

  • 0 survivors
  • irrelevant:
  • killed by journey:
  • Tier4 gate:

CI passed alone is not justification.

Implementation — 3 files, ~130 lines, docs-only

To respect current ownership (AGENTS.md = hard rules + pointers, TESTING.md owns how to run):

File Purpose
TESTING.md Canonical owner — full tiers, demonstrable killer, exact variant, state-transition, clock, concurrency, schema, cross-path, limitation that mutation score is not proof
AGENTS.md Hard rules only: production behavior needs demonstrable killer, observable state not impl text, survivors must be classified, specialized tests cover what cargo-mutants can't model, pointer to TESTING.md
.agents/skills/ship-pr/SKILL.md Actionable PR gate: 1) classify behavior 2) identify killer 3) prove killer reaches path 4) run mutants --in-diff 5) classify survivors 6) CI green

No production Rust code change.

Validation

Iterated by simulating the gate against 60 random historical PRs. The original first-commits failed because they only asserted tip height, used real sleeps, or checked is_err() not exact variant. With this policy applied, they would require the missing assertion before first commit and pass — preventing the later cleanup PRs.

What this is NOT

  • Not "mutant-free = correct". Mutation score is evidence, not proof
  • Not requiring log URLs for every journey — burden proportional, tool attribution preferred
  • Not requiring universal old<->new schema interop — only directions promised by SCHEMA.md / COMPAT.md
  • Not requiring core-functional for every dep bump — classified by dep type

Open questions

  1. Do we want cargo mutants --in-diff advisory in CI or keep it local-only via ship-pr skill?
  2. Should irrelevant classification require reviewer approval in PR template?

Here is the exact diff for the 3-file proposal — only additions, no subtractions needed in current master because AGENTS.md was just slimmed to pointers:

1. AGENTS.md — add 12 lines under hard rules

Add this block where the other hard rules live, right after TESTING.md owns how to run:

- Every new production behavior must have a demonstrable killer: test/journey/check + exact observable assertion that fails when behavior is removed/inverted. Name alone is not proof.
- Assert observable state, not implementation text.
- Mutation survivors must be classified: 0 / irrelevant with reason / killed by journey with demonstration / Tier4 fixture. "CI passed" alone is insufficient.
- Specialized / property tests cover behavior classes `cargo-mutants` cannot model well (concurrency, ordering, cancellation, time).
- Full policy lives in `TESTING.md`. Do not duplicate tier tables here.

2. TESTING.md — add 1 section, ∼95 lines, canonical owner

Add after the existing journey list, new section:

## Mutation and behavior-killer policy

Every new production behavior must have a demonstrable killer appropriate to its behavior class. Mutation testing verifies the mutations it can model, while specialized tests verify behavior classes it cannot model well. Mutation score is not a proof of correctness.

**What counts as behavior:** new rejection/acceptance decision, error code, RPC field, new file/cleanup path/cap/format/crash recovery, new ranking/selection/timeout/prefill/persistence, new concurrency/cancellation/ordering guarantee. Refactor/move with no behavior change needs no new killer but must show 0 applicable survivors.

**Tier0 Primitives / unsafe / serialization / SIMD / arithmetic:** independent reference/oracle, not copy of optimized impl. Boundary + invalid encoding. Unsafe = narrowest invariant + MIRI where applicable.

**Tier1 Observable decisions (consensus, RPC, mempool, CLI/config):** assert exact variant/code, not `is_err()` or `contains("invalid")`. Exercise real accept/reject path. May reuse existing journey if you can demonstrate it fails without behavior.

**Tier2 Stateful P2P / IBD / overlay / Electrum / Esplora + Time + Concurrency:** assert intermediate state (selected_peer, ranking decision, `take_window`, `last_write`, `nLastTry`, counter, vector content, reopened state) not just final tip height. Time: injected mock clock is default, env-var shortening only with linked issue + reviewer exception. Concurrency/async/cancellation: `cargo-mutants` advisory only, needs property test for ordering/cancellation. Survivor allowed only if tool attributes kill to named journey OR you provide reproducible targeted command. Label alone insufficient.

**Tier3 Store / Query / File / Crash / Schema:** lifecycle = precondition exists + operation + postcondition gone/correct + semantic result. Existence-after alone insufficient. Crash: interrupt at point -> reopen -> assert documented recovery (Ok with expected state OR specific Corrupt). Cap: assert bounded work stops at cap. Schema: test every compatibility direction promised by `SCHEMA.md` / `COMPAT.md` + migration. Cross-path: if invariant spans `sendraw / submitpackage / RPC / Electrum / Esplora / CLI / P2P`, killer must cover each path or justify canonical impl.

**Tier4 Tooling / CI / lint / release / build / deps:** no mutants gate. Use fixture killer: invalid fixture -> lint fails, below-threshold -> gate fails, release fixture -> artifact check. Deps: classify — consensus/protocol/serialization -> functional/compat, runtime/allocator/network -> relevant integration, dev-only -> build, security -> security policy.

**Tier5 Docs-only:** exempt if diff only `*.md` and no `crates/` change. `COMPAT.md` must be updated when shipped behavior changes.

**First-commit gate:**
```bash
git diff origin/master.. --unified=0 > /tmp/pr.diff
cargo test -p <crate> --lib -- --quiet
cargo mutants -p <crate> --in-diff /tmp/pr.diff -j2 -- --skip core -q  # 0 unexplained

Survivors in PR description with reviewer checkbox:

  • 0 survivors
  • irrelevant:
  • killed by journey:
  • Tier4 gate:

### 3. `.agents/skills/ship-pr/SKILL.md` — add 22 lines, actionable gate

Add under `Before first commit` / `Verification`:

```markdown
### Behavior-killer gate

Before first production commit:
1. classify changed production behavior (Tier0-5)
2. identify killer: test/journey/check + exact observable assertion
3. prove killer reaches changed path and fails when behavior removed/inverted
4. run applicable `cargo mutants -p <crate> --in-diff /tmp/pr.diff -j2 -- --skip core -q`
5. classify every applicable survivor (0 / irrelevant with reason / killed by journey with demonstration / Tier4 fixture)
6. run required CI — CI passed alone is not justification

Distinguish:
- `cargo-mutants` = evidence for mutations it generates
- specialized/property tests = behavior it cannot model well (concurrency, ordering, time, file content)

Docs-only change (only `*.md`, no `crates/`): exempt from mutants gate.

Total: ∼12 + ∼95 + ∼22 = ∼129 lines added, 0 lines removed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions