test(picker): release --prs loading marker on parent exit, not a timer

[worktrunk-bot]

Problem

test_switch_picker_prs_shows_loading_marker intermittently times out on test (windows) — it flaked on #3531 (a docs-only change), which is what prompted this fix.

The test asserts a transient frame: while a mocked, delayed gh pr list is in flight, the picker header shows ↳ Loading open PRs… and the #42 row has not yet streamed in. The mock held that loading state for a fixed time (originally 3s). On a loaded Windows runner, the PTY boot sequence — boot_picker_pty's skim-ready wait plus its initial wait_for_stable, together with the async worktree-column churn (· placeholders resolving) — can consume that whole window. By the time the stabilization helper first polls for Loading open PRs, the mocked fetch has already returned and the rows have streamed in, so the marker is gone and the wait times out at STABILIZE_TIMEOUT (30s).

Fix

Per review feedback (comment), tie the marker's lifetime to the picker's, with no timing number at all — the file's "poll, don't pick a time" rule applied to the producer side.

mock-stub gains a hold_until_parent_exit response mode: the mocked gh pr list holds until wt exits instead of sleeping a fixed delay_ms. Detection needs no platform primitive (getppid / OpenProcess):

• While wt lives, its --prs fetch thread runs the mock with a piped stdout and blocks reading that pipe to EOF, so as long as the mock neither writes a full response nor exits, the loading marker stays.
• When the picker aborts, wt detaches the fetch thread (drop(prs_handle)) and exits, which closes the read end of the mock's stdout pipe. The mock's next write then fails with BrokenPipe (Rust ignores SIGPIPE, so the broken write surfaces as an Err rather than killing the process), and the mock exits.

Polling for that write error is a causal parent-death signal — release is tied to wt exiting, not to a timer — and it behaves identically on Unix and Windows, so there's no platform-specific FFI to get right. The one-byte probes written into stdout are never observed (this mode's contract is that the parent aborts without parsing the response, and the fetch thread drains the pipe until process exit). A 60s poll cap bounds a stuck orphan if the pipe somehow never breaks.

This removes the race the earlier 30s bump only widened: the marker is present for the entire window the poll could observe it, on any boot latency, and no orphaned sleeper is left behind — the mock exits the moment the picker does.

Testing

cargo test --test integration --features shell-integration-tests test_switch_picker_prs_shows_loading_marker passes in ~1.3s, confirming the capture-and-abort path stays fast and the mock releases promptly on wt exit. The sibling --prs tests (test_switch_picker_prs_{github,gitlab}_list, ..._rows_survive_alt_x_removal) still pass through the refactored mock_forge_env helper. Reasoning is documented inline at the mock-stub call site and in wait_for_parent_exit.

[max-sixty]

can we poll here? I thought we always polled rather than picked a specific time, or at least have this guidance in our skills?

[worktrunk-bot]

Good catch — you're right to hold this against the "poll, don't pick a time" rule in tests/CLAUDE.md ("producer sleeps to line up with a deadline … every extension just makes the race wider, not correct"). Let me split what's already polling from what isn't:

The test does poll. exec_in_pty_capture_before_abort gates on Some("Loading open PRs") via send_input_awaiting_content → wait_for_stable_until, so it captures the frame the instant the marker appears and aborts — it never waits out the delay. The 30s isn't a fixed test-side sleep; it's how long the mocked gh pr list holds the transient state on screen.

What's still time-based is exactly what you're flagging: the mock's hold has to outlast worst-case picker boot, or the marker clears before the poll starts (that's the 3s Windows flake). Bumping to 30s makes that margin huge — boot is seconds, and 30s matches the file's STABILIZE_TIMEOUT ceiling, so the poll effectively can't outlive the hold — but it's still a picked number vs. boot latency, not a structural guarantee. So: defensible, not race-free.

The structurally race-free version is to make the marker live exactly as long as the picker does, with no number at all: teach mock-stub to hold the response until its parent wt exits (parent-death poll) instead of sleeping a fixed delay_ms. Then abort → wt exits → mock releases; the marker is present for the entire lifetime the poll could observe, on any boot latency, and it also drops the orphaned 30s sleeper this leaves behind.

The cost is that mock-stub is currently dependency-free, so this adds a small platform primitive (getppid on Unix, a parent-handle wait on Windows) — and the Windows path is the one that flakes, so I can't exercise it from Linux CI without care. Two ways forward:

1. I implement the hold-until-parent-exits mode in mock-stub and switch this test to it (the real fix your question points at), or
2. keep the bounded 30s hold and I add a short note to tests/CLAUDE.md documenting this specific transient-frame-mock pattern and why the hold must exceed worst-case boot (your "at least have this guidance in our skills").

I lean toward (1) since it's the honest answer to "can we poll here"; happy to do (2) if you'd rather not grow mock-stub's surface. Which do you prefer?

[max-sixty]

let's try it

[worktrunk-bot]

Done — went with option (1), the parent-death poll, in 568163f.

mock-stub gained a hold_until_parent_exit mode: the mocked gh pr list holds until wt exits instead of sleeping a fixed delay_ms, so the Loading open PRs… marker lives exactly as long as the picker does. No timing number, and it drops the orphaned sleeper the 30s hold left behind.

The one thing I changed from what I floated above: no getppid/OpenProcess platform primitive. While wt lives, its --prs fetch thread blocks reading the mock's piped stdout to EOF; when the picker aborts (drop(prs_handle)) and wt exits, the pipe's read end closes and the mock's next write returns BrokenPipe (Rust ignores SIGPIPE, so it's an Err, not a kill). Polling that write error is the same causal parent-death signal, but it's pure std and behaves identically on Unix and Windows — so there's no platform-specific FFI to get right, and the Windows leg that was flaking is exercised by the same code the Linux leg runs. Windows CI on this push is the real confirmation.

Test finishes in ~1.3s; the three sibling --prs tests still pass through the refactored mock_forge_env. Updated the PR title/description to match the new approach.