fix: skip stdin read when positional message provided

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<T extends Tool.Info> = {
   input: Tool.InferParameters<T>
@@ -454,14 +455,11 @@ export const RunCommand = cmd({
       message = [extractedParts.join("\n\n"), message].filter(Boolean).join("\n\n")
     }
 
-    // altimate_change start — null-safe stdin read. process.stdin can be
-    // undefined in embedded/child runtimes (dev-punia review, PR #937).
-    // Earlier revision used `!process.stdin?.isTTY`, which turned the crash
-    // into a stall: undefined stdin satisfied the guard and we then awaited
-    // Bun.stdin.text() on a stream that would never EOF. Skip the read
-    // entirely when there is no stdin to read from.
-    if (process.stdin && !process.stdin.isTTY) message += "\n" + (await Bun.stdin.text())
-    // altimate_change end
+    // 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.
+    // The helper also handles `process.stdin === undefined` (embedded/child
+    // runtimes — dev-punia review on PR #937) by returning "".
+    message = assembleStdinMessage(message, await readStdinIfAvailable())
 
     if (message.trim().length === 0 && !args.command) {
       UI.error("You must provide a message or a command")

packages/opencode/src/util/stdin.ts

@@ -0,0 +1,194 @@
+import fs from "fs"
+import type { Stats } from "fs"
+
+const STDIN_TIMEOUT_ENV = "ALTIMATE_STDIN_TIMEOUT_MS"
+const DEFAULT_FIRST_BYTE_TIMEOUT_MS = 500
+
+type Stat = Pick<Stats, "isFIFO" | "isFile" | "isSocket">
+
+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<string>
+  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.
+//
+// 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 >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<string> {
+  // `process.stdin` can be undefined in embedded / child runtimes (flagged
+  // 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()
+  const warn = deps.warn ?? defaultWarn
+
+  if (isTTY) return ""
+
+  try {
+    const stat = fstat()
+    if (!stat.isFIFO() && !stat.isFile() && !stat.isSocket()) return ""
+  } catch {
+    return ""
+  }
+
+  const result = await readStdin(timeoutMs)
+
+  if (result === "") {
+    // An empty result can come from the timer firing (slow producer), a
+    // clean `end` with zero bytes (intentionally empty pipe), or a stream
+    // error. The previous version included a `set ${STDIN_TIMEOUT_ENV}=N`
+    // tip; cubic-dev-ai flagged it as misleading for the error and the
+    // dominant inherited-idle subprocess cases. The env var stays
+    // documented in code/docs for the rare slow-producer scenario.
+    warn(`altimate-code: stdin produced no data; proceeding without it.`)
+  }
+
+  return result
+}
+
+// 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<string> {
+  return new Promise<string>((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<typeof setTimeout> | 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("")
+    }
+  })
+}

packages/opencode/test/util/stdin-e2e.test.ts

@@ -0,0 +1,130 @@
+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> | number }
+
+async function runFixture(opts: {
+  writeStdin?: (sink: FileSink) => Promise<void>
+  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<typeof setTimeout> | 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 3s so
+        // the parent's writer isn't garbage-collected; by then the child
+        // should have already exited via the 500ms first-byte timeout.
+        writeStdin: async (sink) => {
+          await new Promise((r) => setTimeout(r, 3000))
+          try {
+            await sink.end()
+          } catch {}
+        },
+        killAfterMs: 8000,
+      })
+      expect(code).toBe(0)
+      expect(result).toBe("")
+      // 1500 = timeout (500ms) × 3 for CI scheduler headroom. Well under
+      // "infinite hang" — the assertion guards the wedge fix, not perf.
+      expect(elapsed).toBeLessThan(1500)
+    },
+    15000,
+  )
+
+  // 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,
+  )
+
+  // 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 ~300ms after spawn)",
+    async () => {
+      const { code, result } = await runFixture({
+        writeStdin: async (sink) => {
+          await new Promise((r) => setTimeout(r, 300))
+          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).
+  // 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, 1200))
+          try {
+            sink.write("too late")
+            await sink.end()
+          } catch {}
+        },
+      })
+      expect(code).toBe(0)
+      expect(result).toBe("")
+    },
+    15000,
+  )
+})

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 }))