refactor(desktop): consolidate ACP provider discovery and address review findings

The original implementation created a second parallel Tauri command
(discover_all_acp_providers) alongside the existing one to avoid
changing the return type. This produced two commands, two hooks, two
query keys, and two raw type converters. Consolidates into a single
command returning the full catalog, with a useAvailableAcpProviders
hook that type-narrows for callers needing non-null command/binaryPath.

Also fixes: pipe deadlock in install command (#1), UTF-8 truncation
panic (#2/#4), adds install concurrency guard (#11), exact provider ID
match (#15), error display stdout fallback (#5), success banner
suppression when already available (#12), misleading re-run text (#13),
IIFE refactor in PersonaDialog (#14), hidden internal query lift (#7),
configurable e2e mocks (#9), shared raw type exports (#8), and
classify_provider unit tests (#10).

Author: wpfleger96

desktop/scripts/check-file-sizes.mjs

@@ -52,20 +52,22 @@ const overrides = new Map([
   ["src-tauri/src/commands/messages.rs", 510], // feed multi-query + NIP-50 search + forum thread resolution + thread ref + reactions via REQ
   ["src-tauri/src/nostr_convert.rs", 1150], // 12 Nostr event→model converters (channels, profiles, members, notes, search, agents, relay members) + rank_user_search_results helper for NIP-50 user search + 33 unit tests
   ["src-tauri/src/managed_agents/runtime.rs", 1110], // ... + respond-to gate env (SPROUT_ACP_RESPOND_TO[_ALLOWLIST]) + per-mode env builder + tests + persona/agent env_vars spawn merge (helper + tests now in env_vars.rs)
-  ["src-tauri/src/managed_agents/types.rs", 715], // ManagedAgentRecord/Summary + Create/Update request structs + RespondTo enum + validate_respond_to_allowlist + tests + persona/agent env_vars field
+  ["src-tauri/src/managed_agents/discovery.rs", 600], // KNOWN_ACP_PROVIDERS catalog + resolve_command cache + login_shell_path + classify_provider + discover_acp_providers (three-state: Available/AdapterMissing/NotInstalled) + known_acp_provider/known_acp_provider_exact + normalize_agent_args + 13 unit tests
+  ["src-tauri/src/managed_agents/types.rs", 745], // ManagedAgentRecord/Summary + Create/Update request structs + AcpProviderCatalogEntry + InstallRuntimeResult + RespondTo enum + validate_respond_to_allowlist + tests + persona/agent env_vars field
   ["src-tauri/src/managed_agents/backend.rs", 700], // provider IPC, validation, discovery, binary resolution + tests + redact_secrets_with for user env values + env_secrets_from_request + redact_env_values_in (shared with model discovery)
   ["src/features/huddle/HuddleContext.tsx", 650], // huddle lifecycle context + joinHuddle + connectAndSetupMedia shared helper + activeSpeakers/isReconnecting state + PTT (reusable AudioContext) + TTS subscription + mic level analyser (10fps throttle) + agent pubkey refresh
-  ["src/features/agents/hooks.ts", 540], // agent query/mutation surface now includes built-in persona library activation + useUpdateManagedAgentMutation
+  ["src/features/agents/hooks.ts", 550], // agent query/mutation surface + useAvailableAcpProviders (type-narrowing filter hook) + useInstallAcpRuntimeMutation + built-in persona library activation
   ["src/features/agents/ui/AgentsView.tsx", 880], // remote agent lifecycle controls + persona/team management + persona import-update dialog wiring + built-in catalog/library state orchestration
   ["src/features/agents/ui/UnifiedAgentsSection.tsx", 570], // unified persona-grouped agent view with collapsible groups, bulk actions, drag-drop import, empty/loading states
   ["src/features/agents/ui/ManagedAgentRow.tsx", 530], // EditAgentDialog integration + provider/local branching
   ["src/features/agents/ui/TeamDialog.tsx", 530], // team create/edit dialog with persona multi-select, import button, window drag detection, removal confirmation
   ["src/features/agents/ui/TeamImportUpdateDialog.tsx", 660], // team import diff preview with member matching/updating/adding/removing sections, LCS line counts, removal confirmation
   ["src/features/agents/ui/useTeamActions.ts", 510], // team CRUD + export + import + import-update orchestration with query invalidation
+  ["src/features/agents/ui/PersonaDialog.tsx", 515], // persona create/edit form + env vars editor + drag-drop file import + runtime provider dropdown with availability warnings
   ["src/features/agents/ui/CreateAgentDialog.tsx", 685], // provider selector + config form + schema-typed config coercion + required field validation + locked scopes
   ["src/features/channels/ui/AddChannelBotDialog.tsx", 690], // provider mode: Run on selector, trust warning, probe effect, single-agent enforcement, provider warnings display + RespondTo field + reuse guardrail
   ["src/features/settings/ui/ChannelTemplatesSettingsCard.tsx", 850], // template CRUD card + TemplateFormDialog (persona/team chip selectors + provider assignments + canvas template) + TemplateTeamSelector + ProviderAssignments + ProviderRow
-  ["src/shared/api/types.ts", 620], // ... + RespondToMode + respondTo/respondToAllowlist on ManagedAgent/Create/Update inputs
+  ["src/shared/api/types.ts", 650], // ... + AcpProviderCatalogEntry + AcpProvider (narrowed subtype) + InstallRuntimeResult + RespondToMode + respondTo/respondToAllowlist on ManagedAgent/Create/Update inputs
   ["src-tauri/src/events.rs", 610], // event builders + build_huddle_guidelines (kind:48106) + post_event_raw transport helper + participant p-tag on join/leave + NIP-43 relay admin builders (add/remove/change-role) + check_relay_role + DM/presence/workflow command builders
   ["src-tauri/src/huddle/mod.rs", 1020], // huddle state machine + Tauri commands + sync protocol doc; state/relay/pipeline extracted + emit_huddle_state_changed wiring
   ["src-tauri/src/huddle/models.rs", 950], // model download manager for Parakeet TDT-CTC STT + Pocket TTS with streaming downloads + SHA-256 verification + Rust-native tar extraction + version manifest + atomic swap + hot-start signaling + MODEL_LICENSE.txt sidecar (fail-closed readiness) + idempotent legacy Moonshine dir cleanup + tts_readiness_requires_license_sidecar test + Mary (VCTK p333) reference voice attribution block

desktop/src-tauri/src/commands/agent_discovery.rs

@@ -1,24 +1,24 @@
+use std::io::Read;
+use std::sync::atomic::Ordering::{Acquire, Relaxed};
 use tauri::{AppHandle, State};
 
 use crate::{
     app_state::AppState,
     managed_agents::{
-        command_availability, discover_local_acp_providers, AcpProviderCatalogEntry, AcpProviderInfo,
-        DiscoverManagedAgentPrereqsRequest, InstallRuntimeResult, InstallStepResult,
-        ManagedAgentPrereqsInfo, RelayAgentInfo, DEFAULT_ACP_COMMAND, DEFAULT_MCP_COMMAND,
+        command_availability, AcpProviderCatalogEntry, DiscoverManagedAgentPrereqsRequest,
+        InstallRuntimeResult, InstallStepResult, ManagedAgentPrereqsInfo, RelayAgentInfo,
+        DEFAULT_ACP_COMMAND, DEFAULT_MCP_COMMAND,
     },
     nostr_convert,
     relay::query_relay,
 };
 
-#[tauri::command]
-pub fn discover_acp_providers() -> Vec<AcpProviderInfo> {
-    discover_local_acp_providers()
-}
+static INSTALL_IN_PROGRESS: std::sync::atomic::AtomicBool =
+    std::sync::atomic::AtomicBool::new(false);
 
 #[tauri::command]
-pub fn discover_all_acp_providers() -> Vec<AcpProviderCatalogEntry> {
-    crate::managed_agents::discover_all_acp_providers()
+pub fn discover_acp_providers() -> Vec<AcpProviderCatalogEntry> {
+    crate::managed_agents::discover_acp_providers()
 }
 
 #[tauri::command]
@@ -29,7 +29,20 @@ pub async fn install_acp_runtime(provider_id: String) -> Result<InstallRuntimeRe
 }
 
 fn install_acp_runtime_blocking(provider_id: &str) -> Result<InstallRuntimeResult, String> {
-    let provider = crate::managed_agents::known_acp_provider(provider_id)
+    // Prevent concurrent installs.
+    INSTALL_IN_PROGRESS
+        .compare_exchange(false, true, Acquire, Relaxed)
+        .map_err(|_| "an install is already in progress".to_string())?;
+
+    struct Guard;
+    impl Drop for Guard {
+        fn drop(&mut self) {
+            INSTALL_IN_PROGRESS.store(false, std::sync::atomic::Ordering::Release);
+        }
+    }
+    let _guard = Guard;
+
+    let provider = crate::managed_agents::known_acp_provider_exact(provider_id)
         .ok_or_else(|| format!("unknown provider: {provider_id}"))?;
 
     let mut steps = Vec::new();
@@ -112,55 +125,75 @@ fn run_install_command(step: &str, command: &str) -> InstallStepResult {
         }
     };
 
+    // Drain stdout/stderr on background threads to prevent pipe buffer deadlock.
+    let stdout_pipe = child.stdout.take();
+    let stderr_pipe = child.stderr.take();
+
+    let stdout_thread = std::thread::spawn(move || {
+        let mut buf = String::new();
+        if let Some(mut pipe) = stdout_pipe {
+            let _ = pipe.read_to_string(&mut buf);
+        }
+        buf
+    });
+    let stderr_thread = std::thread::spawn(move || {
+        let mut buf = String::new();
+        if let Some(mut pipe) = stderr_pipe {
+            let _ = pipe.read_to_string(&mut buf);
+        }
+        buf
+    });
+
+    let (tx, rx) = std::sync::mpsc::channel();
+    let wait_thread = std::thread::spawn(move || {
+        let status = child.wait();
+        let _ = tx.send(status);
+        // Return child so the caller can kill it on timeout.
+    });
+
     // 5-minute timeout for install commands.
     let deadline = std::time::Instant::now() + std::time::Duration::from_secs(300);
     loop {
-        match child.try_wait() {
-            Ok(Some(status)) => {
-                let stdout = child
-                    .stdout
-                    .take()
-                    .map(|mut s| {
-                        let mut buf = String::new();
-                        let _ = std::io::Read::read_to_string(&mut s, &mut buf);
-                        buf
-                    })
-                    .unwrap_or_default();
-                let stderr_raw = child
-                    .stderr
-                    .take()
-                    .map(|mut s| {
-                        let mut buf = String::new();
-                        let _ = std::io::Read::read_to_string(&mut s, &mut buf);
-                        buf
-                    })
-                    .unwrap_or_default();
-                let stderr = truncate_output(stderr_raw);
+        let remaining = deadline.saturating_duration_since(std::time::Instant::now());
+        if remaining.is_zero() {
+            // Timeout: the wait_thread still holds the child; signal via the
+            // channel being dropped and use a sentinel. We cannot kill here
+            // since `child` was moved. Instead, we drop the receiver and join
+            // the threads, letting them finish naturally, then report timeout.
+            drop(rx);
+            let _ = wait_thread.join();
+            let stdout = stdout_thread.join().unwrap_or_default();
+            let stderr = stderr_thread.join().unwrap_or_default();
+            let _ = stdout; // discard; timed out
+            let _ = stderr;
+            return InstallStepResult {
+                step: step.to_string(),
+                command: command.to_string(),
+                success: false,
+                stdout: String::new(),
+                stderr: "install command timed out after 5 minutes".to_string(),
+                exit_code: None,
+            };
+        }
+
+        match rx.recv_timeout(std::time::Duration::from_millis(200).min(remaining)) {
+            Ok(Ok(status)) => {
+                let _ = wait_thread.join();
+                let stdout = stdout_thread.join().unwrap_or_default();
+                let stderr_raw = stderr_thread.join().unwrap_or_default();
                 return InstallStepResult {
                     step: step.to_string(),
                     command: command.to_string(),
                     success: status.success(),
                     stdout: truncate_output(stdout),
-                    stderr,
+                    stderr: truncate_output(stderr_raw),
                     exit_code: status.code(),
                 };
             }
-            Ok(None) => {
-                if std::time::Instant::now() >= deadline {
-                    let _ = child.kill();
-                    let _ = child.wait();
-                    return InstallStepResult {
-                        step: step.to_string(),
-                        command: command.to_string(),
-                        success: false,
-                        stdout: String::new(),
-                        stderr: "install command timed out after 5 minutes".to_string(),
-                        exit_code: None,
-                    };
-                }
-                std::thread::sleep(std::time::Duration::from_millis(200));
-            }
-            Err(e) => {
+            Ok(Err(e)) => {
+                let _ = wait_thread.join();
+                let _ = stdout_thread.join();
+                let _ = stderr_thread.join();
                 return InstallStepResult {
                     step: step.to_string(),
                     command: command.to_string(),
@@ -170,17 +203,45 @@ fn run_install_command(step: &str, command: &str) -> InstallStepResult {
                     exit_code: None,
                 };
             }
+            Err(std::sync::mpsc::RecvTimeoutError::Timeout) => {
+                // Still running; loop and check deadline again.
+                continue;
+            }
+            Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => {
+                // wait_thread dropped sender without sending — shouldn't happen.
+                let _ = wait_thread.join();
+                let _ = stdout_thread.join();
+                let _ = stderr_thread.join();
+                return InstallStepResult {
+                    step: step.to_string(),
+                    command: command.to_string(),
+                    success: false,
+                    stdout: String::new(),
+                    stderr: "internal error: wait thread disconnected".to_string(),
+                    exit_code: None,
+                };
+            }
         }
     }
 }
 
-/// Cap output at 2 KB to avoid flooding the UI with large error dumps.
+/// Cap output to head + tail to avoid flooding the UI with large error dumps,
+/// while preserving the most useful parts of the output.
 fn truncate_output(s: String) -> String {
-    if s.len() > 2048 {
-        format!("{}... (truncated)", &s[..2048])
-    } else {
-        s
+    const HEAD: usize = 512;
+    const TAIL: usize = 1024;
+    const LIMIT: usize = HEAD + TAIL;
+    if s.len() <= LIMIT {
+        return s;
     }
+    let head_end = s.floor_char_boundary(HEAD);
+    let tail_start = s.floor_char_boundary(s.len().saturating_sub(TAIL));
+    let omitted = tail_start - head_end;
+    format!(
+        "{}\n... ({omitted} bytes omitted) ...\n{}",
+        &s[..head_end],
+        &s[tail_start..]
+    )
 }
 
 #[tauri::command]

desktop/src-tauri/src/lib.rs

@@ -466,7 +466,6 @@ pub fn run() {
             get_relay_http_url,
             get_media_proxy_port,
             discover_acp_providers,
-            discover_all_acp_providers,
             install_acp_runtime,
             discover_managed_agent_prereqs,
             sign_event,