ci: consolidate CI workflow from 15 jobs to 8 - #1996
Merged
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 filesNo changed packed files. |
This was referenced Aug 24, 2026
fix(test): drain session-event-log writes before rmSync cleanup in save-script transport tests
#1999
Merged
Member
Author
Cross-PR finding with #1994 (currently open, all-green): once both land, the coverage lane breaks. #1994 changes Neither PR is wrong alone; they need one reconciliation before both merge. Detail posted on #1994. Separately, good news: the "known pre-existing flake" this PR documents ( |
thymikee
added a commit
that referenced
this pull request
Aug 24, 2026
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 (subprocess-stub is
serialized and unit-core has drained), 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 runs through `test:fuzz-worker`, which blanks
AGENT_DEVICE_COVERAGE_SHARD and AGENT_DEVICE_COVERAGE_MERGE. #1996 sets
those as job-level env over a single `gate: unit-ci` step, so both legs
would otherwise inherit them. Verified on a local merge of the two
branches: without the blanking the shard fails outright, because Vitest
refuses `--shard=1/2` over this one-file project and the blob reporter
overwrites the instrumented shard's report on the way out. With it, both
shards and the merge job pass and the merged report carries all 1094
files.
Refs #1824
thymikee
added a commit
that referenced
this pull request
Aug 24, 2026
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 (subprocess-stub is
serialized and unit-core has drained), 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 runs through `test:fuzz-worker`, which blanks
AGENT_DEVICE_COVERAGE_SHARD and AGENT_DEVICE_COVERAGE_MERGE. #1996 sets
those as job-level env over a single `gate: unit-ci` step, so both legs
would otherwise inherit them. Verified on a local merge of the two
branches: without the blanking the shard fails outright, because Vitest
refuses `--shard=1/2` over this one-file project and the blob reporter
overwrites the instrumented shard's report on the way out. With it, both
shards and the merge job pass and the merged report carries all 1094
files.
Refs #1824
Member
Author
|
Review of exact head The head is not ready for the label only because its base is stale: current Merge order: #1996 first, then rebase and revalidate #1994, whose environment clearing is a follow-on adjustment. |
thymikee
added a commit
that referenced
this pull request
Aug 24, 2026
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 (subprocess-stub is
serialized and unit-core has drained), 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 runs through `test:fuzz-worker`, which blanks
AGENT_DEVICE_COVERAGE_SHARD and AGENT_DEVICE_COVERAGE_MERGE. #1996 sets
those as job-level env over a single `gate: unit-ci` step, so both legs
would otherwise inherit them. Verified on a local merge of the two
branches: without the blanking the shard fails outright, because Vitest
refuses `--shard=1/2` over this one-file project and the blob reporter
overwrites the instrumented shard's report on the way out. With it, both
shards and the merge job pass and the merged report carries all 1094
files.
Refs #1824
Merge single-gate ubuntu jobs into grouped jobs sharing one checkout and install: Lint & Format (plus the static text assertions), Repo Guards (layering/selector/wiring/maestro/mcp-metadata), Compatibility & Provenance (shared fetch-depth: 0 checkout), Typecheck & Package, and Integration Tests (absorbs the web smoke with step-scoped env). Every gate remains an independently named run-gate step; the gate manifest derives lane ownership structurally. Drop the Bun setup from FreeRange: @chenglou/freerange's bin is a plain Node script. It stays GitHub-owned; only the runtime requirement is retired.
FreeRange runs on plain Node now, so its gate joins Repo Guards as the last step instead of occupying its own worker for the slowest guard. The fixture release matrix filters to entries that will actually build, so a cached-fingerprint PR starts zero release runners.
The macOS lane now builds one unit-test-flagged runner bundle that both the host XCTest run and the replay smoke consume, so the host lane no longer occupies its own macos-26 runner behind a separate queue. The host lane's file moves with it, and check:xctest-selection follows. Coverage shards across two runners via blob reports and merges them on a report job that evaluates thresholds once over the full suite and produces every coverage artifact. The tmpdir leak check runs per shard, since a leak lands on whichever runner executed the file.
A shard evaluates its own half-suite coverage, so the global gate fired per shard. Shards now report without gating; Coverage Report keeps the real thresholds over the full merged suite.
thymikee
force-pushed
the
ci/optimize-checks-2
branch
from
August 24, 2026 14:27
356ce79 to
bc48201
Compare
thymikee
added
the
ready-for-human
Member
Author
|
Re-review of exact head Sequence remains #1996 first, then rebase/revalidate #1994 so its environment clearing applies to the consolidated job topology. |
|
This was referenced Aug 25, 2026
Open
thymikee
added a commit
that referenced
this pull request
Aug 25, 2026
Reviewer feedback on #2036: the standalone four-case test duplicated the harness above it and only two states are meaningful for this regression. Reuse the same parsed workflow, temp dir, resolver stub, and Node stub; keep neither-cached (both platforms, has-work=true) and both-cached (empty matrix, has-work=false, release gated). Drops the single-cache permutations, which exercise #1996's unchanged filtering rather than this fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A typical code PR used to start ~19 ubuntu workers, most running a 25-second gate after a full checkout and install; queueing for workers dominated wall time. This consolidates and re-shards the CI surface with no gate removed or weakened — every check remains a named
run-gatestep, and the gate manifest derives lane ownership structurally from the workflow YAML.Job layout changes:
pnpm check:xctest-selectionfollows it to macos.yml) and the replay smoke. Removes the second macos-26 queue wait, which measured 5–6 min per PR.AGENT_DEVICE_COVERAGE_SHARD/AGENT_DEVICE_COVERAGE_MERGE.@chenglou/freerange's bin is a plain Node script; the Bun setup is gone and the gate joins Repo Guards as its last step. Still GitHub-owned (localRunnable stays false) so the pre-push affected path does not pay its ~3 min.Typical PR check lines drop from 28 to ~18–20 (the four CodeQL lines are repo-settings default setup, intentionally untouched). Ubuntu worker demand drops by roughly half; every non-device ubuntu lane now completes in under 5 minutes.
Validation
Evidence at 356ce79; all 20 checks green on that head:
pnpm check:gate-manifest→ ok (48 checks wired); gate manifest tests 49/49, affected-selector tests 62/62, xctest-selection tests 37/37;pnpm check:affected --run, typecheck, lint, format clean.Two issues surfaced by real runs and fixed here: vitest enforces coverage thresholds per shard (shards report ungated; only the merged run gates), and upload-artifact v4 silently skips dot-prefixed directories without
include-hidden-files.Known pre-existing flake exposed once under shard load (not from this change):
src/daemon/__tests__/request-save-script-transports.test.tshitENOTEMPTYin its own afterEach cleanup racing an async daemon-log write. It passed on re-run and repeatedly in isolation; worth a follow-up issue.