Skip to content

Commit 0602822

Browse files
committed
docs(skill): add a test-audit skill with the mutation discipline that worked
Adapts the upstream test-audit card to this repository. The authoring gate, junk patterns, retention bar, candidate-evidence fields, and subagent dispatch plan are its structure; what changes is everything this repo actually enforces. Junk patterns are the shapes that turned up as real findings here rather than a generic list: type pins that survive erasure (`Equal<A, B> = true`, `X ? true : never`, `satisfies` compared after it stops existing at runtime), a lone `typeof x === 'function'` sitting directly above the test that calls `x`, an ordering claim with no ordering assertion, and a `switch` written inside a test body and compared to its own arms. The retention bar is likewise earned: registry and gate-manifest completeness, cross-language source inspection, `@ts-expect-error` directives (an unused one is a compile error, so they do bite), exact-array facets where a membership scan cannot see a duplicate, identity pins where deep-equality does not subsume `toBe`, and vocabulary pins that survive a production-and-fixture co-update. The mutation rule is the part worth keeping: plant the production edit that represents the bug, show the old body green and the new body red with both counts, revert, and show `git status` clean. Arguing detection is not evidence; measuring it is. Validation commands are this repo's — `--project unit-core`/`apple-runner`, `oxlint`, `tsc -b` per package plus the root project, `check:affected --run`, `check:fallow` — and record that the upstream `run-vitest.mjs` and `check-changed.mjs` helpers do not exist here, so nobody goes looking. It also notes the trap found the hard way: `scripts/**` tests are in no `tsconfig` include, so a helper added there needs an explicit type-check. Routed from the AGENTS.md task table, which stays inside its 10 kB budget at 8.0 kB.
1 parent 769ee23 commit 0602822

2 files changed

Lines changed: 79 additions & 0 deletions

File tree

‎AGENTS.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ Load only the procedures relevant to the task:
1212
| Domain vocabulary | `CONTEXT.md`, `docs/agents/domain.md` |
1313
| Architecture decisions | `docs/adr/README.md` |
1414
| Tests or gate selection | `docs/agents/testing.md` |
15+
| Writing, auditing, or sweeping tests; dispatching test-audit subagents | `skills/test-audit/SKILL.md` |
1516
| Selector capture, polling, or interaction fast paths | `docs/agents/selector-capture.md` |
1617
| Adding or changing a CLI flag | `docs/agents/cli-flags.md` |
1718
| Opening or reviewing a PR | `docs/agents/pull-requests.md` |

‎skills/test-audit/SKILL.md‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
---
2+
name: test-audit
3+
description: Write, change, review, or sweep tests. Authoring gate for new tests, plus an audit workflow and parallel subagent dispatch plan for finding tests that cannot fail, duplicating proof, or test-only production seams.
4+
---
5+
6+
# Test audit
7+
8+
Three modes, one value criterion: a test earns its cost by guarding behavior, credible regressions, or independently meaningful contracts. Optimize for confidence, not deletion count.
9+
10+
## Authoring gate
11+
12+
Before adding a test, answer; if you can't, don't add it yet:
13+
14+
1. Which observable behavior, invariant, or independent contract does it guard?
15+
2. Which credible regression makes it fail? Name the production edit.
16+
3. Why doesn't existing coverage catch it? Each contract has one primary owner at the strongest boundary.
17+
4. Does it need a production seam no production caller needs? Move the test to the real boundary instead.
18+
19+
Bug regressions must fail on pre-fix code for the intended reason. A regression test that never visibly failed is proving the mock.
20+
21+
## Junk patterns
22+
23+
Authoring rejects matches; audits hunt for existing matches:
24+
25+
- assertion-free probes; lone `typeof x === 'function'` followed by a sibling that calls `x`;
26+
- self-comparison (`f(x)` vs `f(x)`, `map.get(k)` vs itself, fixture vs re-spelled fixture);
27+
- expected value from the helper under test (oracle-as-expectation); a `switch` written in the test body;
28+
- byte-identical or renamed `test()` blocks; the same golden replayed through a second entry point;
29+
- type pins posing as runtime assertions: `Equal<A,B> = true`, `X ? true : never`, `satisfies` compared after erasure, `toHaveLength(n)` over test literals. Erase them mentally first; if nothing executable remains, convert to the `void [fixtures]` idiom;
30+
- mocks asserting their own inputs; negative controls passing for the wrong reason;
31+
- names promising more than the input exercises (an ordering claim with no ordering assertion is a repair candidate, not a deletion);
32+
- dead code called only by tests; tests preserving test-only exports.
33+
34+
## Retention criteria
35+
36+
Keep (and refuse to delete) tests enforcing: command-registry/gate-manifest/layering/DI-seam completeness; golden tables and cross-language source inspection (`protocol.test.ts` grepping Obj-C is a real detector); `@ts-expect-error` pins (unused directives fail `tsc`, so they bite); deliberate identity pins where deep-equal doesn't subsume `toBe`; exact-array facets where a membership scan can't see duplicates; vocabulary pins that survive production+fixture co-updates. Static-or-slow isn't deletion-worthy; similarity to implementation isn't either — prove you can't break it.
37+
38+
## Candidate evidence
39+
40+
Record before editing; missing field ⇒ not ready:
41+
42+
- location + test name; detectable failure (or why none exists);
43+
- non-test callers of covered seams; stronger remaining owner-boundary proof (quote both assertion sets, file:line, show superset relation);
44+
- skip guards in candidate and sibling (`skipIf`, `.skip`, `AGENT_DEVICE_*` env, whether any workflow or script sets it);
45+
- which Vitest project includes the file (`unit-core`, `apple-runner`, `provider-integration`, `fuzz-worker`, `interaction-contract`, `output-economy`) — unmatched = never-run finding; note: `macos.yml` names its darwin files explicitly;
46+
- unlock deletion; focused validation command; risk.
47+
48+
## Mutation discipline
49+
50+
The audit's core proof: plant a production edit that represents the bug the test claims to catch; the pre-edit body must stay green while the post-edit body goes red. Report both counts (`8 passed` → `1 failed | 7 passed`). Revert and show `git status` clean before committing. For deletions, prove the sibling catches the mutant. Never argue detection — measure it.
51+
52+
## Validation
53+
54+
This repo's commands (upstream `run-vitest.mjs`/`check-changed.mjs` don't exist):
55+
56+
1. Focused: `npx vitest run --project unit-core <path>` (or `--project apple-runner` for `packages/platform-apple/src/runner/**`).
57+
2. `pnpm format` (whole repo), `npx oxlint . --deny-warnings`, `npx tsc -b packages/...` for package edits, `npx tsc -p tsconfig.json` for `src/`.
58+
3. `pnpm check:affected --run` at the pushed commit; `pnpm check:fallow --base origin/main` when touching production exports.
59+
4. `git diff --numstat`; report production/test deltas separately.
60+
5. Don't edit during Vitest runs; `scripts/**` tests are outside `tsc` scope — type-check new helpers there explicitly.
61+
62+
## Dispatching subagents
63+
64+
For broad scope, run parallel read-only lanes and keep editing ownership exclusive:
65+
66+
- subsystem lanes (platform packages; `src/daemon`; contracts/kernel/registry/capture-kit/selectors; `test/integration` + maestro/ad-replay/session-journal + provider packages; `scripts/**`);
67+
- one cross-cutting pattern sweep (junk-pattern greps across all tests, each hit read and confirmed);
68+
- one never-run inventory (project includes, gate ownership via `scripts/gate` and `.github/workflows`, unsatisfiable guards).
69+
70+
Mandate in every prompt: READ-ONLY; read the production owner and name the bug; verify skip guards and project selection; retention list above verbatim; "a false positive costs more than a miss"; output capped, highest-confidence first, with validation commands.
71+
72+
## Landing
73+
74+
One coherent owner-boundary batch per PR; prefer net-negative production LOC; delete the test-only seam with the test. Push only when authorized; rebase on review feedback and answer findings by fixing the rule, not the cited site. After landing, rebase to `main` and rediscover; don't carry stale candidate lists.
75+
76+
## Handoff
77+
78+
Report: categories deleted; owner simplifications; retained false positives with reasons; mutations run with before/after counts; production vs test LOC; PR state; named follow-ups needing owner judgment (never-run env gates, tsconfig coverage gaps, suspected product bugs).

0 commit comments

Comments
 (0)