docs(skills): prevent junk tests and add subsystem test-pruning campaigns (#156316)

## Problem

Agents keep adding tests that restate implementation or repeat the same contract across layers, and `test-audit` had no repeatable workflow for pruning a whole subsystem's tests.

## Fix

The `test-audit` authoring gate now checks new tests against a shared junk-pattern list (tests the retention bar protects are exempt). It requires one primary test per contract at its strongest boundary and one regression per bug. A new campaign mode (`CAMPAIGN.md`) records the step order from the Telegram cleanup in #155040.

## User impact

No user-visible change. Agents writing tests reject known junk patterns, and maintainers can run the same subsystem-wide test cleanup on other areas.

## Proof

A fresh read-only agent ran the revised skill on `extensions/policy`: 7 lanes, a 329-declaration ledger (296 retain, 26 consolidate, 7 delete), one test-only seam flagged for a maintainer decision, and both deliberately junk test proposals correctly rejected by the authoring gate. Ambiguities it found were fixed in the final commit.

Co-authored-by: Ayaan Zaidi <hi@obviy.us>

.agents/skills/test-audit/CAMPAIGN.md

@@ -0,0 +1,116 @@
+# Test-pruning campaign
+
+Campaign mode prunes one subsystem's whole test surface in one PR: a plugin
+such as `extensions/telegram`, or one core area. The value bar, retention bar,
+candidate evidence, and validation in [SKILL.md](SKILL.md) apply to every
+lane. This file adds the order of work and the lessons of a full campaign.
+Each step ends on its completion criterion; do not start the next step early.
+
+## 1. Baseline
+
+Record the subsystem's test and support line counts and every test file's
+pass/fail state at a pinned `main` SHA. Keep baseline failures in their own
+list: in the Telegram campaign, all three were real delivery bugs, not stale
+tests.
+
+Done when every in-scope test file has a recorded baseline result.
+
+## 2. Lanes and inventory
+
+Split the surface into **lanes** along production owner boundaries, not file
+prefixes. For Telegram these were accounts, commands, context, dispatch,
+inbound, outbound, persistence, transport, shared, harness, and live/QA
+scenarios. Include the subsystem's cases at shared core boundaries and its QA
+and live-proof harness tests.
+
+Done when every test file and QA scenario the subsystem owns belongs to exactly
+one lane.
+
+## 3. Read-only ledger per lane
+
+Give each lane to its own read-only agent. The agent reads every assigned test
+in full, including parameter tables. It also reads the production owners and
+their entry points, callers, history, and CI routing. Each test declaration
+goes into a written **ledger** with one mark. An `it.each` is one declaration
+unless its rows need different marks; then mark each row.
+
+- `R`: retain, naming the contract and the bug it catches; a retained test that
+  only moves to a better-named file stays `R` with the move noted;
+- `F`: retain the contract but repair the assertion, such as a vacuous negative
+  that passes when only one of several items is missing;
+- `C`: consolidate, naming the owner that absorbs the assertion first: a sibling
+  table case, a stronger boundary suite, or the shared owner in another package;
+- `D`: delete, naming the proof that remains, or why no contract exists.
+
+Judge a test by its assertions, not its name. One Telegram test named for
+retiring a progress window asserted the window was _not_ cleared.
+
+Done when every declaration in the lane has a mark and an evidence line.
+
+## 4. Layer plan per lane
+
+Treat the per-test ledger as input, not as the edit list. A second read-only
+pass, starting from the ledger, looks for the redundant **layer**. In Telegram,
+several dispatch suites replayed the same shared compositor through one mocked
+preview, around stronger real-stream and HTTP-fixture suites. Name the
+**keeper** suite for each contract. Prefer the real transport boundary with a
+fake network over a mocked collaborator. Correct any ledger errors this pass
+finds.
+
+Done when each lane plan names its retired files, its keeper per contract, the
+assertions to carry into keepers, and the test-only production seams unlocked.
+
+## 5. Cutover
+
+Edit lane by lane. Serialize changes to shared harnesses and support files
+through one owner. With each lane, remove the test-only production seams it
+unlocks: injection parameters, getters, reset exports, and indirection layers.
+Register moved suites in CI routing and test inventories. Update shrink-only
+line-cap baselines. Put durable test-ownership rules in the subsystem's
+`AGENTS.md`, drawn from mistakes this campaign actually found.
+
+Done when every lane plan is applied and each lane's keepers pass.
+
+## 6. Preservation review
+
+Before claiming completion, have independent reviewers compare deleted
+coverage against the keepers, one reviewer per boundary group. They look for
+contracts that lost their only proof. They also look for new assertions that
+cannot fail, such as a rejection row the production code never reaches. The
+Telegram review found nine real gaps and one unreachable assertion.
+
+For each restored contract, make one deliberate **mutation** of the production
+owner and confirm the keeper goes red. Then restore the source byte for byte.
+
+Done when every reported gap is restored or rejected with source evidence, and
+every restored contract has a caught mutation.
+
+## 7. Product defects
+
+A baseline failure that survives into a keeper is a bug report. Fix it at its
+owner as a separate commit, and prove it through the real user flow, with a
+**control** run that reverts the fix and shows the old behavior. Record
+unrelated product discrepancies you find as follow-ups instead of fixing them
+in the campaign.
+
+Done when each repaired defect has a failing control and a passing candidate
+on the same harness.
+
+## 8. Reconcile and hand off
+
+Campaigns outlive many `main` commits. Merge `main` rather than rebasing a
+long, many-commit campaign. When `main` modified a file the campaign
+deleted, keep the deletion. Port the new contract into the keeper instead, and
+confirm every new regression `main` added still has a home. Rerun the whole
+subsystem suite and repeat live proof on the merged head.
+
+Expect review tooling to see a truncated file list on a diff this large.
+Record maintainer decisions for generic compatibility flags in the PR evidence
+rather than editing gates.
+
+Hand off with the [SKILL.md](SKILL.md) report, plus:
+
+- baseline and final test/support line counts, with production counted separately;
+- lanes, retired layers, and keepers;
+- preservation gaps found and their mutations;
+- product defects with control and candidate proof.

.agents/skills/test-audit/SKILL.md

@@ -5,11 +5,13 @@ description: "Invoke whenever writing, changing, reviewing, or sweeping tests. A
 
 # Test Audit
 
-Two modes, one value bar. Authoring mode gates every new or changed test at
+Three modes, one value bar. Authoring mode gates every new or changed test at
 write time. Audit mode runs focused sweeps of tests that re-assert source,
 duplicate stronger proof, couple behavior to implementation, or keep test-only
 production seams alive. Continue broad audits as separate coherent follow-up
-PRs; optimize for confidence, not deletion count.
+PRs; optimize for confidence, not deletion count. Campaign mode prunes one
+whole subsystem's test surface (every test file a plugin or core area owns);
+before starting one, read [CAMPAIGN.md](CAMPAIGN.md).
 
 ## Authoring gate
 
@@ -18,26 +20,57 @@ add it yet:
 
 1. What observable behavior, invariant, or independent contract does it protect?
 2. What credible regression makes it fail?
-3. Why does existing coverage not already catch that failure? Prefer extending a
-   table-driven case or shared fixture over a near-duplicate test; consolidate
-   duplicated setup in the same change.
+3. Why does existing coverage not already catch that failure? Each contract has
+   one primary test owner at the strongest boundary; another layer needs its
+   own distinct risk, such as a transport or lifecycle failure the owner cannot
+   reach. Prefer extending a table-driven case or shared fixture over a
+   near-duplicate test; consolidate duplicated setup in the same change.
 4. Does it need a production seam (export, flag, wrapper, injection hook) that no
    production caller needs? If yes, move the test to the real boundary instead.
 
-A test that would break under behavior-preserving refactoring is asserting
-implementation, not behavior; rewrite it at the owning boundary before landing
-it.
+Then check the test against every [junk pattern](#junk-patterns); a match fails
+the gate unless the [retention bar](#retention-bar) names the contract it
+independently guards. A test that would break under behavior-preserving
+refactoring is asserting implementation, not behavior; rewrite it at the
+owning boundary before landing it.
 
 Bug regression tests must fail on the pre-fix code for the intended reason and
 pass after the owner-boundary repair. A regression test that never demonstrably
-failed proves the mock, not the fix.
+failed proves the mock, not the fix. One regression at the owner boundary
+covers the bug; do not replay the same scenario at every layer it crosses.
+
+## Junk patterns
+
+The shared checklist for both modes: the authoring gate rejects a new test that
+matches one, and audits hunt for existing tests that do.
+
+- assertion-free coverage probes;
+- self-comparisons and identity copiers;
+- copied fixtures, inventories, manifests, or export lists;
+- exact source, import, or string greps;
+- private predicate or call-shape tests duplicated at real boundaries;
+- duplicate invocations of the same contract;
+- provider-local replays of shared helpers;
+- tests whose only purpose is preserving test-only exports, globals, or wrappers;
+- dead production code whose only callers are tests;
+- expected values produced by the helper or renderer under test;
+- mocks that implement the asserted behavior, or one identical mock standing in
+  for different APIs;
+- fixtures that supply the receipt, admission, or callback ordering the owner
+  should produce, or persistence asserted against a store the path never writes;
+- capability tests that restate declared flags instead of exercising the
+  delivery or acknowledgement the flag promises;
+- negative controls that pass for an unrelated reason, such as a denial from a
+  different guard or a rejection the production path never reaches;
+- names or fixtures that promise more than the input exercises, such as a
+  "retires the window" test asserting the window was not cleared.
 
 ## Value bar
 
 Tests justify their maintenance cost by protecting behavior, a credible
-regression, or an independently meaningful contract. A test that must change
-for behavior-preserving source reorganization is suspect, not automatically
-deletable.
+regression, or an independently meaningful contract. In an audit, an existing
+test that must change for behavior-preserving source reorganization is suspect,
+not automatically deletable; the authoring gate still rejects new ones.
 
 Before judging a candidate, read the complete test and production owner, its
 entry point, callers, callees, sibling implementations, overlapping tests, CI
@@ -55,18 +88,8 @@ run parallel discovery lanes when available:
 - UI, apps, scripts, and tooling;
 - a cross-cutting pattern sweep.
 
-Prefer a few high-confidence candidates over a large speculative inventory.
-Look for:
-
-- assertion-free coverage probes;
-- self-comparisons and identity copiers;
-- copied fixtures, inventories, manifests, or export lists;
-- exact source, import, or string greps;
-- private predicate or call-shape tests duplicated at real boundaries;
-- duplicate invocations of the same contract;
-- provider-local replays of shared helpers;
-- tests whose only purpose is preserving test-only exports, globals, or wrappers;
-- dead production code whose only callers are tests.
+Outside campaign mode, prefer a few high-confidence candidates over a large
+speculative inventory. Hunt for the [junk patterns](#junk-patterns).
 
 ## Retention bar
 
@@ -76,7 +99,11 @@ cross-language, package, release, or architecture contract. Also keep:
 
 - call ordering when order is observable behavior;
 - regressions with a credible failure mode;
-- source inspection when it is the cheapest independent guard.
+- source inspection when it is the cheapest independent guard: it fails when
+  the contract changes (the user-facing key, byte, or path) and survives an
+  identifier-only refactor;
+- a retained test that fails on the baseline: treat it as a possible product
+  bug, reproduce it, and repair the owner rather than deleting it.
 
 Static or slow is not a deletion reason. A test that resembles implementation
 may still be the independent contract; prove otherwise before removing it.