fix(ci): run the fuzz corpus replay outside the coverage lane - #1994
Conversation
|
Size Report
npm unpacked components
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. Top changed packed files
|
938a3fa to
0ac0fcc
Compare
|
Request changes at |
12ab7fa to
8a496ea
Compare
|
Both corrections applied at (1) The membership rule was false — confirmed and removed. Since the proximate cause was never reproduced, I replaced the mechanism-shaped rule with the observed one: a file is admitted only on a record of actually vanishing from the instrumented Coverage lane, with run URLs — a theory about workers is not enough. I also renamed the project and constant (2) I took the removal rather than the test because the genuinely leaky case is not reachable without a seam: the path that strands a live thread is the startup-budget miss, and Diff is now 6 files: Re-validated on the new base: One thing worth flagging from the sweep, since it changes how this class gets triaged: a run that is re-run to green reports |
Cross-PR review finding against #1996 (currently open, all-green): once both land, the coverage lane breaks.
#1996 sets
There is a second wrinkle in the other direction: #1996's Coverage Report job runs Suggested reconciliation (either side can own it): make the shard env apply only to the instrumented leg (e.g. distinct blob names per leg, or move the fuzz-worker invocation out of the shared script into the workflow), and skip the second leg when Also a heads-up: this PR and #1997 both edit the readiness bullet in |
76ae7d2 to
1dd134f
Compare
|
Verified the #1996 finding by merging the two branches locally and running the sharded lane. It is real, and the failure is harder than described — the shared leg doesn't silently produce a wrong report, it takes the shard down:
Fixed at "test:coverage:ci": "vitest run --coverage --project=!fuzz-worker && pnpm test:fuzz-worker",
"test:fuzz-worker": "AGENT_DEVICE_COVERAGE_SHARD= AGENT_DEVICE_COVERAGE_MERGE= vitest run --project fuzz-worker",I put it on this side because the two-leg split is the only thing that knows there are two legs — moving the env from job-level to step-level in #1996 wouldn't help, since both legs live inside the single Re-ran the whole sharded lane on the merged tree with the fix:
Two notes for whoever wires this up:
On the |
|
Reviewed exact head |
1dd134f to
eb169d7
Compare
|
Review of exact head One P3 cleanup remains: Sequence is also required: land #1996 first, then rebase #1994, resolve the real |
|
#1996 is now merged and this branch is conflicting as anticipated. Rebase onto current main, preserve #1996's shard/merge switches plus #1994's fuzz-worker project/env reset, fix the stale bare-coverage rationale noted above, and rerun the combined coverage lane. No new code review is needed until the head changes. |
The Coverage job intermittently ends with no failing test and one file's
results missing:
Test Files 1070 passed (1071)
Errors 1 error
Error: [vitest-pool]: Worker forks emitted error.
Caused by: Error: Worker exited unexpectedly
This is shape (B) of #1824 — the half #1854 did not fix. Scanning every
failed Coverage job across the 120 CI runs since #1854 merged finds the
signature five times, and the vanished file is
scripts/fuzz/corpus-replay.test.ts all five (six for six with #1866's
occurrence): 23% of Coverage failures in that window, ~4% of all CI runs.
The ~40s gap before the error is coverage report generation, not test
time — the pool surfaces its AggregateError only once every task settles.
Control, from a green attempt of the same run: the file passes in 3152ms
at 09:37:35.9 and the summary prints at 09:38:12.5. So the file is not
slow in CI, nothing else is in flight when it dies, and neither a missed
per-case budget nor STARTUP_BUDGET_MS is implicated. Partial test counts
(3/11 and 9/11 reported) place the death mid-file, inside runCases.
So the corpus replay gets its own serialized project that the coverage
run skips, and a second uninstrumented Vitest invocation in
`test:coverage:ci` runs it, keeping the tests on every PR. Measured
against two full runs, this costs zero coverage: the cases execute in
worker threads, a separate isolate the fork's inspector never
instruments, so the lines reported are identical with and without it.
Membership is by demonstrated failure, not by a property of the code:
`session-replay-runtime-maestro.test.ts` also constructs a
node:worker_threads Worker and stays in unit-core, instrumented and
green, so "nests a Worker" is explicitly not the criterion.
The second leg goes through `test:fuzz-worker`, which blanks
AGENT_DEVICE_COVERAGE_SHARD and AGENT_DEVICE_COVERAGE_MERGE. ci.yml sets
those as job-level env over a single `gate: unit-ci` step, so both legs
would otherwise inherit them and the shard would die: Vitest refuses
`--shard=1/2` over this one-file project, and the blob reporter
overwrites the instrumented shard's report on its way out. Verified on
the merged tree — shard 1/2 (549 files), shard 2/2 (548), and the merge
job (1097 files, 90.38% lines) all pass, and the leg still fails without
the blanking.
Refs #1824
eb169d7 to
ca3dd64
Compare
|
Rebased onto The Combined coverage lane, re-run on the real merged tree (not the synthetic merge from before):
549 + 548 = 1097, and the fuzz leg runs on each. Negative control on the same tree, so the guard is demonstrably load-bearing rather than defensive: P3 rationale cleanup. Both sites updated:
Local: Thanks for the |
|
Re-reviewed |
…) (#2007) Deletes the serialized `subprocess-stub` Vitest project and drops SUBPROCESS_STUB_TESTS from unit-core's exclude, so its two real spawners (client-metro.test.ts, harness.test.ts — corpus-replay.test.ts already left for fuzz-worker in #1994) run un-serialized in the default forks pool per #1823's own kill criterion. Revert if a timeout-shaped failure shows up before 20 consecutive CI runs pass clean. The files stay excluded from the mutation lane (SERIALIZED_TESTS): that exclusion is about mutant-rerun cost, independent of Vitest project structure. Updated the comments/docs/scripts that described the old project by name so none of them assert a project that no longer exists. Claude-Session: https://claude.ai/code/session_015YPgKE1xmjdqh7T1q987DA Co-authored-by: Claude <noreply@anthropic.com>
Summary
The Coverage job intermittently ends with no failing test and one file's results missing:
This is shape (B) of #1824 — the half #1854 did not fix, and which that issue said to fix at the source if it recurred. It has.
scripts/fuzz/corpus-replay.test.tsmoves into its own serialized project that the coverage run skips (--project=!fuzz-worker); a second, uninstrumented Vitest invocation intest:coverage:ciruns it, so the tests still execute on every PR.The vanished file is the same one every time
Scanning every failed Coverage job across the 120 CI runs since #1854 merged (2026-08-21 → 08-24): the signature appears 5 times, and the vanished file is
scripts/fuzz/corpus-replay.test.tsin all 5 — 23% of Coverage failures in that window, ~4% of all CI runs. With #1866's occurrence that is six for six.corpus-replay.test.tscorpus-replay.test.tscorpus-replay.test.tscorpus-replay.test.tscorpus-replay.test.tscorpus-replay.test.tsIdentified by subtraction: the parenthesised count is what was collected, the first number is what reported. (
gh run view --logemits ANSI as literal caret notation, so strip with(?:\x1b\[|\^\[\[)[0-9;]*m.) A run re-run to green reportsconclusion: success, so this needs a per-attempt sweep — scanning--status failurealone misses most occurrences.Every one has zero failing tests, exactly one file's results missing, and
scripts/fuzz/harness.test.tsas the last file reported — i.e.corpus-replaywas alone in flight.What the evidence rules out
The ~40s gap before the error is coverage report generation, not test time. The pool surfaces its
AggregateErroronly once every task settles, which on this job is after the report is written. Control, from a green attempt of run32630591037:So the file takes 3.1s in CI, not 40. That rules out:
subprocess-stubis serialized andunit-corehad drained ~40s earlier.terminate()— there is no 5s overrun in a 3s file, and a hang would have failed the test loudly rather than vanishing it.STARTUP_BUDGET_MSbeing too low — a 60s budget is never approached.Partial test counts (3/11 and 9/11 reported) place the death mid-file, inside
runCases.Membership is by demonstrated failure, not by a property
An earlier revision of this PR claimed the criterion was "constructs a
node:worker_threadsWorker inside the Vitest worker". That rule is false, as review caught:src/daemon/handlers/__tests__/session-replay-runtime-maestro.test.tsconstructs one and stays inunit-core, instrumented and green. It is the only other test file in the repo that does, and it has never died.Since the proximate cause was never reproduced, the honest criterion is the observed record: a file is admitted here only when it has actually vanished from the instrumented Coverage lane, with run URLs. A theory about workers is not enough. The project and constant are named
fuzz-worker/FUZZ_WORKER_TESTSso the name does not re-assert the false rule.Measured: this costs zero coverage. Two full local runs, baseline vs. variant, differ by 0 lines — the cases execute in worker threads, a separate isolate the fork's inspector session never instruments, so the file never contributed measured coverage in the first place.
scripts/gate/model.tslearns Vitest's negated--projectsemantics, sounit-cistill provably owns every project instead of being credited with one it skips.Validation
pnpm gate unit-cigreen end-to-end:1094 passed (1094), thenfuzz-worker 1 passed (1).typecheck,lint,format:check,check:fallow --base origin/main,check:layering,check:gate-manifest,check:agent-guidance,check:tmpdir-leaks,mutation:test, and thescripts/gate+scripts/check-affectednode:test suites all pass.Honest limits
poolOptions.forks.execArgv), which would at least turn a silentSIGKILLinto a namedJavaScript heap out of memory—emitUnexpectedExitdiscards the exit code and signal, so today CI cannot tell us which it was.docs/agents/testing.mdis at 9,985 of its 10,000-byte ceiling onmain, so a pointer to the new project does not fit. Per that gate's own rule, the rationale lives with its owner invitest.config.ts.Refs #1824