Windows: pet_watch output_failures_do_not_stop_the_world asserts sink.failed() before the audio path records it

[Hmbown]

Test (windows-latest) fails on v0914-chunk2 (PR #6175), one test out of 15271:

FAIL [0.849s] (12432/15271) codewhale-tui tui::pet_watch::worker::tests::output_failures_do_not_stop_the_world
  panicked at crates\tui\src\tui\pet_watch\worker.rs:412:9:
  assertion failed: sink.failed()
Summary [624.861s] 15271 tests run: 15270 passed (1 leaky), 1 failed, 20 skipped

Passes on ubuntu and macOS. Windows only.

What the test does

let worker = Worker::start(None).unwrap();
let (sink, packets) = audio::Output::capture();
drop(packets);                                       // no receiver left
frame_with_audio(&worker, 0.0, Some(sink.target()));
let before = frame_with_audio(&worker, 400.0, Some(sink.target()));
assert!(sink.failed());                              // <-- fails here on Windows

Why it is probably ordering, not correctness

frame_with_audio sends Command::Advance and then polls worker.latest until a raster appears, with a 10-second bound. So it synchronises on the raster being published — but sink.failed() is a flag on the audio path. A published raster does not, on the face of it, guarantee that the audio send has been attempted and its failure recorded.

If that is right, the test assumes an ordering that holds incidentally on unix thread scheduling and not on Windows, and the fix is to poll sink.failed() with a bounded wait — the same idiom this file already uses for the raster — which would not weaken the assertion.

But that is exactly the assumption someone who owns this code should check rather than me. If the audio failure is supposed to be recorded by the time the raster is published, then Windows is exposing a real defect in the audio path and adding a wait would mask it. I do not know the delivery semantics well enough to tell those apart, so I have not touched it.

Context

This is the second Windows/platform issue in the pet slice tonight; portable (Pet conformance) is also red on the same branch, on a TypeScript↔Rust frame-hash parity mismatch, and has been since a06d2296. There is also uncommitted pet work (pet_sim.rs, pet_widget.rs, pet_cameo.rs) sitting in the codewhale checkout that may already change this code, which is a further reason not to patch it from outside.

Not a blocker for #6175 per the founder's call on the pet slice, but main's Windows CI will be red until this is resolved.

Last touched by c5ea6992cf and 5c70db41f0.

[Hmbown]

Root-caused and fixed in be6b7fe8dc. It is not flakiness — there is a wall clock in the code under test.

Target::send deliberately drops stale audio:

fn current(&self) -> bool { self.requested.elapsed() <= Duration::from_millis(500) }
...
if !self.current() { return Ok(()); }      // returns early, never reaches try_send

and failed is only ever set when try_send observes Disconnected. Since sink.target() stamps requested at call time, the test was implicitly requiring the worker to consume that target within 500 ms. On a slower Windows runner it does not: send takes the stale path, returns Ok(()), no failure is recorded, and the precondition assert fires. Unix hosts win that race consistently, which is why it was green everywhere else.

So my earlier guess in this issue — that it was an ordering race between the raster and the audio flag — was wrong. It is a freshness deadline.

The staleness guard is correct product behaviour. Audio requested half a second ago should not play, so audio.rs is not what needed changing.

The fix is to the precondition only. The real assertion in this test is at the end — after.time_ms > before.time_ms and after.hollow, i.e. output failures do not stop the world. sink.failed() merely establishes that a failure happened at all. That now runs through the same code path with the wall clock removed: a fresh target, a live active(), and a try_send that observes Disconnected because packets was dropped.

Deliberately not weakened — a receiver that failed to disconnect still fails the test — and nothing downstream changed. One assertion plus a comment; no change to audio.rs or to the worker itself, since this is not my subsystem.

Checks: ./scripts/dev-test.sh tui pet_watch — 16 tests run, 16 passed. CI clippy gate exits 0, cargo fmt --check clean. The Windows half rests on the mechanism above and on CI, as this host cannot build for x86_64-pc-windows-msvc.

Worth a look from whoever owns the pet slice: if a target can miss its 500 ms window on a slow host in a test, it can miss it on a loaded machine in production too, and the symptom there is silent — audio simply never plays and nothing is recorded as failed. That is a different question from this test and I have not touched it.

portable (Pet conformance) is still red on the TypeScript↔Rust frame-hash parity mismatch. That one is untouched and still needs an owner.

[Hmbown]

Fixed in be6b7fe8dc (on main at b367f62).

The root cause turned out not to be an assertion-ordering race but a 500 ms freshness window in Target::send: on a host slow enough to miss that window the failure was never recorded, which is exactly why this was red on Windows and green everywhere else.

The precondition now provokes the Disconnected try_send directly — crates/tui/src/tui/pet_watch/worker.rs:420 — so the wall clock is gone from the test path while assert!(sink.failed()) is retained rather than weakened.

Test (windows-latest) is green on b367f62 (run 35624606511). Reopen if it flakes again.