refactor: consolidate duplicated shared logic

docs/frontend-ui-audit-2026-08-11/ActivityGroups.md

@@ -0,0 +1,18 @@
+# Frontend UI Audit — Activity Groups
+
+Scope: `EditActivityGroup`, `TerminalActivityGroup`, and their shared event projection. This is a behavior-preserving component refactor; no rendered styles, copy, layout, focus behavior, or interaction contract changed.
+
+| Line                                                                  | Element                                                                                                              | Verdict          | Reason                                                                                                                                                                                                      | Suggested change                                                  |
+| --------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- | ---------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------- |
+| `src/engines/ChatPanel/ChatItems/activityGroupProjection.tsx:17`      | Event-item projection, intermediate running-state normalization, lazy registry rendering, and tool-usage aggregation | abstract         | Edit and terminal groups previously duplicated the same presentation pipeline. One shared owner prevents loading-state and usage-badge behavior from drifting while leaving domain summaries separate.      | Reuse the shared projection from both activity-group components.  |
+| `src/engines/ChatPanel/ChatItems/EditActivityGroup/index.tsx:111`     | Edit activity stack                                                                                                  | keep with reason | `StackedBlock`, tool icons, workstation diff tokens, and the shared usage badge already implement the design-system contracts. The edit/read and diff-stat summary is specific to edit activity.            | Keep the edit summary local and continue using shared primitives. |
+| `src/engines/ChatPanel/ChatItems/TerminalActivityGroup/index.tsx:140` | Terminal activity stack                                                                                              | keep with reason | The stack uses the same shared primitives, while terminal/MCP/wait counts and durable Work Item result cards are terminal-domain behavior. Moving them into the generic projection would leak domain rules. | Keep terminal summary and Work Item projection local.             |
+| `src/engines/ChatPanel/ChatItems/EditActivityGroup/index.tsx:119`     | Existing summary typography and spacing                                                                              | keep with reason | The existing classes compose established text and diff-stat tokens; this refactor introduces no arbitrary visual value or parallel component style.                                                         | No visual change.                                                 |
+
+## Summary
+
+- Fix: 0
+- Keep with reason: 3
+- Abstract: 1
+- Sweep candidates: 0
+- Accessibility: no semantic or interactive changes; `StackedBlock` retains the existing keyboard/collapse contract.

docs/frontend-ui-audit-2026-08-11/OAuthSessionSetup.md

@@ -0,0 +1,21 @@
+# Frontend UI Audit — OAuth Session Setup
+
+Scope: the Claude Code and Codex OAuth session-setup components plus their extracted shared shell. Provider-specific credential mapping remains outside the shared presentation/lifecycle boundary.
+
+| Line                                                                       | Element                                               | Verdict          | Reason                                                                                                                                                                                                              | Suggested change                                                                                        |
+| -------------------------------------------------------------------------- | ----------------------------------------------------- | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- |
+| `src/features/SessionSetup/components/OAuthSessionSetupShell.tsx:63`       | OAuth idle/browser/loading/error/success presentation | abstract         | Claude Code and Codex duplicated the same section layout, WebView chrome, progress indicator, overlays, alerts, and debug container. A provider-copy contract preserves localized differences without parallel JSX. | Reuse one shell and pass provider copy/test identity explicitly.                                        |
+| `src/features/SessionSetup/components/OAuthSessionSetupShell.tsx:286`      | OAuth browser lifecycle                               | abstract         | Both providers used the same open, delayed native start, success collapse, retry, parent-layout sync, and external-close transitions. One owner makes cleanup and duplicate retry policy consistent.                | Keep transient browser intent in the shared shell; retain provider capture state in the existing hooks. |
+| `src/features/SessionSetup/components/OAuthSessionSetupShell.tsx:123`      | Refresh and close icon buttons                        | fix              | The duplicated icon-only controls had no accessible names. The shared shell can apply localized labels once for both providers.                                                                                     | Add `aria-label` and `title` from existing common actions copy.                                         |
+| `src/features/SessionSetup/components/OAuthSessionSetupShell.tsx:172`      | Loading and error overlays                            | fix              | Visual states existed but did not expose status/alert semantics to assistive technology.                                                                                                                            | Mark loading as `role="status"` and errors as `role="alert"`.                                           |
+| `src/features/SessionSetup/components/OAuthSessionSetupShell.tsx:391`      | Two-step progress indicator                           | fix              | Active styling was only visual.                                                                                                                                                                                     | Expose the active item with `aria-current="step"`.                                                      |
+| `src/features/SessionSetup/components/ClaudeCodeSessionSetup/index.tsx:58` | Claude Code capture and account metadata mapping      | keep with reason | Claude Code owns its callback response and optional organization metadata; generalizing this mapping would weaken provider types.                                                                                   | Keep mapping in the provider adapter and pass normalized capture state to the shell.                    |
+| `src/features/SessionSetup/components/CodexSessionSetup/index.tsx:41`      | Codex capture and required token mapping              | keep with reason | Codex requires refresh and ID tokens and supports initial auto-start. Those are provider contract differences, not presentation variants.                                                                           | Keep mapping and auto-start input in the provider adapter.                                              |
+
+## Summary
+
+- Fix: 3
+- Keep with reason: 2
+- Abstract: 2
+- Sweep candidates: 0
+- Visual behavior: unchanged; accessibility semantics are additive.

src-tauri/crates/agent-core/src/specialization/external_import/commands.rs

@@ -3,6 +3,7 @@
 use std::path::{Path, PathBuf};
 
 use super::detect::detect_all;
+use super::mcp_config::load_external_mcp_config;
 use super::types::{
     frontmatter_declares_readonly, readonly_excluded_tool_names, DetectedItem, ImportItemReport,
     ImportReport, ImportSelection, ImportStatus, ItemKind, SourceScope,
@@ -368,50 +369,6 @@ fn copy_dir_recursive(from: &Path, to: &Path) -> Result<(), String> {
 // MCP import
 // ============================================================
 
-fn load_external_mcp_config(path: &Path) -> Result<McpConfigFile, String> {
-    let raw = std::fs::read_to_string(path)
-        .map_err(|err| format!("Failed to read MCP config {}: {}", path.display(), err))?;
-    let mut value: serde_json::Value = serde_json::from_str(&raw)
-        .map_err(|err| format!("Failed to parse MCP config {}: {}", path.display(), err))?;
-    let Some(servers) = value
-        .get_mut("mcpServers")
-        .and_then(|entry| entry.as_object_mut())
-    else {
-        return Ok(McpConfigFile::default());
-    };
-
-    for server in servers.values_mut() {
-        let Some(server_obj) = server.as_object_mut() else {
-            continue;
-        };
-        if !server_obj.contains_key("type") {
-            let inferred = if server_obj.contains_key("url") {
-                "streamableHttp"
-            } else {
-                "stdio"
-            };
-            server_obj.insert(
-                "type".to_string(),
-                serde_json::Value::String(inferred.to_string()),
-            );
-        }
-        if server_obj.get("type").and_then(|entry| entry.as_str()) == Some("http") {
-            server_obj.insert(
-                "type".to_string(),
-                serde_json::Value::String("streamableHttp".to_string()),
-            );
-        }
-    }
-
-    serde_json::from_value(value).map_err(|err| {
-        format!(
-            "Failed to parse MCP server entries {}: {}",
-            path.display(),
-            err
-        )
-    })
-}
-
 fn apply_mcp_import(
     selection: &ImportSelection,
     target_repo_path: Option<&Path>,

src-tauri/crates/agent-core/src/specialization/external_import/detect/mcp.rs

@@ -6,9 +6,10 @@
 
 use std::path::Path;
 
+use super::super::mcp_config::load_external_mcp_config;
 use super::super::types::{DetectedItem, ItemKind, ItemPreview, SourceAgent, SourceScope};
 use super::helpers::{home_dir, orgii_mcp_exists, path_has_denied_ancestor, MAX_ITEMS_PER_BATCH};
-use crate::specialization::mcp::config::{McpConfigFile, McpTransportType};
+use crate::specialization::mcp::config::McpTransportType;
 
 pub(super) fn detect_mcp_servers(repo_path: Option<&Path>) -> Vec<DetectedItem> {
     let mut out = Vec::new();
@@ -70,50 +71,6 @@ pub(super) fn detect_mcp_servers(repo_path: Option<&Path>) -> Vec<DetectedItem>
     out
 }
 
-fn load_external_mcp_config(path: &Path) -> Result<McpConfigFile, String> {
-    let raw = std::fs::read_to_string(path)
-        .map_err(|err| format!("Failed to read MCP config {}: {}", path.display(), err))?;
-    let mut value: serde_json::Value = serde_json::from_str(&raw)
-        .map_err(|err| format!("Failed to parse MCP config {}: {}", path.display(), err))?;
-    let Some(servers) = value
-        .get_mut("mcpServers")
-        .and_then(|entry| entry.as_object_mut())
-    else {
-        return Ok(McpConfigFile::default());
-    };
-
-    for server in servers.values_mut() {
-        let Some(server_obj) = server.as_object_mut() else {
-            continue;
-        };
-        if !server_obj.contains_key("type") {
-            let inferred = if server_obj.contains_key("url") {
-                "streamableHttp"
-            } else {
-                "stdio"
-            };
-            server_obj.insert(
-                "type".to_string(),
-                serde_json::Value::String(inferred.to_string()),
-            );
-        }
-        if server_obj.get("type").and_then(|entry| entry.as_str()) == Some("http") {
-            server_obj.insert(
-                "type".to_string(),
-                serde_json::Value::String("streamableHttp".to_string()),
-            );
-        }
-    }
-
-    serde_json::from_value(value).map_err(|err| {
-        format!(
-            "Failed to parse MCP server entries {}: {}",
-            path.display(),
-            err
-        )
-    })
-}
-
 fn scan_mcp_config_file(
     path: &Path,
     source_agent: SourceAgent,

src-tauri/crates/agent-core/src/specialization/external_import/mcp_config.rs

@@ -0,0 +1,105 @@
+use std::path::Path;
+
+use crate::specialization::mcp::config::McpConfigFile;
+
+/// Load an MCP config authored by another agent and normalize the transport
+/// spellings that ORGII accepts before deserializing it into the canonical
+/// config model.
+pub(super) fn load_external_mcp_config(path: &Path) -> Result<McpConfigFile, String> {
+    let raw = std::fs::read_to_string(path)
+        .map_err(|err| format!("Failed to read MCP config {}: {}", path.display(), err))?;
+    let mut value: serde_json::Value = serde_json::from_str(&raw)
+        .map_err(|err| format!("Failed to parse MCP config {}: {}", path.display(), err))?;
+    let Some(servers) = value
+        .get_mut("mcpServers")
+        .and_then(|entry| entry.as_object_mut())
+    else {
+        return Ok(McpConfigFile::default());
+    };
+
+    for server in servers.values_mut() {
+        let Some(server_obj) = server.as_object_mut() else {
+            continue;
+        };
+        if !server_obj.contains_key("type") {
+            let inferred = if server_obj.contains_key("url") {
+                "streamableHttp"
+            } else {
+                "stdio"
+            };
+            server_obj.insert(
+                "type".to_string(),
+                serde_json::Value::String(inferred.to_string()),
+            );
+        }
+        if server_obj.get("type").and_then(|entry| entry.as_str()) == Some("http") {
+            server_obj.insert(
+                "type".to_string(),
+                serde_json::Value::String("streamableHttp".to_string()),
+            );
+        }
+    }
+
+    serde_json::from_value(value).map_err(|err| {
+        format!(
+            "Failed to parse MCP server entries {}: {}",
+            path.display(),
+            err
+        )
+    })
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::specialization::mcp::config::McpTransportType;
+    use tempfile::TempDir;
+
+    #[test]
+    fn normalizes_external_transport_variants() {
+        let temp = TempDir::new().expect("create temp dir");
+        let path = temp.path().join("mcp.json");
+        std::fs::write(
+            &path,
+            r#"{
+                "mcpServers": {
+                    "implicit-stdio": { "command": "server" },
+                    "implicit-http": { "url": "https://example.com/mcp" },
+                    "legacy-http": { "type": "http", "url": "https://example.com/legacy" },
+                    "explicit-sse": { "type": "sse", "url": "https://example.com/sse" }
+                }
+            }"#,
+        )
+        .expect("write config");
+
+        let config = load_external_mcp_config(&path).expect("load config");
+
+        assert_eq!(
+            config.mcp_servers["implicit-stdio"].transport_type,
+            McpTransportType::Stdio
+        );
+        assert_eq!(
+            config.mcp_servers["implicit-http"].transport_type,
+            McpTransportType::StreamableHttp
+        );
+        assert_eq!(
+            config.mcp_servers["legacy-http"].transport_type,
+            McpTransportType::StreamableHttp
+        );
+        assert_eq!(
+            config.mcp_servers["explicit-sse"].transport_type,
+            McpTransportType::Sse
+        );
+    }
+
+    #[test]
+    fn treats_missing_mcp_servers_as_empty() {
+        let temp = TempDir::new().expect("create temp dir");
+        let path = temp.path().join("mcp.json");
+        std::fs::write(&path, r#"{ "other": true }"#).expect("write config");
+
+        let config = load_external_mcp_config(&path).expect("load config");
+
+        assert!(config.mcp_servers.is_empty());
+    }
+}

src-tauri/crates/agent-core/src/specialization/external_import/mod.rs

@@ -20,6 +20,7 @@
 
 pub mod commands;
 pub mod detect;
+mod mcp_config;
 pub mod types;
 
 // Wildcard re-export needed: `#[tauri::command]` generates hidden

src-tauri/crates/lsp/src/command_detection.rs

@@ -0,0 +1,53 @@
+use std::process::Command;
+
+/// Check whether a command-line tool is available on the system PATH.
+///
+/// Forward PATH explicitly so app startup code that augments the process
+/// environment is reflected consistently across every LSP discovery surface.
+pub fn command_exists(command_name: &str) -> bool {
+    let current_path = std::env::var_os("PATH");
+
+    #[cfg(unix)]
+    let mut command = {
+        let mut command = Command::new("which");
+        command.arg(command_name);
+        command
+    };
+
+    #[cfg(windows)]
+    let mut command = {
+        let mut command = Command::new("where");
+        command.arg(command_name);
+        command
+    };
+
+    if let Some(path) = current_path {
+        command.env("PATH", path);
+    }
+    app_platform::hide_console(&mut command);
+    command
+        .output()
+        .map(|output| output.status.success())
+        .unwrap_or(false)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn finds_a_platform_shell() {
+        #[cfg(unix)]
+        assert!(command_exists("sh"));
+
+        #[cfg(windows)]
+        assert!(command_exists("cmd"));
+    }
+
+    #[test]
+    fn rejects_a_missing_command() {
+        assert!(!command_exists(
+            "orgii-command-detection-test-definitely-missing"
+        ));
+    }
+}

src-tauri/crates/lsp/src/commands/discovery.rs

@@ -2,8 +2,6 @@
 //!
 //! Tauri commands for detecting installed language servers and lint tools.
 
-use std::process::Command;
-
 use super::cache;
 use crate::lint_tools::LintToolInfo;
 use crate::server_defs::{servers, servers_for_language_id};
@@ -55,31 +53,7 @@ pub const LANGUAGE_DISPLAY_NAMES: &[(&str, &str)] = &[
     ("zig", "Zig"),
 ];
 
-/// Check if a command exists in PATH
-pub fn command_exists(cmd: &str) -> bool {
-    // Explicitly forward PATH so the login-shell-augmented PATH is visible.
-    let current_path = std::env::var("PATH").unwrap_or_default();
-    #[cfg(unix)]
-    {
-        Command::new("which")
-            .arg(cmd)
-            .env("PATH", &current_path)
-            .output()
-            .map(|output| output.status.success())
-            .unwrap_or(false)
-    }
-    #[cfg(windows)]
-    {
-        let mut command = Command::new("where");
-        command.arg(cmd).env("PATH", &current_path);
-        // Suppress console window on Windows.
-        app_platform::hide_console(&mut command);
-        command
-            .output()
-            .map(|output| output.status.success())
-            .unwrap_or(false)
-    }
-}
+pub use crate::command_detection::command_exists;
 
 /// Check if uninstall is supported based on install hint
 fn is_uninstall_supported(install_hint: &str) -> bool {

src-tauri/crates/lsp/src/commands/package_manager.rs

@@ -3,7 +3,7 @@
 //! Utilities for detecting installed package managers and extracting
 //! package names from install hints.
 
-use super::discovery::command_exists;
+use crate::command_detection::command_exists;
 
 #[cfg(test)]
 #[path = "tests/package_manager_tests.rs"]

src-tauri/crates/lsp/src/install_pipeline.rs

@@ -16,7 +16,7 @@ use std::path::{Path, PathBuf};
 use std::process::Stdio;
 use tokio::process::Command;
 
-use super::commands::discovery::command_exists;
+use super::command_detection::command_exists;
 use super::commands::package_manager::detect_package_manager;
 use app_paths::lsp_bin_dir;
 

src-tauri/crates/lsp/src/lib.rs

@@ -7,6 +7,7 @@
 
 pub mod broadcast;
 pub mod codec;
+mod command_detection;
 pub mod commands;
 pub mod config;
 pub mod eslint;