From 70a002da470af3cee9fd2aeffc7d39af930770d9 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 3 Jul 2026 14:49:58 +0200 Subject: [PATCH] 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. --- .changeset/shell-output-drop.md | 5 ++++ packages/core/src/cross-spawn-spawner.ts | 6 +++-- packages/core/src/kilocode/stdio-tap.ts | 34 ++++++++++++++++++++++++ packages/opencode/src/tool/shell.ts | 10 +++++-- 4 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 .changeset/shell-output-drop.md create mode 100644 packages/core/src/kilocode/stdio-tap.ts diff --git a/.changeset/shell-output-drop.md b/.changeset/shell-output-drop.md new file mode 100644 index 00000000000..dfee2ce22a1 --- /dev/null +++ b/.changeset/shell-output-drop.md @@ -0,0 +1,5 @@ +--- +"@kilocode/cli": patch +--- + +Fix shell tool occasionally returning "(no output)" for fast-exiting commands diff --git a/packages/core/src/cross-spawn-spawner.ts b/packages/core/src/cross-spawn-spawner.ts index 5c769569e63..6c29a89dd4e 100644 --- a/packages/core/src/cross-spawn-spawner.ts +++ b/packages/core/src/cross-spawn-spawner.ts @@ -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((resume) => { const signal = Deferred.makeUnsafe() 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) => { diff --git a/packages/core/src/kilocode/stdio-tap.ts b/packages/core/src/kilocode/stdio-tap.ts new file mode 100644 index 00000000000..0bfe8011919 --- /dev/null +++ b/packages/core/src/kilocode/stdio-tap.ts @@ -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() + +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]! +} diff --git a/packages/opencode/src/tool/shell.ts b/packages/opencode/src/tool/shell.ts index b3ef9cb7c14..8736cee8942 100644 --- a/packages/opencode/src/tool/shell.ts +++ b/packages/opencode/src/tool/shell.ts @@ -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)