fix(screenshare): address Codex A23 audit P3s — picker state machine, enum parity, pactl timeout

Triage of Codex's read-only A23 audit (a23-audit-2026-06-26.md). The P2 (pixelpass
--app best-effort fallback) is cross-repo and deferred to a design decision; these
are the three actionable peerspeak-side P3s:

- P3-1: guard the share-startup window. New `share_starting` flag blocks reopening
  the picker (and re-firing StartScreenShare) between ConfirmShareScreen and the
  core's ScreenShareStarted; cleared on Started/Stopped/Error so a failed spawn
  (surfaced as Error, not Stopped) can't wedge it. +2 state-machine tests.
- P3-2: parse_audio_apps now runs each name through sanitize_app_name, so the
  picker never offers a name that host_args would later silently drop (which would
  revert the share to whole-desktop audio = the A23 echo, with no signal). +1 test.
- P3-3: list_audio_apps wraps pactl in a 2s timeout so a wedged enumeration can't
  stall the core command loop (mute/deafen/leave/stop) while the picker opens.

433 lib tests (+3), clippy --all-targets clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/app/mod.rs

@@ -583,6 +583,11 @@ pub struct AppState {
     /// The picker's current selection: `Some(name)` = capture that app's audio,
     /// `None` = "All system audio" (whole desktop; may echo the call).
     share_audio_selection: Option<String>,
+    /// A share start is in flight: `ConfirmShareScreen` was sent but the core
+    /// hasn't yet replied with `ScreenShareStarted`/an error. Blocks reopening
+    /// the picker (and re-confirming) during that startup window. Cleared on
+    /// `ScreenShareStarted`, `ScreenShareStopped`, or any `Error`.
+    share_starting: 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.
@@ -655,6 +660,7 @@ impl AppState {
         self.share_picker_open = false;
         self.share_audio_apps.clear();
         self.share_audio_selection = None;
+        self.share_starting = false;
     }
 
     fn custom_sound_path(&self, sound: Sound) -> &str {
@@ -779,6 +785,7 @@ impl Default for AppState {
             share_picker_open: false,
             share_audio_apps: Vec::new(),
             share_audio_selection: None,
+            share_starting: false,
             drawer_chat_open: false,
             mic_level: 0.0,
             mic_test_active: false,
@@ -1162,12 +1169,14 @@ fn update(state: &mut AppState, message: AppMessage) -> Task<AppMessage> {
         AppMessage::ToggleScreenShare => {
             if state.self_sharing {
                 let _ = state.controller.send(CoreCommand::StopScreenShare);
-            } else {
+            } else if !state.share_starting {
                 // Open the audio picker instead of sharing immediately, so the
                 // user chooses which app's audio to capture rather than the whole
                 // desktop (which echoes the call back to viewers, A23). Default
                 // selection is "All system audio" (None). Kick off a fresh
                 // enumeration so the list reflects what's playing right now.
+                // Suppressed while a start is already in flight (`share_starting`)
+                // so the picker can't be reopened during the startup window.
                 state.share_picker_open = true;
                 state.share_audio_selection = None;
                 state.share_audio_apps.clear();
@@ -1181,12 +1190,18 @@ fn update(state: &mut AppState, message: AppMessage) -> Task<AppMessage> {
             state.share_audio_selection = app;
         }
         AppMessage::ConfirmShareScreen => {
-            state.share_picker_open = false;
+            // Only a confirm from an open picker starts a share; a stray confirm
-            let audio_app = state.share_audio_selection.clone();
+            // (or one arriving while a start is already in flight) is ignored, so
-            let _ = state
+            // we can't double-send StartScreenShare.
-                .controller
+            if state.share_picker_open && !state.share_starting {
-                .send(CoreCommand::StartScreenShare { audio_app });
+                state.share_picker_open = false;
-            state.status_message = "Starting screen share…".to_string();
+                state.share_starting = true;
+                let audio_app = state.share_audio_selection.clone();
+                let _ = state
+                    .controller
+                    .send(CoreCommand::StartScreenShare { audio_app });
+                state.status_message = "Starting screen share…".to_string();
+            }
         }
         AppMessage::WatchShare(ticket) => {
             let _ = state.controller.send(CoreCommand::ViewShare(ticket));
@@ -1359,10 +1374,14 @@ fn update(state: &mut AppState, message: AppMessage) -> Task<AppMessage> {
                 }
                 UiEvent::ScreenShareStarted => {
                     state.self_sharing = true;
+                    state.share_starting = false;
+                    // Defensive: ensure no picker lingers across a successful start.
+                    state.share_picker_open = false;
                     state.status_message = "Sharing your screen".to_string();
                 }
                 UiEvent::ScreenShareStopped => {
                     state.self_sharing = false;
+                    state.share_starting = false;
                     state.status_message = "Screen share stopped".to_string();
                 }
                 UiEvent::IdentityStatus { node_id, persisted, error } => {
@@ -1407,6 +1426,10 @@ fn update(state: &mut AppState, message: AppMessage) -> Task<AppMessage> {
                     }
                 }
                 UiEvent::Error(err) => {
+                    // A failed share start (spawn error) surfaces here, not via
+                    // ScreenShareStopped, so clear the in-flight flag to let the
+                    // user retry instead of being wedged.
+                    state.share_starting = false;
                     state.status_message = format!("Error: {}", err);
                 }
             }
@@ -6128,8 +6151,9 @@ impl Program<AppMessage> for Icon {
 mod tests {
     use super::{
         attachment_default_name, format_duration, initial_window_position, reconnect_attempt_chime,
-        reconnected_chime, set_peer_gate_config, set_peer_volume_config, AppConfig, AppState,
+        reconnected_chime, set_peer_gate_config, set_peer_volume_config, update, AppConfig,
-        AttachmentCache, AttachmentState, ChatEntry, GateMeter, METER_MAX,
+        AppMessage, AppState, AttachmentCache, AttachmentState, ChatEntry, GateMeter, METER_MAX,
+        UiEvent,
     };
     use iroh::SecretKey;
 
@@ -6292,6 +6316,7 @@ mod tests {
         state.share_picker_open = true;
         state.share_audio_apps = vec!["Firefox".to_string()];
         state.share_audio_selection = Some("Firefox".to_string());
+        state.share_starting = true;
         state.clip_status.lock().unwrap().playing_id = Some(attachment_id);
 
         state.reset_room_state();
@@ -6315,6 +6340,7 @@ mod tests {
         assert!(!state.share_picker_open);
         assert!(state.share_audio_apps.is_empty());
         assert!(state.share_audio_selection.is_none());
+        assert!(!state.share_starting);
 
         for _ in 0..50 {
             if crate::audio::clip_player::status_snapshot(&state.clip_status).playing_id.is_none() {
@@ -6325,6 +6351,64 @@ mod tests {
         panic!("clip player did not stop during room reset");
     }
 
+    #[test]
+    fn share_picker_startup_window_is_guarded() {
+        // P3-1: between confirming the picker and the core's ScreenShareStarted,
+        // self_sharing is still false. The picker must not be reopenable in that
+        // window, and a stray confirm must not re-fire StartScreenShare.
+        // Picker open, user confirms a selection.
+        let mut state = AppState {
+            share_picker_open: true,
+            share_audio_selection: Some("mpv".to_string()),
+            ..Default::default()
+        };
+        let _ = update(&mut state, AppMessage::ConfirmShareScreen);
+        assert!(state.share_starting, "confirm should mark a start in flight");
+        assert!(!state.share_picker_open, "confirm should close the picker");
+        assert!(!state.self_sharing, "core hasn't acked the start yet");
+
+        // Clicking Share again during startup must NOT reopen the picker.
+        let _ = update(&mut state, AppMessage::ToggleScreenShare);
+        assert!(!state.share_picker_open, "picker must stay closed while starting");
+        assert!(state.share_starting);
+
+        // A stray confirm during startup is ignored (no double-start).
+        let _ = update(&mut state, AppMessage::ConfirmShareScreen);
+        assert!(state.share_starting);
+
+        // Core acks: flag clears, sharing begins, no picker lingers.
+        let _ = update(
+            &mut state,
+            AppMessage::UiEventReceived(UiEvent::ScreenShareStarted),
+        );
+        assert!(!state.share_starting);
+        assert!(state.self_sharing);
+        assert!(!state.share_picker_open);
+    }
+
+    #[test]
+    fn share_start_failure_clears_in_flight_flag() {
+        // A failed spawn surfaces as UiEvent::Error (not ScreenShareStopped); the
+        // in-flight flag must still clear so the user can retry.
+        let mut state = AppState {
+            share_picker_open: true,
+            ..Default::default()
+        };
+        let _ = update(&mut state, AppMessage::ConfirmShareScreen);
+        assert!(state.share_starting);
+
+        let _ = update(
+            &mut state,
+            AppMessage::UiEventReceived(UiEvent::Error("boom".into())),
+        );
+        assert!(!state.share_starting, "error must un-wedge the start flag");
+        assert!(!state.self_sharing);
+
+        // And now the picker can be opened again.
+        let _ = update(&mut state, AppMessage::ToggleScreenShare);
+        assert!(state.share_picker_open);
+    }
+
     #[test]
     fn peer_gate_persists_when_on_and_clears_when_off() {
         let mut config = AppConfig::default();

src/screenshare/mod.rs

@@ -145,28 +145,38 @@ pub fn sanitize_app_name(name: &str) -> Option<String> {
     ok.then(|| name.to_string())
 }
 
+/// Hard cap on how long enumeration waits for `pactl`. It runs inline on the core
+/// command loop (the picker awaits it before opening), so a wedged/slow `pactl`
+/// must not stall mute/deafen/leave/stop. On timeout we treat it like any other
+/// failure: empty list → "All system audio" only.
+const LIST_APPS_TIMEOUT: Duration = Duration::from_secs(2);
+
 /// Enumerate the apps currently sending audio to a sink, deduplicated by
 /// `application.name`. Mirrors how pixelpass itself builds its interactive
 /// picker (`pactl -f json list sink-inputs`), so the names we return are exactly
 /// the ones `--app` matches against. Returns an empty list on any error (pactl
-/// missing, non-PipeWire host, nothing playing) — a normal, handled state that
+/// missing, non-PipeWire host, nothing playing, or [`LIST_APPS_TIMEOUT`] elapsed)
-/// leaves the picker showing only "All system audio".
+/// — a normal, handled state that leaves the picker showing only "All system
+/// audio".
 pub async fn list_audio_apps() -> Vec<String> {
-    let output = Command::new("pactl")
+    let run = Command::new("pactl")
         .args(["-f", "json", "list", "sink-inputs"])
         .stdin(Stdio::null())
         .stdout(Stdio::piped())
         .stderr(Stdio::null())
-        .output()
+        .output();
-        .await;
+    match tokio::time::timeout(LIST_APPS_TIMEOUT, run).await {
-    match output {
+        Ok(Ok(o)) if o.status.success() => parse_audio_apps(&o.stdout),
-        Ok(o) if o.status.success() => parse_audio_apps(&o.stdout),
         _ => Vec::new(),
     }
 }
 
 /// 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
+/// offers names that will actually survive [`host_args`]; otherwise a name that
+/// parses here but fails sanitization later would be selectable yet silently
+/// drop the `--app` flag and revert the share to whole-desktop audio (A23 echo).
 pub fn parse_audio_apps(stdout: &[u8]) -> Vec<String> {
     let Ok(entries) = serde_json::from_slice::<Vec<SinkInput>>(stdout) else {
         return Vec::new();
@@ -174,8 +184,7 @@ pub fn parse_audio_apps(stdout: &[u8]) -> Vec<String> {
     let mut names: Vec<String> = entries
         .into_iter()
         .filter_map(|e| e.properties.application_name)
-        .map(|n| n.trim().to_string())
+        .filter_map(|n| sanitize_app_name(&n))
-        .filter(|n| !n.is_empty())
         .collect();
     names.sort_unstable();
     names.dedup();
@@ -532,6 +541,24 @@ mod tests {
         assert_eq!(parse_audio_apps(b"[]"), Vec::<String>::new());
     }
 
+    #[test]
+    fn parse_audio_apps_drops_names_host_args_would_reject() {
+        // Names that parse from pactl but fail `sanitize_app_name` (control chars,
+        // overlong) must NOT be offered in the picker — otherwise the user could
+        // pick one, `host_args` would silently drop `--app`, and the share would
+        // revert to whole-desktop audio (A23 echo) with no signal. The valid name
+        // survives; the control-char and overlong ones are filtered out.
+        let overlong = "x".repeat(MAX_APP_NAME_LEN + 1);
+        let stdout = format!(
+            r#"[
+                {{"index":1,"properties":{{"application.name":"mpv"}}}},
+                {{"index":2,"properties":{{"application.name":"bad\nname"}}}},
+                {{"index":3,"properties":{{"application.name":"{overlong}"}}}}
+            ]"#
+        );
+        assert_eq!(parse_audio_apps(stdout.as_bytes()), vec!["mpv".to_string()]);
+    }
+
     #[test]
     fn sanitize_ticket_accepts_pixelpass_endpoint_ticket_shape() {
         let ticket = "endpointaabwxjexzensznfvuudiapn5tyzws3angd2merarm";