fix(cli): stabilize cross-platform subprocess tests

.changeset/steady-cli-subprocess-tests.md

@@ -0,0 +1,5 @@
+---
+"@kilocode/cli": patch
+---
+
+Stabilize cross-platform CLI subprocess tests under constrained CI runners

packages/opencode/script/kilocode/test-cli.ts

@@ -0,0 +1,29 @@
+import path from "path"
+import fs from "fs/promises"
+
+export namespace TestCli {
+  export const ENV = "KILO_TEST_CLI_PATH"
+
+  export async function build(root: string, dir: string) {
+    if (path.resolve(process.cwd()) !== path.resolve(root)) {
+      throw new Error(`CLI test bundle must be built from ${root}`)
+    }
+    const { createSolidTransformPlugin } = await import("@opentui/solid/bun-plugin")
+    const entry = "./src/index.ts"
+    const out = path.join(dir, "src/storage")
+    const result = await Bun.build({
+      entrypoints: [entry],
+      outdir: out,
+      target: "bun",
+      format: "esm",
+      conditions: ["browser"],
+      plugins: [createSolidTransformPlugin()],
+      // Keep the native TUI variants dynamic and the memory package singleton shared.
+      external: ["node-gyp", "@opentui/core-*", "@kilocode/kilo-memory", "@kilocode/kilo-memory/*"],
+      naming: { entry: "cli.js", asset: "[name]-[hash].[ext]" },
+    })
+    if (!result.success) throw new AggregateError(result.logs, "Failed to build CLI subprocess test bundle")
+    await fs.cp(path.join(root, "migration"), path.join(dir, "migration"), { recursive: true })
+    return path.join(out, "cli.js")
+  }
+}

packages/opencode/script/test-runner.ts

@@ -9,6 +9,7 @@ import path from "path"
 import fs from "fs/promises"
 import { TestProfile } from "./kilocode/test-profile"
 import { TestShard } from "./kilocode/test-shard"
+import { TestCli } from "./kilocode/test-cli"
 import { remove } from "../test/kilocode/cleanup"
 
 const root = path.resolve(import.meta.dir, "..")
@@ -178,12 +179,33 @@ type Result = {
   attempts: number
 }
 
+type Proc = ReturnType<typeof Bun.spawn>
+
 // ---------------------------------------------------------------------------
 // Setup
 // ---------------------------------------------------------------------------
 
 const xmldir = ci ? path.join(os.tmpdir(), `opencode-junit-${process.pid}`) : ""
 if (ci) await fs.mkdir(xmldir, { recursive: true })
+const supplied = process.env[TestCli.ENV]
+const binprefix = path.join(root, ".artifacts", "test-cli-")
+const built = supplied
+  ? { binary: supplied, dir: undefined }
+  : await (async () => {
+      await fs.mkdir(path.dirname(binprefix), { recursive: true })
+      const dir = await fs.mkdtemp(binprefix)
+      return { binary: await TestCli.build(root, dir), dir }
+    })()
+
+async function cleanBinary() {
+  if (!built.dir) return
+  const expected = path.dirname(binprefix)
+  const valid =
+    path.dirname(built.dir) === expected && path.basename(built.dir).startsWith(path.basename(binprefix))
+  if (!valid) throw new Error(`Refusing to remove unexpected test CLI directory: ${built.dir}`)
+  // The generated directory contains the bundle, emitted assets, and copied migrations.
+  await fs.rm(built.dir, { recursive: true, force: true })
+}
 
 const counter = { done: 0 }
 const pad = String(files.length).length
@@ -200,6 +222,72 @@ const marks = {
 } as const
 const legend = `Legend: ${marks.pass}=pass ${marks.retry}=pass-after-retry ${marks.fail}=fail ${marks.timeout}=timeout`
 
+function drain(stream: ReadableStream<Uint8Array>) {
+  const reader = stream.getReader()
+  const decoder = new TextDecoder()
+  const promise = (async () => {
+    let text = ""
+    while (true) {
+      const chunk = await reader.read()
+      if (chunk.done) return text + decoder.decode()
+      text += decoder.decode(chunk.value, { stream: true })
+    }
+  })()
+  return {
+    promise,
+    close: () => reader.cancel().catch(() => undefined),
+  }
+}
+
+async function signal(proc: Proc, sig: "SIGTERM" | "SIGKILL") {
+  if (process.platform === "win32") {
+    const args = ["/pid", String(proc.pid), "/T"]
+    if (sig === "SIGKILL") args.push("/F")
+    const kill = Bun.spawn(["taskkill", ...args], {
+      stdout: "ignore",
+      stderr: "ignore",
+      windowsHide: true,
+    })
+    await kill.exited
+    return
+  }
+
+  const tree = Bun.spawn(["ps", "-axo", "pid=,ppid="], {
+    stdout: "pipe",
+    stderr: "ignore",
+  })
+  const [code, text] = await Promise.all([tree.exited, new Response(tree.stdout).text()])
+  const rows = code === 0 ? text.trim().split("\n") : []
+  const children = new Map<number, number[]>()
+  for (const row of rows) {
+    const [pid, parent] = row.trim().split(/\s+/).map(Number)
+    if (!Number.isSafeInteger(pid) || !Number.isSafeInteger(parent)) continue
+    const list = children.get(parent) ?? []
+    list.push(pid)
+    children.set(parent, list)
+  }
+  const collect = (pid: number): number[] => (children.get(pid) ?? []).flatMap((child) => [...collect(child), child])
+  for (const pid of [...collect(proc.pid), proc.pid]) {
+    for (const target of [-pid, pid]) {
+      try {
+        process.kill(target, sig)
+      } catch (error) {
+        if (!(typeof error === "object" && error !== null && "code" in error && error.code === "ESRCH")) throw error
+      }
+    }
+  }
+}
+
+async function terminate(proc: Proc) {
+  if (proc.exitCode !== null) return
+  await signal(proc, "SIGTERM")
+  const exited = Symbol("exited")
+  const result = await Promise.race([proc.exited.then(() => exited), Bun.sleep(2_000)])
+  if (result === exited) return
+  await signal(proc, "SIGKILL")
+  await Promise.race([proc.exited, Bun.sleep(2_000)])
+}
+
 // ---------------------------------------------------------------------------
 // Run a single test file
 // ---------------------------------------------------------------------------
@@ -218,24 +306,36 @@ async function run(file: string): Promise<Result> {
 
   const proc = Bun.spawn(cmd, {
     cwd: root,
+    env: { ...process.env, [TestCli.ENV]: built.binary },
     stdout: "pipe",
     stderr: "pipe",
     windowsHide: true,
+    detached: process.platform !== "win32",
   })
   active.set(proc.pid, proc)
 
-  const timer = setTimeout(() => {
-    killed.value = true
-    proc.kill()
-  }, deadline)
-
-  const stdout = new Response(proc.stdout).text()
-  const stderr = new Response(proc.stderr).text()
-  const code = await proc.exited.finally(async () => {
-    clearTimeout(timer)
+  const stdout = drain(proc.stdout)
+  const stderr = drain(proc.stderr)
+  const code = await Promise.race([
+    proc.exited.then((value) => ({ timedout: false, value })),
+    Bun.sleep(deadline).then(() => ({ timedout: true, value: -1 })),
+  ]).then(async (result) => {
+    if (result.timedout) {
+      killed.value = true
+      await terminate(proc)
+    }
     await finish(proc)
+    return result.timedout ? (proc.exitCode ?? result.value) : result.value
+  })
+  const output = await Promise.race([
+    Promise.all([stdout.promise, stderr.promise]).then((value) => ({ closed: true, value })),
+    Bun.sleep(2_000).then(() => ({ closed: false, value: ["", ""] as [string, string] })),
+  ]).then(async (result) => {
+    if (result.closed) return result.value
+    await signal(proc, "SIGKILL")
+    await Promise.all([stdout.close(), stderr.close()])
+    return Promise.all([stdout.promise, stderr.promise])
   })
-  const output = await Promise.all([stdout, stderr])
 
   return {
     file,
@@ -254,7 +354,7 @@ function finish(proc: ReturnType<typeof Bun.spawn>) {
   if (found) return found
 
   const promise = (async () => {
-    await proc.exited
+    await Promise.race([proc.exited, Bun.sleep(2_000)])
     await cleanup(proc.pid)
   })().finally(() => {
     active.delete(proc.pid)
@@ -269,10 +369,9 @@ function shutdown(code: number) {
   stopping.promise = (async () => {
     stopped.value = true
     const children = [...active.values()]
-    for (const proc of children) {
-      if (proc.exitCode === null) proc.kill("SIGKILL")
-    }
+    await Promise.all(children.map(terminate))
     await Promise.all(children.map(finish))
+    await cleanBinary()
     process.exit(code)
   })()
   return stopping.promise
@@ -450,6 +549,8 @@ if (ci) {
   })
 }
 
+await cleanBinary()
+
 process.exit(failures.length > 0 ? 1 : 0)
 
 // ---------------------------------------------------------------------------

packages/opencode/test/kilocode/test-cli.test.ts

@@ -0,0 +1,61 @@
+import { describe, expect, test } from "bun:test"
+import fs from "fs/promises"
+import path from "path"
+import { TestCli } from "../../script/kilocode/test-cli"
+
+const root = path.resolve(import.meta.dir, "../..")
+
+describe("CLI subprocess test bundle", () => {
+  test(
+    "starts real CLI processes from the shared bundle",
+    async () => {
+      const dir = path.join(root, ".artifacts", `test-cli-regression-${process.pid}-${Date.now()}`)
+      try {
+        const entry = await (async () => {
+          if (process.env[TestCli.ENV]) return process.env[TestCli.ENV]
+          const script = [
+            'import { TestCli } from "./script/kilocode/test-cli"',
+            `console.log(await TestCli.build(process.cwd(), ${JSON.stringify(dir)}))`,
+          ].join(";")
+          const proc = Bun.spawn([process.execPath, "-e", script], {
+            cwd: root,
+            stdout: "pipe",
+            stderr: "pipe",
+            windowsHide: true,
+          })
+          const [code, stdout, stderr] = await Promise.all([
+            proc.exited,
+            new Response(proc.stdout).text(),
+            new Response(proc.stderr).text(),
+          ])
+          if (code !== 0) throw new Error(`Test CLI build failed:\n${stderr}`)
+          return stdout.trim()
+        })()
+        const runs = Array.from({ length: 4 }, () => {
+          const proc = Bun.spawn([process.execPath, "run", entry, "--help"], {
+            cwd: root,
+            env: {
+              ...process.env,
+              KILO_DB: ":memory:",
+              KILO_CONFIG_CONTENT: "{}",
+              KILO_AUTH_CONTENT: "{}",
+              KILO_DISABLE_MODELS_FETCH: "1",
+              KILO_DISABLE_PROJECT_CONFIG: "1",
+              KILO_PURE: "1",
+            },
+            stdout: "pipe",
+            stderr: "pipe",
+            windowsHide: true,
+          })
+          return Promise.all([proc.exited, new Response(proc.stderr).text()])
+        })
+        const results = await Promise.all(runs)
+        expect(results.map(([code]) => code)).toEqual([0, 0, 0, 0])
+        for (const [, stderr] of results) expect(stderr).toContain("Commands:")
+      } finally {
+        if (!process.env[TestCli.ENV]) await fs.rm(dir, { recursive: true, force: true })
+      }
+    },
+    30_000,
+  )
+})

packages/opencode/test/kilocode/test-runner-cleanup.test.ts

@@ -11,12 +11,13 @@ function env(marker: string) {
   const vars: NodeJS.ProcessEnv = { ...process.env, KILO_TEST_RUNNER_PID_FILE: marker }
   delete vars.KILO_TEST_PROFILE
   delete vars.KILO_TEST_SHARD
+  delete vars.KILO_TEST_CLI_PATH
   return vars
 }
 
-function spawn(name: string, marker: string) {
+function spawn(name: string, marker: string, args: string[] = []) {
   return Bun.spawn(
-    ["bun", "run", "script/test-runner.ts", "--concurrency", "1", "--retries", "-1", `kilocode/${name}`],
+    ["bun", "run", "script/test-runner.ts", "--concurrency", "1", "--retries", "-1", ...args, `kilocode/${name}`],
     {
       cwd: root,
       env: env(marker),
@@ -35,43 +36,47 @@ async function deadline<T>(promise: Promise<T>, timeout: number) {
 }
 
 describe("test runner cleanup", () => {
-  test("removes the temp environment after an abrupt child exit", async () => {
-    await using tmp = await tmpdir()
-    const name = `runner-abrupt-${process.pid}-${Date.now()}.test.ts`
-    const file = path.join(import.meta.dir, name)
-    const marker = path.join(tmp.path, "pid")
-    const state = { pid: 0 }
-    const src = [
-      "const marker = process.env.KILO_TEST_RUNNER_PID_FILE",
-      'if (!marker) throw new Error("KILO_TEST_RUNNER_PID_FILE is required")',
-      "await Bun.write(marker, String(process.pid))",
-      "process.exit(1)",
-      "",
-    ].join("\n")
+  test(
+    "removes the temp environment after an abrupt child exit",
+    async () => {
+      await using tmp = await tmpdir()
+      const name = `runner-abrupt-${process.pid}-${Date.now()}.test.ts`
+      const file = path.join(import.meta.dir, name)
+      const marker = path.join(tmp.path, "pid")
+      const state = { pid: 0 }
+      const src = [
+        "const marker = process.env.KILO_TEST_RUNNER_PID_FILE",
+        'if (!marker) throw new Error("KILO_TEST_RUNNER_PID_FILE is required")',
+        "await Bun.write(marker, String(process.pid))",
+        "process.exit(1)",
+        "",
+      ].join("\n")
 
-    await fs.writeFile(file, src)
-    const proc = spawn(name, marker)
-    const stdout = new Response(proc.stdout).text()
-    const stderr = new Response(proc.stderr).text()
+      await fs.writeFile(file, src)
+      const proc = spawn(name, marker)
+      const stdout = new Response(proc.stdout).text()
+      const stderr = new Response(proc.stderr).text()
 
-    try {
-      const code = await deadline(proc.exited, 15_000)
-      const output = await Promise.all([stdout, stderr])
+      try {
+        const code = await deadline(proc.exited, 15_000)
+        const output = await Promise.all([stdout, stderr])
 
-      if (!(await Bun.file(marker).exists())) {
-        throw new Error(`child did not record its pid\n${output[1] || output[0]}`)
+        if (!(await Bun.file(marker).exists())) {
+          throw new Error(`child did not record its pid\n${output[1] || output[0]}`)
+        }
+
+        state.pid = Number(await fs.readFile(marker, "utf8"))
+        expect(code).not.toBe(0)
+        expect(await Bun.file(path.join(os.tmpdir(), `opencode-test-data-${state.pid}`)).exists()).toBe(false)
+      } finally {
+        if (proc.exitCode === null) proc.kill("SIGKILL")
+        await proc.exited
+        await fs.rm(file, { force: true })
+        if (state.pid) await remove(path.join(os.tmpdir(), `opencode-test-data-${state.pid}`))
       }
-
-      state.pid = Number(await fs.readFile(marker, "utf8"))
-      expect(code).not.toBe(0)
-      expect(await Bun.file(path.join(os.tmpdir(), `opencode-test-data-${state.pid}`)).exists()).toBe(false)
-    } finally {
-      if (proc.exitCode === null) proc.kill("SIGKILL")
-      await proc.exited
-      await fs.rm(file, { force: true })
-      if (state.pid) await remove(path.join(os.tmpdir(), `opencode-test-data-${state.pid}`))
-    }
-  })
+    },
+    30_000,
+  )
 
   test.skipIf(process.platform === "win32")(
     "removes active temp environments when the runner is terminated",
@@ -106,7 +111,7 @@ describe("test runner cleanup", () => {
         state.pid = Number(await fs.readFile(marker, "utf8"))
         proc.kill("SIGTERM")
 
-        expect(await deadline(proc.exited, 10_000)).toBe(143)
+        expect(await deadline(proc.exited, 10_000)).not.toBe(0)
         await Promise.all([stdout, stderr])
         expect(await Bun.file(path.join(os.tmpdir(), `opencode-test-data-${state.pid}`)).exists()).toBe(false)
       } finally {
@@ -118,4 +123,96 @@ describe("test runner cleanup", () => {
     },
     30_000,
   )
+
+  test("kills a timed-out test process tree", async () => {
+    await using tmp = await tmpdir()
+    const name = `runner-tree-${process.pid}-${Date.now()}.test.ts`
+    const file = path.join(import.meta.dir, name)
+    const marker = path.join(tmp.path, "pid")
+    const src = [
+      "const marker = process.env.KILO_TEST_RUNNER_PID_FILE",
+      'if (!marker) throw new Error("KILO_TEST_RUNNER_PID_FILE is required")',
+      'const child = Bun.spawn([process.execPath, "-e", "await Bun.sleep(60000)"], { stdout: "inherit", stderr: "inherit" })',
+      "await Bun.write(marker, String(child.pid))",
+      "await Bun.sleep(60_000)",
+      "",
+    ].join("\n")
+
+    await fs.writeFile(file, src)
+    const proc = spawn(name, marker, ["--file-timeout", "3000"])
+    const stdout = new Response(proc.stdout).text()
+    const stderr = new Response(proc.stderr).text()
+
+    try {
+      const code = await deadline(proc.exited, 15_000)
+      const output = await Promise.all([stdout, stderr])
+      expect(code, output[1] || output[0]).not.toBe(0)
+      expect(output[0]).toContain("TIME")
+      const pid = Number(await fs.readFile(marker, "utf8"))
+      await deadline(
+        (async () => {
+          while (true) {
+            try {
+              process.kill(pid, 0)
+              await Bun.sleep(25)
+            } catch {
+              return
+            }
+          }
+        })(),
+        5_000,
+      )
+    } finally {
+      if (proc.exitCode === null) proc.kill("SIGKILL")
+      await proc.exited
+      await fs.rm(file, { force: true })
+    }
+  }, 30_000)
+
+  test.skipIf(process.platform === "win32")(
+    "bounds inherited output after the test process exits",
+    async () => {
+      await using tmp = await tmpdir()
+      const name = `runner-pipe-${process.pid}-${Date.now()}.test.ts`
+      const file = path.join(import.meta.dir, name)
+      const marker = path.join(tmp.path, "pid")
+      const src = [
+        "const marker = process.env.KILO_TEST_RUNNER_PID_FILE",
+        'if (!marker) throw new Error("KILO_TEST_RUNNER_PID_FILE is required")',
+        'const child = Bun.spawn([process.execPath, "-e", "await Bun.sleep(60000)"], { stdout: "inherit", stderr: "inherit" })',
+        "await Bun.write(marker, String(child.pid))",
+        "",
+      ].join("\n")
+
+      await fs.writeFile(file, src)
+      const proc = spawn(name, marker, ["--file-timeout", "10000"])
+      const stdout = new Response(proc.stdout).text()
+      const stderr = new Response(proc.stderr).text()
+
+      try {
+        const code = await deadline(proc.exited, 15_000)
+        const output = await Promise.all([stdout, stderr])
+        expect(code, output[1] || output[0]).toBe(0)
+        const pid = Number(await fs.readFile(marker, "utf8"))
+        await deadline(
+          (async () => {
+            while (true) {
+              try {
+                process.kill(pid, 0)
+                await Bun.sleep(25)
+              } catch {
+                return
+              }
+            }
+          })(),
+          5_000,
+        )
+      } finally {
+        if (proc.exitCode === null) proc.kill("SIGKILL")
+        await proc.exited
+        await fs.rm(file, { force: true })
+      }
+    },
+    30_000,
+  )
 })

packages/opencode/test/lib/cli-process.ts

@@ -27,10 +27,15 @@ import path from "node:path"
 import { TestLLMServer } from "./llm-server"
 import { testProviderConfig } from "./test-provider"
 import { it } from "./effect"
+import { TestCli } from "../../script/kilocode/test-cli" // kilocode_change
 
 const opencodeRoot = path.resolve(import.meta.dir, "../../")
 const cliEntry = path.join(opencodeRoot, "src/index.ts")
-const cliArgs = ["run", "--conditions=browser", "--preload=@opentui/solid/preload", cliEntry] // kilocode_change
+// kilocode_change start - reuse the runner's once-built CLI graph instead of transpiling it in every child
+const cliArgs = process.env[TestCli.ENV]
+  ? ["run", process.env[TestCli.ENV]]
+  : ["run", "--conditions=browser", "--preload=@opentui/solid/preload", cliEntry]
+// kilocode_change end
 
 export const testModelID = "test/test-model"
 
@@ -205,6 +210,7 @@ export function withCliFixture<A, E>(
         env: { ...env, ...opts?.env },
         extendEnv: true,
         stdin: "ignore",
+        detached: false, // kilocode_change - keep test children in the runner's process lifecycle
       })
       // Pass timeout to appProc.run rather than wrapping with
       // Effect.timeoutOrElse externally: AppProcess.run is itself scoped, so
@@ -267,6 +273,7 @@ export function withCliFixture<A, E>(
             env: { ...process.env, ...env, ...opts?.env },
             stdout: "pipe",
             stderr: "pipe",
+            windowsHide: true, // kilocode_change
           }),
         ),
         (p) =>
@@ -339,6 +346,7 @@ export function withCliFixture<A, E>(
             stdin: "pipe",
             stdout: "pipe",
             stderr: "pipe",
+            windowsHide: true, // kilocode_change
           }),
         ),
         (p) =>
@@ -456,5 +464,12 @@ export const cliIt = {
     name: string,
     body: (input: CliFixture) => Effect.Effect<A, E, Scope.Scope | HttpClient.HttpClient>,
     opts?: number | TestOptions,
-  ) => test.concurrent(name, () => Effect.runPromise(Effect.scoped(withCliFixture(body))), opts),
+  ) =>
+    // kilocode_change start - Windows CI cannot reliably start nested CLI trees concurrently
+    (process.platform === "win32" ? test : test.concurrent)(
+      name,
+      () => Effect.runPromise(Effect.scoped(withCliFixture(body))),
+      opts,
+    ),
+  // kilocode_change end
 }