You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
GitHub CI split coverage into a job that runs in parallel with build (#2159), which took the workflow from ~17 minutes to roughly the length of build alone. The local pre-push gate (npm run local:gate, renamed off ci in #2146) still runs them serially:
validate and coverage are independent in the same way there — coverage consumes nothing validate produces; every client's test:coverage builds whatever it needs itself. So the same win should be available locally, where it is paid for on every push by a human who is waiting.
Why this is not just validate & coverage & wait
CI's split is safe for a reason that does not transfer: the two jobs get separate runners, and therefore separate filesystems and separate CPUs. Run locally they share one working tree and one machine, which adds two hazards CI does not have. The workflow comment on the coverage job already says as much — "Do NOT 'optimize' this back into one job by backgrounding the two commands" — and that warning is about CI, but the filesystem half of it is worse locally, not better.
1. They write the same artifacts. Concurrent, non-atomic writes to the same output paths:
Artifact
Written by (validate side)
Written by (coverage side)
test-servers/build (+ its .tsbuildinfo, pinned inside build/)
validate:web → pretest → test-servers:build
coverage:webandcoverage:cli, each tsc -p ../../test-servers --noCheck
clients/cli/build
validate:cli → pretest → npm run build (tsup)
coverage:cli → npm run build (tsup)
The output is deterministic and identical, so this is not a wrong output risk — it is a torn-read risk: a reader can observe a partially written .js, and two tsc processes can corrupt one .tsbuildinfo. Intermittent and misattributed when it happens, which is the same failure shape #2111 was about.
2. Two vitest fleets on one CPU. Each half spawns workers sized to the core count, so on an 8-core box the two together oversubscribe it — the exact condition already recorded as timing tests out at the 5s default (two concurrent gate runs did this). Vitest 4 exposes no environment knob for this (there is no VITEST_MAX_THREADS in its dist), so capping means passing --maxWorkers inside each client's test script.
Proposed shape
scripts/run-local-gate.mjs — spawns npm run validate and npm run coverage concurrently, line-prefixes each half's output so an interleaved failure is still attributable, lets both finish rather than killing the survivor (so one run surfaces every failure), and exits non-zero if either did. local:gate calls it in place of the two serial steps.
Cap the workers so the two fleets together stay at roughly one core each, via --maxWorkers on the vitest invocations.
Measure it. The claim to verify is not just "it passes" but "it is meaningfully faster and does not flake" — several consecutive green runs, with before/after wall-clock. If the win turns out to be small (much of validate is builds and lint, which compete for the same cores coverage wants), that is a legitimate reason to close this rather than ship it.
Acceptance criteria
npm run local:gate runs validate and coverage concurrently and fails if either fails, with output that says which half failed.
No two processes can write test-servers/build or clients/cli/build at the same time, and neither build becomes conditional.
Wall-clock before/after recorded in the PR, over enough consecutive runs to show it does not flake.
No change to what either half checks.
Notes
Came out of #2146 (the rename), where this was raised and deliberately kept out of that PR: #2146 is a mechanical rename plus a guard and docs, and this is a change that can cause intermittent build corruption and test-timeout flakes. It deserves its own review and its own gate runs.
Problem
GitHub CI split
coverageinto a job that runs in parallel withbuild(#2159), which took the workflow from ~17 minutes to roughly the length ofbuildalone. The local pre-push gate (npm run local:gate, renamed offciin #2146) still runs them serially:validateandcoverageare independent in the same way there —coverageconsumes nothingvalidateproduces; every client'stest:coveragebuilds whatever it needs itself. So the same win should be available locally, where it is paid for on every push by a human who is waiting.Why this is not just
validate & coverage & waitCI's split is safe for a reason that does not transfer: the two jobs get separate runners, and therefore separate filesystems and separate CPUs. Run locally they share one working tree and one machine, which adds two hazards CI does not have. The workflow comment on the
coveragejob already says as much — "Do NOT 'optimize' this back into one job by backgrounding the two commands" — and that warning is about CI, but the filesystem half of it is worse locally, not better.1. They write the same artifacts. Concurrent, non-atomic writes to the same output paths:
validateside)coverageside)test-servers/build(+ its.tsbuildinfo, pinned insidebuild/)validate:web→pretest→test-servers:buildcoverage:webandcoverage:cli, eachtsc -p ../../test-servers --noCheckclients/cli/buildvalidate:cli→pretest→npm run build(tsup)coverage:cli→npm run build(tsup)The output is deterministic and identical, so this is not a wrong output risk — it is a torn-read risk: a reader can observe a partially written
.js, and twotscprocesses can corrupt one.tsbuildinfo. Intermittent and misattributed when it happens, which is the same failure shape #2111 was about.2. Two vitest fleets on one CPU. Each half spawns workers sized to the core count, so on an 8-core box the two together oversubscribe it — the exact condition already recorded as timing tests out at the 5s default (two concurrent gate runs did this). Vitest 4 exposes no environment knob for this (there is no
VITEST_MAX_THREADSin its dist), so capping means passing--maxWorkersinside each client's test script.Proposed shape
scripts/run-local-gate.mjs— spawnsnpm run validateandnpm run coverageconcurrently, line-prefixes each half's output so an interleaved failure is still attributable, lets both finish rather than killing the survivor (so one run surfaces every failure), and exits non-zero if either did.local:gatecalls it in place of the two serial steps.clients/webandclients/cli'stest-servers:buildat the existingscripts/lib/ensure-test-servers.mjs— which is already documented as "the one placetest-servers/buildis produced for a script consumer" (The smokes rebuild test-servers only when it is MISSING, so a stale fixture silently tests the wrong thing #2111), and where the test scripts are the remaining non-consumers — and give it a cross-process lock.proper-lockfileis already a root dependency andwithSecretFileLockis the in-repo precedent. Do the same for theclients/clitsup collision. Keep the build unconditional: The smokes rebuild test-servers only when it is MISSING, so a stale fixture silently tests the wrong thing #2111's whole point is that presence is not freshness, so this must serialize the build, never skip it.--maxWorkerson the vitest invocations.validateis builds and lint, which compete for the same corescoveragewants), that is a legitimate reason to close this rather than ship it.Acceptance criteria
npm run local:gaterunsvalidateandcoverageconcurrently and fails if either fails, with output that says which half failed.test-servers/buildorclients/cli/buildat the same time, and neither build becomes conditional.Notes
Came out of #2146 (the rename), where this was raised and deliberately kept out of that PR: #2146 is a mechanical rename plus a guard and docs, and this is a change that can cause intermittent build corruption and test-timeout flakes. It deserves its own review and its own gate runs.