Skip to content

Commit 8eac6fa

Browse files
author
Haider
committed
stdin: address cubic P2 + P3 on commit 7fceb85
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.
1 parent 9cf83d9 commit 8eac6fa

2 files changed

Lines changed: 34 additions & 3 deletions

File tree

packages/opencode/src/util/stdin.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,13 @@ export async function readStdinIfAvailable(deps: ReadStdinDeps = {}): Promise<st
9898
const result = await readStdin(timeoutMs)
9999

100100
if (result === "") {
101+
// An empty result can come from the timer firing (slow producer), a
102+
// clean `end` with zero bytes (intentionally empty pipe), or an error
103+
// event. Wording is cause-neutral; the env-var tip is still useful for
104+
// the slow-producer case and harmless otherwise.
101105
warn(
102-
`altimate-code: stdin appears piped but no data received within ${timeoutMs}ms; ` +
103-
`proceeding without it. Set ${STDIN_TIMEOUT_ENV}=N (ms) to wait longer.`,
106+
`altimate-code: stdin produced no data; proceeding without it. ` +
107+
`Tip: set ${STDIN_TIMEOUT_ENV}=N (ms) higher if upstream is a slow producer.`,
104108
)
105109
}
106110

@@ -132,6 +136,11 @@ export function assembleStdinMessage(positional: string, stdinInput: string): st
132136
function defaultReadStdin(timeoutMs: number): Promise<string> {
133137
return new Promise<string>((resolve) => {
134138
const stdin = process.stdin
139+
// Defensive: a caller can reach this path with `deps.isTTY` set but
140+
// `process.stdin` undefined (embedded/child runtime). The outer
141+
// `process.stdin?.isTTY` guard short-circuits the default case but not
142+
// this one. Treat absence as "no data" to match the function's contract.
143+
if (!stdin) return resolve("")
135144
const chunks: Buffer[] = []
136145
let firstByteReceived = false
137146
let firstByteTimer: ReturnType<typeof setTimeout> | undefined

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,28 @@ describe("readStdinIfAvailable", () => {
138138
}
139139
})
140140

141+
// cubic P2 on commit 7fceb85a1: a caller that passes `deps.isTTY` bypasses
142+
// the outer `process.stdin?.isTTY` guard. If `process.stdin` is undefined,
143+
// the default reader path used to crash on `stdin.on(...)`. The defensive
144+
// check inside `defaultReadStdin` covers this.
145+
test("default reader returns empty when process.stdin is undefined even when deps.isTTY is passed", async () => {
146+
const original = process.stdin
147+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
148+
;(process as any).stdin = undefined
149+
try {
150+
const out = await readStdinIfAvailable({
151+
isTTY: false,
152+
fstat: () => fifo,
153+
// no readStdin → exercises defaultReadStdin
154+
warn: () => {},
155+
})
156+
expect(out).toBe("")
157+
} finally {
158+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
159+
;(process as any).stdin = original
160+
}
161+
})
162+
141163
// Sury PR #935 review #2: silent drop is bad UX. When fd 0 looked like
142164
// real input but no first byte arrived, warn so the user knows.
143165
test("warns when stdin looked like input but readStdin returned empty (silent-drop fix)", async () => {
@@ -150,7 +172,7 @@ describe("readStdinIfAvailable", () => {
150172
})
151173
expect(out).toBe("")
152174
expect(seen).toHaveLength(1)
153-
expect(seen[0]).toContain("stdin appears piped")
175+
expect(seen[0]).toContain("stdin produced no data")
154176
expect(seen[0]).toContain("ALTIMATE_STDIN_TIMEOUT_MS")
155177
})
156178

0 commit comments

Comments
 (0)