From a1ad65e5229156bfa7d404fcbaa98ad70462197f Mon Sep 17 00:00:00 2001 From: Marius Date: Fri, 31 Jul 2026 13:27:34 +0200 Subject: [PATCH] fix(cli): stabilize Windows CI tests and rebalance slow shards (#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. --- .changeset/windows-locked-file-retry.md | 5 +++ packages/opencode/src/util/filesystem.ts | 35 +++++++++++++++---- .../kilocode/server/httpapi-kilo-edit.test.ts | 11 +++++- .../kilocode/server/httpapi-memory.test.ts | 11 +++++- .../opencode/test/server/httpapi-cors.test.ts | 11 +++++- .../test/server/httpapi-exercise/backend.ts | 8 ++++- .../httpapi-instance-route-auth.test.ts | 5 +++ .../opencode/test/server/httpapi-ui.test.ts | 6 ++++ 8 files changed, 82 insertions(+), 10 deletions(-) create mode 100644 .changeset/windows-locked-file-retry.md diff --git a/.changeset/windows-locked-file-retry.md b/.changeset/windows-locked-file-retry.md new file mode 100644 index 0000000000..b6bc07c285 --- /dev/null +++ b/.changeset/windows-locked-file-retry.md @@ -0,0 +1,5 @@ +--- +"@kilocode/cli": patch +--- + +Retry transient locked-file errors (EPERM/EACCES/EBUSY) on Windows when atomically saving config and other files. Background plugin installs and Windows Defender/indexer can briefly hold the temp file during the rename step, which previously surfaced as a 500 error. A short backoff now retries the rename so config writes succeed without surfacing the contention. diff --git a/packages/opencode/src/util/filesystem.ts b/packages/opencode/src/util/filesystem.ts index ed80262345..985fae9b1d 100644 --- a/packages/opencode/src/util/filesystem.ts +++ b/packages/opencode/src/util/filesystem.ts @@ -66,6 +66,21 @@ function isEnoent(e: unknown): e is { code: "ENOENT" } { return typeof e === "object" && e !== null && "code" in e && (e as { code: string }).code === "ENOENT" } +// kilocode_change start - Windows transient locked-file errors on atomic rename +// Defender/indexer and concurrent writers (e.g. background plugin install) can +// briefly hold the temp file, making MoveFileEx fail with EPERM/EACCES/EBUSY. +// Retry with a short backoff instead of surfacing a 500; POSIX renames are atomic +// so the retry path only fires under contention and never changes success semantics. +function isLocked(e: unknown): boolean { + return ( + typeof e === "object" && + e !== null && + "code" in e && + ["EBUSY", "EACCES", "EPERM"].includes(String((e as { code: string }).code)) + ) +} +// kilocode_change end + export async function write(p: string, content: string | Buffer | Uint8Array, mode?: number): Promise { // kilocode_change start - atomic write via temp-file + rename to avoid partial reads on concurrent saves // Include a random suffix so that concurrent writes to the same path never share a temp file, @@ -79,15 +94,23 @@ export async function write(p: string, content: string | Buffer | Uint8Array, mo } await rename(tmp, p) } - try { - await doWrite() - } catch (e) { - if (isEnoent(e)) { - await mkdir(dirname(p), { recursive: true }) + const attempts = process.platform === "win32" ? 8 : 1 + for (let attempt = 1; ; attempt++) { + try { await doWrite() return + } catch (e) { + if (isEnoent(e)) { + await mkdir(dirname(p), { recursive: true }) + await doWrite() + return + } + if (isLocked(e) && attempt < attempts) { + await Bun.sleep(50 * attempt) + continue + } + throw e } - throw e } // kilocode_change end } diff --git a/packages/opencode/test/kilocode/server/httpapi-kilo-edit.test.ts b/packages/opencode/test/kilocode/server/httpapi-kilo-edit.test.ts index 0b63dc0513..c89061aefb 100644 --- a/packages/opencode/test/kilocode/server/httpapi-kilo-edit.test.ts +++ b/packages/opencode/test/kilocode/server/httpapi-kilo-edit.test.ts @@ -30,7 +30,16 @@ const edit = { function app() { const handler = HttpRouter.toWebHandler( - HttpApiServer.routes.pipe(Layer.provide(ConfigProvider.layer(ConfigProvider.fromUnknown({})))), + // kilocode_change - keep the filewatcher-disable flag visible (see httpapi-instance-route-auth.test.ts) + HttpApiServer.routes.pipe( + Layer.provide( + ConfigProvider.layer( + ConfigProvider.fromUnknown({ + KILO_EXPERIMENTAL_DISABLE_FILEWATCHER: process.env.KILO_EXPERIMENTAL_DISABLE_FILEWATCHER ?? "true", + }), + ), + ), + ), { disableLogger: true }, ).handler diff --git a/packages/opencode/test/kilocode/server/httpapi-memory.test.ts b/packages/opencode/test/kilocode/server/httpapi-memory.test.ts index 767c1c9d94..6c5af0ef1e 100644 --- a/packages/opencode/test/kilocode/server/httpapi-memory.test.ts +++ b/packages/opencode/test/kilocode/server/httpapi-memory.test.ts @@ -14,7 +14,16 @@ type Json = Record function app() { const handler = HttpRouter.toWebHandler( - HttpApiServer.routes.pipe(Layer.provide(ConfigProvider.layer(ConfigProvider.fromUnknown({})))), + // kilocode_change - keep the filewatcher-disable flag visible (see httpapi-instance-route-auth.test.ts) + HttpApiServer.routes.pipe( + Layer.provide( + ConfigProvider.layer( + ConfigProvider.fromUnknown({ + KILO_EXPERIMENTAL_DISABLE_FILEWATCHER: process.env.KILO_EXPERIMENTAL_DISABLE_FILEWATCHER ?? "true", + }), + ), + ), + ), { disableLogger: true }, ).handler diff --git a/packages/opencode/test/server/httpapi-cors.test.ts b/packages/opencode/test/server/httpapi-cors.test.ts index bdb733767c..3ba9804db4 100644 --- a/packages/opencode/test/server/httpapi-cors.test.ts +++ b/packages/opencode/test/server/httpapi-cors.test.ts @@ -64,7 +64,16 @@ describe("HttpApi CORS", () => { Effect.gen(function* () { const handler = HttpRouter.toWebHandler( HttpApiApp.createRoutes().pipe( - Layer.provide(ConfigProvider.layer(ConfigProvider.fromUnknown({ KILO_SERVER_PASSWORD: "secret" }))), + // kilocode_change start - keep the filewatcher-disable flag visible (see httpapi-instance-route-auth.test.ts) + Layer.provide( + ConfigProvider.layer( + ConfigProvider.fromUnknown({ + KILO_SERVER_PASSWORD: "secret", + KILO_EXPERIMENTAL_DISABLE_FILEWATCHER: process.env.KILO_EXPERIMENTAL_DISABLE_FILEWATCHER ?? "true", + }), + ), + ), + // kilocode_change end ), { disableLogger: true }, ).handler diff --git a/packages/opencode/test/server/httpapi-exercise/backend.ts b/packages/opencode/test/server/httpapi-exercise/backend.ts index 7dd3330938..e2277817fc 100644 --- a/packages/opencode/test/server/httpapi-exercise/backend.ts +++ b/packages/opencode/test/server/httpapi-exercise/backend.ts @@ -59,9 +59,15 @@ function app(modules: Runtime, options: CallOptions) { const web = HttpRouter.toWebHandler( modules.HttpApiApp.routes.pipe( Layer.provide( + // kilocode_change start - keep the filewatcher-disable flag visible (see httpapi-instance-route-auth.test.ts) ConfigProvider.layer( - ConfigProvider.fromUnknown({ KILO_SERVER_PASSWORD: password, KILO_SERVER_USERNAME: username }), + ConfigProvider.fromUnknown({ + KILO_SERVER_PASSWORD: password, + KILO_SERVER_USERNAME: username, + KILO_EXPERIMENTAL_DISABLE_FILEWATCHER: process.env.KILO_EXPERIMENTAL_DISABLE_FILEWATCHER ?? "true", + }), ), + // kilocode_change end ), ), { disableLogger: true, memoMap: modules.memoMap }, diff --git a/packages/opencode/test/server/httpapi-instance-route-auth.test.ts b/packages/opencode/test/server/httpapi-instance-route-auth.test.ts index a51ac14574..642762e62e 100644 --- a/packages/opencode/test/server/httpapi-instance-route-auth.test.ts +++ b/packages/opencode/test/server/httpapi-instance-route-auth.test.ts @@ -14,10 +14,15 @@ function app(input: { password?: string; username?: string }) { HttpApiApp.routes.pipe( Layer.provide( ConfigProvider.layer( + // kilocode_change start - keep the filewatcher-disable flag visible so the + // @parcel/watcher Windows backend does not subscribe on temp dirs that + // the tmpdir fixture deletes mid-test (throws "Invalid handle"). ConfigProvider.fromUnknown({ KILO_SERVER_PASSWORD: input.password, KILO_SERVER_USERNAME: input.username, + KILO_EXPERIMENTAL_DISABLE_FILEWATCHER: process.env.KILO_EXPERIMENTAL_DISABLE_FILEWATCHER ?? "true", }), + // kilocode_change end ), ), ), diff --git a/packages/opencode/test/server/httpapi-ui.test.ts b/packages/opencode/test/server/httpapi-ui.test.ts index 90802ca02f..08d84acd2e 100644 --- a/packages/opencode/test/server/httpapi-ui.test.ts +++ b/packages/opencode/test/server/httpapi-ui.test.ts @@ -52,12 +52,15 @@ function app(input?: { password?: string; username?: string }) { const handler = HttpRouter.toWebHandler( HttpApiApp.routes.pipe( Layer.provide( + // kilocode_change start - keep the filewatcher-disable flag visible (see httpapi-instance-route-auth.test.ts) ConfigProvider.layer( ConfigProvider.fromUnknown({ KILO_SERVER_PASSWORD: input?.password, KILO_SERVER_USERNAME: input?.username, + KILO_EXPERIMENTAL_DISABLE_FILEWATCHER: process.env.KILO_EXPERIMENTAL_DISABLE_FILEWATCHER ?? "true", }), ), + // kilocode_change end ), ), { disableLogger: true }, @@ -100,12 +103,15 @@ function uiApp(input?: { input?.client ?? httpClient(new Response("ui")), RuntimeFlags.layer({ disableEmbeddedWebUi: input?.disableEmbeddedWebUi ?? false }), HttpServer.layerServices, + // kilocode_change start - keep the filewatcher-disable flag visible (see httpapi-instance-route-auth.test.ts) ConfigProvider.layer( ConfigProvider.fromUnknown({ KILO_SERVER_PASSWORD: input?.password, KILO_SERVER_USERNAME: input?.username, + KILO_EXPERIMENTAL_DISABLE_FILEWATCHER: process.env.KILO_EXPERIMENTAL_DISABLE_FILEWATCHER ?? "true", }), ), + // kilocode_change end ]), ), { disableLogger: true },