fix(ci): reduce Windows CLI test contention - #13077
Conversation
Windows CLI unit tests ran 4-at-a-time on the 4-vCPU runner (custom test-runner default = min(4, cpus)), oversubscribing CPU so heavy real-server test files blew their per-test timeouts. - Cap KILO_TEST_CONCURRENCY=2 on Windows (each file gets ~2 vCPU). - Grow Windows shards 4 -> 6 to absorb the lower per-shard parallelism. - Shard by observed duration instead of file size, so the two heaviest files no longer stack in one shard. - Raise the Windows per-file kill deadline to 600s (KILO_TEST_FILE_TIMEOUT); the heaviest file runs ~270s, only ~30s under the old 300s default. Scoped to the @kilocode/cli custom runner on Windows only. Linux/macOS unchanged; the non-CLI/core suite runs via plain 'bun test' and is not affected (separate lever).
There was a problem hiding this comment.
Pull request overview
This PR reduces CI flakiness on Windows for the @kilocode/cli unit suite by lowering per-shard parallelism and improving shard balancing so heavy, real-server test files don’t compete for limited CPU on 4‑vCPU runners.
Changes:
- Add
KILO_TEST_CONCURRENCYandKILO_TEST_FILE_TIMEOUTenvironment overrides to the CLItest-runner.ts(while keeping CLI flags as highest priority). - Switch shard weighting from file size to duration-aware weighting using a small set of per-file duration hints (with size as fallback).
- Update the CI unit-test matrix to use 6 Windows shards (was 4) and set Windows-only env overrides (
concurrency=2,file timeout=600000ms).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/opencode/script/test-runner.ts |
Adds env-configurable concurrency and per-file deadline, and uses duration-based shard weighting to spread slow tests across shards. |
.github/workflows/test.yml |
Adjusts Windows sharding to 6 and applies Windows-only runner env overrides to reduce CPU contention/timeouts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| } | ||
| const weight = (file: string) => Bun.file(path.join(root, "test", file)).size | ||
| // kilocode_change start - shard by estimated DURATION, not file size. File size is a poor | ||
| // proxy: run-process.test.ts is ~7 KB but ~230s, while config-overlay is the single slowest |
There was a problem hiding this comment.
SUGGESTION: Comment says run-process.test.ts is "~7 KB", but the file is ~19 KB (19,664 bytes) in the current tree.
The rhetorical point (small file, ~230s runtime) still holds, but since the block asks future maintainers to refresh these from observed data, it's worth keeping the numbers accurate.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| // DURATION_HINTS are max observed per-file durations (ms) from real Windows CI runs; the LPT | ||
| // splitter places the highest-weight files first, so hinted heavy files get spread across | ||
| // distinct shards. Unhinted files fall back to size (a fine proxy among the fast majority); | ||
| // hint values (tens of thousands of ms) dominate byte sizes, so heavy files always sort first. |
There was a problem hiding this comment.
SUGGESTION: The claim that "hint values ... dominate byte sizes, so heavy files always sort first" is only true for the largest hints.
Several unhinted files outweigh the smaller hints by byte size today: provider/transform.test.ts (~201 KB) sorts above the snapshot.test.ts hint (165_000), and kilocode/sessions/remote-sender.test.ts (~131 KB) sorts above the session/prompt.test.ts hint (128_000); ~8 unhinted files exceed the tool/task.test.ts hint (64_000). The primary goal — keeping the two heaviest files (config-overlay 270s, run-process 233s) in distinct shards — is unaffected since those are the top two weights, and the CI evidence backs the change. Just consider softening "always" (e.g. "the heaviest hinted files sort first") so future readers don't over-trust the guarantee.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
The change is well-scoped and mechanically sound: env delivery through turbo works ( Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Reviewed by kimi-k3 · Input: 70K · Output: 19.6K · Cached: 896.9K Review guidance: REVIEW.md from base branch |
The comment claimed per-shard concurrency was already at its min(4, cpus) cap with no headroom. That describes this branch's base, not main: PR #13077 landed KILO_TEST_CONCURRENCY=2 and KILO_TEST_FILE_TIMEOUT=600000 for Windows CLI tests because the default 4 blew per-test timeouts on the 4-vCPU runner. That matters beyond accuracy. This stack is based on #13068's head, which predates #13077, so it currently carries neither env knob -- rebasing onto main must preserve them or the extra shards buy wall-clock back by reintroducing the flakiness #13077 fixed. The 733s anchor these projections scale from was itself measured with the throttle in place, so the numbers already assume it.
Two conflicts, both where main and this branch fixed the same problem. test-runner.ts: main added a hand-maintained DURATION_HINTS table because file size is a bad shard weight; this branch reads real per-file durations from junit history for the same reason. Kept both -- measured history wins when --history supplied any, and the hints stay as the no-history default, which is the case on a first run or a cold cache. test.yml: main took Windows 4->6 shards to absorb #13077's KILO_TEST_CONCURRENCY=2 throttle; this branch already takes it to 10 for the same throttle, so this side supersedes it. The dangerous part of this merge was not either conflict. Git cleanly auto-merged the "Run CLI unit tests" step by taking this branch's side, which predates #13077, silently dropping KILO_TEST_CONCURRENCY and KILO_TEST_FILE_TIMEOUT -- exactly the regression the shard-count comment warns about, and one that would have reintroduced #13077's Windows flakiness at 10 shards. Both are restored. Also fixes a defect this branch introduced: the batch allowlist drift warning measured staleness against the selected files, so every sharded run printed "256 allowlist entries are not in this run". It now measures against the whole suite and names the offenders. Verified on the merged tree: typecheck clean, 10/10 batch tests, 38/38 across the sharding suites, the matrix generator still emits valid JSON (20 entries), and shard 1/8 is 50/50 green in 99.7s.
Sharding buys parallelism by duplicating a fixed cost. On Windows that cost is 118s per shard -- 83s of it Setup Bun, because Windows deliberately skips the dep cache -- so seven shards pay it seven times. Scaling up pays it once. Blacksmith bills strictly by vCPU x minutes (1 Windows 2vCPU minute = 2 x64 2vCPU minutes, applied proportionally at higher vCPU counts), so with wall = overhead + max(heaviest file, work/lanes) over the 1315s of test work measured in run 31657477161, in x64 2vCPU-minute units: 7 x 4vCPU, 2 lanes each wall 306s billed 143 1 x 8vCPU, 4 lanes wall 447s billed 60 1 x 16vCPU, 8 lanes wall 306s billed 82 1 x 32vCPU, 16 lanes wall 306s billed 163 16 vCPU holds the wall clock at exactly today's 306s for 43 percent less, since six of the seven Setup Bun + install cycles stop existing. 32 buys nothing: past 8 lanes the wall is pinned by the heaviest single file (cli/run/run-process.test.ts, 188s), not by throughput. KILO_TEST_CONCURRENCY goes 2 -> 8 because what #13077 actually fixed was per-lane starvation, not lane count: 4 vCPU / 2 lanes and 16 vCPU / 8 lanes are both 2 vCPU per lane, so the per-lane budget its timeouts depend on is unchanged. Going past 8 would re-create the starvation it diagnosed. The job is now named `unit (windows)` rather than `unit (windows, i/7)`, following the existing single-job macOS precedent. The aggregate gates depend on the whole `unit` matrix rather than on individual shard names, so they are unaffected -- but any branch-protection rule naming a Windows shard directly would need updating.
…-in-process fix(ci): reduce Windows CLI test contention
fix(ci): reduce Windows CLI test contention
Problem
Windows unit jobs are the biggest CI flakiness source (~62% of test-job failures). The
@kilocode/clitests run through our customtest-runner.tsatmin(4, cpus)= 4 files at once on the 4-vCPU Windows runner; heavy real-server files get ~1 vCPU each and blow their per-test timeouts.In scope (Windows,
@kilocode/clirunner only)KILO_TEST_CONCURRENCY=2) — each file gets ~2 vCPU.KILO_TEST_FILE_TIMEOUT) — the heaviest file runs ~270s, only ~30s under the old 300s default.Linux/macOS unchanged.
Out of scope
@opencode-ai/coresuite — runs via plainbun test, so these knobs don't reach it; throttling it is a separate lever (per @marius-kilocode's note).Evidence — 6 runs, all green
This PR's own CI run — a real
pull_request(cache-miss = genuine execution, real PR conditions): 31545193495 — all 6 Windows shards pass.Plus 5 prior
workflow_dispatchruns: 31536957068 · 31539345090 · 31539347494 · 31541429209 · 31541432642Across the 4 instrumented runs: 24 Windows shard-jobs / 2,640 file-runs → 0 failures, 0 timeouts, 0 flaky. Per-file timeout headroom went from ~30s to ~340s.
Cost / tradeoff
This buys stability with a bit more time — only on real (cache-miss) runs:
When no relevant code changed, the turbo cache replays in
1s, so it's almost free (+3 VM-min, negligible wall-clock). Still far under the 45-min job cap.Validation
workflow_dispatchruns mostly hit the turbo cache; the real proof is on-main PR traffic. Plan: merge, then watch the Windows failure rate over the following week.