Double the test timeout on Windows CI runners - #16
Merged
Conversation
The post-merge main run failed with a single 30s test timeout on the 4-vCPU Windows runner: resume.test.ts "recovers each intermediate phase" -- the git-heaviest test in the suite (five git-initialized fixtures plus per-resume snapshots, roughly fifty blocking git spawns) -- while 1,647 of 1,648 tests passed. Windows process spawn costs several times more than Linux, and the second vitest worker now runs the subprocess-heavy v1.2 driver suites concurrently, so the test''s long-standing margin inside 30s is gone: aggregate Windows test time was 1,102s in that run versus ~160s on ubuntu. A timeout is a slowness budget, not a correctness assertion. The budget doubles to 60s exactly where the slowness lives -- CI AND win32 -- and stays 30s for Linux, macOS, and local runs, protecting every borderline git-heavy test instead of patching this one with a per-test override.
7 tasks
HelloThisWorld
added a commit
that referenced
this pull request
Aug 20, 2026
…TP client (#19) The shared model-API HTTP client composed its total timeout as AbortSignal.any([AbortSignal.timeout(ms), external]) built inline in the fetch call. On Node 20 the composite holds only weak references to its source signals, so an otherwise-unreferenced timeout signal can be garbage collected before its timer fires -- and a request against an endpoint that never answers then hangs forever instead of timing out. This is the root cause of the recurring node-20 CI failure where "a timeout aborts the request deterministically" (contract: ~1.5 s) burned the full 30-second Vitest budget: the abort simply never happened. The flake predates v1.3 (identical failures on the #14 and #16 merge commits) and is GC-timing dependent, which is why it appeared intermittently and almost always on the node 20 runners. Every consumer of safeHttpRequest was exposed: Ollama, OpenAI-compatible endpoints, the managed local model, and registry downloads. The client now creates one explicit AbortController with a real timer per request -- no GC dependence on any Node version -- wired to the external signal and released in a finally, which guarantees the timer and listener never outlive the request. This also fixes the 'abort' listener any() leaked on long-lived external signals (one per request, never unsubscribed), and makes the timeout genuinely TOTAL across redirect hops and body streaming, as the contract always documented. New regression tests pin the replacement mechanism at the client level: never-responding and headers-then-stall endpoints time out deterministically, external aborts classify as cancelled, sequential requests leave zero leaked listeners, and createBoundedAbort's fire/release/propagation semantics are covered directly. The plugin dist bundles are regenerated. Full suite: 1788 tests pass; lint, typecheck, snapshots, smoke-relevant bundle checks all verify.
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
The post-merge
mainrun failed on Windows/node 20 with a single 30s test timeout —resume.test.ts > recovers each intermediate phase— while 1,647 of 1,648 tests passed (failing job). Nothing is functionally wrong: that test is the git-heaviest in the suite (five separate git-initialized fixtures plus a snapshot per resume — roughly fifty blockingexecFileSyncgit spawns inside one budget), Windows process spawn costs several times more than Linux, and the second vitest worker now runs the subprocess-heavy v1.2 driver suites concurrently. The run''s aggregate Windows test time was 1,102s versus ~160s on Ubuntu — the test''s long-standing margin inside 30s is simply gone on that runner.A timeout is a slowness budget, not a correctness assertion, and the config already documents this class ("slow CI runners regularly exceed the 5s default"). The budget doubles to 60s scoped exactly to where the slowness lives —
CIANDwin32— and stays 30s for Linux, macOS, and every local run, so real hangs still fail fast where machines are fast. Scoping it globally for Windows CI (rather than a per-test override) protects every borderline git-heavy test instead of whack-a-moling the current slowest one.For the reviewer''s context, this is the third distinct Windows-CI failure mode since v1.2 landed, each with a different root cause and fix: the forks-pool IPC timeout (fixed by the threads pool), the bundle-verifier partial-line race (fixed in #15), and now a genuine slowness-budget shortfall. No
onTaskUpdateerror and no verifier failure occurred in this run — those fixes hold.Checklist
pnpm lint,pnpm typecheck, andpnpm testpass locally — config-only change; the affected file (tests/orchestration/resume.test.ts) passes locally, and the full suite was green on this tree at the Fix flaky bundle verifier: stop parsing partial stdout lines #15 mergepnpm check:public-contractspasses — no contract touched