diff --git a/src/app/mod.rs b/src/app/mod.rs index 04a55af..1a383de 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -589,10 +589,21 @@ pub struct AppState { /// `ScreenShareStarted`, `ScreenShareStopped`, or any `Error`. share_starting: bool, /// While sharing a specific app's audio (A23 strict mode): `true` when that - /// app's audio has stopped, so viewers currently hear silence. Drives a - /// transient warning. Always `false` for whole-desktop shares (pixelpass - /// emits no `app_audio` events then) and when not sharing. + /// app's audio has stopped (or hasn't started yet), so viewers currently hear + /// silence. Drives a transient warning. Always `false` for whole-desktop + /// shares (pixelpass emits no `app_audio` events then) and when not sharing. share_audio_dropped: bool, + /// Whether the current share is a specific-app capture (vs whole-desktop). + /// Set from the confirmed selection on `ScreenShareStarted`, cleared on + /// stop/reset. Gates applying `ShareAudioActive`, so a late event from a + /// just-killed host can't flip the warning on a new whole-desktop share or + /// after stop (audit P3, unscoped events). + share_audio_app_active: bool, + /// Whether the resolved pixelpass supports `--strict-audio` (per-app audio). + /// `false` ⇒ the picker offers whole-desktop only, because a per-app share + /// would pass a flag an older pixelpass rejects (audit P2). Optimistic `true` + /// until the core's `AudioAppsListed` reports otherwise. + share_app_audio_supported: bool, /// Whether the Chat drawer is open (drawer layout only). drawer_chat_open: bool, /// Raw mic level (normalized RMS, `0.0..=1.0`) for the settings meter. @@ -667,6 +678,8 @@ impl AppState { self.share_audio_selection = None; self.share_starting = false; self.share_audio_dropped = false; + self.share_audio_app_active = false; + self.share_app_audio_supported = true; } fn custom_sound_path(&self, sound: Sound) -> &str { @@ -793,6 +806,8 @@ impl Default for AppState { share_audio_selection: None, share_starting: false, share_audio_dropped: false, + share_audio_app_active: false, + share_app_audio_supported: true, drawer_chat_open: false, mic_level: 0.0, mic_test_active: false, @@ -1365,24 +1380,35 @@ fn update(state: &mut AppState, message: AppMessage) -> Task { state.attachments.insert(key, AttachmentState::Failed(error.clone()), None); state.status_message = format!("Attachment failed: {error}"); } - UiEvent::AudioAppsListed(apps) => { + UiEvent::AudioAppsListed { apps, app_audio_supported } => { // Only meaningful while the picker is open; if the user - // already cancelled, drop it. Keep the current selection if it - // still exists in the refreshed list, else fall back to "All - // system audio". + // already cancelled, drop it. if state.share_picker_open { - if let Some(sel) = &state.share_audio_selection - && !apps.iter().any(|a| a == sel) - { + state.share_app_audio_supported = app_audio_supported; + if app_audio_supported { + // Keep the current selection if it still exists in the + // refreshed list, else fall back to "All system audio". + if let Some(sel) = &state.share_audio_selection + && !apps.iter().any(|a| a == sel) + { + state.share_audio_selection = None; + } + state.share_audio_apps = apps; + } else { + // Older pixelpass: per-app capture would hard-fail + // (--strict-audio unknown). Force whole-desktop only. + state.share_audio_apps.clear(); state.share_audio_selection = None; } - state.share_audio_apps = apps; } } UiEvent::ScreenShareStarted => { state.self_sharing = true; state.share_starting = false; state.share_audio_dropped = false; + // Remember whether this share captures a specific app, so we + // only apply `app_audio` warnings to app shares (P3). + state.share_audio_app_active = state.share_audio_selection.is_some(); // Defensive: ensure no picker lingers across a successful start. state.share_picker_open = false; state.status_message = "Sharing your screen".to_string(); @@ -1391,11 +1417,17 @@ fn update(state: &mut AppState, message: AppMessage) -> Task { state.self_sharing = false; state.share_starting = false; state.share_audio_dropped = false; + state.share_audio_app_active = false; state.status_message = "Screen share stopped".to_string(); } UiEvent::ShareAudioActive(active) => { - // Per-app audio routed/lost (only while sharing a chosen app). - state.share_audio_dropped = !active; + // Per-app audio routed/lost. Apply only while we're actually + // sharing a specific app: a late event from a just-killed host + // must not flip the warning after stop or on a whole-desktop + // share (audit P3, unscoped events). + if state.self_sharing && state.share_audio_app_active { + state.share_audio_dropped = !active; + } } UiEvent::IdentityStatus { node_id, persisted, error } => { state.self_node_id = Some(node_id); @@ -4098,7 +4130,7 @@ fn view(state: &AppState) -> Element<'_, AppMessage> { column![ badge, text( - "⚠ Shared app's audio stopped — viewers hear silence until it plays again" + "⚠ Shared app isn't sending audio — viewers hear silence until it plays" ) .size(11) .color(color_yellow), @@ -5501,7 +5533,18 @@ fn with_share_picker<'a>( )); } - let list: Element<'_, AppMessage> = if state.share_audio_apps.is_empty() { + let list: Element<'_, AppMessage> = if !state.share_app_audio_supported { + // Older pixelpass without --strict-audio: per-app capture is unavailable + // (it would hard-fail), so only whole-desktop is offered. Nudge to upgrade. + column![ + options, + text("Update pixelpass to capture a single app's audio (avoids echoing the call to viewers).") + .size(11) + .color(yellow), + ] + .spacing(8) + .into() + } else if state.share_audio_apps.is_empty() { column![ options, text("No other apps are playing audio right now.") @@ -6347,6 +6390,8 @@ mod tests { state.share_audio_selection = Some("Firefox".to_string()); state.share_starting = true; state.share_audio_dropped = true; + state.share_audio_app_active = true; + state.share_app_audio_supported = false; state.clip_status.lock().unwrap().playing_id = Some(attachment_id); state.reset_room_state(); @@ -6372,6 +6417,8 @@ mod tests { assert!(state.share_audio_selection.is_none()); assert!(!state.share_starting); assert!(!state.share_audio_dropped); + assert!(!state.share_audio_app_active); + assert!(state.share_app_audio_supported, "reset is optimistic by default"); for _ in 0..50 { if crate::audio::clip_player::status_snapshot(&state.clip_status).playing_id.is_none() { @@ -6444,14 +6491,19 @@ mod tests { fn share_audio_dropped_tracks_app_audio_events() { // While sharing a specific app, app_audio lost/routed toggles the warning // flag; start and stop both reset it so it can't linger across sessions. - let mut state = AppState::default(); + // A specific app was chosen in the picker, so the share is app-specific. + let mut state = AppState { + share_audio_selection: Some("mpv".to_string()), + ..Default::default() + }; - // Start sharing — flag is clear. + // Start sharing — flag is clear, and the share is marked app-specific. let _ = update( &mut state, AppMessage::UiEventReceived(UiEvent::ScreenShareStarted), ); assert!(!state.share_audio_dropped); + assert!(state.share_audio_app_active); // The chosen app's audio stops → warning on. let _ = update( @@ -6478,6 +6530,80 @@ mod tests { AppMessage::UiEventReceived(UiEvent::ScreenShareStopped), ); assert!(!state.share_audio_dropped); + assert!(!state.share_audio_app_active); + } + + #[test] + fn share_audio_active_ignored_unless_app_sharing() { + // P3 (unscoped events): a late app_audio event from a just-killed host + // must not flip the warning when we're not sharing a specific app — + // neither after stop nor on a whole-desktop share. + + // (a) Whole-desktop share (no app selected): events are ignored. + let mut state = AppState::default(); + let _ = update( + &mut state, + AppMessage::UiEventReceived(UiEvent::ScreenShareStarted), + ); + assert!(!state.share_audio_app_active, "no app selected ⇒ not app-specific"); + let _ = update( + &mut state, + AppMessage::UiEventReceived(UiEvent::ShareAudioActive(false)), + ); + assert!(!state.share_audio_dropped, "whole-desktop share ignores app_audio"); + + // (b) After stop: a straggling event can't resurrect the warning. + let mut state = AppState { + share_audio_selection: Some("mpv".to_string()), + ..Default::default() + }; + let _ = update( + &mut state, + AppMessage::UiEventReceived(UiEvent::ScreenShareStarted), + ); + let _ = update( + &mut state, + AppMessage::UiEventReceived(UiEvent::ScreenShareStopped), + ); + let _ = update( + &mut state, + AppMessage::UiEventReceived(UiEvent::ShareAudioActive(false)), + ); + assert!(!state.share_audio_dropped, "post-stop event is ignored"); + } + + #[test] + fn old_pixelpass_picker_offers_whole_desktop_only() { + // P2 (version skew): when the resolved pixelpass lacks --strict-audio, the + // picker must drop all per-app options and force the whole-desktop choice, + // so a per-app share (which would pass the unknown flag) can't be started. + let mut state = AppState { + share_picker_open: true, + share_audio_selection: Some("Firefox".to_string()), + share_audio_apps: vec!["Firefox".to_string(), "mpv".to_string()], + ..Default::default() + }; + let _ = update( + &mut state, + AppMessage::UiEventReceived(UiEvent::AudioAppsListed { + apps: vec!["Firefox".to_string(), "mpv".to_string()], + app_audio_supported: false, + }), + ); + assert!(!state.share_app_audio_supported); + assert!(state.share_audio_apps.is_empty(), "no per-app rows offered"); + assert!(state.share_audio_selection.is_none(), "forced to whole-desktop"); + + // A supported pixelpass keeps the app list and a valid selection. + let _ = update( + &mut state, + AppMessage::UiEventReceived(UiEvent::AudioAppsListed { + apps: vec!["Firefox".to_string(), "mpv".to_string()], + app_audio_supported: true, + }), + ); + assert!(state.share_app_audio_supported); + assert_eq!(state.share_audio_apps.len(), 2); } #[test] diff --git a/src/core/messages.rs b/src/core/messages.rs index 2a920c7..448d963 100644 --- a/src/core/messages.rs +++ b/src/core/messages.rs @@ -149,8 +149,11 @@ pub enum UiEvent { AttachmentFailed { from: EndpointId, id: crate::files::AttachmentId, error: String }, /// The apps currently producing audio, for the screen-share audio picker /// (A23). Sorted, deduplicated `application.name`s; empty when nothing is - /// playing or enumeration isn't available. - AudioAppsListed(Vec), + /// playing or enumeration isn't available. `app_audio_supported` reports + /// whether the resolved pixelpass understands `--strict-audio`: when `false` + /// (an older pixelpass) the picker must offer whole-desktop audio only, since + /// a per-app share would pass a flag that older binary rejects (audit P2). + AudioAppsListed { apps: Vec, app_audio_supported: bool }, /// Our own screen share started; the UI flips the Share button to "Stop". ScreenShareStarted, /// Our own screen share stopped (or failed to start). diff --git a/src/core/mod.rs b/src/core/mod.rs index 2add083..0d254f8 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -2635,8 +2635,25 @@ async fn run_core_loop( } CoreCommand::ListAudioApps => { - let apps = crate::screenshare::list_audio_apps().await; - let _ = ui_tx.send(UiEvent::AudioAppsListed(apps)).await; + // Probe whether this pixelpass supports `--strict-audio` before + // offering per-app capture: an older binary would reject the flag + // and hard-fail the share (audit P2). When unsupported (or + // pixelpass is missing), skip enumeration and let the picker show + // whole-desktop audio only — never a best-effort `--app` that + // would reopen the A23 echo. + let app_audio_supported = + match crate::screenshare::pixelpass_path(pixelpass_override.as_deref()) { + Some(bin) => crate::screenshare::supports_strict_audio(&bin).await, + None => false, + }; + let apps = if app_audio_supported { + crate::screenshare::list_audio_apps().await + } else { + Vec::new() + }; + let _ = ui_tx + .send(UiEvent::AudioAppsListed { apps, app_audio_supported }) + .await; } CoreCommand::StartScreenShare { audio_app } => { diff --git a/src/screenshare/mod.rs b/src/screenshare/mod.rs index 7294ed5..345249b 100644 --- a/src/screenshare/mod.rs +++ b/src/screenshare/mod.rs @@ -183,6 +183,10 @@ pub async fn list_audio_apps() -> Vec { .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::null()) + // On [`LIST_APPS_TIMEOUT`] the `output()` future is dropped, which drops + // the child — `kill_on_drop(true)` then SIGKILLs and reaps it so a wedged + // `pactl` can't linger/accumulate across picker opens (audit P3). + .kill_on_drop(true) .output(); match tokio::time::timeout(LIST_APPS_TIMEOUT, run).await { Ok(Ok(o)) if o.status.success() => parse_audio_apps(&o.stdout), @@ -190,6 +194,41 @@ pub async fn list_audio_apps() -> Vec { } } +/// Hard cap on the capability probe (`pixelpass --help`). Conservative: a slow or +/// hung pixelpass degrades to "strict audio unsupported" → whole-desktop-only +/// picker (safe), never a stalled core loop. +const HELP_PROBE_TIMEOUT: Duration = Duration::from_secs(2); + +/// Whether the resolved pixelpass understands `--strict-audio` (added in pixelpass +/// `85fdebe`). peerspeak only offers per-app audio capture when it does: a per-app +/// share always appends `--strict-audio`, and an **older** pixelpass would have +/// clap reject the unknown flag → the host spawn hard-fails and the share is +/// broken (audit P2, version skew). When unsupported the picker degrades to +/// whole-desktop audio only — we never silently drop to best-effort `--app`, which +/// would reintroduce the call echo (A23). +/// +/// Any probe failure/timeout returns `false` (degrade to the safe path). The +/// `--help` child is `kill_on_drop` so a hung pixelpass can't linger. +pub async fn supports_strict_audio(bin: &Path) -> bool { + let run = Command::new(bin) + .arg("--help") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .kill_on_drop(true) + .output(); + match tokio::time::timeout(HELP_PROBE_TIMEOUT, run).await { + Ok(Ok(o)) => help_mentions_strict_audio(&o.stdout), + _ => false, + } +} + +/// Pure check: does `pixelpass --help` advertise `--strict-audio`? Matches the +/// flag token rather than a whole line, since clap may wrap/realign help text. +pub fn help_mentions_strict_audio(help_stdout: &[u8]) -> bool { + String::from_utf8_lossy(help_stdout).contains("--strict-audio") +} + /// Parse `pactl -f json list sink-inputs` stdout into a sorted, deduplicated list /// of `application.name`s. Pure: no I/O. Unparseable input yields an empty list. /// Each name is passed through [`sanitize_app_name`] so the picker only ever @@ -594,6 +633,19 @@ mod tests { assert_eq!(parse_audio_apps(stdout.as_bytes()), vec!["mpv".to_string()]); } + #[test] + fn help_probe_detects_strict_audio_flag() { + // A new pixelpass advertises the flag; an old one doesn't. The probe must + // match the token even when clap wraps the option onto its own line. + let new_help = b"Options:\n --app \n --strict-audio\n With --app, never fall back..."; + assert!(help_mentions_strict_audio(new_help)); + let old_help = b"Options:\n --app \n --output \n -h, --help"; + assert!(!help_mentions_strict_audio(old_help)); + // Garbage / empty output degrades to "unsupported" (safe path). + assert!(!help_mentions_strict_audio(b"")); + assert!(!help_mentions_strict_audio(&[0xff, 0xfe, 0x00])); + } + #[test] fn sanitize_ticket_accepts_pixelpass_endpoint_ticket_shape() { let ticket = "endpointaabwxjexzensznfvuudiapn5tyzws3angd2merarm";