From 3101ebd99205816acb95489746b8dd97eaff42df Mon Sep 17 00:00:00 2001 From: postoso Date: Fri, 26 Jun 2026 17:25:23 -0400 Subject: [PATCH] fix(command-mode): drain process output concurrently to prevent TerminalService pipe deadlock --- Sources/Fluid/Services/TerminalService.swift | 17 ++++++++-- .../DictationE2ETests.swift | 33 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/Sources/Fluid/Services/TerminalService.swift b/Sources/Fluid/Services/TerminalService.swift index 47bf79d4..1baaef26 100644 --- a/Sources/Fluid/Services/TerminalService.swift +++ b/Sources/Fluid/Services/TerminalService.swift @@ -108,12 +108,23 @@ final class TerminalService { } } + // Drain stdout and stderr concurrently while the process is still + // running. A child that writes more than the pipe buffer (~64KB) + // blocks on write() until its output is read, so reading only after + // waitUntilExit() would deadlock until the timeout fired and return + // truncated output. Background reads keep both pipes drained so the + // process can run to completion. + let outputHandle = outputPipe.fileHandleForReading + let errorHandle = errorPipe.fileHandleForReading + async let pendingOutput = Task.detached { outputHandle.readDataToEndOfFile() }.value + async let pendingError = Task.detached { errorHandle.readDataToEndOfFile() }.value + + let outputData = await pendingOutput + let errorData = await pendingError + process.waitUntilExit() timeoutTask.cancel() - let outputData = outputPipe.fileHandleForReading.readDataToEndOfFile() - let errorData = errorPipe.fileHandleForReading.readDataToEndOfFile() - let output = String(data: outputData, encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" let errorOutput = String(data: errorData, encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines) diff --git a/Tests/FluidDictationIntegrationTests/DictationE2ETests.swift b/Tests/FluidDictationIntegrationTests/DictationE2ETests.swift index 3f97b50a..8b3216fa 100644 --- a/Tests/FluidDictationIntegrationTests/DictationE2ETests.swift +++ b/Tests/FluidDictationIntegrationTests/DictationE2ETests.swift @@ -691,6 +691,39 @@ final class DictationE2ETests: XCTestCase { ) } + func testTerminalServiceExecute_largeStdoutIsNotTruncatedAndReturnsQuickly() async { + // Regression for a pipe-buffer deadlock: execute() used to call + // waitUntilExit() before draining stdout, so a command writing more than + // the ~64KB pipe buffer blocked on write() until the 30s timeout fired, + // returning truncated output with success == false. Draining stdout and + // stderr concurrently lets the command run to completion. + let service = TerminalService() + let lineCount = 200_000 + + let result = await service.execute(command: "seq 1 \(lineCount)") + + XCTAssertTrue(result.success, "Expected success, got exitCode \(result.exitCode) error \(result.error ?? "nil")") + XCTAssertEqual(result.exitCode, 0) + XCTAssertGreaterThan(result.output.utf8.count, 64 * 1024, "Output should exceed the 64KB pipe buffer") + XCTAssertTrue(result.output.hasSuffix("\(lineCount)"), "Last line should be complete, output was truncated") + XCTAssertEqual(result.output.split(separator: "\n").count, lineCount) + XCTAssertLessThan(result.executionTimeMs, 15000, "Should return promptly, not after the ~30s timeout") + } + + func testTerminalServiceExecute_largeStderrIsNotTruncated() async throws { + // The same deadlock applied to stderr; both pipes must be drained concurrently. + let service = TerminalService() + let lineCount = 200_000 + + let result = await service.execute(command: "seq 1 \(lineCount) 1>&2") + + XCTAssertTrue(result.success, "Expected success, got exitCode \(result.exitCode)") + let stderr = try XCTUnwrap(result.error) + XCTAssertGreaterThan(stderr.utf8.count, 64 * 1024, "Stderr should exceed the 64KB pipe buffer") + XCTAssertTrue(stderr.hasSuffix("\(lineCount)"), "Last stderr line should be complete, output was truncated") + XCTAssertLessThan(result.executionTimeMs, 15000, "Should return promptly, not after the ~30s timeout") + } + private static func modelDirectoryForRun() -> URL { // Use a stable path on CI so GitHub Actions cache can speed up runs. if ProcessInfo.processInfo.environment["GITHUB_ACTIONS"] == "true" ||