From 04a005e1cd820e3525616694e62dcf017adeb6b9 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 18:43:02 +1000 Subject: [PATCH] F4: real-sink discovery fallback closes the cold-boot race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex flagged a real-but-rare race in `try_bind_default_metadata`: the daemon installs the metadata listener then immediately writes `default.audio.sink = headroom-processed`, relying on PipeWire to deliver the prior value to the listener before our write. In practice this works (pw-metadata replays current state to a freshly-installed listener), but two failure modes leak through: 1. **Prior daemon left the world dirty.** If a previous daemon run set default to `headroom-processed` and didn't restore before exiting, the listener replays "headroom-processed" — `on_metadata_property` recognises that as our own promotion and returns early. `real_sink.name` is never captured. Bypass routes log "no real sink known" forever. 2. **No replay event.** If the listener doesn't fire for any reason — broken PipeWire, an event-bus hiccup, pipewire-rs's `add_listener_local` racing with our write — same outcome. Fix: instead of trying to win the listener race (pipewire-rs has no synchronous metadata getter, and `Core::sync` needs an async done-callback we don't have plumbing for), make `try_capture_real_sink` self-heal. When `real_sink.name` is still `None` and we see *any* non-processed `Audio/Sink` on the registry, adopt it as the fallback real sink. A subsequent `default.audio.sink` event will refine the choice via the existing `adopt_new_real_sink` path if WP picks something else. This is a belt-and-braces patch — the listener path stays the primary mechanism, the fallback only kicks in when that path hasn't produced a name. In steady-state (the common case where listener replay works) it changes nothing. Verified Cold start with PipeWire's `default.audio.sink` set to the Mbox: daemon logs `preferred_real_sink updated sink=Mbox` via the listener path; the fallback's `adopting first available Audio/Sink as fallback` log does not fire. No regression for the steady state. 188 tests pass; clippy clean at -D warnings --all-targets. --- crates/headroom-core/src/pw/registry.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/headroom-core/src/pw/registry.rs b/crates/headroom-core/src/pw/registry.rs index ecc19e2..32631e8 100644 --- a/crates/headroom-core/src/pw/registry.rs +++ b/crates/headroom-core/src/pw/registry.rs @@ -496,6 +496,17 @@ impl RoutingState { /// `sinks_by_name`. If the captured name matches the active /// `preferred_real_sink`, populate `real_sink.node_id` so `status` /// reports it and downstream ops can route by id. + /// + /// **Cold-boot fallback (F4).** If we haven't captured a real-sink + /// name yet — either because the metadata listener's initial + /// replay hasn't fired, or because `default.audio.sink` was + /// already `headroom-processed` from a prior daemon run that + /// didn't clean up — adopt the first non-processed Audio/Sink we + /// see as the real sink. This avoids the failure mode Codex + /// flagged where `real_sink.name = None` indefinitely and bypass + /// routes log "no real sink known" forever. A subsequent + /// `default.audio.sink` event will refine the choice via + /// `adopt_new_real_sink` if the user/WP picks a different one. fn try_capture_real_sink(&mut self, global: &GlobalObject<&DictRef>) { let Some(props) = &global.props else { return }; let dict: &DictRef = props; @@ -510,7 +521,15 @@ impl RoutingState { } self.sinks_by_name.insert(name.to_owned(), global.id); let mut s = self.daemon.lock(); - if s.real_sink.name.as_deref() == Some(name) && s.real_sink.node_id != Some(global.id) { + if s.real_sink.name.is_none() { + tracing::info!( + node_id = global.id, + name, + "no preferred_real_sink yet; adopting first available Audio/Sink as fallback" + ); + s.real_sink.name = Some(name.to_owned()); + s.real_sink.node_id = Some(global.id); + } else if s.real_sink.name.as_deref() == Some(name) && s.real_sink.node_id != Some(global.id) { tracing::info!( node_id = global.id, name,