drain command stdout and stderr concurrently

5 files changed, 82 insertions, 14 deletions

diff --git a/Sources/bar/Models/Runner.swift b/Sources/bar/Models/Runner.swift
index 6c90d54..4b6694f 100644
--- a/Sources/bar/Models/Runner.swift
+++ b/Sources/bar/Models/Runner.swift
@@ -19,14 +19,15 @@ enum Runner {
 	///
 	/// - Returns: Parsed stdout on success; ["ERROR"] on launch failure or
 	///   nonzero exit, or [""] if successful output cannot be decoded as UTF-8.
-	///   Captured stderr is forwarded only after a nonzero exit.
-	static func run(_ command: String, env: [String: String] = [:]) -> [String] {
+	///   Captured stderr is forwarded regardless of exit status.
+	static func run(_ command: String, env: [String: String] = [:],
+	                errorOutput: FileHandle = .standardError) -> [String] {
 		// Detached tasks do not run inside AppKit's main event-loop pool.
 		// Drain temporary Foundation objects at the end of each command.
-		autoreleasepool { execute(command, env: env) }
+		autoreleasepool { execute(command, env: env, errorOutput: errorOutput) }
 	}
 
-	private static func execute(_ command: String, env: [String: String]) -> [String] {
+	private static func execute(_ command: String, env: [String: String], errorOutput: FileHandle) -> [String] {
 		let process = Process()
 		process.executableURL = URL(fileURLWithPath: "/bin/sh")
 		process.arguments = ["-c", command]
@@ -53,14 +54,21 @@ enum Runner {
 		}
 		timer.resume()
 
+		// Drain stderr alongside stdout so neither full pipe can stall the child.
+		// Use GCD for this blocking read, not Swift's cooperative executor.
+		let diagnostics = DispatchGroup()
+		DispatchQueue.global(qos: .utility).async(group: diagnostics) {
+			autoreleasepool {
+				write(stderr.fileHandleForReading.readDataToEndOfFile(), to: errorOutput)
+			}
+		}
 		let data = stdout.fileHandleForReading.readDataToEndOfFile()
-		let errData = stderr.fileHandleForReading.readDataToEndOfFile()
 		process.waitUntilExit()
+		diagnostics.wait()
 
 		timer.cancel()
 
 		guard process.terminationStatus == 0 else {
-			write(errData, to: .standardError)
 			return ["ERROR"]
 		}
 
diff --git a/Sources/bar/bar.docc/command.md b/Sources/bar/bar.docc/command.md
index 3196011..c2ad917 100644
--- a/Sources/bar/bar.docc/command.md
+++ b/Sources/bar/bar.docc/command.md
@@ -29,13 +29,13 @@ On the contrary, if you run `bar` from a launch agent, you'll probably not have
 ## Error handling
 
 If the command exits with a non-zero status or cannot be launched, the block displays a single cell containing the text `ERROR`.
-After a non-zero exit, captured stderr is forwarded to the `bar` process stderr.
-Stderr from a successful command is discarded, and launch failures do not forward a diagnostic.
+Captured stderr is forwarded to the `bar` process stderr, including on successful exits.
+Launch failures do not forward a diagnostic.
 Successful stdout that cannot be decoded as UTF-8 produces a blank cell.
 
 After 30 seconds, *bar* requests termination of the shell with SIGTERM.
 This is not a hard deadline: a shell can ignore the signal, and child processes can keep the output pipes open.
-The current runner reads stdout and then stderr before waiting for exit, so commands with enough stderr output can also block while stdout is being read.
+Stdout and stderr are drained concurrently so a full output pipe does not stall the command.
 
 ```shell
 # Example: simulate a command failure
diff --git a/Tests/barTests/phbarTests.swift b/Tests/barTests/phbarTests.swift
index 6a2c4d5..982e728 100644
--- a/Tests/barTests/phbarTests.swift
+++ b/Tests/barTests/phbarTests.swift
@@ -1,8 +1,53 @@
+import Foundation
+import Synchronization
 import Testing
 @testable import bar
 
-@Test func example() async throws {
-    // Write your test here and use APIs like `#expect(...)` to check expected conditions.
-    // Swift Testing Documentation
-    // https://developer.apple.com/documentation/testing
+/// Blocking Runner work stays off the cooperative executor. The outer deadline
+/// also fails promptly if a regression prevents Runner from returning.
+private func runCommand(_ command: String, env: [String: String] = [:]) throws -> (lines: [String], diagnostics: Data) {
+    let url = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    FileManager.default.createFile(atPath: url.path, contents: nil)
+    let output = try FileHandle(forWritingTo: url)
+    defer { try? FileManager.default.removeItem(at: url) }
+    let result = Mutex<[String]?>(nil)
+    let finished = DispatchSemaphore(value: 0)
+    DispatchQueue.global(qos: .utility).async {
+        let lines = Runner.run(command, env: env, errorOutput: output)
+        try? output.close()
+        result.withLock { $0 = lines }
+        finished.signal()
+    }
+    let completed = finished.wait(timeout: .now() + 5) == .success
+    try #require(completed, "Runner exceeded the five-second outer deadline")
+    return (try #require(result.withLock { $0 }), try Data(contentsOf: url))
+}
+
+@Test func drainsLargeStderrBeforeStdout() throws {
+    let result = try runCommand("/usr/bin/head -c 1048576 /dev/zero >&2; printf ' ready \\n'")
+    #expect(result.lines == [" ready "])
+    #expect(result.diagnostics == Data(repeating: 0, count: 1048576))
+}
+
+@Test func drainsSimultaneousLargeStreams() throws {
+    let result = try runCommand("/usr/bin/head -c 1048576 /dev/zero >&2 & /usr/bin/head -c 1048576 /dev/zero; wait")
+    #expect(result.lines == [String(repeating: "\0", count: 1048576)])
+    #expect(result.diagnostics == Data(repeating: 0, count: 1048576))
+}
+
+@Test func emptyCommandOutput() throws {
+    let result = try runCommand(":")
+    #expect(result.lines.isEmpty)
+    #expect(result.diagnostics.isEmpty)
+}
+
+@Test func failedCommandForwardsDiagnostics() throws {
+    let result = try runCommand("printf 'ignored'; printf 'failure\\n' >&2; exit 7")
+    #expect(result.lines == ["ERROR"])
+    #expect(result.diagnostics == Data("failure\n".utf8))
+}
+
+@Test func shellEnvironmentAndWhitespaceArePreserved() throws {
+    let result = try runCommand("printf ' %s \\n\\nlast\\n' \"$BAR_RUNNER_TEST\"; test -n \"$PATH\"", env: ["BAR_RUNNER_TEST": "value"])
+    #expect(result.lines == [" value ", "", "last"])
 }
diff --git a/tasks/20261014-190252/TASK.md b/tasks/20261014-190252/TASK.md
index abbeb72..16a404d 100644
--- a/tasks/20261014-190252/TASK.md
+++ b/tasks/20261014-190252/TASK.md
@@ -1,6 +1,6 @@
 # Drain command stdout and stderr concurrently
 
-- STATUS: OPEN
+- STATUS: CLOSED
 - PRIORITY: 10
 - TAGS: review, bug, core
 
@@ -13,3 +13,12 @@ Approach: Drain both pipes concurrently using a small, clearly owned implementat
 Completion checks: A command writes substantially more than pipe capacity to stderr and then emits stdout; it finishes promptly and stdout is correct. Check simultaneous large stdout/stderr, normal empty output, and nonzero exit diagnostics. Add focused regression tests with an outer deadline so a failure cannot hang the test suite. Coordinate timeout ownership with the separate timeout task.
 
 Design constraint: Follow README.md: keep source-based configuration, no dependencies or plugin framework, and use the smallest readable change that makes the existing behavior reliable. This is a suggestion for individual review, not a deadline. Findings were recorded on 2026-10-14; re-check the current code before implementing.
+
+Implemented 2026-10-15: the blocking caller drains stdout while one GCD worker
+reads and forwards stderr, including on success. A DispatchGroup joins the
+worker before returning. Existing timeout behavior is unchanged.
+
+Validation: `swift test` passes all 13 tests. Regression cases cover 1 MiB
+stderr before stdout, simultaneous 1 MiB streams, empty output, nonzero exit
+diagnostics, and shell environment/whitespace preservation. Each uses a
+five-second outer wait so a stalled Runner fails the test promptly.
diff --git a/tasks/20261014-191209/TASK.md b/tasks/20261014-191209/TASK.md
index 2509308..bd127bc 100644
--- a/tasks/20261014-191209/TASK.md
+++ b/tasks/20261014-191209/TASK.md
@@ -13,3 +13,9 @@ Approach: First reproduce a SIGTERM-ignoring command and a shell with a descenda
 Completion checks: Normal commands complete without later signals. Both reproductions return ERROR within timeout plus documented grace, release readers, and leave no owned child running. Allow a short timeout in tests without adding a user-facing configuration system. Review together with concurrent pipe draining.
 
 Design constraint: Follow README.md: keep source-based configuration, no dependencies or plugin framework, and use the smallest readable change that makes the existing behavior reliable. This is a suggestion for individual review, not a deadline. Findings were recorded on 2026-10-14; re-check the current code before implementing.
+
+Integration note (2026-10-15): task 20261014-190252 now drains stderr on one GCD
+worker while the caller drains stdout, and joins the worker with a DispatchGroup.
+Runner still owns the single termination timer and cancels it after both readers
+and process waiting finish. Bounded cleanup must release both readers; the
+concurrent-draining change does not alter the existing timeout guarantees.