From 0e718abe274d49856200124f3f81995176230554 Mon Sep 17 00:00:00 2001 From: atagen Date: Thu, 21 May 2026 18:40:02 +1000 Subject: [PATCH] F2: reapply routing on profile / rule changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex flagged that `profile use`, `profile reload`, `route set`, and `route unset` updated overlay state and (sometimes) propagated DSP configs but never asked the registry thread to re-route existing streams. The new policy only applied to *future* connections; anything already routed kept its old explicit links until the app disconnected. The plumbing was actually already in place from F1 — the bypass toggle posted `PwCommand::ReevaluateAll`, the registry handled it, and `reevaluate_all` iterated the `known_streams` cache. This commit is just the missing call sites: a `post_reevaluate(state)` helper that reads `state.pw_command_tx` and sends `ReevaluateAll`, called after each of the four mutating IPC ops. `execute_reload` (which the profile-watcher also calls) gets the post too, so editing a TOML on disk now re-routes live streams. Tests All 188 still pass; clippy clean. Live verification Sine flowing through `headroom-processed` while the daemon is on the `layer-a-test` profile (default_route = processed): - `headroom profile use bypass-all` → pw-cat's explicit link flips from processed → Mbox within ~50 ms (one drain tick). - `headroom profile use layer-a-test` → flips back to processed. - Layer A tap link survives both transitions (orthogonal, unaffected by bus rerouting — same invariant as F1). Adjacent issue noted (not in F2 scope) `headroom route set ` only writes the rule's `process_binary` field. Streams that don't advertise `application.process.binary` (pw-cat is one) can't be matched by this single-field rule even though they have an `application.name`. The fix is either to widen `route.set` into a smarter "match by app label" verb (which would either need a new OR-across-fields matcher kind or a CLI flag to pick which field) or to teach the materialiser to produce both process_binary AND application_name rules with the same name, with the matcher then OR'd. Either way it's a separate UX bug; filed as a follow-up. --- crates/headroom-core/src/ipc/ops.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/headroom-core/src/ipc/ops.rs b/crates/headroom-core/src/ipc/ops.rs index 1230e43..3b480b0 100644 --- a/crates/headroom-core/src/ipc/ops.rs +++ b/crates/headroom-core/src/ipc/ops.rs @@ -193,6 +193,7 @@ fn profile_use(id: u64, name: &str, state: &SharedState) -> Response { publish_profile_changed(&mut s, name); let control = s.filter_control.clone(); let snap = build_dsp_configs(&s); + post_reevaluate(&s); drop(s); push_dsp_update(control.as_ref(), snap); ok(id, &json!({ "name": name })) @@ -234,6 +235,7 @@ pub(crate) fn execute_reload( publish_profile_reloaded(&mut s, &report.loaded); let control = s.filter_control.clone(); let snap = build_dsp_configs(&s); + post_reevaluate(&s); drop(s); push_dsp_update(control.as_ref(), snap); Ok(report) @@ -245,6 +247,7 @@ fn route_set(id: u64, app: &str, to: Route, state: &SharedState) -> Response { Ok(()) => { tracing::info!(app, ?to, "route.set applied"); publish_rule_changed(&mut s); + post_reevaluate(&s); drop(s); ok(id, &Value::Null) } @@ -258,6 +261,7 @@ fn route_unset(id: u64, app: &str, state: &SharedState) -> Response { Ok(()) => { tracing::info!(app, "route.unset applied"); publish_rule_changed(&mut s); + post_reevaluate(&s); drop(s); ok(id, &Value::Null) } @@ -271,6 +275,25 @@ fn publish_rule_changed(state: &mut crate::state::DaemonState) { } } +/// Ask the PipeWire main loop to re-run `routing::evaluate` against +/// every known stream. Called after any IPC mutation that changes +/// the inputs to that decision: active profile, profile contents +/// reloaded from disk, or a `route.set` / `route.unset` overlay +/// edit. Without this, the new policy only applies to *future* +/// streams; everything already routed keeps its old links until the +/// app reconnects. A stale or duplicate post is harmless — the +/// handler reads current state at apply time and is idempotent +/// when nothing changed. +fn post_reevaluate(state: &crate::state::DaemonState) { + let Some(tx) = state.pw_command_tx.as_ref() else { + tracing::debug!("no PipeWire command channel; reevaluation skipped (test mode)"); + return; + }; + if tx.send(PwCommand::ReevaluateAll).is_err() { + tracing::warn!("PipeWire command channel closed; reevaluation lost"); + } +} + fn publish_profile_changed(state: &mut crate::state::DaemonState, name: &str) { if let Ok(event) = Event::new(Topic::Profile, "used", &json!({ "name": name })) { state.broadcaster.publish(Topic::Profile, event);