Two latent defects found while reviewing the fix for #317 (branch fix/watch-startup-race). Neither causes a failure today; both are traps for future test authors.
Correction (2026-08-26): line references below are updated to reflect the state after PR #323 merged.
1. Complete stderr capture is no longer guaranteed
The tap implementation moved to crates/mds-cli/tests/common/mod.rs. bytes() at :63-65 is still a non-blocking clone, and the drain thread's JoinHandle is discarded at spawn (:93) — so no caller can join it. There is no happens-before guarantee that the drain thread has finished appending to the buffer before a snapshot is taken. A test asserting on late-arriving stderr content could read a truncated buffer and fail intermittently.
Real analogues of the old cli_watch.rs:515 / :3372 refs are cli_watch.rs:582-585 and :3463-3465. PR #323 widened exposure to 21 stderr_tap.bytes() call sites.
Fix: join the drain thread before snapshotting, or have bytes() join internally before returning the slice.
2. Latent stdout deadlock in spawn_watch_ready
The test is at cli_watch.rs:1355 (not :1278). The helper blocks waiting for the readiness marker at :1361, and the stdout drain thread only starts afterwards at :1379. Today the payload is ~18 bytes, well under the pipe buffer, so nothing blocks.
Important: readiness now travels over a temp file named by MDS_TEST_READY (common/mod.rs:136-165) — not over stderr. The hazard remains reachable because startup compile_and_write is at watch.rs:1158 and emit_ready_marker() at :1326: a future test with a large stdout payload would block the child before it writes the ready marker, deadlocking until the helper's timeout fires.
Fix: start draining stdout before blocking on the readiness marker, so stdout never fills the pipe buffer regardless of payload size.
Notes
Two latent defects found while reviewing the fix for #317 (branch
fix/watch-startup-race). Neither causes a failure today; both are traps for future test authors.Correction (2026-08-26): line references below are updated to reflect the state after PR #323 merged.
1. Complete stderr capture is no longer guaranteed
The tap implementation moved to
crates/mds-cli/tests/common/mod.rs.bytes()at:63-65is still a non-blocking clone, and the drain thread'sJoinHandleis discarded at spawn (:93) — so no caller can join it. There is no happens-before guarantee that the drain thread has finished appending to the buffer before a snapshot is taken. A test asserting on late-arriving stderr content could read a truncated buffer and fail intermittently.Real analogues of the old
cli_watch.rs:515/:3372refs arecli_watch.rs:582-585and:3463-3465. PR #323 widened exposure to 21stderr_tap.bytes()call sites.Fix: join the drain thread before snapshotting, or have
bytes()join internally before returning the slice.2. Latent stdout deadlock in
spawn_watch_readyThe test is at
cli_watch.rs:1355(not:1278). The helper blocks waiting for the readiness marker at:1361, and the stdout drain thread only starts afterwards at:1379. Today the payload is ~18 bytes, well under the pipe buffer, so nothing blocks.Important: readiness now travels over a temp file named by
MDS_TEST_READY(common/mod.rs:136-165) — not over stderr. The hazard remains reachable because startupcompile_and_writeis atwatch.rs:1158andemit_ready_marker()at:1326: a future test with a large stdout payload would block the child before it writes the ready marker, deadlocking until the helper's timeout fires.Fix: start draining stdout before blocking on the readiness marker, so stdout never fills the pipe buffer regardless of payload size.
Notes