From a387257789c15ee8787e4f5494ffe4ed6b1aac68 Mon Sep 17 00:00:00 2001 From: Haider Date: Fri, 12 Jun 2026 03:27:01 +0530 Subject: [PATCH 1/6] fix: skip stdin read when positional message provided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `!process.stdin.isTTY` alone is too broad — it matches not just "someone piped input in" but also "the parent process inherited stdin from somewhere upstream and never closed its end." In the second case, `Bun.stdin.text()` waits forever for an EOF that never arrives. The process sits at 0% CPU with no session created, no log activity, no error — a silent hang. This surfaces in every subprocess invocation pattern that passes a positional message and inherits stdin: Claude Code's Bash tool, Python `subprocess.run(..., stdin=None)`, CI runners, plugin hosts that don't pin stdin to `/dev/null`. Downstream callers have been working around it with `stdio: "ignore"` on spawn (see e.g. altimate-opencode-plugin `plugins/altimate-code/index.ts:28`), but anything that forgets to do that still hits the wedge. Fix: only read stdin when no positional `message` was provided. Matches conventional CLI semantics (positional overrides stdin) and unblocks every subprocess caller. Pipe-only invocations without a positional arg continue to work unchanged. Closes #934 --- packages/opencode/src/cli/cmd/run.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index dc59c4c86..06e1a2f2e 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -430,7 +430,16 @@ export const RunCommand = cmd({ message = [extractedParts.join("\n\n"), message].filter(Boolean).join("\n\n") } - if (!process.stdin.isTTY) message += "\n" + (await Bun.stdin.text()) + // Only read stdin when no positional message was provided. The original + // unconditional `!isTTY` guard matched a non-TTY parent process even when + // it inherited (but never closed) stdin — `Bun.stdin.text()` would then + // wait forever for an EOF that never arrives, producing a silent 0% CPU + // hang for any subprocess / CI / agent caller that passed a positional + // message. Positional-overrides-stdin matches conventional CLI semantics; + // pipe-only invocations without a positional arg still work as before. + if (!process.stdin.isTTY && message.trim().length === 0) { + message += "\n" + (await Bun.stdin.text()) + } if (message.trim().length === 0 && !args.command) { UI.error("You must provide a message or a command") From cc222db2ac8d51e1e5a7d46a33cb11d27b69b9ad Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 15 Jun 2026 19:01:42 +0530 Subject: [PATCH 2/6] fix: replace timeout race with first-byte gate + add full edge coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous fix (a387257789) replaced the unbounded `Bun.stdin.text()` call with a `Promise.race` against a 100ms timer. PR #935 reviewer flagged a silent stdin-drop regression; a consensus self-review found two more regressions introduced by the race: the orphaned read kept fd 0 open (`~1s` exit delay verified on Bun 1.3.11), and the whole-stream timeout silently truncated slow-but-legitimate piped producers. This commit replaces the race with a first-byte gate over `process.stdin` events. The timeout now caps "time until first readable byte," not full drain — so producers that flush within the window are not truncated, and the no-data path explicitly removes listeners and `unref`s the stream so an idle inherited fd no longer pins process exit. Other changes: - Accept socket-backed stdin (`isSocket()`) — was a silent narrowing. - Use `Pick` instead of a hand-rolled `Stat` type. - Move helper from `src/cli/cmd/run-stdin.ts` to `src/util/stdin.ts`. - Extract `assembleStdinMessage` as a pure function so the positional + stdin concatenation case from PR #935 is unit-testable. - 4 spawn-based E2E tests for the actual fd-0 wedge / pipe paths. --- packages/opencode/src/cli/cmd/run.ts | 14 +- packages/opencode/src/util/stdin.ts | 137 +++++++++++++++++ packages/opencode/test/util/stdin-e2e.test.ts | 129 ++++++++++++++++ packages/opencode/test/util/stdin-fixture.ts | 9 ++ packages/opencode/test/util/stdin.test.ts | 145 ++++++++++++++++++ 5 files changed, 424 insertions(+), 10 deletions(-) create mode 100644 packages/opencode/src/util/stdin.ts create mode 100644 packages/opencode/test/util/stdin-e2e.test.ts create mode 100644 packages/opencode/test/util/stdin-fixture.ts create mode 100644 packages/opencode/test/util/stdin.test.ts diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index 06e1a2f2e..87ed0bcfd 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -29,6 +29,7 @@ import { TodoWriteTool } from "../../tool/todo" import { Locale } from "../../util/locale" import { Tracer, FileExporter, HttpExporter, type TraceExporter } from "../../altimate/observability/tracing" import { Config } from "../../config/config" +import { readStdinIfAvailable, assembleStdinMessage } from "../../util/stdin" type ToolProps = { input: Tool.InferParameters @@ -430,16 +431,9 @@ export const RunCommand = cmd({ message = [extractedParts.join("\n\n"), message].filter(Boolean).join("\n\n") } - // Only read stdin when no positional message was provided. The original - // unconditional `!isTTY` guard matched a non-TTY parent process even when - // it inherited (but never closed) stdin — `Bun.stdin.text()` would then - // wait forever for an EOF that never arrives, producing a silent 0% CPU - // hang for any subprocess / CI / agent caller that passed a positional - // message. Positional-overrides-stdin matches conventional CLI semantics; - // pipe-only invocations without a positional arg still work as before. - if (!process.stdin.isTTY && message.trim().length === 0) { - message += "\n" + (await Bun.stdin.text()) - } + // Read piped/redirected stdin without wedging on inherited-but-idle fds. + // See `src/util/stdin.ts` for the failure mode and the first-byte-race fix. + message = assembleStdinMessage(message, await readStdinIfAvailable()) if (message.trim().length === 0 && !args.command) { UI.error("You must provide a message or a command") diff --git a/packages/opencode/src/util/stdin.ts b/packages/opencode/src/util/stdin.ts new file mode 100644 index 000000000..44511e610 --- /dev/null +++ b/packages/opencode/src/util/stdin.ts @@ -0,0 +1,137 @@ +import fs from "fs" +import type { Stats } from "fs" + +const FIRST_BYTE_TIMEOUT_MS = 100 + +type Stat = Pick + +export interface ReadStdinDeps { + isTTY?: boolean + fstat?: () => Stat + // Returns "" if no first byte arrives within timeoutMs; otherwise drains + // stdin to EOF and returns the full content. Default implementation uses + // process.stdin events so that a wedged-after-first-byte case still has a + // well-defined behavior (waits for `end`); callers can inject a faster + // implementation in tests. + readStdin?: (timeoutMs: number) => Promise + timeoutMs?: number +} + +// Read piped/redirected stdin without wedging on an inherited-but-idle fd. +// +// The failure mode this guards against: subprocess callers (Claude Code's +// Bash tool, Python `subprocess.run(..., stdin=None)`, CI, plugin hosts) +// leave stdin attached to a parent pipe that is never written to and never +// closed. A blind `Bun.stdin.text()` waits forever for an EOF that never +// arrives. +// +// Strategy — two gates: +// +// 1. fstat gate: only FIFOs (pipes), regular files (redirects), and +// sockets (process supervisors, socket activation, `nc -l`) can carry +// real input. TTYs and character devices (e.g. `< /dev/null`) skip. +// +// 2. First-byte timeout: instead of bounding the whole-stream drain, we +// wait up to `timeoutMs` for the first readable byte. If no byte +// arrives in that window, we treat stdin as inherited-idle and skip. +// If a byte arrives, we drain to EOF without further deadline — so a +// slow producer that takes >100ms total but flushes its first chunk +// within the window is not truncated. This avoids the two pitfalls of +// a whole-stream race: (a) the orphaned `Bun.stdin.text()` continuing +// to hold fd 0 open after the loser is abandoned, and (b) silent +// mid-stream truncation of legitimate slow / large producers. +export async function readStdinIfAvailable(deps: ReadStdinDeps = {}): Promise { + const isTTY = deps.isTTY ?? Boolean(process.stdin.isTTY) + const fstat = deps.fstat ?? (() => fs.fstatSync(0) as Stat) + const readStdin = deps.readStdin ?? defaultReadStdin + const timeoutMs = deps.timeoutMs ?? FIRST_BYTE_TIMEOUT_MS + + if (isTTY) return "" + + try { + const stat = fstat() + if (!stat.isFIFO() && !stat.isFile() && !stat.isSocket()) return "" + } catch { + return "" + } + + return readStdin(timeoutMs) +} + +// Compose the final prompt from a positional message and stdin input. +// Extracted as a pure function so the regression case from PR #935 +// (`echo ctx | run "prompt"` must concatenate, not silently drop ctx) can +// be unit-tested without spawning the full run command. +export function assembleStdinMessage(positional: string, stdinInput: string): string { + if (stdinInput.trim().length === 0) return positional + if (positional.length === 0) return stdinInput + return positional + "\n" + stdinInput +} + +// Default implementation of the first-byte race over `process.stdin`. +// +// Why not `Bun.stdin.text()`: that reads the entire stream as a single +// uncancellable Promise. If we race it against a timer and the timer wins, +// the read still holds fd 0 open until the producer eventually closes, +// blocking process exit (the original wedge moved to teardown). +// +// Using `process.stdin` event listeners lets us: +// - bind the timeout to "first byte" rather than "full drain", so slow +// producers and large payloads aren't truncated; +// - cleanly remove our listeners and `unref` the stream on the no-data +// path, so an inherited-open fd doesn't pin the event loop. +function defaultReadStdin(timeoutMs: number): Promise { + return new Promise((resolve) => { + const stdin = process.stdin + const chunks: Buffer[] = [] + let firstByteReceived = false + let firstByteTimer: ReturnType | undefined + let settled = false + + const cleanup = () => { + stdin.off("data", onData) + stdin.off("end", onEnd) + stdin.off("error", onError) + if (firstByteTimer) clearTimeout(firstByteTimer) + try { + stdin.pause() + } catch {} + try { + stdin.unref?.() + } catch {} + } + + const settle = (result: string) => { + if (settled) return + settled = true + cleanup() + resolve(result) + } + + const onData = (chunk: Buffer) => { + if (!firstByteReceived) { + firstByteReceived = true + if (firstByteTimer) { + clearTimeout(firstByteTimer) + firstByteTimer = undefined + } + } + chunks.push(chunk) + } + const onEnd = () => settle(Buffer.concat(chunks).toString("utf8")) + const onError = () => settle(Buffer.concat(chunks).toString("utf8")) + + firstByteTimer = setTimeout(() => { + if (!firstByteReceived) settle("") + }, timeoutMs) + + stdin.on("data", onData) + stdin.on("end", onEnd) + stdin.on("error", onError) + try { + stdin.resume() + } catch { + settle("") + } + }) +} diff --git a/packages/opencode/test/util/stdin-e2e.test.ts b/packages/opencode/test/util/stdin-e2e.test.ts new file mode 100644 index 000000000..7512a2128 --- /dev/null +++ b/packages/opencode/test/util/stdin-e2e.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, test } from "bun:test" +import path from "path" + +// The fixture imports the real helper and prints { result, elapsed } as JSON. +// Spawning it lets us exercise the actual `process.stdin` event path against +// real fd 0 conditions — something dependency injection can't cover. +const FIXTURE = path.join(__dirname, "stdin-fixture.ts") + +type FileSink = { write(chunk: string | Uint8Array): number; end(): Promise | number } + +async function runFixture(opts: { + writeStdin?: (sink: FileSink) => Promise + killAfterMs?: number +}): Promise<{ code: number | null; result?: string; elapsed?: number; stdout: string; stderr: string }> { + const proc = Bun.spawn(["bun", "run", FIXTURE], { + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + }) + + if (opts.writeStdin) { + await opts.writeStdin(proc.stdin as unknown as FileSink) + } + + let killTimer: ReturnType | undefined + if (opts.killAfterMs) { + killTimer = setTimeout(() => proc.kill(), opts.killAfterMs) + } + const code = await proc.exited + if (killTimer) clearTimeout(killTimer) + + const stdout = await new Response(proc.stdout).text() + const stderr = await new Response(proc.stderr).text() + + try { + const parsed = JSON.parse(stdout) + return { code, result: parsed.result, elapsed: parsed.elapsed, stdout, stderr } + } catch { + return { code, stdout, stderr } + } +} + +describe("readStdinIfAvailable (spawned subprocess)", () => { + // M1-regression — the canonical wedge: an inherited pipe that's never + // written to and never closed. Pre-fix this hung forever; with the + // previous Promise.race fix, the await released at 100ms but fd 0 stayed + // open until the parent closed. With the first-byte event race + unref, + // the child exits promptly. + test( + "exits promptly with empty result when stdin is an inherited-but-idle pipe", + async () => { + const { code, result, elapsed } = await runFixture({ + // Don't write — leave the pipe open and silent. Close after 1s so + // the parent's writer isn't garbage-collected; by then the child + // should have already exited via the first-byte timeout. + writeStdin: async (sink) => { + await new Promise((r) => setTimeout(r, 1000)) + try { + await sink.end() + } catch {} + }, + killAfterMs: 5000, + }) + expect(code).toBe(0) + expect(result).toBe("") + expect(elapsed).toBeLessThan(500) + }, + 10000, + ) + + // MAJOR-regression from PR #935: `echo ctx | run "prompt"` must still + // deliver "ctx". Verified here by writing data + closing the pipe. + test( + "returns piped data when producer writes and closes", + async () => { + const { code, result } = await runFixture({ + writeStdin: async (sink) => { + sink.write("context data") + await sink.end() + }, + }) + expect(code).toBe(0) + expect(result).toBe("context data") + }, + 10000, + ) + + // M2-regression: a producer that takes >100ms to flush first byte was + // truncated by the old Promise.race timeout. The first-byte gate must + // accept the byte once it arrives and then drain the rest without a + // deadline. + test( + "preserves data from slow producer (first byte arrives just before timeout)", + async () => { + const { code, result } = await runFixture({ + writeStdin: async (sink) => { + // Sleep close to but under the 100ms first-byte budget, then + // flush. The whole-stream-race fix would have returned "". + await new Promise((r) => setTimeout(r, 60)) + sink.write("slow ctx") + await sink.end() + }, + }) + expect(code).toBe(0) + expect(result).toBe("slow ctx") + }, + 10000, + ) + + // Negative control: a producer slow enough to miss the first-byte window + // should yield "" (intentional cutoff, no truncation of in-flight data). + test( + "returns empty when first byte arrives after the first-byte timeout", + async () => { + const { code, result } = await runFixture({ + writeStdin: async (sink) => { + await new Promise((r) => setTimeout(r, 400)) + try { + sink.write("too late") + await sink.end() + } catch {} + }, + }) + expect(code).toBe(0) + expect(result).toBe("") + }, + 10000, + ) +}) diff --git a/packages/opencode/test/util/stdin-fixture.ts b/packages/opencode/test/util/stdin-fixture.ts new file mode 100644 index 000000000..4be684958 --- /dev/null +++ b/packages/opencode/test/util/stdin-fixture.ts @@ -0,0 +1,9 @@ +// Subprocess fixture used by stdin-e2e.test.ts. +// Imports the real helper, invokes it against real fd 0, and prints the +// result as JSON so the test can assert on it. +import { readStdinIfAvailable } from "../../src/util/stdin" + +const start = Date.now() +const result = await readStdinIfAvailable() +const elapsed = Date.now() - start +process.stdout.write(JSON.stringify({ result, elapsed })) diff --git a/packages/opencode/test/util/stdin.test.ts b/packages/opencode/test/util/stdin.test.ts new file mode 100644 index 000000000..49252d239 --- /dev/null +++ b/packages/opencode/test/util/stdin.test.ts @@ -0,0 +1,145 @@ +import { describe, expect, test } from "bun:test" +import { readStdinIfAvailable, assembleStdinMessage } from "../../src/util/stdin" + +const fifo = { isFIFO: () => true, isFile: () => false, isSocket: () => false } +const file = { isFIFO: () => false, isFile: () => true, isSocket: () => false } +const sock = { isFIFO: () => false, isFile: () => false, isSocket: () => true } +const charDev = { isFIFO: () => false, isFile: () => false, isSocket: () => false } + +describe("readStdinIfAvailable", () => { + test("returns empty when stdin is a TTY (no fstat, no read)", async () => { + let read = false + const out = await readStdinIfAvailable({ + isTTY: true, + fstat: () => { + throw new Error("fstat should not run when isTTY") + }, + readStdin: async () => { + read = true + return "should not happen" + }, + }) + expect(out).toBe("") + expect(read).toBe(false) + }) + + test("returns empty for `< /dev/null` (character device — neither FIFO, file, nor socket)", async () => { + let read = false + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => charDev, + readStdin: async () => { + read = true + return "should not happen" + }, + }) + expect(out).toBe("") + expect(read).toBe(false) + }) + + test("returns empty when fstat throws (e.g. EBADF — fd 0 not open)", async () => { + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => { + throw new Error("EBADF") + }, + readStdin: async () => "should not happen", + }) + expect(out).toBe("") + }) + + // MAJOR-regression: `echo ctx | run "prompt"` must still deliver "ctx". + test("returns piped data when stdin is a FIFO with available bytes", async () => { + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => fifo, + readStdin: async () => "context data", + }) + expect(out).toBe("context data") + }) + + test("returns redirected file contents (run < file.txt)", async () => { + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => file, + readStdin: async () => "from-file", + }) + expect(out).toBe("from-file") + }) + + // m1 fix: sockets are now accepted (used by process supervisors, socket + // activation, `nc -l`). Pre-fix the helper silently skipped them. + test("accepts socket-backed stdin (process supervisors, socket activation, nc -l)", async () => { + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => sock, + readStdin: async () => "via-socket", + }) + expect(out).toBe("via-socket") + }) + + // M1-regression: timed-out wait must propagate as "" (no orphan, no wedge). + test("returns empty when readStdin times out (idle inherited FIFO)", async () => { + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => fifo, + readStdin: async () => "", // simulates first-byte timeout + }) + expect(out).toBe("") + }) + + // M2-regression: a producer that takes >100ms total but flushes its first + // byte inside the window must NOT be truncated. The helper trusts readStdin + // to wait for EOF after first byte; we verify the no-truncation contract by + // injecting a readStdin that resolves slowly with full content. + test("returns full content from slow-but-pre-timeout producer", async () => { + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => fifo, + readStdin: () => new Promise((r) => setTimeout(() => r("ctx after 80ms"), 80)), + timeoutMs: 100, + }) + expect(out).toBe("ctx after 80ms") + }) + + test("returns empty for FIFO with immediate EOF (empty pipe)", async () => { + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => fifo, + readStdin: async () => "", + }) + expect(out).toBe("") + }) + + test("returns whitespace-only stdin verbatim (caller decides to trim)", async () => { + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => fifo, + readStdin: async () => " \n\t ", + }) + expect(out).toBe(" \n\t ") + }) +}) + +describe("assembleStdinMessage", () => { + // MAJOR-regression from PR #935: positional must NOT silently override stdin. + test("concatenates positional + stdin with newline", () => { + expect(assembleStdinMessage("summarize:", "context data")).toBe("summarize:\ncontext data") + }) + + test("returns stdin when positional is empty", () => { + expect(assembleStdinMessage("", "stdin only")).toBe("stdin only") + }) + + test("returns positional when stdin is empty", () => { + expect(assembleStdinMessage("msg", "")).toBe("msg") + }) + + test("ignores whitespace-only stdin", () => { + expect(assembleStdinMessage("msg", " \n ")).toBe("msg") + }) + + test("returns empty string when both are empty", () => { + expect(assembleStdinMessage("", "")).toBe("") + }) +}) From 7fceb85a19dc3dadb133f7f0521c7b8969b576d6 Mon Sep 17 00:00:00 2001 From: Haider Date: Thu, 18 Jun 2026 06:22:45 +0530 Subject: [PATCH 3/6] fix: bump first-byte timeout, env override, stderr note, NPE safety Addresses suryaiyer95's review of `cc222db2a`: - Default first-byte timeout bumped from 100ms to 500ms. The previous value was aggressive against realistic slow-to-first-byte producers (DB queries that need to plan, decompression headers, network calls with DNS+TLS handshake). 500ms is generous for those without sacri- ficing the inherited-idle wedge fix. - New `ALTIMATE_STDIN_TIMEOUT_MS=N` env override so a user hitting an even slower pipeline can bump it without code changes. - Stderr note when fd 0 looked like real input (FIFO / file / socket) but readStdin came back empty. Silent drop was the original UX failure that motivated this whole review thread. The note fires on the dominant inherited-idle path too; that's intentional since stderr is captured but not surfaced for most subprocess callers (Claude Code's Bash tool, CI), and a single line of noise per call is worth the explicitness for the slow-producer case. - `process.stdin` null-safety folded into `readStdinIfAvailable`: in embedded / child runtimes (dev-punia review on PR #937) accessing `process.stdin.isTTY` could throw. Treat undefined stdin as "no input to read". - 6 new injection tests: env override + invalid env fallback, warning fires/doesn't fire across each gate, undefined-stdin guard. - E2E thresholds adjusted for the 500ms timeout (idle-exit assertion bumped to 1500ms, post-timeout sleep bumped to 1200ms). --- packages/opencode/src/util/stdin.ts | 65 ++++++++-- packages/opencode/test/util/stdin-e2e.test.ts | 33 +++--- packages/opencode/test/util/stdin.test.ts | 112 ++++++++++++++++++ 3 files changed, 186 insertions(+), 24 deletions(-) diff --git a/packages/opencode/src/util/stdin.ts b/packages/opencode/src/util/stdin.ts index 44511e610..9d9d024d4 100644 --- a/packages/opencode/src/util/stdin.ts +++ b/packages/opencode/src/util/stdin.ts @@ -1,7 +1,8 @@ import fs from "fs" import type { Stats } from "fs" -const FIRST_BYTE_TIMEOUT_MS = 100 +const STDIN_TIMEOUT_ENV = "ALTIMATE_STDIN_TIMEOUT_MS" +const DEFAULT_FIRST_BYTE_TIMEOUT_MS = 500 type Stat = Pick @@ -15,6 +16,25 @@ export interface ReadStdinDeps { // implementation in tests. readStdin?: (timeoutMs: number) => Promise timeoutMs?: number + // Called once with a human-readable note when fd 0 looked like a real + // input source (FIFO / file / socket) but no first byte arrived within + // the timeout — so a user whose pipe was silently dropped finds out. + // Default writes to stderr; tests inject a no-op to silence. + warn?: (msg: string) => void +} + +function resolveTimeoutMs(): number { + const raw = process.env[STDIN_TIMEOUT_ENV] + if (raw === undefined) return DEFAULT_FIRST_BYTE_TIMEOUT_MS + const parsed = Number.parseInt(raw, 10) + if (!Number.isFinite(parsed) || parsed < 0) return DEFAULT_FIRST_BYTE_TIMEOUT_MS + return parsed +} + +function defaultWarn(msg: string): void { + try { + process.stderr?.write(msg + "\n") + } catch {} } // Read piped/redirected stdin without wedging on an inherited-but-idle fd. @@ -35,16 +55,36 @@ export interface ReadStdinDeps { // wait up to `timeoutMs` for the first readable byte. If no byte // arrives in that window, we treat stdin as inherited-idle and skip. // If a byte arrives, we drain to EOF without further deadline — so a -// slow producer that takes >100ms total but flushes its first chunk -// within the window is not truncated. This avoids the two pitfalls of -// a whole-stream race: (a) the orphaned `Bun.stdin.text()` continuing -// to hold fd 0 open after the loser is abandoned, and (b) silent -// mid-stream truncation of legitimate slow / large producers. +// slow producer that takes >500ms total but flushes its first chunk +// within the window is not truncated. The 500ms default is chosen to +// cover realistic slow-to-first-byte producers (DB queries that need +// to plan, decompression headers, network calls with DNS+TLS handshake) +// while still failing fast enough to keep the inherited-idle wedge fix +// effective. Users can override via `ALTIMATE_STDIN_TIMEOUT_MS=N` (ms) +// for environments with even slower producers. This avoids the two +// pitfalls of a whole-stream race: (a) the orphaned `Bun.stdin.text()` +// continuing to hold fd 0 open after the loser is abandoned, and +// (b) silent mid-stream truncation of legitimate slow / large producers. +// +// 3. Stderr note on silent drop: if we got past the fstat gate (fd 0 +// looked like real input) but the read came back empty, we write one +// line to stderr so a user whose pipe was dropped finds out instead +// of silently getting no input. The note also tells them how to bump +// the timeout. This fires on the dominant inherited-idle path too — +// that's intentional, since stderr is captured but not surfaced for +// most subprocess callers (Claude Code's Bash tool, CI) and a single +// line of noise per call is worth the explicitness for the +// slow-producer case. export async function readStdinIfAvailable(deps: ReadStdinDeps = {}): Promise { + // `process.stdin` can be undefined in embedded / child runtimes (flagged + // by dev-punia on PR #937). Treat absence as "no stdin to read." + if (deps.isTTY === undefined && !process.stdin) return "" + const isTTY = deps.isTTY ?? Boolean(process.stdin.isTTY) const fstat = deps.fstat ?? (() => fs.fstatSync(0) as Stat) const readStdin = deps.readStdin ?? defaultReadStdin - const timeoutMs = deps.timeoutMs ?? FIRST_BYTE_TIMEOUT_MS + const timeoutMs = deps.timeoutMs ?? resolveTimeoutMs() + const warn = deps.warn ?? defaultWarn if (isTTY) return "" @@ -55,7 +95,16 @@ export async function readStdinIfAvailable(deps: ReadStdinDeps = {}): Promise { "exits promptly with empty result when stdin is an inherited-but-idle pipe", async () => { const { code, result, elapsed } = await runFixture({ - // Don't write — leave the pipe open and silent. Close after 1s so + // Don't write — leave the pipe open and silent. Close after 3s so // the parent's writer isn't garbage-collected; by then the child - // should have already exited via the first-byte timeout. + // should have already exited via the 500ms first-byte timeout. writeStdin: async (sink) => { - await new Promise((r) => setTimeout(r, 1000)) + await new Promise((r) => setTimeout(r, 3000)) try { await sink.end() } catch {} }, - killAfterMs: 5000, + killAfterMs: 8000, }) expect(code).toBe(0) expect(result).toBe("") - expect(elapsed).toBeLessThan(500) + // 1500 = timeout (500ms) × 3 for CI scheduler headroom. Well under + // "infinite hang" — the assertion guards the wedge fix, not perf. + expect(elapsed).toBeLessThan(1500) }, - 10000, + 15000, ) // MAJOR-regression from PR #935: `echo ctx | run "prompt"` must still @@ -85,18 +87,16 @@ describe("readStdinIfAvailable (spawned subprocess)", () => { 10000, ) - // M2-regression: a producer that takes >100ms to flush first byte was - // truncated by the old Promise.race timeout. The first-byte gate must - // accept the byte once it arrives and then drain the rest without a - // deadline. + // Sury PR #935 review #2: 500ms must cover realistic slow-first-byte + // producers (DB queries that need to plan, decompression, network calls + // with DNS+TLS handshake). 300ms is comfortably under the 500ms budget; + // the previous 100ms config would have truncated this. test( - "preserves data from slow producer (first byte arrives just before timeout)", + "preserves data from slow producer (first byte arrives ~300ms after spawn)", async () => { const { code, result } = await runFixture({ writeStdin: async (sink) => { - // Sleep close to but under the 100ms first-byte budget, then - // flush. The whole-stream-race fix would have returned "". - await new Promise((r) => setTimeout(r, 60)) + await new Promise((r) => setTimeout(r, 300)) sink.write("slow ctx") await sink.end() }, @@ -109,12 +109,13 @@ describe("readStdinIfAvailable (spawned subprocess)", () => { // Negative control: a producer slow enough to miss the first-byte window // should yield "" (intentional cutoff, no truncation of in-flight data). + // 1200ms > 500ms timeout with margin for CI scheduler latency. test( "returns empty when first byte arrives after the first-byte timeout", async () => { const { code, result } = await runFixture({ writeStdin: async (sink) => { - await new Promise((r) => setTimeout(r, 400)) + await new Promise((r) => setTimeout(r, 1200)) try { sink.write("too late") await sink.end() @@ -124,6 +125,6 @@ describe("readStdinIfAvailable (spawned subprocess)", () => { expect(code).toBe(0) expect(result).toBe("") }, - 10000, + 15000, ) }) diff --git a/packages/opencode/test/util/stdin.test.ts b/packages/opencode/test/util/stdin.test.ts index 49252d239..7f7701cfb 100644 --- a/packages/opencode/test/util/stdin.test.ts +++ b/packages/opencode/test/util/stdin.test.ts @@ -84,6 +84,7 @@ describe("readStdinIfAvailable", () => { isTTY: false, fstat: () => fifo, readStdin: async () => "", // simulates first-byte timeout + warn: () => {}, // silence the stderr note in test output }) expect(out).toBe("") }) @@ -107,6 +108,7 @@ describe("readStdinIfAvailable", () => { isTTY: false, fstat: () => fifo, readStdin: async () => "", + warn: () => {}, }) expect(out).toBe("") }) @@ -119,6 +121,116 @@ describe("readStdinIfAvailable", () => { }) expect(out).toBe(" \n\t ") }) + + // PR #937 / dev-punia review: `process.stdin` can be undefined in + // embedded/child runtimes. The helper must not throw when accessing + // `process.stdin.isTTY` in that case. + test("returns empty when process.stdin is undefined (embedded/child runtime)", async () => { + const original = process.stdin + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ;(process as any).stdin = undefined + try { + const out = await readStdinIfAvailable() + expect(out).toBe("") + } finally { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ;(process as any).stdin = original + } + }) + + // Sury PR #935 review #2: silent drop is bad UX. When fd 0 looked like + // real input but no first byte arrived, warn so the user knows. + test("warns when stdin looked like input but readStdin returned empty (silent-drop fix)", async () => { + const seen: string[] = [] + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => fifo, + readStdin: async () => "", + warn: (msg) => seen.push(msg), + }) + expect(out).toBe("") + expect(seen).toHaveLength(1) + expect(seen[0]).toContain("stdin appears piped") + expect(seen[0]).toContain("ALTIMATE_STDIN_TIMEOUT_MS") + }) + + test("does NOT warn when isTTY (no pipe to drop in the first place)", async () => { + const seen: string[] = [] + const out = await readStdinIfAvailable({ + isTTY: true, + readStdin: async () => "", + warn: (msg) => seen.push(msg), + }) + expect(out).toBe("") + expect(seen).toHaveLength(0) + }) + + test("does NOT warn when fstat says char device (/dev/null) — intentional skip", async () => { + const seen: string[] = [] + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => charDev, + readStdin: async () => "", + warn: (msg) => seen.push(msg), + }) + expect(out).toBe("") + expect(seen).toHaveLength(0) + }) + + test("does NOT warn when stdin delivered data (no drop happened)", async () => { + const seen: string[] = [] + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => fifo, + readStdin: async () => "ctx", + warn: (msg) => seen.push(msg), + }) + expect(out).toBe("ctx") + expect(seen).toHaveLength(0) + }) + + // Sury PR #935 review #2: env override for slow-pipeline users. + test("ALTIMATE_STDIN_TIMEOUT_MS env override is forwarded to readStdin", async () => { + const original = process.env["ALTIMATE_STDIN_TIMEOUT_MS"] + process.env["ALTIMATE_STDIN_TIMEOUT_MS"] = "2500" + let observed: number | undefined + try { + await readStdinIfAvailable({ + isTTY: false, + fstat: () => fifo, + readStdin: async (ms) => { + observed = ms + return "data" + }, + warn: () => {}, + }) + } finally { + if (original === undefined) delete process.env["ALTIMATE_STDIN_TIMEOUT_MS"] + else process.env["ALTIMATE_STDIN_TIMEOUT_MS"] = original + } + expect(observed).toBe(2500) + }) + + test("ALTIMATE_STDIN_TIMEOUT_MS falls back to default when unparseable", async () => { + const original = process.env["ALTIMATE_STDIN_TIMEOUT_MS"] + process.env["ALTIMATE_STDIN_TIMEOUT_MS"] = "not-a-number" + let observed: number | undefined + try { + await readStdinIfAvailable({ + isTTY: false, + fstat: () => fifo, + readStdin: async (ms) => { + observed = ms + return "data" + }, + warn: () => {}, + }) + } finally { + if (original === undefined) delete process.env["ALTIMATE_STDIN_TIMEOUT_MS"] + else process.env["ALTIMATE_STDIN_TIMEOUT_MS"] = original + } + expect(observed).toBe(500) + }) }) describe("assembleStdinMessage", () => { From 9cf83d97316dc6c8ec358bb62f070f9b176cc741 Mon Sep 17 00:00:00 2001 From: Haider Date: Thu, 18 Jun 2026 06:52:44 +0530 Subject: [PATCH 4/6] stdin: use optional chaining for process.stdin NPE guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Matches the literal form suryaiyer95 proposed in review #3 on PR #935: `process.stdin?.isTTY` instead of an early-return + plain access. Functionally equivalent — the fstat try/catch already returns "" when fd 0 isn't open, so the optional chaining alone covers the embedded- runtime case. --- packages/opencode/src/util/stdin.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/opencode/src/util/stdin.ts b/packages/opencode/src/util/stdin.ts index 9d9d024d4..72dbfc9ad 100644 --- a/packages/opencode/src/util/stdin.ts +++ b/packages/opencode/src/util/stdin.ts @@ -77,10 +77,10 @@ function defaultWarn(msg: string): void { // slow-producer case. export async function readStdinIfAvailable(deps: ReadStdinDeps = {}): Promise { // `process.stdin` can be undefined in embedded / child runtimes (flagged - // by dev-punia on PR #937). Treat absence as "no stdin to read." - if (deps.isTTY === undefined && !process.stdin) return "" - - const isTTY = deps.isTTY ?? Boolean(process.stdin.isTTY) + // by dev-punia on PR #937, suryaiyer95 review on PR #935 #3). Optional + // chaining keeps the access safe; the fstat catch below covers the case + // where fd 0 itself isn't open. + const isTTY = deps.isTTY ?? Boolean(process.stdin?.isTTY) const fstat = deps.fstat ?? (() => fs.fstatSync(0) as Stat) const readStdin = deps.readStdin ?? defaultReadStdin const timeoutMs = deps.timeoutMs ?? resolveTimeoutMs() From 8eac6fa477c2895a7b7eb4477c57576e41a45ce2 Mon Sep 17 00:00:00 2001 From: Haider Date: Thu, 18 Jun 2026 07:09:58 +0530 Subject: [PATCH 5/6] stdin: address cubic P2 + P3 on commit 7fceb85a1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P2 (NPE guard incomplete): a caller that passes `deps.isTTY` bypasses the outer `process.stdin?.isTTY` guard, and if `process.stdin` is undefined the default reader crashes on `stdin.on(...)`. Add a defensive check inside `defaultReadStdin` that returns "" when stdin is absent. New regression test exercises the path with `deps.isTTY` set + undefined `process.stdin` + no `readStdin` injection. P3 (warning text misleading for non-timeout empty results): an empty result can come from the timer firing, a clean `end` with zero bytes, or an error event — the previous "no data received within Nms" wording implied only the first. Reword to cause-neutral "stdin produced no data" while keeping the env-var tip for slow-producer cases. --- packages/opencode/src/util/stdin.ts | 13 ++++++++++-- packages/opencode/test/util/stdin.test.ts | 24 ++++++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/opencode/src/util/stdin.ts b/packages/opencode/src/util/stdin.ts index 72dbfc9ad..4aef948be 100644 --- a/packages/opencode/src/util/stdin.ts +++ b/packages/opencode/src/util/stdin.ts @@ -98,9 +98,13 @@ export async function readStdinIfAvailable(deps: ReadStdinDeps = {}): Promise { return new Promise((resolve) => { const stdin = process.stdin + // Defensive: a caller can reach this path with `deps.isTTY` set but + // `process.stdin` undefined (embedded/child runtime). The outer + // `process.stdin?.isTTY` guard short-circuits the default case but not + // this one. Treat absence as "no data" to match the function's contract. + if (!stdin) return resolve("") const chunks: Buffer[] = [] let firstByteReceived = false let firstByteTimer: ReturnType | undefined diff --git a/packages/opencode/test/util/stdin.test.ts b/packages/opencode/test/util/stdin.test.ts index 7f7701cfb..b6bcc7d0f 100644 --- a/packages/opencode/test/util/stdin.test.ts +++ b/packages/opencode/test/util/stdin.test.ts @@ -138,6 +138,28 @@ describe("readStdinIfAvailable", () => { } }) + // cubic P2 on commit 7fceb85a1: a caller that passes `deps.isTTY` bypasses + // the outer `process.stdin?.isTTY` guard. If `process.stdin` is undefined, + // the default reader path used to crash on `stdin.on(...)`. The defensive + // check inside `defaultReadStdin` covers this. + test("default reader returns empty when process.stdin is undefined even when deps.isTTY is passed", async () => { + const original = process.stdin + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ;(process as any).stdin = undefined + try { + const out = await readStdinIfAvailable({ + isTTY: false, + fstat: () => fifo, + // no readStdin → exercises defaultReadStdin + warn: () => {}, + }) + expect(out).toBe("") + } finally { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ;(process as any).stdin = original + } + }) + // Sury PR #935 review #2: silent drop is bad UX. When fd 0 looked like // real input but no first byte arrived, warn so the user knows. test("warns when stdin looked like input but readStdin returned empty (silent-drop fix)", async () => { @@ -150,7 +172,7 @@ describe("readStdinIfAvailable", () => { }) expect(out).toBe("") expect(seen).toHaveLength(1) - expect(seen[0]).toContain("stdin appears piped") + expect(seen[0]).toContain("stdin produced no data") expect(seen[0]).toContain("ALTIMATE_STDIN_TIMEOUT_MS") }) From 320be762cc14a391abd0d975f4cd0832bce2e647 Mon Sep 17 00:00:00 2001 From: Haider Date: Thu, 18 Jun 2026 20:12:02 +0530 Subject: [PATCH 6/6] stdin: drop env-var tip from no-data warning (cubic P2) The cause-neutral wording from 8eac6fa47 still kept the `set ALTIMATE_STDIN_TIMEOUT_MS=N` tip. cubic flagged that the tip points users at timeout tuning even when the real cause is a stream error or the dominant inherited-idle subprocess case. Drop the tip; the env var stays documented in code/docs for the rare slow-producer scenario. Final wording: "altimate-code: stdin produced no data; proceeding without it." --- packages/opencode/src/util/stdin.ts | 13 ++++++------- packages/opencode/test/util/stdin.test.ts | 1 - 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/opencode/src/util/stdin.ts b/packages/opencode/src/util/stdin.ts index 4aef948be..be2054840 100644 --- a/packages/opencode/src/util/stdin.ts +++ b/packages/opencode/src/util/stdin.ts @@ -99,13 +99,12 @@ export async function readStdinIfAvailable(deps: ReadStdinDeps = {}): Promise { expect(out).toBe("") expect(seen).toHaveLength(1) expect(seen[0]).toContain("stdin produced no data") - expect(seen[0]).toContain("ALTIMATE_STDIN_TIMEOUT_MS") }) test("does NOT warn when isTTY (no pipe to drop in the first place)", async () => {