ci: split test.yml into four concurrent jobs behind a fail-closed result gate - #2075
Merged
Conversation
…ult gate The single per-platform job ran ~14 steps sequentially, so its wall clock was the sum of every step rather than the longest dependent chain. Measured PR wall clock was 452 s (434-483 s, n=3 healthy runs), with the Windows leg on the critical path. Split the work into four independent jobs, keeping together only the steps that consume a previous step's build output: suites build package -> unit -> integration 121 s / 195 s agent-suite native bindings -> coding-agent vitest 126 s / 232 s release-archive build package -> binaries -> smoke 74 s / 149 s static-checks typecheck, file length, docs, contracts 30 s (Linux) Estimated wall clock is ~247 s, a ~205 s (45 %) cut, for ~35 % more runner-seconds (709 s -> ~957 s). Job id `test` keeps its name expression and its two matrix rows and becomes a result gate, so the required contexts test (blacksmith-4vcpu-ubuntu-2404, linux-x64) test (blacksmith-4vcpu-windows-2025, windows-x64) stay byte-identical and no ruleset edit is needed. The gate carries `if: always()` and fails on failure, cancelled, and skipped: a job whose `needs` failed is skipped, and GitHub counts a skipped required check as satisfied. No test-runner flag changed. `--parallel` implies `--isolate`, and 20 files in test/unit import 108 sibling *.test.ts files, so a fresh module registry per file re-executes them: 5407 executions against 4426 distinct tests, each duplicate scored twice by the duration guard, once under contention. `--shard` is deterministic but buys no wall clock while Windows agent-suite is the critical path. test/unit/subagents-attempt-watchdog.test.ts and its helpers are untouched; the plan never runs the unit suite under contention. Every suite still reaches scripts/run-flaky-test-suite.ts through an unmodified `bun run <script>`, so budget resolution, blind detection and the 40 %/70 % ratios are unchanged. Diagnostics artifact names became job-unique because actions/upload-artifact@v4+ fails a run on duplicates. test:ci-contracts gains six topology contracts covering the gate, the per-job caps, the build couplings, artifact-name uniqueness and the absence of parallel flags. Moving that suite to a Linux-only job would have dropped its only CRLF checkout, so a new contract requires every workflow read to go through the newline-normalizing reader. Assistant-model: Claude Opus 5
The per-job caps came from four sequential-job runs. The first split run (30527771985) measured `suites` Windows at 348 s and `agent-suite` Windows at 349 s, against caps of 8 and 9 minutes, because Windows vitest took 221 s against a 142 s sample, the Windows native build 63 s against 42 s, and the unit step fired its one bounded flake retry on both platforms. Raise both Windows caps to 12 and `suites` Linux to 8, keeping the documented "hang detector at roughly 2x measured p100" rule against the newer measurement. A cap that cancels a passing retried run is worse than a late hang detection. Every cap still sits under the 15-minute Windows blanket it replaced, and the contract test now enforces that ceiling as well as the exact values. Record the observed per-job durations and queue delays in docs/ci.md: all seven work jobs started within 68 s of run creation, so the runner-concurrency risk did not materialize, but the whole run took 433 s against a 452 s baseline rather than the estimated 247 s. The wall clock is now dominated by two steps instead of fourteen, which is what makes sharding coding-agent vitest the next lever rather than a diluted one. Assistant-model: Claude Opus 5
Run 30528920082 finished in 440 s with no flake retry, so the two runs bracket the real figure: 433 s and 440 s against a 452 s baseline, a saving of about 15 s rather than the estimated 205 s. On both runs the critical path was Windows `agent-suite`, whose real cost is 349-380 s rather than the 232 s the estimate assumed; the sequential-job sampling understated every Windows step by roughly 1.5x. State that plainly next to the estimate instead of leaving the 247 s figure standing, and keep the structural result that does hold: wall clock is now dominated by two steps instead of fourteen, which is what makes sharding coding-agent vitest a direct lever rather than a diluted one. Assistant-model: Claude Opus 5
flora131
force-pushed
the
ci/parallelize-checks
branch
from
July 30, 2026 15:55
053618d to
dfef077
Compare
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
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.
What this does
.github/workflows/test.ymlran ~14 steps sequentially in one job per platform, so its wall clock was the sum of every step rather than the longest dependent chain. This splits the work into four independent jobs and keeps job idtestas a fail-closed result gate.suitesagent-suiterelease-archivebuild-binaries.sh→ archive smokestatic-checks(Linux only)test(gate, 2 legs)successSteps stay together only when one consumes another's build output: unit tests stay behind the package build (
test/unit/pi-0.82.1-artifacts.test.tssilentlytest.skips withoutdist/), integration stays behind the package build plus Node, vitest stays behind the natives build, andrelease-archiverebuilds natives itself becausescripts/build-binaries.shreuses or rebuildspackages/natives/native/*.node. Rust is dropped fromsuitesandstatic-checks; Node is dropped fromstatic-checks. The blanket 10/15-minute caps become measured per-job caps (suites6/8,agent-suite6/9,release-archive5/6,static-checks6, gate 5), each leaving room for the one bounded flake retry its job owns.Before / after wall clock
The critical path today is the Windows job at 452 s. After the split its longest chain is 48 s setup + 42 s native bindings + 142 s coding-agent vitest = 232 s, plus a ~15 s Linux result gate. The runner-second rise is the deliberate price of the wall-clock cut, not a surprise.
Both runs were green, but the estimate above did not hold. Recording it honestly:
static-checks (linux-x64)release-archiveLinux / WindowssuitesLinux / Windowsagent-suiteLinux / Windowstestgate, both legsBaseline for comparison: 452 s avg (434–483 s, n=3 healthy). Queue delays on run 1 were 9–10 s on Linux and 42–68 s on Windows.
The topology worked exactly as designed. All seven work jobs started within 68 s of run creation, so Blacksmith does not cap concurrency below seven and risk #1 (queueing) did not materialize. Both required contexts appeared with byte-identical names, the gate ran in 3–4 s, and
static-checkswas green in 32 s — feedback on typecheck and file length that previously arrived only at the end of a 257 s job.The saving is ~15 s, not ~205 s. The sequential-job sampling the plan was built on understated every Windows step by roughly 1.5×:
coding-agent vitestcoding-agent vitestOn both runs the critical path was Windows
agent-suite, whose real cost is 349–380 s rather than the 232 s the estimate assumed. That single number is where the plan's 247 s estimate came from, so the estimate cannot be recovered by tuning anything else.On run 1 the unit step's one bounded flake retry fired on both platforms, from two different pre-existing flakes, each of which passed on the retry (run 2 had no retry at all):
isolated default main lists and executes engine-only /workflow and /workflows while the host has no extensions(13.1 s, "Timed out waiting for fixture report") ininteractive-engine-default-main.test.ts.raw child stdout and stderr share the bounded telemetry budget— one of the tests the plan already identified as timing-sensitive.Neither is caused by this PR.
test/unitis untouched by the diff and each job owns its own runner, so nothing here runs under added contention. They are reported, not repaired: repairing them is out of scope and would mean weakening assertions. The retry machinery behaved correctly and the duration guard printed its table on both platforms.The useful structural result still stands: wall clock is now dominated by two steps instead of fourteen, so sharding
coding-agent vitest(the plan's phase 2) now has a direct effect rather than a diluted one, andstatic-checksreturns typecheck/file-length feedback in 32–50 s instead of at the end of a 257 s job. This PR deliberately does not shard anything, per the brief.docs/ci.mdcarries both runs so the next decision starts from measurements rather than from the estimate.Follow-up commit: two Windows caps widened
suitesWindows at 348 s was 72 % of its 8-minute cap on the very first run, with the retry. A cap that cancels a passing retried run is worse than a late hang detection, so the second commit applies the plan's own "roughly 2× measured p100" rule to the newer measurement:suites6/8 → 8/12,agent-suite6/9 → 6/12,release-archiveunchanged at 5/6,static-checks6, gate 5. Every cap still sits under the 15-minute Windows blanket it replaced, and the contract test now enforces that ceiling as well as the exact values.Executed vs distinct test counts
Measured in this worktree, Bun 1.3.14,
bun installfresh.test/ci— the suite this PR changesbun run test:ci-contracts)--shard=1/2--shard=2/2All three full runs and the union of the two shards produce byte-identical 31-name sets, so the suite is order- and partition-independent. Distinct coverage grows 25 → 31: one test (
test workflow preserves its two-platform matrix and deterministic contracts) is replaced by six stronger ones, and every assertion it made is still made. Nothing was removed.test/unit— unchanged by this PR, re-run to prove itscripts/run-flaky-test-suite.ts -- bun run test:unit(The 4427th record is the same
test.skipname printed twice; Bun's own footer reportsRan 4426 tests across 581 filesin every run.)test/integration: 289 executed, 289 distinct, pass, through the flaky-suite runner.The 950 duplicate executions
They are real re-execution, not a reporting artifact, and this PR avoids them by not using
--parallelanywhere.--parallelimplies--isolate. 20 files intest/unitimport 108 sibling*.test.tsfiles (e.g.executor.test.tsimports 20+ siblings), and Bun also collects those 108 files in their own right. Sequentially the module registry is shared so the second collection is a no-op; under--isolateevery file gets a fresh registry and each imported file's tests run twice. The arithmetic checks out: the sequential log attributes 607 records to the 20 aggregator headers and 360 to the 108 children = 967, against an observed excess of 981. A clean 8-file/24-test probe under--parallel=4executed each test exactly once, so--paralleldoes not duplicate by itself. Thesubagents-attempt-watchdog.test.tsheader appearing 9 times is separate and benign: Bun's parallel reporter reprints the file header on every worker flush.Consequence for the duration guard: every duplicated test is scored twice, once under contention, so the 70 % fail ratio starts firing on tests that are healthy sequentially. That alone disqualifies
--parallelfor this repository. A follow-up issue is worth filing for the redundant aggregator files, which already make "which file owns this test" depend on collection order.The 600 ms watchdog test
Nothing was changed about it.
test/unit/subagents-attempt-watchdog.test.tsandtest/unit/subagents-attempt-watchdog-helpers.tsare not deleted, skipped, weakened, or excluded, and the diff does not touch them.Under a saturation proxy (
--parallel=12on 12 cores, the same one-worker-per-core oversubscription--parallel=4would create on a 4 vCPU runner) the failure is a class, not one test, and it is unstable: three identical invocations failed 4, 3 and 3 tests, and the failing set changed between them. Fixing only the test named in the brief would not have made the first run green. The fixture sets a 1000 ms idle bound and a 4000 ms attempt cap for the whole file and individual tests tighten the wall cap to 600–700 ms; those bounds also apply to the healthy fallback attempt, so under contention the good child cannot produce first activity inside the idle window and the watchdog kills it. The wall-cap test already spends 928 ms of its 600 ms cap sequentially on an idle machine (330 ms of that is pure double child spawn).The test is not wrong — the environment stops being able to serve a sub-second deadline. So this PR simply never runs the unit suite under contention. If parallel execution is revisited, the correct fix is to derive the fixture's idle bound from a spawn-latency baseline measured once per suite (
idleMs = max(1000, k × measured spawn ms)) while leaving every assertion exact. Raising a magic constant would only move the cliff.Duration-headroom guard
Unchanged, because nothing it reads changed. Each split job still runs its own
scripts/run-flaky-test-suite.tsinvocation over an unmodifiedbun run test:unit/bun run test:integration/bun run --cwd packages/coding-agent --bun test, so budget resolution,blinddetection and the 40 %/70 % ratios are untouched. Verified end to end in this worktree:.ci-diagnostics/<suite>-durations.mdtable on a green run.A new contract asserts the three retried suites still reach the guard through unmodified
bun runcommands and that--parallel,--shard,--concurrentand--max-concurrencyappear nowhere in the workflow, so a future edit cannot silently reshape the records the guard scores..ci-diagnosticsartifact names are now job-unique (test-diagnostics-<job>-<binary_platform>);actions/upload-artifact@v4+fails the whole run on a duplicate name.Required check names — no maintainer action needed
No ruleset edit is required and no check name changes. Ruleset
9310196requires exactly:test (blacksmith-4vcpu-ubuntu-2404, linux-x64)test (blacksmith-4vcpu-windows-2025, windows-x64)Job id
testkeeps those two matrix rows and the samename: test (${{ matrix.os }}, ${{ matrix.binary_platform }})expression, so both strings are byte-identical. It becomes a result gate withif: always()that fails onfailure,cancelledandskipped.The
skippedcase is the one that matters: a job whoseneedsfailed is skipped, and GitHub counts a skipped required check as satisfied. Withoutalways()plus an explicit result check, a failing suite would turn the required context green — strictly worse than today. Becauseneeds.<job>.resultcollapses a matrix to one value, each gate leg asserts every platform's work jobs, which is strictly stronger than the per-platform meaning the context had before. The gate does no platform work, so both legs run on the Linux runner rather than burning a Windows runner and its measured 33 s queue.If maintainers later prefer real per-job required contexts (which would remove the ~15 s gate), that is a separate deliberate change: replace the two contexts with the eight work-job contexts in the same window as the workflow merge. Do not do both at once.
One thing to verify on the first run of this PR: the estimate assumes 7 jobs start about as promptly as 2 do today (measured queue: Linux 9 s, Windows 33 s). Read
gh api .../jobs --jq '.jobs[]|{name,created_at,started_at}'; if Windows queueing exceeds ~60 s, mergerelease-archiveback intosuiteson Windows.Contract-test changes
test/ci/ci-workflow-contracts.test.tsshrinks: its workflow-text helpers move to a newtest/ci/workflow-text.tsshared by all four CI contract suites, and the topology contracts land in a newtest/ci/test-workflow-topology.test.ts:if: always(), itsneedsset, and that its single step fails onfailure|cancelled|skipped;timeout_minutes: 10|15may not come back;agent-suite, and the smoke steps insiderelease-archive, so neither depends on file-order adjacency across jobs;useblacksmith/actionscheckout pair is asserted for each of the three cross-platform jobs, and the gate is asserted to check out nothing;Moving
test:ci-contractsinto a Linux-only job drops its only CRLF checkout (.gitattributesmarks*.ymlastextwithouteol=lf), so a new contract requires every CI contract suite to read workflow text through the newline-normalizing reader. That converts a platform-dependent trap into one Linux can enforce;release-publisher-contracts.test.tsandrelease-recovery-contracts.test.tswere converted to comply.docs/ci.mdis rewritten for the new job graph, the gate's role, the exact required contexts, and the measured basis for each cap.No
packages/*/CHANGELOG.mdentry: this is CI-only infrastructure perAGENTS.md.Validation
All run in a dedicated worktree with Bun 1.3.14 after a fresh
bun install:bun run typecheck✅ ·bun run lint✅ ·bun run check:file-length✅ (max 500; largest touched file is 398 lines)bun run test:ci-contracts✅ ×3 plus--shard=1/2and--shard=2/2, identical 31-name setsbun run scripts/run-flaky-test-suite.ts ... -- bun run test:unit✅ 4426 tests, first attempt, guard not blindbun run scripts/run-flaky-test-suite.ts ... -- bun run test:integration✅ 289 tests, guard not blindbun run --cwd packages/coding-agent docs:check✅Bun.YAML.parseon the new workflow, and the gate's shell logic exercised againstsuccess,failure,cancelled,skippedand empty inputs (empty fails closed)bun run test:unit, passed on commitNo unrelated failures were observed.
Open PR #2073 (
ci: guard Blacksmith runner coverage and trim workflow comments) also edits.github/workflows/test.ymlandtest/ci/ci-workflow-contracts.test.ts. Whichever of the two merges second will need a rebase.Greptile Summary
This change separates CI work into concurrent suite, native-agent, release-archive, and static-check jobs while preserving the two existing required
testcheck names through a result gate. The gate was exercised with successful, failed, cancelled, skipped, and empty dependency-result sets: only the all-success case passed, and every incomplete or unsuccessful result failed closed. The repository’s CI workflow contract suite completed with 32 passing tests.Confidence Score: 5/5
What T-Rex did
Reviews (4): Last reviewed commit: "docs(ci): record the second split run an..." | Re-trigger Greptile