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

[marius-kilocode]

This fixes a non-deterministic test

test/tool/shell.test.ts intermittently fails in CI, e.g. the original run reported:

(fail) tool.shell > falls back from terminal-only configured shell [159.79ms]
expect(result.output).toContain("fallback")
Received: "(no output)"

Locally it reproduced at ~13% (2/15 runs failing) across two test cases (falls back from terminal-only configured shell and captures stderr in output), so it is a genuine timing race, not a one-off flake. The failure is non-deterministic because it depends on whether the reader fiber attaches before the process exits, which varies with scheduler load.

Root cause

Bun's child_process silently discards buffered stdout/stderr once the child emits close. Our CrossSpawnSpawner attaches stream readers lazily (the Effect stream consumer runs a tick or more after spawn), so any process that exits before the reader attaches loses all of its output. A standalone repro confirms a reader attached even 5ms after spawn gets zero bytes from echo.

The same path serves the live shell tool, so fast commands can return (no output) in the real product too — the test is correctly catching that, which is why it is worth fixing rather than deleting.

Fix

1. packages/core/src/kilocode/stdio-tap.ts (new) — tap(proc) pipes proc.stdout/stderr into PassThrough streams synchronously at spawn time, before the event loop yields. The taps buffer the data (bounded by PassThrough highWaterMark) so it survives until the lazy reader attaches. Storage uses a WeakMap so taps are GC'd with the process.

2. packages/core/src/cross-spawn-spawner.ts — call tap(proc) in the same tick as launch(...), and swap setupOutput's evaluate callbacks to read the tapped streams via tapped(proc, "stdout"/"stderr") (falling back to the raw stream). Four single-line kilocode_change markers; no signature or structural changes, to keep upstream merges trivial.

3. packages/opencode/src/tool/shell.ts — the spawner fix retains the data, but scope teardown after exit can still interrupt the reader fiber before it drains the final chunks. Keep the reader fiber and Fiber.await it (3s timeout, errors ignored) after the exit/abort/timeout race resolves, so trailing output is not dropped.

Verification

• test/tool/shell.test.ts: 30/30 green (was ~13% failure locally: 2/15 runs failing before the fix).
• Core package: 485 pass / 0 fail.
• test/tool/: 310 pass / 0 fail.
• Both packages typecheck; check-opencode-annotations.ts clean.

This affects upstream OpenCode equally (they also run on Bun), so it is a candidate to upstream, which would let us drop the markers.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Both previously reported issues remain fixed at the current commit:

• packages/core/src/kilocode/stdio-tap.ts attaches a no-op error listener on the tap PassThrough (out.on("error", () => {})), preventing the unhandled-error crash.
• packages/core/src/cross-spawn-spawner.ts imports the tap function as tapStdio, resolving the shadowing of the local tap variable in setupFds.

No new issues found.

Files Reviewed (4 files)

• .changeset/shell-output-drop.md
• packages/core/src/cross-spawn-spawner.ts
• packages/core/src/kilocode/stdio-tap.ts
• packages/opencode/src/tool/shell.ts

Previous Review Summaries (2 snapshots, latest commit 71a0ad7)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 71a0ad7)

Status: No Issues Found | Recommendation: Merge

Both previously reported issues are fixed in this update:

• packages/core/src/kilocode/stdio-tap.ts now attaches a no-op error listener on the tap PassThrough (out.on("error", () => {})), preventing the unhandled-error crash.
• packages/core/src/cross-spawn-spawner.ts now imports the tap function as tapStdio, resolving the shadowing of the local tap variable in setupFds.

No new issues found in the incremental diff.

Files Reviewed (4 files)

• .changeset/shell-output-drop.md
• packages/core/src/cross-spawn-spawner.ts
• packages/core/src/kilocode/stdio-tap.ts
• packages/opencode/src/tool/shell.ts

Previous review (commit ffb0093)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	1

Issue Details (click to expand)

WARNING

File	Line	Issue
packages/core/src/kilocode/stdio-tap.ts	15	out.destroy(err) can emit an unhandled error on the tap PassThrough before a downstream consumer attaches, crashing the process

SUGGESTION

File	Line	Issue
packages/core/src/cross-spawn-spawner.ts	5	New tap import shadows an existing local const tap in setupFds

Files Reviewed (4 files)

• .changeset/shell-output-drop.md - 0 issues
• packages/core/src/cross-spawn-spawner.ts - 1 issue
• packages/core/src/kilocode/stdio-tap.ts - 1 issue
• packages/opencode/src/tool/shell.ts - 0 issues

The root-cause analysis and fix approach (synchronous stdio tap at spawn time + awaiting the reader fiber before scope teardown) are sound, and the diff follows fork-merge hygiene well (Kilo-specific logic isolated in packages/core/src/kilocode/, minimal single-line kilocode_change markers in shared files). The changeset is a good user-facing description.

Fix these issues in Kilo Cloud

---

_{Reviewed by claude-sonnet-5-20260630 · Input: 34 · Output: 11.4K · Cached: 802.5K}

_{Review guidance: REVIEW.md from base branch main}