fix(test): make tool/shell.test.ts deterministic (fast commands lost output)

tool/shell.test.ts intermittently failed in CI (e.g. falls back from
terminal-only configured shell [159.79ms]) with result.output being
"(no output)". Reproduced locally at ~13% across two test cases, so
this is a real timing race rather than a one-off flake.

Bun's child_process discards buffered stdout/stderr once the child emits
"close", and our CrossSpawnSpawner attaches stream readers lazily, so
fast-exiting processes lose all output before the reader attaches. The
same path serves the live shell tool, so the test is correctly catching
a product bug.

Tap stdout/stderr into PassThroughs synchronously at spawn time, and
await the reader fiber after the process exits so scope teardown cannot
interrupt it before trailing chunks are drained.
This commit is contained in:
marius-kilocode
2026-07-03 14:49:58 +02:00
parent 0cd75205f6
commit 70a002da47
4 changed files with 51 additions and 4 deletions
+5
View File
@@ -0,0 +1,5 @@
---
"@kilocode/cli": patch
---
Fix shell tool occasionally returning "(no output)" for fast-exiting commands
+4 -2
View File
@@ -2,6 +2,7 @@ import type * as Arr from "effect/Array"
import { NodeFileSystem, NodeSink, NodeStream } from "@effect/platform-node"
import * as NodePath from "@effect/platform-node/NodePath"
import { prepareCommand as prepareSandbox } from "@kilocode/sandbox" // kilocode_change
import { tap as tapStdio, tapped } from "./kilocode/stdio-tap" // kilocode_change - Bun drops buffered stdio on close
import * as Deferred from "effect/Deferred"
import * as Effect from "effect/Effect"
import * as Exit from "effect/Exit"
@@ -246,13 +247,13 @@ export const make = Effect.gen(function* () {
) => {
let stdout = proc.stdout
? NodeStream.fromReadable({
evaluate: () => proc.stdout!,
evaluate: () => tapped(proc, "stdout"), // kilocode_change - read the spawn-time tap
onError: (cause) => toPlatformError("fromReadable(stdout)", toError(cause), command),
})
: Stream.empty
let stderr = proc.stderr
? NodeStream.fromReadable({
evaluate: () => proc.stderr!,
evaluate: () => tapped(proc, "stderr"), // kilocode_change - read the spawn-time tap
onError: (cause) => toPlatformError("fromReadable(stderr)", toError(cause), command),
})
: Stream.empty
@@ -267,6 +268,7 @@ export const make = Effect.gen(function* () {
Effect.callback<readonly [NodeChildProcess.ChildProcess, ExitSignal], PlatformError.PlatformError>((resume) => {
const signal = Deferred.makeUnsafe<readonly [code: number | null, signal: NodeJS.Signals | null]>()
const proc = launch(command.command, command.args, opts)
tapStdio(proc) // kilocode_change - must run in the same tick as spawn
let end = false
let exit: readonly [code: number | null, signal: NodeJS.Signals | null] | undefined
proc.on("error", (err) => {
+34
View File
@@ -0,0 +1,34 @@
import type * as NodeChildProcess from "node:child_process"
import { PassThrough, type Readable } from "node:stream"
// Bun's child_process drops buffered stdio data once the child emits "close", so
// stream readers that attach lazily (a tick or more after spawn) lose the output of
// fast-exiting processes entirely. To retain it, stdout/stderr are piped into
// PassThroughs synchronously at spawn time, before yielding to the event loop.
// PassThrough backpressure (default highWaterMark) keeps unconsumed output bounded.
const map = new WeakMap<NodeChildProcess.ChildProcess, { stdout: PassThrough | null; stderr: PassThrough | null }>()
const wrap = (src: Readable | null) => {
if (!src) return null
const out = new PassThrough()
// A destroy(err) before the lazy consumer attaches would otherwise emit an
// unhandled "error" event and crash the process (a hazard that also existed
// when readers attached lazily to the raw stdio streams). Consumers attached
// by then still get the error via their own listeners; in the rare pre-attach
// window the error is dropped instead of crashing the CLI.
out.on("error", () => {})
src.on("error", (err) => out.destroy(err instanceof Error ? err : new Error(String(err))))
src.pipe(out)
return out
}
/** Tap a freshly spawned process. Must be called in the same tick as spawn. */
export function tap(proc: NodeChildProcess.ChildProcess) {
map.set(proc, { stdout: wrap(proc.stdout), stderr: wrap(proc.stderr) })
}
/** The tapped stream for a process, falling back to the raw stdio stream. */
export function tapped(proc: NodeChildProcess.ChildProcess, fd: "stdout" | "stderr") {
return map.get(proc)?.[fd] ?? proc[fd]!
}
+8 -2
View File
@@ -1,4 +1,4 @@
import { Effect, Stream } from "effect"
import { Effect, Fiber, Stream } from "effect" // kilocode_change - Fiber
import os from "os"
import { createWriteStream } from "node:fs"
import * as Tool from "./tool"
@@ -548,7 +548,7 @@ export const ShellTool = Tool.define(
yield* Effect.addFinalizer(closeSink)
const handle = yield* spawner.spawn(cmd(input.shell, input.command, input.cwd, input.env))
yield* Effect.forkScoped(
const reader = yield* Effect.forkScoped( // kilocode_change - keep the fiber so trailing output can be drained
Stream.runForEach(Stream.decodeText(handle.all), (chunk) => {
const size = Buffer.byteLength(chunk, "utf-8")
list.push({ text: chunk, size })
@@ -621,6 +621,12 @@ export const ShellTool = Tool.define(
yield* handle.kill({ forceKillAfter: "3 seconds" }).pipe(Effect.orDie)
}
// kilocode_change start - closing the scope interrupts the reader fiber, which can drop
// buffered output that arrived just before the process exited. Wait for the stream to
// finish (it ends once stdio closes) so fast commands do not lose their final chunks.
yield* Fiber.await(reader).pipe(Effect.timeout("3 seconds"), Effect.ignore)
// kilocode_change end
return exit.kind === "exit" ? exit.code : null
}),
).pipe(Effect.orDie)