fix(cli): stabilize Windows CI tests and rebalance slow shards by marius-kilocode · Pull Request #12723 · Kilo-Org/kilocode

marius-kilocode · 2026-07-31T09:38:18Z

Problem

Windows CLI unit tests were unreliable:

server/httpapi-instance-route-auth.test.ts failed on Windows with an uncaught error: Invalid handle (no stack), killing the PTY websocket auth test. This broke main CI (run 30560035944, windows shard 1/4) and PRs.
kilocode/server/config-overlay.test.ts intermittently returned HTTP 500 (UnknownError) on Windows (PR feat(agent-manager): run setup scripts in panel terminal #12703), and three files needed whole-file retries (run-process, instance-bootstrap, httpapi-session) — retry overhead dominated the slowest shard (PR feat(agent-manager): run setup scripts in panel terminal #12703 windows 4/4 ran 12m50s).

Root causes and fixes

1. "Invalid handle" — config-provider blinding enabled the Windows file watcher on deleted temp dirs.

The test built its HttpApi app with ConfigProvider.layer(ConfigProvider.fromUnknown({ KILO_SERVER_PASSWORD, KILO_SERVER_USERNAME })). In Effect, ConfigProvider.layer replaces the ambient provider, so every Config.* read inside the app graph resolved against that two-key map and process.env became invisible. That blinded KILO_EXPERIMENTAL_DISABLE_FILEWATCHER, which test/preload.ts and CI set to \"true\" precisely to keep @parcel/watcher out of unit tests.

With the flag hidden, the watcher's Windows backend subscribed on the temp repo's .git during instance bootstrap. The await using tmp fixture then deleted that directory while the never-disposed per-test app runtime still held the subscription, and CreateFileW failed with the hardcoded WatcherError(\"Invalid handle\") (the exact string lives in @parcel/watcher/src/windows/WindowsBackend.cc:93). The napi rejection carries no JS stack, which matches the bare error: Invalid handle line in CI exactly. POSIX watcher backends (fs-events/inotify) tolerate a deleted watched dir; Windows does not, so the failure is Windows-only.

Fix: add KILO_EXPERIMENTAL_DISABLE_FILEWATCHER to every test config map that boots instances through the HttpApi app (httpapi-instance-route-auth, httpapi-cors, httpapi-ui, the exerciser backend.ts, httpapi-kilo-edit, httpapi-memory). The flag is read from env with a \"true\" default, matching preload, so the watcher stays disabled in these tests. Auth semantics are unchanged — fromUnknown treats undefined values as absent, and preload already scrubs KILO_SERVER_PASSWORD/KILO_SERVER_USERNAME from env.

2. config-overlay 500 — Windows locked-file errors on the atomic rename.

Filesystem.write writes to a temp file then renames it into place. On POSIX that rename is atomic regardless of other activity in the directory. On Windows, MoveFileEx fails transiently with EPERM/EACCES/EBUSY when Windows Defender/the search indexer or a concurrent writer (the detached Arborist plugin install that config.loadInstanceState kicks into the same tmpdir) briefly holds the freshly written temp file. The throw became an Effect defect → the error boundary's UnknownError 500. The repo already fights this exact Windows class in test/kilocode/cleanup.ts (60× retry with Bun.gc), but Filesystem.write had no retry.

Fix: mirror the trusted cleanup.ts locked() pattern — retry EBUSY/EACCES/EPERM with a short linear backoff (50ms × attempt, up to 8 attempts on Windows, 1 elsewhere). POSIX is byte-identical (single attempt); the retry only fires under Windows contention and never changes success semantics. This also hardens real Windows users' config saves against the same Defender/indexer race.

Follow-up (not in this PR)

The Windows shards are also imbalanced (the slowest concentrates the spawn/FS-heavy files), which adds wall time. Runtime-weighted sharding would fix it, but a committed timings manifest is a maintenance burden (it goes stale as tests change) and no static size heuristic can place small-but-slow subprocess outliers like run-process.test.ts (7kb but 112s). A maintainable fix is a self-updating CI cache fed from the junit artifacts CI already uploads, which is tracked separately. The reliability fixes above already eliminate the failures and the retry overhead that dominated the 12m50s shard.

Three Windows-only instabilities in the CLI unit suite: 1. httpapi-instance-route-auth.test.ts failed with an uncaught "Invalid handle" error. The test's ConfigProvider.layer( fromUnknown(...)) replaced the ambient config provider, blinding KILO_EXPERIMENTAL_DISABLE_FILEWATCHER=true that CI/preload sets. With the flag hidden, the @parcel/watcher Windows backend subscribed on the temp repo's .git; the tmpdir fixture then deleted that directory while the never-disposed per-test runtime still held the subscription, and CreateFileW failed with the hardcoded "Invalid handle" (napi rejection with no JS stack). Add the disable-filewatcher flag to every test config map that boots instances via the HttpApi app (instance-route-auth, cors, ui, exercise backend, kilo-edit, memory). 2. config-overlay.test.ts intermittently returned HTTP 500 on Windows. Filesystem.write's atomic temp-file+rename had no retry for Windows transient locked-file errors (EPERM/EACCES/EBUSY) from Defender/indexer and the detached background plugin install racing the rename in the same tmpdir. Mirror the proven cleanup.ts locked-error retry pattern with a short backoff, Windows-only. 3. Windows shards were badly imbalanced: the sharder weighted files by byte size, which concentrated every slow spawn/FS/lock-heavy file (snapshot, prompt, provider, run-process, instance-bootstrap, httpapi-session) into one shard (~612s vs ~356s siblings), and the resulting contention forced whole-file retries that doubled cost. Add TestShard.timedWeight and a committed test-timings.json seeded from CI junit data so shards balance by measured runtime (spread collapses from ~200s to ~18s) and contention-prone files spread across shards. Platforms without manifest entries fall back to size weighting.

kilo-code-bot · 2026-07-31T09:46:08Z

Code Review Summary

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	2

Issue Details (click to expand)

WARNING

File	Line	Issue
`packages/opencode/src/util/filesystem.ts`	108	`isLocked` treats permanent `EACCES`/`EPERM` (read-only target, denied ACL, target is a directory, non-writable dir) as transient, so genuine permission failures sleep ~1.4s across 8 attempts before rethrowing; the retry also re-runs `writeFile` rather than scoping to the `rename` that actually contends

SUGGESTION

File	Line	Issue
`packages/opencode/src/util/filesystem.ts`	105	ENOENT recovery calls `doWrite()` directly, so the first write into a just-created directory gets no Windows retry — the exact fresh-tmpdir/fresh-config-dir case the PR targets
`packages/opencode/test/server/httpapi-instance-route-auth.test.ts`	23	Re-adding one env key to six config maps treats the symptom; `ConfigProvider.layerAdd(..., { asPrimary: true })` would stop `ConfigProvider.layer` from blinding the ambient env provider so the next env-gated flag can't regress the same way

Files Reviewed (3 files this pass)

Incremental review of 25affff..e2cb0b5:

packages/opencode/script/kilocode/test-shard.ts - 0 issues (revert)
packages/opencode/script/kilocode/test-timings.json - 0 issues (deleted)
packages/opencode/script/test-runner.ts - 0 issues (revert)

Notes and assumptions

Resolved since the last review: e2cb0b5 reverts the runtime-weighted sharding work entirely (timedWeight, test-timings.json, and the test-runner.ts timings wiring are gone; test-runner.ts and test-shard.ts are now identical to main). Verified no dangling references remain (timedWeight / test-timings have zero matches in the package or .github/, and TestShard.parse/split/order are still imported and used). The previous "no unit coverage for timedWeight" suggestion no longer applies.
No new issues in the incremental diff — it is a clean revert. The three findings above are re-verified against current HEAD in files this pass did not touch, and remain summary-only (no inline comments posted).
The PR title still says "rebalance slow shards" and the body's follow-up section reads as if sharding work is part of this branch, but only the two Windows reliability fixes (filewatcher flag visibility, locked-file retry) remain. Worth retitling before a squash merge so the commit history matches the diff.
Read-only session, so Windows retry behavior was reviewed by reading only, not measured.
CI-owned checks (lint/prettier, typecheck, tests, kilocode_change markers, changeset presence) were intentionally not commented on.

Fix these issues in Kilo Cloud

Previous Review Summaries (2 snapshots, latest commit 25affff)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit `25affff`)

Status: 4 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	3

Issue Details (click to expand)

WARNING

File	Line	Issue
`packages/opencode/src/util/filesystem.ts`	108	`isLocked` treats permanent `EACCES`/`EPERM` (read-only target, denied ACL, target is a directory, non-writable dir) as transient, so genuine permission failures sleep ~1.4s across 8 attempts before rethrowing; the retry also re-runs `writeFile` rather than scoping to the `rename` that actually contends

SUGGESTION

File	Line	Issue
`packages/opencode/src/util/filesystem.ts`	105	ENOENT recovery calls `doWrite()` directly, so the first write into a just-created directory gets no Windows retry — the exact fresh-tmpdir/fresh-config-dir case the PR targets
`packages/opencode/script/kilocode/test-shard.ts`	50	No unit coverage for `timedWeight` even though `test/kilocode/test-shard.test.ts` exists and `size` is injectable — the stale-entry path added in `25affff` is exactly the kind of logic a test would pin
`packages/opencode/test/server/httpapi-instance-route-auth.test.ts`	23	Re-adding one env key to six config maps treats the symptom; `ConfigProvider.layerAdd(..., { asPrimary: true })` would stop `ConfigProvider.layer` from blinding the ambient env provider so the next env-gated flag can't regress the same way

Files Reviewed (1 file this pass)

Incremental review of 06c1078..25affff:

packages/opencode/script/kilocode/test-shard.ts - 0 new issues

Notes and assumptions

Resolved since the last review: the dead try/catch around size(file) in timedWeight is gone; stale manifest entries are now skipped via if (s <= 0) continue, so their runtime no longer inflates the time/byte scale. Verified size is Bun.file(...).size in script/test-runner.ts:159, which returns 0 for missing paths, and that totalS <= 0 still falls back to plain size weighting when every manifest entry is stale.
No new issues in the incremental diff; the remaining findings are re-verified against current HEAD in files this pass did not touch.
Read-only session, so shard balance and Windows retry behavior were reviewed by reading only, not measured.
CI-owned checks (lint/prettier, typecheck, tests, kilocode_change markers, changeset presence) were intentionally not commented on.

Fix these issues in Kilo Cloud

Previous review (commit `06c1078`)

Status: 5 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	4

Issue Details (click to expand)

WARNING

File	Line	Issue
`packages/opencode/src/util/filesystem.ts`	108	`isLocked` treats permanent `EACCES`/`EPERM` (read-only target, denied ACL, target is a directory, non-writable dir) as transient, so genuine permission failures now sleep ~1.4s across 8 attempts before rethrowing; the retry also re-runs `writeFile` rather than scoping to the `rename` that actually contends

SUGGESTION

File	Line	Issue
`packages/opencode/src/util/filesystem.ts`	105	ENOENT recovery calls `doWrite()` directly, so the first write into a just-created directory gets no Windows retry — the exact fresh-tmpdir/fresh-config-dir case the PR targets
`packages/opencode/script/kilocode/test-shard.ts`	61	`Bun.file().size` returns 0 for missing paths, so the `try`/`catch` is dead code and stale manifest entries add runtime with zero size, inflating `scale` and overweighting unknown files
`packages/opencode/script/kilocode/test-shard.ts`	50	No unit coverage for `timedWeight` even though `test/kilocode/test-shard.test.ts` exists and `size` is injectable
`packages/opencode/test/server/httpapi-instance-route-auth.test.ts`	23	Re-adding one env key to six config maps treats the symptom; `ConfigProvider.layerAdd(..., { asPrimary: true })` would stop `ConfigProvider.layer` from blinding the ambient env provider so the next env-gated flag can't regress the same way

Files Reviewed (11 files)

packages/opencode/src/util/filesystem.ts - 2 issues
packages/opencode/script/kilocode/test-shard.ts - 2 issues
packages/opencode/test/server/httpapi-instance-route-auth.test.ts - 1 issue
packages/opencode/script/test-runner.ts
packages/opencode/script/kilocode/test-timings.json
packages/opencode/test/server/httpapi-cors.test.ts
packages/opencode/test/server/httpapi-ui.test.ts
packages/opencode/test/server/httpapi-exercise/backend.ts
packages/opencode/test/kilocode/server/httpapi-kilo-edit.test.ts
packages/opencode/test/kilocode/server/httpapi-memory.test.ts
.changeset/windows-locked-file-retry.md

Notes and assumptions

Verified the stated root causes against the code: test/preload.ts:43 sets KILO_EXPERIMENTAL_DISABLE_FILEWATCHER ??= "true", src/kilocode/watcher.ts:61 reads it via Flag, and script/test-runner.ts:124 normalizes discovered paths to forward slashes so the test-timings.json keys do match on Windows.
Could not execute anything in this read-only session, so shard-balance and Windows retry behavior were reviewed by reading only, not measured.
CI-owned checks (lint/prettier line length, typecheck, tests, kilocode_change markers, changeset presence) were intentionally not commented on. The changeset text reads as user-facing, which is right.
test-timings.json is a hand-refreshed manifest with no automation to keep it current; the size fallback makes that safe, but expect the weights to drift as slow files are added.

Fix these issues in Kilo Cloud

_{Reviewed by claude-opus-5 · Input: 32 · Output: 8.3K · Cached: 703.6K}

_{Review guidance: REVIEW.md from base branch main}

Bun.file().size returns 0 (never throws) for missing paths, so the try/catch in timedWeight was dead code and stale/renamed manifest entries added their time to the scale numerator with zero size, inflating the size-to-time ratio that estimates unknown files. Skip entries with a non-positive on-disk size instead of catching a throw that never happens.

The committed test-timings.json (482 entries) was a maintenance burden: it goes stale as tests are added/renamed and no size-based heuristic can replace it (slow subprocess outliers like run-process.test.ts are 7kb but 112s, 10x the runtime-per-byte of other files). Revert the timing-weighted sharding to the prior size-based LPT. The Windows reliability fixes (ConfigProvider filewatcher flag + Filesystem.write locked-file retry) remain and are what eliminate the failures and the ~360s of retry overhead that dominated the 12m50s shard. A maintainable runtime-based rebalance (self-updating CI cache fed from the junit artifacts CI already uploads) is a separate follow-up.

…Org#12723) * fix(cli): stabilize Windows CI tests and rebalance slow shards Three Windows-only instabilities in the CLI unit suite: 1. httpapi-instance-route-auth.test.ts failed with an uncaught "Invalid handle" error. The test's ConfigProvider.layer( fromUnknown(...)) replaced the ambient config provider, blinding KILO_EXPERIMENTAL_DISABLE_FILEWATCHER=true that CI/preload sets. With the flag hidden, the @parcel/watcher Windows backend subscribed on the temp repo's .git; the tmpdir fixture then deleted that directory while the never-disposed per-test runtime still held the subscription, and CreateFileW failed with the hardcoded "Invalid handle" (napi rejection with no JS stack). Add the disable-filewatcher flag to every test config map that boots instances via the HttpApi app (instance-route-auth, cors, ui, exercise backend, kilo-edit, memory). 2. config-overlay.test.ts intermittently returned HTTP 500 on Windows. Filesystem.write's atomic temp-file+rename had no retry for Windows transient locked-file errors (EPERM/EACCES/EBUSY) from Defender/indexer and the detached background plugin install racing the rename in the same tmpdir. Mirror the proven cleanup.ts locked-error retry pattern with a short backoff, Windows-only. 3. Windows shards were badly imbalanced: the sharder weighted files by byte size, which concentrated every slow spawn/FS/lock-heavy file (snapshot, prompt, provider, run-process, instance-bootstrap, httpapi-session) into one shard (~612s vs ~356s siblings), and the resulting contention forced whole-file retries that doubled cost. Add TestShard.timedWeight and a committed test-timings.json seeded from CI junit data so shards balance by measured runtime (spread collapses from ~200s to ~18s) and contention-prone files spread across shards. Platforms without manifest entries fall back to size weighting. * fix(cli): skip stale manifest entries in timed shard weighting Bun.file().size returns 0 (never throws) for missing paths, so the try/catch in timedWeight was dead code and stale/renamed manifest entries added their time to the scale numerator with zero size, inflating the size-to-time ratio that estimates unknown files. Skip entries with a non-positive on-disk size instead of catching a throw that never happens. * revert(cli): drop hardcoded test-timings manifest The committed test-timings.json (482 entries) was a maintenance burden: it goes stale as tests are added/renamed and no size-based heuristic can replace it (slow subprocess outliers like run-process.test.ts are 7kb but 112s, 10x the runtime-per-byte of other files). Revert the timing-weighted sharding to the prior size-based LPT. The Windows reliability fixes (ConfigProvider filewatcher flag + Filesystem.write locked-file retry) remain and are what eliminate the failures and the ~360s of retry overhead that dominated the 12m50s shard. A maintainable runtime-based rebalance (self-updating CI cache fed from the junit artifacts CI already uploads) is a separate follow-up.

Kilo-Org deleted a comment from kilo-code-bot Bot Jul 31, 2026

marius-kilocode enabled auto-merge (squash) July 31, 2026 10:05

marius-kilocode disabled auto-merge July 31, 2026 10:08

marius-kilocode enabled auto-merge (squash) July 31, 2026 10:35

chrarnoldus approved these changes Jul 31, 2026

View reviewed changes

marius-kilocode merged commit a1ad65e into main Jul 31, 2026
42 of 45 checks passed

marius-kilocode deleted the investigate-window-test-performance-and-instabilit branch July 31, 2026 11:27

kilo-maintainer Bot mentioned this pull request Jul 31, 2026

release(jetbrains): v7.0.12-rc.3 #12742

Merged

github-actions Bot mentioned this pull request Aug 1, 2026

docs: auto-sync with merged PRs (through 2026-08-01) #12716

Merged

kilo-maintainer Bot mentioned this pull request Aug 1, 2026

release(jetbrains): v7.0.12 #12766

Merged

Navigation Menu

Uh oh!

fix(cli): stabilize Windows CI tests and rebalance slow shards - #12723

fix(cli): stabilize Windows CI tests and rebalance slow shards#12723
marius-kilocode merged 3 commits into
mainfrom
investigate-window-test-performance-and-instabilit

marius-kilocode commented Jul 31, 2026 •

edited

Loading

Uh oh!

kilo-code-bot Bot commented Jul 31, 2026 •

edited

Loading

WARNING

SUGGESTION

Previous review (commit `25affff`)

Overview

WARNING

SUGGESTION

Previous review (commit `06c1078`)

Overview

WARNING

SUGGESTION

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

marius-kilocode commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root causes and fixes

Follow-up (not in this PR)

Uh oh!

kilo-code-bot Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

Overview

WARNING

SUGGESTION

Previous review (commit 25affff)

Overview

WARNING

SUGGESTION

Previous review (commit 06c1078)

Overview

WARNING

SUGGESTION

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

marius-kilocode commented Jul 31, 2026 •

edited

Loading

kilo-code-bot Bot commented Jul 31, 2026 •

edited

Loading

Previous review (commit `25affff`)

Previous review (commit `06c1078`)