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.
This commit is contained in:
Marius
2026-07-31 13:27:34 +02:00
committed by GitHub
parent 5678c09cf5
commit a1ad65e522
8 changed files with 82 additions and 10 deletions
+5
View File
@@ -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.
+29 -6
View File
@@ -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<void> {
// 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
}
@@ -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
@@ -14,7 +14,16 @@ type Json = Record<string, unknown>
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
@@ -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
@@ -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 },
@@ -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
),
),
),
@@ -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 },