⚠️ 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:
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
- Do we want
cargo mutants --in-diff advisory in CI or keep it local-only via ship-pr skill?
- 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:
### 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.
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:
is_err()passedExamples: #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:
Killer = test / journey / check + exact observable assertion.
What counts as behavior:
Refactor / move with no behavior change needs no new killer, but must show 0 applicable survivors.
Tiers
Tier0 Primitives / unsafe / serialization / SIMD / arithmetic
cargo mutants -p <crate> -f <file> --in-diff /tmp/pr.diff -j2 -- --skip core -q-> 0 unexplainedTier1 Observable decisions (consensus, RPC, mempool, CLI/config)
is_err()orcontains("invalid")Tier2 Stateful P2P / IBD / overlay / Electrum / Esplora + Time + Concurrency
selected_peer, ranking decision,take_window,last_write, counter, vector content, reopened state — not just final tip heightcargo-mutantsis advisory only. Needs property test for ordering / cancellationTier3 Store / Query / File / Crash / Schema
SCHEMA.md/COMPAT.md+ migrationsendraw / submitpackage / RPC / Electrum / Esplora / CLI, killer must cover each path or justify canonical implTier4 Tooling / CI / lint / release / build / deps
Tier5 Docs-only
*.mdand nocrates/change.COMPAT.mdmust be updated when shipped behavior changesFirst-commit gate
Survivor classification in PR description with reviewer checkbox:
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):TESTING.mdAGENTS.mdcargo-mutantscan't model, pointer toTESTING.md.agents/skills/ship-pr/SKILL.mdNo 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
SCHEMA.md/COMPAT.mdcore-functionalfor every dep bump — classified by dep typeOpen questions
cargo mutants --in-diffadvisory in CI or keep it local-only via ship-pr skill?irrelevantclassification require reviewer approval in PR template?Here is the exact diff for the 3-file proposal — only additions, no subtractions needed in current
masterbecauseAGENTS.mdwas just slimmed to pointers:1.
AGENTS.md— add 12 lines under hard rulesAdd this block where the other hard rules live, right after
TESTING.md owns how to run:2.
TESTING.md— add 1 section, ∼95 lines, canonical ownerAdd after the existing journey list, new section:
Survivors in PR description with reviewer checkbox:
Total: ∼12 + ∼95 + ∼22 = ∼129 lines added, 0 lines removed.