chore(test): remove the contention retry (0 fires since 07-29) and shrink the subprocess-stub project to its 3 real spawners (#1781 A4)

[thymikee]

#1781 item A4. Two changes, one theme: keep the part of the contention story that still pays, delete the part that never fired.

(a) The contention single-retry policy is removed

It has fired 0 times since it landed on 2026-07-29 (#1448):

• 0 of 234 sampled Coverage-job lane envelopes (2026-08-11 → 2026-08-18, pulled through the Artifacts API) had retryCount > 0. 20 of those runs failed; none was retried.
• 0 of 17 recent failed CI runs were retried: 5 refused with "a failure landed outside the enumerated retry list", 4 with "the run failed for a reason a rerun cannot re-check (unhandled error)", the rest n/a.
• All three trackers its 10 entries pointed at (Slow-test ratchet: convert real-time waits to injected budgets #1098, test: nightly fuzz lane — parser inputs must fail as typed AppErrors, never hang #1414, ci: single-retry policy for enumerated contention-flaky files (timeouts only) #1419) are closed/completed, so no entry had a live owner. None had expired (REVIEW_BY = 2026-10-31), so the expiry gate would not have surfaced this for another 74 days.

What it cost: ~1,454 LOC deleted — scripts/lib/contention-retry{,-args,-blockers,-lane,-reporter,-run}.ts (747), contention-retry-policy.test.ts (531), runner-timeout-meta.ts + scripts/vitest-runner-timeout-setup.ts + run-blocker-bus.ts (71), test/contention-retry-fixtures/* (75), config (~30) — plus a per-run secret marker threaded through a setup file on every Vitest project, and a standing obligation for every future gate reporter to call recordRunBlocker.

Also removed: the check:contention-retry package script and its place in check:unit, the contention-retry gate (catalog + CI step), the contention-retry-envelope artifact upload, and the vitest-runner-timeout-setup.ts setup-file wiring. test:coverage:ci is a plain vitest run --coverage again — the exact argv the wrapper passed through for a coverage run (reporters come from vitest.config.ts, which no lane overrides).

Kept, with a note: scripts/lib/lane-envelope.ts (#1430) is not retry-specific — the mutation lane (scripts/mutation/run.ts), the fuzz lane (scripts/fuzz/envelope.ts) and the concurrency-torture lane all build their envelopes from it. Only the retry's consumer (contention-retry-lane.ts) and the retry's artifact-upload step are gone. run-blocker-bus.ts is deleted: its only consumer was the retry's failure sink, and its only publisher (the slow-test ratchet) already fails the run by setting process.exitCode = 1.

One follow-on: OPAQUE_RUNNERS loses its test:coverage:ci entry, because a bare vitest run --coverage is something the gate-manifest loader reads directly. That makes "a Vitest project no registered check runs" unrepresentable rather than merely detected, so scripts/gate/audit-coverage.test.ts now constructs that state by project-scoping the coverage script in its model mutation.

(b) The subprocess-stub project stays, shrunk to the 3 files that really spawn

The list had 10 entries; 6 were serialized. Measured in CI (run 32127451031) and locally:

kept	why	CI
src/__tests__/client-metro.test.ts	real Metro-stub server + child spawn per case	4.7s (567-922ms/case)
scripts/fuzz/harness.test.ts	the SUT is the subprocess watchdog; one target hangs on purpose	13.2-14.0s
scripts/fuzz/corpus-replay.test.ts	same watchdog, per-case budget	2.2-2.4s

dropped	CI	why
src/platforms/android/__tests__/app-deployment.test.ts	31ms	shell stubs in a tmpdir; nothing to serialize
src/__tests__/platform-runtime-runtime-hints.test.ts	212ms	env-driven stub exit codes, no sleeps
src/platforms/apple/core/__tests__/apps.test.ts	277ms	already vi.mocks the exec seam

All three now run in unit-core and pass there (verified locally: 3 files / 72 tests, 7.8s, slowest case 1.57s — inside the 2.5s slow-test budget). The 4 retry-only entries (request-router-open.test.ts, runner-client.test.ts, runner-xctestrun.test.ts — whose stated reason was false: it vi.mocks node:child_process and nothing spawns — and help-conformance-bench.test.ts) simply cease to be listed anywhere. The optional in-process conversions for them are deliberately not done here.

SUBPROCESS_STUB_TESTS is now a plain 3-entry array in vitest.config.ts with one line per entry saying why it spawns; fileParallelism: false, isolate: true, maxWorkers: 1 are unchanged.

Lane accounting

subprocess-stub project — retained

• Catches: CPU contention starving real stub-binary spawns past an internal budget.
• Evidence: pre-split android/index.test.ts aggregation per the config comment; the 3 members measured above genuinely spawn per case. (process.env isolation, the other stated motive, already comes from pool: forks + isolate: true.)
• Cost: ~15 config lines; ~0 CI wall clock (concurrent project, unit-core is the 296s critical path); ~25-37s serialized chain locally.
• Kill-criterion: subprocess-stub vitest project: members and kill criterion (#1781 A4) #1823 — if the three run un-serialized in the default pool for 20 consecutive CI runs with no timeout-shaped failure, delete the project.

Contention retry — removed. Catches: runner-proven timeouts in enumerated files only / Evidence: 0 fires in ~3 weeks, refused every observed failure class / Cost: ~1.45k LOC + a per-run secret marker + a bus every future gate reporter must call / Killed.

Issues filed

• subprocess-stub vitest project: members and kill criterion (#1781 A4) #1823 — subprocess-stub vitest project: members and kill criterion (the live tracker the config comment points at; all three old trackers were closed).
• Coverage job: "Worker exited unexpectedly" kills one fork in the last ~40s of the run (4 of 17 recent CI failures) #1824 — Coverage job: "Worker exited unexpectedly" kills one fork in the last ~40s of the run. 4 of the 17 recent CI failures (24%), Coverage job only, exactly one file's results vanish each time, all stub files ✓ before it. The retry refused this class by design and is now gone, so it has to be fixed at the source.

Test plan

• pnpm test:unit — 891 files / 6,847 tests pass, including the three files moved back into unit-core.
• pnpm test:coverage:ci — run locally; writes coverage/lcov.info.
• pnpm typecheck, pnpm lint, pnpm format:check — green.
• pnpm check:gate-manifest (46 checks across 33 lanes) and pnpm check:gate-manifest:test (37 tests) — green.
• pnpm check:affected:test (36 tests), pnpm check:layering (183 tests + guard), pnpm check:fallow — green.
• pnpm check:production-exports reports the same 17 pre-existing exports as clean origin/main (verified by stashing).

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-18 13:36 UTC

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	2.28 MB	2.28 MB	-3.0 kB
JS gzip	749.3 kB	748.3 kB	-952 B
npm tarball	871.2 kB	870.1 kB	-1.1 kB
npm unpacked	3.04 MB	3.04 MB	-3.6 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	22.0 ms	22.9 ms	+0.8 ms
CLI --help	54.2 ms	53.5 ms	-0.6 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/snapshot.js	-1.1 kB	-434 B
dist/src/internal/daemon.js	-885 B	-290 B
dist/src/prepare-kind.js	-207 B	-30 B
dist/src/runtime.js	-6 B	-8 B
dist/src/session2.js	0 B	+4 B

[thymikee]

Adversarial pass over #1827 (checked out origin/chore/1781-a4-contention-retry @ 57b93d6 in a throwaway worktree, pnpm install, then ran the actual gates rather than trusting the PR-body claims).

Verified against the diff (no leftover references found)

Grepped the whole tree for contention-retry, contentionRetry, CONTENTION_RETRY, runner-timeout/RUNNER_TIMEOUT/runnerTimedOut/takeRunnerTimeoutToken, run-blocker-bus/recordRunBlocker/drainRunBlockers, serializedStub, timeout-provenance, and #1419/#1430 across .ts/.md/.yml/.json. Only hits:

• docs/agents/testing.md:506 — the intended "Removed 2026-08-18 (#1781 A4)" historical note.
• src/platforms/apple/core/__tests__/screenshot.test.ts — a mkdtempForTest('agent-device-ios-runner-timeout-') fixture-name string, unrelated (predates this PR, not touched by it). False positive.
• #1430 hits are all the kept lane-envelope.ts consumers (mutation/fuzz/concurrency-torture), consistent with the PR's claim that lane-envelope itself is untouched.

No dangling doc anchors, no orphaned imports, nothing in .github/, scripts/gate/declarations.ts, scripts/check-affected/{checks,model}.ts, .fallowrc.json, package.json scripts, or either vitest config still points at a deleted symbol/file.

Ran the gates myself (all green, matching the PR-body numbers exactly)

• pnpm check:gate-manifest → 46 checks wired across 33 lanes — matches the body verbatim.
• pnpm check:gate-manifest:test → 37/37 pass — matches.
• pnpm check:affected:test → 36/36 pass — matches.
• pnpm check:fallow → No issues in 25 changed files.
• node --test scripts/gate/audit-coverage.test.ts → 6/6 pass, including the new projectScoped case that makes "an unrun Vitest project" representable now that OPAQUE_RUNNERS is empty (scripts/gate/declarations.ts:6, consumed by scripts/gate/model.ts:199).
• vitest run --project unit-core src/platforms/android/__tests__/app-deployment.test.ts src/__tests__/platform-runtime-runtime-hints.test.ts src/platforms/apple/core/__tests__/apps.test.ts → 3 files / 72 tests pass in unit-core. Confirms item (3): vitest.config.ts's unit-core.exclude: [...SUBPROCESS_STUB_TESTS] is derived dynamically from the same shrunk 3-entry array the subprocess-stub project's include uses, so the 3 dropped files fall straight into unit-core via its existing src/**/*.test.ts glob rather than being silently unlisted anywhere.
• oxlint on every touched non-deleted file: clean.

test:coverage:ci argv/reporter equivalence (item 2) — confirmed correct

Old contention-retry-run.ts's firstRunArgs was called with modes.projects derived from an unset --project CLI flag (package.json never passed one to the wrapper), so projectArgs([]) = [], and the first run was literally vitest exec vitest run --coverage with reporters resolved from reporters(env) where the only difference from bare config was appending contentionRetryReporter() (now correctly deleted along with the mechanism it fed). New "test:coverage:ci": "vitest run --coverage" is byte-for-byte the same argv the old wrapper produced for its first run, and vitest.config.ts's top-level reporters: ['default', slowTestGateReporter()] is unchanged — so the claim "reporters come from vitest.config.ts, which no lane overrides" holds.

Downstream: "Enforce changed-line coverage gate" in ci.yml still runs unconditionally after unit-ci and still reuses the lcov the Coverage step wrote — coverage.reporter in vitest.config.ts (untouched by this diff) still includes 'lcov'. The deleted "Upload contention-retry envelope" artifact step had zero other consumers in the repo (grepped), consistent with the mechanism it fed being gone.

Issues #1823 / #1824

Both open and match the PR body's claims precisely — #1823 lists the same 3 kept members + measured durations + the same 20-consecutive-CI-runs kill criterion; #1824 states the same "Worker exited unexpectedly" signature, run IDs, and "the retry refused this class by design and is now gone" framing. No discrepancy.

Minor / non-blocking

• package.json: test:coverage ("vitest run --coverage") and test:coverage:ci (also now "vitest run --coverage") are byte-identical scripts. Harmless — one is the CI gate entrypoint (scripts/gate/declarations.ts no longer needs to special-case it) and one is the local ad-hoc command — but a future reader diffing the two will wonder why there are two names for one command. Not worth blocking on.
• I chased a hunch that shrinking SUBPROCESS_STUB_TESTS (which vitest.mutation.config.ts:6,38 also consumes for its exclude) would let PATH-mutating files reach Stryker's thread-pool mutation runner unprotected. Empirically two of the three dropped files (app-deployment.test.ts, real process.env.PATH = mutation with try/finally restore) are already reachable via vitest related from the current stryker.config.json mutate list — so this isn't hypothetical, it's live today. But scripts/mutation/test-scope.ts's own stated thread-pool exclusion criterion is only chdir/worker_threads usage, never generic env mutation, and the files restore PATH synchronously in finally with no test.concurrent in the file (global maxConcurrency doesn't even apply since there's no concurrent test), so there's no realistic race. Checked, not a bug — noting only so nobody re-treads this.

Verdict

Everything the PR body claims reproduces exactly under a real re-run of the gates it cites, and an exhaustive grep across the whole tree turns up no leftover reference to any deleted symbol, file, or gate. The evidence for deleting the retry (0/234 fires, all three trackers closed) and for shrinking the stub list (measured per-file durations) is solid and independently checked here. This is about as clean a deletion PR as they come.

VERDICT: approve
One-line summary: exhaustive grep + re-running every gate the PR cites (check:gate-manifest, check:gate-manifest:test, check:affected:test, check:fallow, the 3 moved-back unit files, both linked issues) all confirm the PR body's claims with zero leftover references found.

[thymikee]

Reviewed exact head 57b93d6: clean. The contention retry is removed end-to-end without leaving stale workflow/gate/docs ownership; the bare coverage command preserves the wrapper’s first-run invocation. Subprocess-stub membership now has one source of truth and retains only the three genuine subprocess/watchdog tests, with gate and linked-issue evidence covering the decision. No device evidence is needed for this tooling-only change. Code review is ready-for-human; iOS Smoke is still in progress and the PR remains draft.