Browse Source

fix(cli): prevent hang when spawned without TTY (#9073)

* fix(cli): prevent hang when spawned without TTY

When the CLI is spawned as a child process without a TTY (e.g., from
spawn() in smoke tests or CI), process.stdin.isTTY is false even when
nothing is piped to stdin. This caused readStdinIfPiped() to wait up
to 5 minutes for input that would never arrive.

Fix by using fs.fstatSync(0) to check if stdin is actually a FIFO
(pipe) or file before waiting. This correctly handles:
- Spawned processes without TTY → returns immediately
- Actual piped input (echo "x" | cline) → waits and reads
- stdin from /dev/null → returns immediately

* chore: add changeset

* test(cli): add tests for stdin type detection
Robin Newhouse 2 months ago
parent
commit
b1a8db2
3 changed files with 93 additions and 1 deletions
  1. 5 0
      .changeset/cli-stdin-hang.md
  2. 70 0
      cli/src/utils/piped.test.ts
  3. 18 1
      cli/src/utils/piped.ts

+ 5 - 0
.changeset/cli-stdin-hang.md

@@ -0,0 +1,5 @@
+---
+"cline": patch
+---
+
+fix(cli): prevent hang when spawned without TTY

+ 70 - 0
cli/src/utils/piped.test.ts

@@ -1,7 +1,17 @@
 import { EventEmitter } from "node:events"
+import * as fs from "node:fs"
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
 import { readStdinIfPiped } from "./piped"
 
+// Mock fs.fstatSync to simulate pipe detection
+vi.mock("node:fs", async () => {
+	const actual = await vi.importActual("node:fs")
+	return {
+		...actual,
+		fstatSync: vi.fn(),
+	}
+})
+
 describe("readStdinIfPiped", () => {
 	let originalStdin: typeof process.stdin
 	let mockStdin: EventEmitter & {
@@ -20,6 +30,12 @@ describe("readStdinIfPiped", () => {
 			setEncoding: vi.fn(),
 			resume: vi.fn(),
 		})
+
+		// Default: simulate a real pipe (FIFO)
+		vi.mocked(fs.fstatSync).mockReturnValue({
+			isFIFO: () => true,
+			isFile: () => false,
+		} as fs.Stats)
 	})
 
 	afterEach(() => {
@@ -73,6 +89,60 @@ describe("readStdinIfPiped", () => {
 		})
 	})
 
+	describe("stdin type detection (fstat)", () => {
+		it("should return null when stdin is not a FIFO or file (spawned without TTY)", async () => {
+			setTTY(false)
+			// Simulate a character device or socket (not a pipe)
+			vi.mocked(fs.fstatSync).mockReturnValue({
+				isFIFO: () => false,
+				isFile: () => false,
+			} as fs.Stats)
+
+			const result = await readStdinIfPiped()
+			expect(result).toBeNull()
+		})
+
+		it("should return null when fstatSync throws (detached stdin)", async () => {
+			setTTY(false)
+			vi.mocked(fs.fstatSync).mockImplementation(() => {
+				throw new Error("EBADF: bad file descriptor")
+			})
+
+			const result = await readStdinIfPiped()
+			expect(result).toBeNull()
+		})
+
+		it("should read from stdin when it is a FIFO (pipe)", async () => {
+			setTTY(false)
+			vi.mocked(fs.fstatSync).mockReturnValue({
+				isFIFO: () => true,
+				isFile: () => false,
+			} as fs.Stats)
+
+			const promise = readStdinIfPiped()
+			emitData("piped content")
+			emitEnd()
+			const result = await promise
+
+			expect(result).toBe("piped content")
+		})
+
+		it("should read from stdin when it is a regular file (redirected)", async () => {
+			setTTY(false)
+			vi.mocked(fs.fstatSync).mockReturnValue({
+				isFIFO: () => false,
+				isFile: () => true,
+			} as fs.Stats)
+
+			const promise = readStdinIfPiped()
+			emitData("file content")
+			emitEnd()
+			const result = await promise
+
+			expect(result).toBe("file content")
+		})
+	})
+
 	describe("piped input scenarios", () => {
 		interface TestCase {
 			name: string

+ 18 - 1
cli/src/utils/piped.ts

@@ -1,3 +1,5 @@
+import * as fs from "node:fs"
+
 /**
  * Read piped input from stdin (non-blocking)
  *
@@ -9,11 +11,26 @@
  * for EOF which signals that the previous command has finished writing.
  */
 export async function readStdinIfPiped(): Promise<string | null> {
-	// Check if stdin is a TTY (interactive) or piped
+	// Check if stdin is a TTY (interactive) - no piped input
 	if (process.stdin.isTTY) {
 		return null
 	}
 
+	// When spawned as a child process without TTY (e.g., from spawn()), stdin.isTTY
+	// is false but there's no actual piped input. Check if stdin is a real pipe/file
+	// by testing if we can get stats on fd 0. A real pipe will have stats, while
+	// a detached stdin may throw or have unusual properties.
+	try {
+		const stats = fs.fstatSync(0)
+		// If it's not a FIFO (pipe) or regular file, treat as no input
+		if (!stats.isFIFO() && !stats.isFile()) {
+			return null
+		}
+	} catch {
+		// If we can't stat stdin, treat as no input
+		return null
+	}
+
 	// Use async approach - more reliable for piped input from other commands
 	// The synchronous readFileSync(0) can fail with EAGAIN when the pipe
 	// isn't ready yet (common when piping from another cline command)