Fix macOS command output truncated mid-table by draining PTY on child exit - #14635
Fix macOS command output truncated mid-table by draining PTY on child exit#14635warp-agent-staging[bot] wants to merge 1 commit into
Conversation
…output On macOS, a command that prints a large table in a fast burst is sometimes cut off mid-table when run in a Warp dev build launched via `script/run`. Root cause: the PTY reader event loop breaks out of its loop the instant it observes the child process has exited (both the SIGCHLD `child_event_token` path and the Windows `Message::ChildExited` channel path), without first draining output still buffered in the PTY. When the child writes a large final burst and exits quickly, the exit notification can win the race against reading that buffer, so the tail of the output is never read/parsed and is dropped. This is intermittent and especially visible on macOS due to SIGCHLD delivery ordering. Fix: add `EventLoop::drain_pty_after_exit`, which loops `pty_read` until the PTY reports it can no longer be read (WouldBlock/EOF), and call it before `terminal.exit()` at both child-exit sites. The PTY leader is non-blocking so this cannot block; `DRAIN_MAX_READS` bounds it as a safety net. Adds a regression test (event_loop_tests.rs) with a mock EventedPty holding a burst larger than MAX_LOCKED_READ, asserting the drain reads all buffered bytes, plus a control test showing a single pty_read stops early. CHANGELOG-BUG-FIX: Fixed command output (e.g. large tables) sometimes being cut off mid-print on macOS. Co-Authored-By: Oz <oz-agent@warp.dev> Co-Authored-By: Warp <agent@warp.dev>
![warp-agent-staging[bot]](https://avatars.githubusercontent.com/in/2196476?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR fixes intermittent mid-table truncation on macOS by draining output still buffered in the PTY after the child exits — at both child-exit sites, before the terminal is marked exited. The PTY-layer change and its regression test are correct; the only blocker is that the user-facing behavior in APP-5099's acceptance criteria has not been confirmed on the affected platform.
Concerns
This is a user-facing change (the observable effect is complete vs. truncated terminal output), and APP-5099's acceptance criteria explicitly require manual macOS confirmation of a full table across repeated large/fast bursts, with no Linux/Windows regression. That confirmation is outstanding: the factory run executed on Linux, where ./script/run uses cargo run rather than the macOS .app foreground-launch path, so the affected path could not be exercised here — the author disclosed this correctly. Before merge, a human on macOS should run the ticket's repro (a large/fast table burst such as docker ps / ps aux, repeated) and confirm no mid-table truncation and no regression on Linux/Windows.
Minor, optional (non-blocking): drain_pty_after_exit silently stops if the DRAIN_MAX_READS (~32 MiB) bound is ever reached — a debug!/warn! there would aid future diagnosis; and the regression test exercises the drain method directly but not that the two exit sites actually invoke it (left to code inspection). Neither affects correctness.
Verdict
Checks: build ✅ (CI release-compilation Linux/macOS/wasm green) · tests ⏳ (CI test jobs pending, none red; local cargo nextest -p warp could not run — the warp crate compile is OOM-killed under this sandbox's memory limit) · CI ✅ so far (no failures) · visual proof ❌ (macOS path not exercisable here — outstanding for the human reviewer)
Found: 0 critical, 0 important, 0 suggestions (1 human verification gate)
Request changes
Review run
https://oz.staging.warp.dev/runs/019fc30b-2747-78c6-a3c4-7129b90856fc
1 participant
Description
On macOS, a command that prints a large table (e.g.
docker ps,kubectl get pods -A,ps aux) run inside a Warp dev build launched via./script/runis sometimes cut off mid-table. This is a longstanding, intermittent, macOS-specific defect.Root cause
The PTY reader event loop (
app/src/terminal/local_tty/event_loop.rs) breaks out of its loop the instant it observes the child process has exited — at both the SIGCHLDchild_event_tokenpath and the WindowsMessage::ChildExitedchannel path — without first draining output still buffered in the PTY.When a command writes a large final burst and then exits quickly, the child-exit notification can win the race against reading that buffered output. The loop tears down and the tail of the output is never read or parsed, so it is dropped — the table is cut off part-way through. It is intermittent and especially visible on macOS because
SIGCHLDdelivery ordering there frequently beats draining the PTY buffer on a fast, large burst.Fix
Add
EventLoop::drain_pty_after_exit, which loopspty_readuntil the PTY reports it can no longer be read (WouldBlock/EOF), and call it immediately beforeterminal.exit()at both child-exit sites. The PTY leader is non-blocking, so this cannot block; aDRAIN_MAX_READSbound is added as a safety net. This guarantees the complete output is read and rendered before the terminal is marked exited.Linked Issue
APP-5099
Originating thread: https://warpdev.slack.com/archives/C0BDQDW8V5E/p1785681506675889
Testing
app/src/terminal/local_tty/event_loop_tests.rs:drain_pty_after_exit_reads_all_buffered_output— a mockEventedPtyholds an output burst larger thanMAX_LOCKED_READ; asserts the drain reads every buffered byte on child exit. Verified this fails before the fix (0 of 262,152 bytes drained) and passes after.single_pty_read_stops_before_draining_large_burst— control test showing a singlepty_readintentionally stops early (afterMAX_LOCKED_READ), demonstrating why the drain loop is required.cargo nextest run -p warp <both tests>— pass../script/format— clean.cargo clippy -p warp --all-targets --tests -- -D warnings— clean../script/check_no_inline_test_modules— pass.Screenshots / Videos
Not included. The defect is macOS-specific and intermittent, and this factory run executes on Linux (where
./script/runusescargo runrather than the macOS.app-bundle + foreground-launch path), so the running UI could not be exercised to reproduce it here. Verification is via the deterministic regression test above, which exercises the exact affected code path (PTY drain on child exit). Visual confirmation on macOS with the repro in the ticket remains outstanding for the reviewer.Agent Mode
Conversation: https://staging.warp.dev/conversation/06cd1095-c452-4eec-84ca-ae8a7ddf6c94
Run: https://oz.staging.warp.dev/runs/019fc2f0-9d51-7993-b41b-061df79e637f
This PR was generated with Oz.