fix(mcp): read pipe data before waitUntilExit to prevent deadlock

[SebTardif]

What Problem This Solves

ShellTool.execute() calls process.waitUntilExit() before reading from the stdout/stderr pipes. When a child command writes more than the kernel pipe buffer (~64 KB on macOS), write() blocks until someone drains the pipe. Because the parent is stuck in waitUntilExit(), nobody drains, and both parent and child block indefinitely.

Any shell command producing moderate output (a long ls, cat of a config file, build logs) can trigger this.

Call chain

ShellTool.execute()
  -> Process.run()
  -> process.waitUntilExit()     // blocks waiting for child exit
  -> outputPipe...readDataToEndOfFile()  // never reached

The child is blocked in write() waiting for the pipe to drain, while the parent is blocked in waitUntilExit() waiting for the child to exit.

Bug origin

Introduced in 95ecdd4 (2025-11-14) in the initial PeekabooAgentRuntime module creation. Present for ~8 months.

Evidence

Fix

Move readDataToEndOfFile() calls before process.waitUntilExit(). This is the standard POSIX pattern: drain the pipe first, then wait for the child to exit.

 do {
     try process.run()
-    process.waitUntilExit()
-
+    // Read pipe data BEFORE waitUntilExit to avoid deadlock.
     let outputData = outputPipe.fileHandleForReading.readDataToEndOfFile()
     let errorData = errorPipe.fileHandleForReading.readDataToEndOfFile()
+    process.waitUntilExit()

Red-green verification

Red step (without fix): timeout 30 swift test --filter "Shell tool does not deadlock" -> exit code 124 (killed by timeout; test deadlocked as expected).

Green step (with fix): same test -> passed in 0.077 seconds.

Regression test

The test generates 128 KB of output (head -c 131072 /dev/zero | base64), exceeding the ~64 KB pipe buffer. Without the fix, the test deadlocks. With the fix, it completes in under a second.

Prior art

The same Process pipe buffer deadlock pattern has been fixed in other Swift projects:

• NSEvent/xbox-controller-mapper#83
• diogot/Xproject#49

Python's subprocess documentation explicitly warns against this pattern: "This will deadlock when using stdout=PIPE or stderr=PIPE and the child process generates enough output to a pipe such that it blocks waiting for the OS pipe buffer to accept more data."

Real behavior proof

Command tested: head -c 131072 /dev/zero | base64 via ShellTool MCP interface

Environment: macOS, Swift Testing framework, PeekabooCore test suite

Proof type: Red-green regression test

Red (without fix): Test deadlocks; killed by 30-second timeout (exit 124)

Green (with fix): Test passes in 0.077 seconds; output size >100,000 characters verified

Reproducibility: Deterministic. Any command producing >64 KB output triggers the deadlock on the unfixed code.

[clawsweeper]

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(mcp): read pipe data before waitUntilExit to prevent deadlock This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

[steipete]

Maintainer proof on exact head ff2bffa5763a92b448a492e5960e4ee4a3245007 (base 88606409bc0ed14259400aa33cbfe5afdfc0047b).

Verdict: ready to land. The reported deadlock is real, but the original patch only fixed the stdout-only case: reading stdout and stderr sequentially can still deadlock when stderr fills while stdout remains open. I pushed ff2bffa5 to drain both pipes concurrently and added a dual-pipe regression, preserving Sebastien's original commit and authorship.

Red/green evidence:

• Original waitUntilExit() ordering plus the submitted large-stdout test: build completed in 18.68s, then the test hung until the 30.035s process-group timeout (exit 124).
• Submitted head 8eba94f7: large stdout passed in 0.073s, but a command writing 128 KiB to stderr before 128 KiB to stdout hung until the 30.017s timeout (exit 124).
• Repaired head: both large-output regressions passed in 0.015s total.

Exact-head validation:

• swift test --package-path Core/PeekabooCore --filter MCP --no-parallel: 157 tests across 23 suites passed.
• pnpm run test:safe: release-policy/artifact checks passed; 755 Swift tests passed, with one expected live-daemon skip.
• pnpm run format: zero files changed.
• pnpm run lint: zero serious violations (13 pre-existing warnings).
• Full-branch autoreview against origin/main: clean, no accepted/actionable findings (0.97 confidence).
• Hosted macOS CI run 29977478168: PeekabooCore, CLI, Tachikoma, macOS apps, and SwiftLint all green on ff2bffa5.

Provenance is clear: the blocking order was introduced with the original ShellTool in 95ecdd423 on 2025-11-14. No broader refactor is needed for this fix.