From b0b476c9e5158f6c5685b675eaef8ef44baf5666 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Wed, 22 Apr 2026 19:18:52 +0200 Subject: [PATCH] fix: harden external AI tool setup --- docs/ARCHITECTURE.md | 30 +- docs/GETTING-STARTED.md | 8 +- ...setup-and-least-privilege-desktop-scope.md | 36 +++ docs/adr/README.md | 3 +- src-tauri/src/ai_agents.rs | 15 +- src-tauri/src/commands/system.rs | 25 +- src-tauri/src/commands/vault/scan_cmds.rs | 6 +- src-tauri/src/lib.rs | 59 +++- src-tauri/src/mcp.rs | 224 +++++++++++---- src-tauri/src/menu.rs | 2 +- src-tauri/tauri.conf.json | 14 +- src/App.test.tsx | 31 +++ src/App.tsx | 48 +++- src/components/McpSetupDialog.test.tsx | 65 +++++ src/components/McpSetupDialog.tsx | 109 ++++++++ src/components/StatusBar.test.tsx | 19 +- src/components/status-bar/StatusBarBadges.tsx | 3 +- src/hooks/commands/settingsCommands.ts | 9 +- src/hooks/useCommandRegistry.test.ts | 13 +- src/hooks/useMcpStatus.test.ts | 257 +++++++++--------- src/hooks/useMcpStatus.ts | 98 ++++--- 21 files changed, 781 insertions(+), 293 deletions(-) create mode 100644 docs/adr/0074-explicit-external-ai-tool-setup-and-least-privilege-desktop-scope.md create mode 100644 src/components/McpSetupDialog.test.tsx create mode 100644 src/components/McpSetupDialog.tsx diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 6f46cadf..416c2fd8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -211,7 +211,7 @@ Full agent mode — spawns the selected local CLI agent as a subprocess with too 1. **Frontend** (`AiPanel` + `useCliAiAgent` + `aiAgents.ts`) — streaming UI with reasoning blocks, tool action cards, response display, onboarding, and default-agent selection 2. **Backend** (`ai_agents.rs`) — normalizes agent availability and streaming, dispatching to per-agent adapters -3. **Agent adapters** — Claude Code still uses `claude_cli.rs`; Codex runs through `codex exec --json` +3. **Agent adapters** — Claude Code still uses `claude_cli.rs`; Codex runs through `codex exec --json` with the CLI's normal approval / sandbox defaults 4. **MCP Integration** — Claude receives the generated MCP config file path, while Codex receives the same Tolaria MCP server via transient `-c mcp_servers.tolaria.*` config overrides #### Agent Event Flow @@ -305,13 +305,13 @@ The MCP server (`mcp-server/`) exposes vault operations as tools for AI assistan - Port **9710**: Tool bridge — AI/Claude clients call vault tools here - Port **9711**: UI bridge — Frontend listens for UI action broadcasts from MCP tools -### Auto-Registration +### Explicit External Tool Setup -On app startup, Tolaria automatically registers itself as an MCP server in: +Tolaria can register itself as an MCP server in: - `~/.claude/mcp.json` (Claude Code) - `~/.cursor/mcp.json` (Cursor) -Registration is non-destructive (additive, preserves other servers) and uses `upsert` semantics. The `useMcpStatus` hook tracks registration state (`checking | installed | not_installed | no_claude_cli`). +That setup is user-initiated through the status bar / command palette flow, not a startup side effect. Registration is non-destructive (additive, preserves other servers), uses `upsert` semantics, and can be reversed by removing Tolaria's entry again. The `useMcpStatus` hook tracks whether the active vault is explicitly connected (`checking | installed | not_installed`). ### Architecture @@ -330,7 +330,7 @@ flowchart TD end TAURI["Tauri (mcp.rs)"] -->|"spawn on startup"| MCP - TAURI -->|"auto-register"| CFG["~/.claude/mcp.json\n~/.cursor/mcp.json"] + UI["Status bar / Command Palette"] -->|"explicit setup or disconnect"| CFG["~/.claude/mcp.json\n~/.cursor/mcp.json"] ``` ### WebSocket Bridge @@ -357,10 +357,11 @@ flowchart LR | Function | Purpose | |----------|---------| | `spawn_ws_bridge(vault_path)` | Spawns `ws-bridge.js` as child process with VAULT_PATH env | -| `register_mcp(vault_path)` | Writes Tolaria entry to Claude Code and Cursor MCP configs | +| `register_mcp(vault_path)` | Writes Tolaria entry to Claude Code and Cursor MCP configs on explicit user request | +| `remove_mcp()` | Removes Tolaria's MCP entry from Claude Code and Cursor configs | | `upsert_mcp_config(path, entry)` | Atomic config file update (create/merge, preserves others) | -The `WsBridgeChild` state wrapper in `lib.rs` ensures the bridge process is killed on app exit via `RunEvent::Exit` handler. +The `WsBridgeChild` state wrapper in `lib.rs` ensures the bridge process is killed on app exit via `RunEvent::Exit` handler. The same desktop layer now keeps the Tauri asset protocol scoped to the active vault instead of every filesystem path. ## Search @@ -486,7 +487,7 @@ sequenceDiagram participant MCP as MCP Server participant U as User - T->>T: run_startup_tasks()
(register MCP) + T->>T: run_startup_tasks()
(migrate + seed only) T->>MCP: spawn_ws_bridge() — ports 9710 + 9711 T->>A: App mounts @@ -495,10 +496,10 @@ sequenceDiagram A-->>U: WelcomeScreen else Vault found A->>VL: useVaultLoader fires - VL->>T: invoke('list_vault') → scan_vault_cached() + VL->>T: invoke('reload_vault') → sync active vault asset scope + scan_vault_cached() T-->>VL: VaultEntry[] VL->>T: invoke('get_modified_files') - VL->>T: useMcpStatus — register if needed + A->>T: useMcpStatus — check explicit MCP setup state VL-->>A: entries ready end @@ -599,7 +600,7 @@ The vault backend (`src-tauri/src/vault/`) is split into focused submodules: | `search.rs` | Keyword search — walkdir-based vault file scan | | `ai_agents.rs` | Shared CLI-agent detection, stream normalization, and adapter dispatch | | `claude_cli.rs` | Claude Code subprocess spawning + NDJSON stream parsing | -| `mcp.rs` | MCP server spawning + config registration | +| `mcp.rs` | MCP server spawning + explicit config registration/removal | | `commands/` | Tauri command handlers (split into submodules) | | `settings.rs` | App settings persistence | | `vault_config.rs` | Per-vault UI config | @@ -623,7 +624,7 @@ The vault backend (`src-tauri/src/vault/`) is split into focused submodules: | `sync_note_title` | Legacy helper: rewrite `title` frontmatter from filename → `bool` (modified); not used by the normal note-open flow | | `batch_archive_notes` | Archive multiple notes | | `batch_delete_notes` | Permanently delete notes from disk | -| `reload_vault` | Invalidate cache and full rescan from filesystem → `Vec` | +| `reload_vault` | Sync the active vault asset scope, invalidate cache, and full rescan from filesystem → `Vec` | | `reload_vault_entry` | Re-read a single file from disk → `VaultEntry` | | `check_vault_exists` | Check if vault path exists | | `create_empty_vault` | Create a git-backed vault, then seed root `AGENTS.md`, `CLAUDE.md`, `type.md`, and `note.md` defaults | @@ -682,8 +683,9 @@ The vault backend (`src-tauri/src/vault/`) is split into focused submodules: | `check_claude_cli` | Check if Claude CLI is available | | `get_ai_agents_status` | Check Claude Code + Codex availability | | `stream_ai_agent` | Stream the selected CLI agent through the normalized event layer | -| `register_mcp_tools` | Register MCP in Claude/Cursor config | -| `check_mcp_status` | Check MCP registration state | +| `register_mcp_tools` | Register MCP in Claude/Cursor config for the active vault | +| `remove_mcp_tools` | Remove Tolaria's MCP entry from Claude/Cursor config | +| `check_mcp_status` | Check whether the active vault is explicitly registered in Claude/Cursor config | The desktop MCP WebSocket bridge is intentionally local-only. `mcp-server/ws-bridge.js` binds both bridge ports to loopback, rejects non-loopback clients, accepts browser/Tauri origins only on the UI bridge, and rejects browser-origin requests on the tool bridge so remote pages cannot drive vault tools directly. diff --git a/docs/GETTING-STARTED.md b/docs/GETTING-STARTED.md index e05e6b1d..fe325338 100644 --- a/docs/GETTING-STARTED.md +++ b/docs/GETTING-STARTED.md @@ -111,7 +111,7 @@ tolaria/ │ │ ├── useOnboarding.ts # First-launch flow │ │ ├── useCodeMirror.ts # CodeMirror raw editor │ │ ├── useMcpBridge.ts # MCP WebSocket client -│ │ ├── useMcpStatus.ts # MCP registration status +│ │ ├── useMcpStatus.ts # Explicit external AI tool connection status + connect/disconnect actions │ │ ├── useUpdater.ts # In-app updates │ │ └── ... │ │ @@ -165,7 +165,7 @@ tolaria/ │ │ ├── search.rs # Keyword search (walkdir-based) │ │ ├── ai_agents.rs # Shared CLI-agent detection + stream adapters │ │ ├── claude_cli.rs # Claude CLI subprocess management -│ │ ├── mcp.rs # MCP server lifecycle + registration +│ │ ├── mcp.rs # MCP server lifecycle + explicit config registration/removal │ │ ├── app_updater.rs # Alpha/stable updater endpoint selection │ │ ├── settings.rs # App settings persistence │ │ ├── vault_config.rs # Per-vault UI config @@ -234,7 +234,7 @@ tolaria/ | `src-tauri/src/frontmatter/ops.rs` | YAML manipulation — how properties are updated/deleted in files. | | `src-tauri/src/git/` | All git operations (clone, commit, pull, push, conflicts, pulse, add-remote). | | `src-tauri/src/search.rs` | Keyword search — scans vault files with walkdir. | -| `src-tauri/src/ai_agents.rs` | Shared CLI-agent availability checks, Codex adapter, and stream normalization. | +| `src-tauri/src/ai_agents.rs` | Shared CLI-agent availability checks, safe-default Codex adapter, and stream normalization. | | `src-tauri/src/claude_cli.rs` | Claude CLI subprocess spawning + NDJSON stream parsing. | | `src-tauri/src/app_updater.rs` | Desktop updater bridge — selects alpha/stable manifests and streams install progress. | @@ -389,4 +389,4 @@ BASE_URL="http://localhost:5173" npx playwright test tests/smoke/.spec.ts 2. **Context building**: Edit `src/utils/ai-context.ts` for what data is sent to the agent 3. **Tool action display**: Edit `src/components/AiActionCard.tsx` 4. **Claude CLI arguments**: Edit `src-tauri/src/claude_cli.rs` (`run_agent_stream()`) -5. **Shared agent adapters / Codex args**: Edit `src-tauri/src/ai_agents.rs` +5. **Shared agent adapters / Codex args**: Edit `src-tauri/src/ai_agents.rs` (keep Codex on the normal approval/sandbox path unless you are intentionally designing an advanced mode) diff --git a/docs/adr/0074-explicit-external-ai-tool-setup-and-least-privilege-desktop-scope.md b/docs/adr/0074-explicit-external-ai-tool-setup-and-least-privilege-desktop-scope.md new file mode 100644 index 00000000..8591d814 --- /dev/null +++ b/docs/adr/0074-explicit-external-ai-tool-setup-and-least-privilege-desktop-scope.md @@ -0,0 +1,36 @@ +--- +type: ADR +id: "0074" +title: "Explicit external AI tool setup and least-privilege desktop scope" +status: active +date: 2026-04-22 +--- + +## Context + +Tolaria's first MCP integration optimized for zero setup: desktop startup auto-registered the Tolaria MCP server in Claude Code and Cursor config files, the Tauri asset protocol allowed every local path, and app-managed Codex sessions launched with the CLI's dangerous bypass flag. That made the product feel convenient, but it also widened trust by default in places that users could not see or consent to clearly. + +The product direction now favors least-privilege defaults. Fresh installs should not silently edit third-party config files, external AI tool setup must be intentional and reversible, and the desktop shell should only expose the filesystem paths that the active vault actually needs. + +## Decision + +**Tolaria now treats external AI tool wiring as an explicit user action and keeps the desktop shell scoped to the active vault.** + +- The app still spawns its local MCP WebSocket bridge on desktop startup, but it no longer auto-registers third-party MCP config files. +- External MCP registration is exposed through a keyboard-accessible setup flow reachable from the command palette and status surfaces. Confirming the flow upserts Tolaria's MCP entry for the current vault; cancel leaves external config untouched; disconnect removes Tolaria's entry again. +- The Tauri asset protocol remains enabled for local vault images, but its static config scope is empty. Tolaria grants recursive asset access only to the active vault at runtime when that vault is reloaded. +- App-managed Codex sessions use the CLI's normal approval and sandbox path by default instead of opting into the dangerous bypass mode automatically. + +## Options considered + +- **Explicit setup + runtime vault-only scope** (chosen): aligns with least-privilege defaults, keeps command-palette discoverability, preserves image loading and external-tool support, and makes every privileged step visible and reversible. +- **Keep startup auto-registration and global asset scope**: lowest friction, but it silently mutates third-party config and leaves the desktop shell effectively open to every local file path. +- **Disable external MCP registration entirely**: safest on paper, but it removes a valuable workflow for Claude Code, Cursor, and other MCP-compatible tools that Tolaria intentionally supports. + +## Consequences + +- Fresh installs no longer modify `~/.claude/mcp.json` or `~/.cursor/mcp.json` until the user confirms setup. +- Switching vaults does not silently retarget external MCP clients; users reconnect explicitly when they want a different vault exposed. +- Desktop asset access is constrained to the active vault instead of all filesystem paths, while note images and attachments continue to load normally. +- The command palette and status bar now expose an explicit external AI tools setup/remove flow that supports keyboard-only QA. +- Codex agent sessions are safer by default, at the cost of relying on the CLI's normal approval path instead of bypassing it automatically. diff --git a/docs/adr/README.md b/docs/adr/README.md index ed6ae32c..69590669 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -66,7 +66,7 @@ proposed → active → superseded | [0008](0008-underscore-system-properties.md) | Underscore convention for system properties | active | | [0009](0009-keyword-only-search.md) | Keyword-only search (remove semantic indexing) | active | | [0010](0010-dynamic-wikilink-relationship-detection.md) | Dynamic wikilink relationship detection | active | -| [0011](0011-mcp-server-for-ai-integration.md) | MCP server for AI tool integration | active | +| [0011](0011-mcp-server-for-ai-integration.md) | MCP server for AI tool integration | superseded → [0074](0074-explicit-external-ai-tool-setup-and-least-privilege-desktop-scope.md) | | [0012](0012-claude-cli-for-ai-agent.md) | Claude CLI subprocess for AI agent | active | | [0013](0013-remove-theming-system.md) | Remove vault-based theming system | active | | [0014](0014-git-based-vault-cache.md) | Git-based incremental vault cache | active | @@ -129,3 +129,4 @@ proposed → active → superseded | [0071](0071-external-vault-refresh-and-clean-tab-reopen.md) | External vault updates reload derived state and reopen the clean active note | active | | [0072](0072-confirmed-vault-paths-gate-startup-state.md) | Confirmed vault paths gate startup state | active | | [0073](0073-persistent-linkify-protocol-registry-across-editor-remounts.md) | Persistent linkify protocol registry across editor remounts | active | +| [0074](0074-explicit-external-ai-tool-setup-and-least-privilege-desktop-scope.md) | Explicit external AI tool setup and least-privilege desktop scope | active | diff --git a/src-tauri/src/ai_agents.rs b/src-tauri/src/ai_agents.rs index 0fde2d6c..3051a372 100644 --- a/src-tauri/src/ai_agents.rs +++ b/src-tauri/src/ai_agents.rs @@ -246,7 +246,6 @@ fn build_codex_args(request: &AiAgentStreamRequest) -> Result, Strin Ok(vec![ "exec".into(), "--json".into(), - "--dangerously-bypass-approvals-and-sandbox".into(), "-C".into(), request.vault_path.clone(), "-c".into(), @@ -406,6 +405,20 @@ mod tests { assert!(prompt.contains("User request:\nRename the note")); } + #[test] + fn build_codex_args_uses_safe_default_permissions() { + if let Ok(args) = build_codex_args(&AiAgentStreamRequest { + agent: AiAgentId::Codex, + message: "Rename the note".into(), + system_prompt: None, + vault_path: "/tmp/vault".into(), + }) { + assert!(!args.contains(&"--dangerously-bypass-approvals-and-sandbox".to_string())); + assert!(args.contains(&"--json".to_string())); + assert!(args.contains(&"-C".to_string())); + } + } + #[test] fn dispatch_codex_command_events_maps_to_bash_events() { let mut events = Vec::new(); diff --git a/src-tauri/src/commands/system.rs b/src-tauri/src/commands/system.rs index c7a1d1fc..a97cba3c 100644 --- a/src-tauri/src/commands/system.rs +++ b/src-tauri/src/commands/system.rs @@ -26,10 +26,21 @@ pub async fn register_mcp_tools(vault_path: String) -> Result { #[cfg(desktop)] #[tauri::command] -pub async fn check_mcp_status() -> Result { - tokio::task::spawn_blocking(crate::mcp::check_mcp_status) +pub async fn remove_mcp_tools() -> Result { + tokio::task::spawn_blocking(crate::mcp::remove_mcp) .await - .map_err(|e| format!("MCP status check failed: {e}")) + .map_err(|e| format!("Removal task failed: {e}")) +} + +#[cfg(desktop)] +#[tauri::command] +pub async fn check_mcp_status(vault_path: String) -> Result { + let vault_path = super::expand_tilde(&vault_path).into_owned(); + Ok( + tokio::task::spawn_blocking(move || crate::mcp::check_mcp_status(&vault_path)) + .await + .map_err(|e| format!("MCP status check failed: {e}"))?, + ) } // ── MCP commands (mobile stubs) ───────────────────────────────────────────── @@ -42,7 +53,13 @@ pub async fn register_mcp_tools(_vault_path: String) -> Result { #[cfg(mobile)] #[tauri::command] -pub async fn check_mcp_status() -> Result { +pub async fn remove_mcp_tools() -> Result { + Err("MCP is not available on mobile".into()) +} + +#[cfg(mobile)] +#[tauri::command] +pub async fn check_mcp_status(_vault_path: String) -> Result { Ok(crate::mcp::McpStatus::NotInstalled) } diff --git a/src-tauri/src/commands/vault/scan_cmds.rs b/src-tauri/src/commands/vault/scan_cmds.rs index 3885ed74..8848a071 100644 --- a/src-tauri/src/commands/vault/scan_cmds.rs +++ b/src-tauri/src/commands/vault/scan_cmds.rs @@ -22,8 +22,12 @@ pub fn reload_vault_entry( } #[tauri::command] -pub async fn reload_vault(path: String) -> Result, String> { +pub async fn reload_vault( + app_handle: tauri::AppHandle, + path: String, +) -> Result, String> { let path = expand_tilde(&path).into_owned(); + crate::sync_vault_asset_scope(&app_handle, Path::new(&path))?; tokio::task::spawn_blocking(move || { vault::invalidate_cache(Path::new(&path)); vault::scan_vault_cached(Path::new(&path)) diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index bcb64987..0adee2a3 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -13,6 +13,8 @@ pub mod telemetry; pub mod vault; pub mod vault_list; +#[cfg(desktop)] +use std::path::Path; #[cfg(desktop)] use std::process::Child; #[cfg(desktop)] @@ -21,6 +23,9 @@ use std::sync::Mutex; #[cfg(desktop)] struct WsBridgeChild(Mutex>); +#[cfg(desktop)] +struct ActiveAssetScopeRoot(Mutex>); + #[cfg(desktop)] fn log_startup_result(label: &str, result: Result) { match result { @@ -48,12 +53,6 @@ fn run_startup_tasks() { vault::migrate_agents_md(vp_str); // Seed AGENTS.md and starter type definitions at vault root if missing vault::seed_config_files(vp_str); - - // Register Tolaria MCP server in Claude Code and Cursor configs - match mcp::register_mcp(vp_str) { - Ok(status) => log::info!("MCP registration: {status}"), - Err(e) => log::warn!("MCP registration failed: {e}"), - } } #[cfg(desktop)] @@ -148,6 +147,49 @@ fn setup_app(app: &mut tauri::App) -> Result<(), Box> { Ok(()) } +#[cfg(desktop)] +pub(crate) fn sync_vault_asset_scope( + app_handle: &tauri::AppHandle, + vault_path: &Path, +) -> Result<(), String> { + use tauri::Manager; + + let canonical_vault_path = std::fs::canonicalize(vault_path).map_err(|e| { + format!( + "Failed to resolve asset scope for {}: {e}", + vault_path.display() + ) + })?; + let scope = app_handle.asset_protocol_scope(); + let state: tauri::State<'_, ActiveAssetScopeRoot> = app_handle.state(); + let mut active_root = state + .0 + .lock() + .map_err(|_| "Failed to lock active asset scope state".to_string())?; + + if active_root.as_ref() == Some(&canonical_vault_path) { + return Ok(()); + } + + scope + .allow_directory(&canonical_vault_path, true) + .map_err(|e| { + format!( + "Failed to allow asset access for {}: {e}", + canonical_vault_path.display() + ) + })?; + + if let Some(previous_root) = active_root.as_ref() { + if previous_root != &canonical_vault_path { + let _ = scope.forbid_directory(previous_root, true); + } + } + + *active_root = Some(canonical_vault_path); + Ok(()) +} + macro_rules! app_invoke_handler { () => { tauri::generate_handler![ @@ -217,6 +259,7 @@ macro_rules! app_invoke_handler { commands::check_vault_exists, commands::get_default_vault_path, commands::register_mcp_tools, + commands::remove_mcp_tools, commands::check_mcp_status, commands::repair_vault, commands::reinit_telemetry, @@ -250,7 +293,9 @@ pub fn run() { let builder = tauri::Builder::default(); #[cfg(desktop)] - let builder = builder.manage(WsBridgeChild(Mutex::new(None))); + let builder = builder + .manage(WsBridgeChild(Mutex::new(None))) + .manage(ActiveAssetScopeRoot(Mutex::new(None))); with_invoke_handler(builder) .setup(setup_app) diff --git a/src-tauri/src/mcp.rs b/src-tauri/src/mcp.rs index 32e1ae86..43e8369c 100644 --- a/src-tauri/src/mcp.rs +++ b/src-tauri/src/mcp.rs @@ -11,10 +11,8 @@ const LEGACY_MCP_SERVER_NAME: &str = "laputa"; pub enum McpStatus { /// MCP is registered in Claude config and server files exist. Installed, - /// MCP server files or config are missing but can be installed. + /// MCP server files or config are missing for the active vault. NotInstalled, - /// Claude CLI is not installed — must install that first. - NoClaudeCli, } /// Find the `node` binary path at runtime. @@ -116,8 +114,14 @@ pub fn spawn_ws_bridge(vault_path: &str) -> Result { Ok(child) } -fn claude_mcp_config_path() -> Option { - dirs::home_dir().map(|home| home.join(".claude").join("mcp.json")) +fn mcp_config_paths() -> Vec { + [ + dirs::home_dir().map(|home| home.join(".claude").join("mcp.json")), + dirs::home_dir().map(|home| home.join(".cursor").join("mcp.json")), + ] + .into_iter() + .flatten() + .collect() } fn read_registered_mcp_entry(config_path: &Path) -> Option { @@ -142,6 +146,21 @@ fn entry_index_js_exists(entry: &serde_json::Value) -> bool { .is_some_and(|index_js| Path::new(index_js).exists()) } +fn entry_targets_vault(entry: &serde_json::Value, vault_path: &Path) -> bool { + let Some(entry_vault_path) = entry["env"]["VAULT_PATH"].as_str() else { + return false; + }; + + let Ok(expected) = std::fs::canonicalize(vault_path) else { + return false; + }; + let Ok(actual) = std::fs::canonicalize(entry_vault_path) else { + return false; + }; + + actual == expected +} + /// Build the MCP server entry JSON for a given vault path and index.js path. fn build_mcp_entry(index_js: &str, vault_path: &str) -> serde_json::Value { serde_json::json!({ @@ -172,15 +191,7 @@ pub fn register_mcp(vault_path: &str) -> Result { let entry = build_mcp_entry(&index_js, vault_path); - let configs: Vec = [ - dirs::home_dir().map(|h| h.join(".claude").join("mcp.json")), - dirs::home_dir().map(|h| h.join(".cursor").join("mcp.json")), - ] - .into_iter() - .flatten() - .collect(); - - Ok(register_mcp_to_configs(&entry, &configs)) + Ok(register_mcp_to_configs(&entry, &mcp_config_paths())) } /// Insert or update the Tolaria entry in an MCP config file. @@ -222,29 +233,79 @@ fn upsert_mcp_config(config_path: &Path, entry: &serde_json::Value) -> Result String { + let mut removed_any = false; + for config_path in config_paths { + match remove_mcp_from_config(config_path) { + Ok(true) => removed_any = true, + Ok(false) => {} + Err(e) => log::warn!("Failed to update {}: {}", config_path.display(), e), + } + } + + if removed_any { + "removed".to_string() + } else { + "already_absent".to_string() + } +} + +fn remove_mcp_from_config(config_path: &Path) -> Result { + if !config_path.exists() { + return Ok(false); + } + + let raw = std::fs::read_to_string(config_path) + .map_err(|e| format!("Cannot read {}: {e}", config_path.display()))?; + let mut config: serde_json::Value = serde_json::from_str(&raw) + .map_err(|e| format!("Invalid JSON in {}: {e}", config_path.display()))?; + + let Some(config_object) = config.as_object_mut() else { + return Err("Config is not a JSON object".into()); + }; + + let Some(servers_value) = config_object.get_mut("mcpServers") else { + return Ok(false); + }; + + let Some(servers) = servers_value.as_object_mut() else { + return Err("mcpServers is not a JSON object".into()); + }; + + let removed_primary = servers.remove(MCP_SERVER_NAME).is_some(); + let removed_legacy = servers.remove(LEGACY_MCP_SERVER_NAME).is_some(); + if !removed_primary && !removed_legacy { + return Ok(false); + } + + if servers.is_empty() { + config_object.remove("mcpServers"); + } + + let json = serde_json::to_string_pretty(&config) + .map_err(|e| format!("Failed to serialize config: {e}"))?; + std::fs::write(config_path, json) + .map_err(|e| format!("Cannot write {}: {e}", config_path.display()))?; + + Ok(true) +} + +pub fn remove_mcp() -> String { + remove_mcp_from_configs(&mcp_config_paths()) +} + /// Check whether the MCP server is properly installed and registered. /// -/// Returns `Installed` when the Tolaria entry exists in `~/.claude/mcp.json` -/// and the referenced index.js file is present. Returns `NoClaudeCli` when -/// the Claude CLI binary cannot be found. Otherwise returns `NotInstalled`. -pub fn check_mcp_status() -> McpStatus { - // Check Claude CLI first — no point installing MCP if Claude isn't available - if crate::claude_cli::find_claude_binary().is_err() { - return McpStatus::NoClaudeCli; - } - - let Some(config_path) = claude_mcp_config_path() else { - return McpStatus::NotInstalled; - }; - if !config_path.exists() { - return McpStatus::NotInstalled; - } - - let Some(entry) = read_registered_mcp_entry(&config_path) else { - return McpStatus::NotInstalled; - }; - - if entry_index_js_exists(&entry) { +/// Returns `Installed` when the Tolaria entry exists for the active vault in +/// Claude Code or Cursor config and the referenced index.js file is present. +/// Otherwise returns `NotInstalled`. +pub fn check_mcp_status(vault_path: &str) -> McpStatus { + let active_vault_path = Path::new(vault_path); + if mcp_config_paths().into_iter().any(|config_path| { + read_registered_mcp_entry(&config_path).is_some_and(|entry| { + entry_index_js_exists(&entry) && entry_targets_vault(&entry, active_vault_path) + }) + }) { McpStatus::Installed } else { McpStatus::NotInstalled @@ -260,6 +321,12 @@ mod tests { serde_json::from_str(&raw).unwrap() } + fn write_index_js(dir: &Path) -> PathBuf { + let index_js = dir.join("index.js"); + std::fs::write(&index_js, "console.log('ok');").unwrap(); + index_js + } + #[test] fn build_mcp_entry_produces_correct_json() { let entry = build_mcp_entry("/path/to/index.js", "/my/vault"); @@ -566,18 +633,79 @@ mod tests { } #[test] - fn check_mcp_status_returns_valid_variant() { - // On a dev machine with Claude CLI and MCP registered, this should be Installed. - // On CI without Claude it might be NoClaudeCli. Either way it must not panic. - let status = check_mcp_status(); - assert!( - matches!( - status, - McpStatus::Installed | McpStatus::NotInstalled | McpStatus::NoClaudeCli - ), - "unexpected status: {:?}", - status - ); + fn remove_mcp_from_config_removes_primary_and_legacy_entries() { + let tmp = tempfile::tempdir().unwrap(); + let config_path = tmp.path().join("mcp.json"); + let config = serde_json::json!({ + "mcpServers": { + "tolaria": { "command": "node", "args": ["/index.js"] }, + "laputa": { "command": "node", "args": ["/legacy.js"] }, + "other-server": { "command": "other", "args": [] } + } + }); + std::fs::write(&config_path, serde_json::to_string(&config).unwrap()).unwrap(); + + let removed = remove_mcp_from_config(&config_path).unwrap(); + assert!(removed); + + let updated = read_config(&config_path); + assert!(updated["mcpServers"][MCP_SERVER_NAME].is_null()); + assert!(updated["mcpServers"][LEGACY_MCP_SERVER_NAME].is_null()); + assert!(updated["mcpServers"]["other-server"].is_object()); + } + + #[test] + fn remove_mcp_from_config_returns_false_when_entry_missing() { + let tmp = tempfile::tempdir().unwrap(); + let config_path = tmp.path().join("mcp.json"); + let config = serde_json::json!({ + "mcpServers": { + "other-server": { "command": "other", "args": [] } + } + }); + std::fs::write(&config_path, serde_json::to_string(&config).unwrap()).unwrap(); + + let removed = remove_mcp_from_config(&config_path).unwrap(); + assert!(!removed); + } + + #[test] + fn check_mcp_status_returns_installed_for_matching_vault() { + let tmp = tempfile::tempdir().unwrap(); + let vault_path = tmp.path().join("vault"); + std::fs::create_dir_all(&vault_path).unwrap(); + let index_js = write_index_js(tmp.path()); + let config_path = tmp.path().join("mcp.json"); + let config = serde_json::json!({ + "mcpServers": { + "tolaria": { + "command": "node", + "args": [index_js.to_string_lossy()], + "env": { "VAULT_PATH": vault_path.to_string_lossy() } + } + } + }); + std::fs::write(&config_path, serde_json::to_string(&config).unwrap()).unwrap(); + + let entry = read_registered_mcp_entry(&config_path).unwrap(); + assert!(entry_targets_vault(&entry, &vault_path)); + assert!(entry_index_js_exists(&entry)); + } + + #[test] + fn entry_targets_vault_requires_matching_existing_directory() { + let tmp = tempfile::tempdir().unwrap(); + let first_vault = tmp.path().join("vault-a"); + let second_vault = tmp.path().join("vault-b"); + std::fs::create_dir_all(&first_vault).unwrap(); + std::fs::create_dir_all(&second_vault).unwrap(); + + let entry = serde_json::json!({ + "env": { "VAULT_PATH": first_vault.to_string_lossy() } + }); + + assert!(entry_targets_vault(&entry, &first_vault)); + assert!(!entry_targets_vault(&entry, &second_vault)); } #[test] @@ -586,7 +714,5 @@ mod tests { assert_eq!(json, r#""installed""#); let json = serde_json::to_string(&McpStatus::NotInstalled).unwrap(); assert_eq!(json, r#""not_installed""#); - let json = serde_json::to_string(&McpStatus::NoClaudeCli).unwrap(); - assert_eq!(json, r#""no_claude_cli""#); } } diff --git a/src-tauri/src/menu.rs b/src-tauri/src/menu.rs index 59153494..08726d5c 100644 --- a/src-tauri/src/menu.rs +++ b/src-tauri/src/menu.rs @@ -367,7 +367,7 @@ fn build_vault_menu(app: &App) -> MenuResult { let view_changes = MenuItemBuilder::new("View Pending Changes") .id(VAULT_VIEW_CHANGES) .build(app)?; - let install_mcp = MenuItemBuilder::new("Restore MCP Server") + let install_mcp = MenuItemBuilder::new("Set Up External AI Tools…") .id(VAULT_INSTALL_MCP) .build(app)?; let reload = MenuItemBuilder::new("Reload Vault") diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index cad1d7dc..91dcfc27 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -27,12 +27,18 @@ } ], "security": { - "csp": null, + "csp": { + "default-src": "'self' ipc: http://ipc.localhost", + "connect-src": "'self' ipc: http://ipc.localhost ws://localhost:9710 ws://127.0.0.1:9710 ws://localhost:9711 ws://127.0.0.1:9711 https:", + "img-src": "'self' asset: http://asset.localhost data: blob: https:", + "style-src": "'self' 'unsafe-inline'", + "font-src": "'self' data:", + "media-src": "'self' data: blob: https:", + "object-src": "'none'" + }, "assetProtocol": { "enable": true, - "scope": [ - "**" - ] + "scope": [] } } }, diff --git a/src/App.test.tsx b/src/App.test.tsx index 5f503023..6fab9ab7 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -328,6 +328,7 @@ vi.mock('./components/tolariaEditorFormatting', () => ({ import App from './App' +const AI_AGENTS_ONBOARDING_DISMISSED_KEY = 'tolaria:ai-agents-onboarding-dismissed' const CLAUDE_CODE_ONBOARDING_DISMISSED_KEY = 'tolaria:claude-code-onboarding-dismissed' describe('App', () => { @@ -379,6 +380,36 @@ describe('App', () => { }) }) + it('shows the external AI setup dialog from the menu when AI onboarding is active', async () => { + localStorage.removeItem(AI_AGENTS_ONBOARDING_DISMISSED_KEY) + localStorage.removeItem(CLAUDE_CODE_ONBOARDING_DISMISSED_KEY) + mockCommandResults.get_ai_agents_status = { + claude_code: { installed: true, version: '2.1.90' }, + codex: { installed: true, version: '0.122.0-alpha.1' }, + } + mockCommandResults.check_mcp_status = 'installed' + + render() + + await waitFor(() => { + expect(screen.getByText('AI agents ready')).toBeInTheDocument() + }) + + await waitFor(() => { + expect(typeof window.__laputaTest?.dispatchBrowserMenuCommand).toBe('function') + }) + + act(() => { + window.__laputaTest?.dispatchBrowserMenuCommand?.('vault-install-mcp') + }) + + await waitFor(() => { + expect(screen.getByText('Manage External AI Tools')).toBeInTheDocument() + }) + expect(screen.getByTestId('mcp-setup-dialog')).toBeInTheDocument() + expect(screen.queryByText('AI agents ready')).not.toBeInTheDocument() + }) + it('shows onboarding after telemetry consent when no active vault is configured', async () => { mockCommandResults.get_settings = { auto_pull_interval_minutes: null, diff --git a/src/App.tsx b/src/App.tsx index 29f94aea..e99cb040 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -19,6 +19,7 @@ import { WelcomeScreen } from './components/WelcomeScreen' import { AiAgentsOnboardingPrompt } from './components/AiAgentsOnboardingPrompt' import { TelemetryConsentDialog } from './components/TelemetryConsentDialog' import { FeedbackDialog } from './components/FeedbackDialog' +import { McpSetupDialog } from './components/McpSetupDialog' import { useTelemetry } from './hooks/useTelemetry' import { useMcpStatus } from './hooks/useMcpStatus' import { useAiAgentsOnboarding } from './hooks/useAiAgentsOnboarding' @@ -222,6 +223,8 @@ function App() { const dialogs = useDialogs() const { showAIChat, toggleAIChat } = dialogs const [showFeedback, setShowFeedback] = useState(false) + const [showMcpSetupDialog, setShowMcpSetupDialog] = useState(false) + const [mcpDialogAction, setMcpDialogAction] = useState<'connect' | 'disconnect' | null>(null) const openFeedback = useCallback(() => setShowFeedback(true), []) const closeFeedback = useCallback(() => setShowFeedback(false), []) const networkStatus = useNetworkStatus() @@ -405,9 +408,38 @@ function App() { trackEvent('vault_opened', { has_git: gitRepoState === 'ready' ? 1 : 0, note_count: vault.entries.length }) } }, [vault.entries.length, gitRepoState, resolvedPath]) - const { mcpStatus, installMcp } = useMcpStatus(resolvedPath, setToastMessage) + const { mcpStatus, connectMcp, disconnectMcp } = useMcpStatus(resolvedPath, setToastMessage) const gitRemoteStatus = useGitRemoteStatus(resolvedPath) + const openMcpSetupDialog = useCallback(() => { + setShowMcpSetupDialog(true) + }, []) + + const closeMcpSetupDialog = useCallback(() => { + if (mcpDialogAction !== null) return + setShowMcpSetupDialog(false) + }, [mcpDialogAction]) + + const handleConnectMcp = useCallback(async () => { + setMcpDialogAction('connect') + try { + const didConnect = await connectMcp() + if (didConnect) setShowMcpSetupDialog(false) + } finally { + setMcpDialogAction(null) + } + }, [connectMcp]) + + const handleDisconnectMcp = useCallback(async () => { + setMcpDialogAction('disconnect') + try { + const didDisconnect = await disconnectMcp() + if (didDisconnect) setShowMcpSetupDialog(false) + } finally { + setMcpDialogAction(null) + } + }, [disconnectMcp]) + // Detect external file renames on window focus const [detectedRenames, setDetectedRenames] = useState([]) useEffect(() => { @@ -1168,7 +1200,7 @@ function App() { isGettingStartedHidden: vaultSwitcher.isGettingStartedHidden, vaultCount: vaultSwitcher.allVaults.length, mcpStatus, - onInstallMcp: installMcp, + onInstallMcp: openMcpSetupDialog, onOpenAiAgents: dialogs.openSettings, aiAgentsStatus, vaultAiGuidanceStatus, @@ -1258,7 +1290,12 @@ function App() { return } - if (!noteWindowParams && onboarding.state.status === 'ready' && aiAgentsOnboarding.showPrompt) { + if ( + !noteWindowParams + && onboarding.state.status === 'ready' + && aiAgentsOnboarding.showPrompt + && !showMcpSetupDialog + ) { return ( <> - handleSetSelection({ kind: 'filter', filter: 'changes' })} onClickPulse={() => handleSetSelection({ kind: 'filter', filter: 'pulse' })} onCommitPush={handleCommitPush} isOffline={networkStatus.isOffline} isGitVault={isGitVault} syncStatus={autoSync.syncStatus} lastSyncTime={autoSync.lastSyncTime} conflictCount={autoSync.conflictFiles.length} remoteStatus={autoSync.remoteStatus} onTriggerSync={autoSync.triggerSync} onPullAndPush={autoSync.pullAndPush} onOpenConflictResolver={conflictFlow.handleOpenConflictResolver} zoomLevel={zoom.zoomLevel} onZoomReset={zoom.zoomReset} buildNumber={buildNumber} onCheckForUpdates={handleCheckForUpdates} onRemoveVault={vaultSwitcher.removeVault} mcpStatus={mcpStatus} onInstallMcp={installMcp} aiAgentsStatus={aiAgentsStatus} vaultAiGuidanceStatus={vaultAiGuidanceStatus} defaultAiAgent={aiAgentPreferences.defaultAiAgent} onSetDefaultAiAgent={aiAgentPreferences.setDefaultAiAgent} onRestoreVaultAiGuidance={() => { void restoreVaultAiGuidance() }} /> + handleSetSelection({ kind: 'filter', filter: 'changes' })} onClickPulse={() => handleSetSelection({ kind: 'filter', filter: 'pulse' })} onCommitPush={handleCommitPush} isOffline={networkStatus.isOffline} isGitVault={isGitVault} syncStatus={autoSync.syncStatus} lastSyncTime={autoSync.lastSyncTime} conflictCount={autoSync.conflictFiles.length} remoteStatus={autoSync.remoteStatus} onTriggerSync={autoSync.triggerSync} onPullAndPush={autoSync.pullAndPush} onOpenConflictResolver={conflictFlow.handleOpenConflictResolver} zoomLevel={zoom.zoomLevel} onZoomReset={zoom.zoomReset} buildNumber={buildNumber} onCheckForUpdates={handleCheckForUpdates} onRemoveVault={vaultSwitcher.removeVault} mcpStatus={mcpStatus} onInstallMcp={openMcpSetupDialog} aiAgentsStatus={aiAgentsStatus} vaultAiGuidanceStatus={vaultAiGuidanceStatus} defaultAiAgent={aiAgentPreferences.defaultAiAgent} onSetDefaultAiAgent={aiAgentPreferences.setDefaultAiAgent} onRestoreVaultAiGuidance={() => { void restoreVaultAiGuidance() }} /> setToastMessage(null)} /> @@ -1405,6 +1442,7 @@ function App() { /> + {deleteActions.confirmDelete && ( { + it('renders the explicit setup flow without mutating config by default', () => { + render( + , + ) + + expect(screen.getByText('Set Up External AI Tools')).toBeInTheDocument() + expect(screen.getByText(/will not touch third-party config files until you confirm here/i)).toBeInTheDocument() + expect(screen.getByTestId('mcp-setup-connect')).toHaveTextContent('Connect External AI Tools') + expect(screen.queryByTestId('mcp-setup-disconnect')).not.toBeInTheDocument() + }) + + it('renders reconnect and disconnect actions for an already connected vault', () => { + render( + , + ) + + expect(screen.getByText('Manage External AI Tools')).toBeInTheDocument() + expect(screen.getByTestId('mcp-setup-connect')).toHaveTextContent('Reconnect External AI Tools') + expect(screen.getByTestId('mcp-setup-disconnect')).toHaveTextContent('Disconnect') + }) + + it('routes actions through the dialog buttons', () => { + const onClose = vi.fn() + const onConnect = vi.fn() + const onDisconnect = vi.fn() + + render( + , + ) + + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })) + fireEvent.click(screen.getByTestId('mcp-setup-connect')) + fireEvent.click(screen.getByTestId('mcp-setup-disconnect')) + + expect(onClose).toHaveBeenCalledOnce() + expect(onConnect).toHaveBeenCalledOnce() + expect(onDisconnect).toHaveBeenCalledOnce() + }) +}) diff --git a/src/components/McpSetupDialog.tsx b/src/components/McpSetupDialog.tsx new file mode 100644 index 00000000..2ba48343 --- /dev/null +++ b/src/components/McpSetupDialog.tsx @@ -0,0 +1,109 @@ +import { ShieldCheck } from 'lucide-react' +import { Button } from '@/components/ui/button' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog' +import type { McpStatus } from '../hooks/useMcpStatus' + +interface McpSetupDialogProps { + open: boolean + status: McpStatus + busyAction: 'connect' | 'disconnect' | null + onClose: () => void + onConnect: () => void + onDisconnect: () => void +} + +function isConnected(status: McpStatus): boolean { + return status === 'installed' +} + +function actionCopy(status: McpStatus) { + if (isConnected(status)) { + return { + description: 'Tolaria is already connected to external AI tools for this vault. Reconnect to refresh the configuration, or disconnect to remove Tolaria from those third-party config files.', + primaryLabel: 'Reconnect External AI Tools', + secondaryLabel: 'Disconnect', + title: 'Manage External AI Tools', + } + } + + return { + description: 'Tolaria can add its MCP server to external AI tools for this vault, but it will not touch third-party config files until you confirm here.', + primaryLabel: 'Connect External AI Tools', + secondaryLabel: null, + title: 'Set Up External AI Tools', + } +} + +export function McpSetupDialog({ + open, + status, + busyAction, + onClose, + onConnect, + onDisconnect, +}: McpSetupDialogProps) { + const copy = actionCopy(status) + const connectBusy = busyAction === 'connect' + const disconnectBusy = busyAction === 'disconnect' + const buttonsDisabled = busyAction !== null || status === 'checking' + + return ( + { if (!next) onClose() }}> + + + + + {copy.title} + + {copy.description} + + +
+

+ Confirming this action will write or update Tolaria's single tolaria MCP entry in: +

+
+
~/.claude/mcp.json
+
~/.cursor/mcp.json
+
+

+ The entry points those tools at the current vault. Cancel leaves both files untouched, reconnect is idempotent, and disconnect removes Tolaria's entry again. +

+
+ + + + {copy.secondaryLabel ? ( + + ) : null} + + + + + ) +} diff --git a/src/components/StatusBar.test.tsx b/src/components/StatusBar.test.tsx index 666e321c..c70e8abf 100644 --- a/src/components/StatusBar.test.tsx +++ b/src/components/StatusBar.test.tsx @@ -316,15 +316,7 @@ describe('StatusBar', () => { ) expect(screen.getByTestId('status-mcp')).toBeInTheDocument() - await expectTooltip(screen.getByRole('button', { name: 'MCP server not installed — click to install' }), 'MCP server not installed — click to install') - }) - - it('shows MCP warning badge when status is no_claude_cli', async () => { - render( - - ) - expect(screen.getByTestId('status-mcp')).toBeInTheDocument() - await expectTooltip(screen.getByRole('button', { name: 'Claude CLI not found — install it first' }), 'Claude CLI not found — install it first') + await expectTooltip(screen.getByRole('button', { name: 'External AI tools not connected — click to set up' }), 'External AI tools not connected — click to set up') }) it('hides MCP badge when status is installed', () => { @@ -357,15 +349,6 @@ describe('StatusBar', () => { expect(onInstallMcp).toHaveBeenCalledOnce() }) - it('does not call onInstallMcp when clicking MCP badge with no_claude_cli status', () => { - const onInstallMcp = vi.fn() - render( - - ) - fireEvent.click(screen.getByTestId('status-mcp')) - expect(onInstallMcp).not.toHaveBeenCalled() - }) - it('shows Pull required label when syncStatus is pull_required', () => { render( diff --git a/src/components/status-bar/StatusBarBadges.tsx b/src/components/status-bar/StatusBarBadges.tsx index ef2b0524..6623ee35 100644 --- a/src/components/status-bar/StatusBarBadges.tsx +++ b/src/components/status-bar/StatusBarBadges.tsx @@ -40,8 +40,7 @@ const SYNC_COLORS: Record = { } const MCP_TOOLTIPS: Partial> = { - not_installed: 'MCP server not installed — click to install', - no_claude_cli: 'Claude CLI not found — install it first', + not_installed: 'External AI tools not connected — click to set up', } const CLAUDE_INSTALL_URL = 'https://docs.anthropic.com/en/docs/claude-code' diff --git a/src/hooks/commands/settingsCommands.ts b/src/hooks/commands/settingsCommands.ts index ef3c1390..a1692d0d 100644 --- a/src/hooks/commands/settingsCommands.ts +++ b/src/hooks/commands/settingsCommands.ts @@ -59,7 +59,14 @@ function buildMaintenanceCommands({ onRepairVault, }: Pick): CommandAction[] { return [ - { id: 'install-mcp', label: mcpStatus === 'installed' ? 'Restore MCP Server' : 'Install MCP Server', group: 'Settings', keywords: ['mcp', 'claude', 'ai', 'tools', 'install', 'restore', 'fix', 'repair'], enabled: true, execute: () => onInstallMcp?.() }, + { + id: 'install-mcp', + label: mcpStatus === 'installed' ? 'Manage External AI Tools…' : 'Set Up External AI Tools…', + group: 'Settings', + keywords: ['mcp', 'ai', 'tools', 'external', 'setup', 'connect', 'disconnect', 'claude', 'codex', 'cursor', 'consent'], + enabled: true, + execute: () => onInstallMcp?.(), + }, { id: 'reload-vault', label: 'Reload Vault', group: 'Settings', keywords: ['reload', 'refresh', 'rescan', 'sync', 'filesystem', 'cache'], enabled: !!onReloadVault, execute: () => onReloadVault?.() }, { id: 'repair-vault', label: 'Repair Vault', group: 'Settings', keywords: ['repair', 'fix', 'restore', 'config', 'agents', 'themes', 'missing', 'reset', 'flatten', 'structure'], enabled: !!onRepairVault, execute: () => onRepairVault?.() }, ] diff --git a/src/hooks/useCommandRegistry.test.ts b/src/hooks/useCommandRegistry.test.ts index 1554a99b..85081f74 100644 --- a/src/hooks/useCommandRegistry.test.ts +++ b/src/hooks/useCommandRegistry.test.ts @@ -462,15 +462,15 @@ describe('install-mcp command', () => { const cmd = findCommand(result.current, 'install-mcp') expect(cmd).toBeDefined() expect(cmd!.enabled).toBe(true) - expect(cmd!.label).toBe('Install MCP Server') + expect(cmd!.label).toBe('Set Up External AI Tools…') }) - it('is enabled when mcpStatus is installed and handler provided (restore use case)', () => { + it('is enabled when mcpStatus is installed and handler provided (manage use case)', () => { const config = makeConfig({ mcpStatus: 'installed', onInstallMcp: vi.fn() }) const { result } = renderHook(() => useCommandRegistry(config)) const cmd = findCommand(result.current, 'install-mcp') expect(cmd!.enabled).toBe(true) - expect(cmd!.label).toBe('Restore MCP Server') + expect(cmd!.label).toBe('Manage External AI Tools…') }) it('is enabled even when mcpStatus is checking', () => { @@ -487,13 +487,14 @@ describe('install-mcp command', () => { expect(cmd!.enabled).toBe(true) }) - it('has restore keyword for discoverability', () => { + it('has setup keywords for discoverability', () => { const config = makeConfig({ mcpStatus: 'installed', onInstallMcp: vi.fn() }) const { result } = renderHook(() => useCommandRegistry(config)) const cmd = findCommand(result.current, 'install-mcp') - expect(cmd!.keywords).toContain('restore') + expect(cmd!.keywords).toContain('setup') + expect(cmd!.keywords).toContain('external') expect(cmd!.keywords).toContain('mcp') - expect(cmd!.keywords).toContain('claude') + expect(cmd!.keywords).toContain('cursor') }) it('executes onInstallMcp callback', () => { diff --git a/src/hooks/useMcpStatus.test.ts b/src/hooks/useMcpStatus.test.ts index caad674f..3bbd72eb 100644 --- a/src/hooks/useMcpStatus.test.ts +++ b/src/hooks/useMcpStatus.test.ts @@ -13,166 +13,151 @@ vi.mock('../mock-tauri', () => ({ const { mockInvoke } = await import('../mock-tauri') as { mockInvoke: ReturnType } +function mockCommands(handlers: Partial>) { + mockInvoke.mockImplementation((command: string) => { + if (command in handlers) return Promise.resolve(handlers[command]) + return Promise.resolve(null) + }) +} + +function renderSubject(onToast = vi.fn()) { + return renderHook(() => useMcpStatus('/vault', onToast)) +} + +function mockStatusFlow( + initialStatus: 'installed' | 'not_installed', + overrides: Partial> = {}, +) { + mockInvoke.mockImplementation((command: string) => { + if (command === 'check_mcp_status') return Promise.resolve(initialStatus) + if (command in overrides) { + const result = overrides[command as keyof typeof overrides] + if (result instanceof Error) return Promise.reject(result) + return Promise.resolve(result) + } + return Promise.resolve(null) + }) +} + +async function renderReadySubject(initialStatus: 'installed' | 'not_installed') { + const onToast = vi.fn() + const hook = renderSubject(onToast) + await waitFor(() => { + expect(hook.result.current.mcpStatus).toBe(initialStatus) + }) + return { onToast, ...hook } +} + +async function runMutationScenario({ + action, + initialStatus, + overrideKey, + overrideValue, +}: { + action: 'connect' | 'disconnect' + initialStatus: 'installed' | 'not_installed' + overrideKey: 'register_mcp_tools' | 'remove_mcp_tools' + overrideValue: unknown +}) { + mockStatusFlow(initialStatus, { [overrideKey]: overrideValue }) + const hook = await renderReadySubject(initialStatus) + + await act(async () => { + if (action === 'connect') { + await hook.result.current.connectMcp() + return + } + await hook.result.current.disconnectMcp() + }) + + return hook +} + describe('useMcpStatus', () => { beforeEach(() => { vi.clearAllMocks() }) - it('starts in checking state and resolves to installed', async () => { - mockInvoke.mockImplementation((cmd: string) => { - if (cmd === 'check_mcp_status') return Promise.resolve('installed') - if (cmd === 'register_mcp_tools') return Promise.resolve('updated') - return Promise.resolve(null) + it('checks the active vault status without auto-registering on mount', async () => { + mockCommands({ + check_mcp_status: 'installed', }) - const onToast = vi.fn() - const { result } = renderHook(() => useMcpStatus('/vault', onToast)) + const { result } = renderSubject() expect(result.current.mcpStatus).toBe('checking') await waitFor(() => { expect(result.current.mcpStatus).toBe('installed') }) + + expect(mockInvoke).toHaveBeenCalledWith('check_mcp_status', { vaultPath: '/vault' }) + expect(mockInvoke).not.toHaveBeenCalledWith('register_mcp_tools', { vaultPath: '/vault' }) }) - it('resolves to not_installed when check returns not_installed', async () => { - mockInvoke.mockImplementation((cmd: string) => { - if (cmd === 'check_mcp_status') return Promise.resolve('not_installed') - if (cmd === 'register_mcp_tools') return Promise.reject(new Error('fail')) - return Promise.resolve(null) + it('resolves to not_installed when the active vault is not connected', async () => { + mockCommands({ + check_mcp_status: 'not_installed', }) - const onToast = vi.fn() - const { result } = renderHook(() => useMcpStatus('/vault', onToast)) + const { result } = renderSubject() await waitFor(() => { expect(result.current.mcpStatus).toBe('not_installed') }) }) - it('resolves to no_claude_cli when check returns no_claude_cli', async () => { - mockInvoke.mockImplementation((cmd: string) => { - if (cmd === 'check_mcp_status') return Promise.resolve('no_claude_cli') - if (cmd === 'register_mcp_tools') return Promise.reject(new Error('no cli')) - return Promise.resolve(null) + it.each([ + { + action: 'connect' as const, + commandArgs: { vaultPath: '/vault' }, + expectedStatus: 'installed' as const, + initialStatus: 'not_installed' as const, + name: 'connects external AI tools for the current vault on demand', + overrideKey: 'register_mcp_tools' as const, + overrideValue: 'registered', + toastFragment: 'Tolaria external AI tools connected successfully', + }, + { + action: 'connect' as const, + commandArgs: { vaultPath: '/vault' }, + expectedStatus: 'not_installed' as const, + initialStatus: 'not_installed' as const, + name: 'reports setup failures without mutating the active status silently', + overrideKey: 'register_mcp_tools' as const, + overrideValue: new Error('disk full'), + toastFragment: 'External AI tool setup failed', + }, + { + action: 'disconnect' as const, + commandArgs: undefined, + expectedStatus: 'not_installed' as const, + initialStatus: 'installed' as const, + name: 'disconnects external AI tools explicitly', + overrideKey: 'remove_mcp_tools' as const, + overrideValue: 'removed', + toastFragment: 'Tolaria external AI tools disconnected successfully', + }, + { + action: 'disconnect' as const, + commandArgs: undefined, + expectedStatus: 'installed' as const, + initialStatus: 'installed' as const, + name: 'refreshes status after a disconnect failure', + overrideKey: 'remove_mcp_tools' as const, + overrideValue: new Error('permission denied'), + toastFragment: 'External AI tool disconnect failed', + }, + ])('$name', async ({ action, commandArgs, expectedStatus, initialStatus, overrideKey, overrideValue, toastFragment }) => { + const { result, onToast } = await runMutationScenario({ + action, + initialStatus, + overrideKey, + overrideValue, }) - const onToast = vi.fn() - const { result } = renderHook(() => useMcpStatus('/vault', onToast)) - - await waitFor(() => { - expect(result.current.mcpStatus).toBe('no_claude_cli') - }) - }) - - it('install action calls register_mcp_tools and updates status', async () => { - // Auto-register fails (e.g. node not found), leaving status as not_installed - mockInvoke.mockImplementation((cmd: string) => { - if (cmd === 'check_mcp_status') return Promise.resolve('not_installed') - if (cmd === 'register_mcp_tools') return Promise.reject(new Error('no node')) - return Promise.resolve(null) - }) - - const onToast = vi.fn() - const { result } = renderHook(() => useMcpStatus('/vault', onToast)) - - await waitFor(() => { - expect(result.current.mcpStatus).toBe('not_installed') - }) - - // Now manual install succeeds - mockInvoke.mockImplementation((cmd: string) => { - if (cmd === 'register_mcp_tools') return Promise.resolve('registered') - return Promise.resolve(null) - }) - - await act(async () => { - await result.current.installMcp() - }) - - expect(result.current.mcpStatus).toBe('installed') - expect(onToast).toHaveBeenCalledWith('Tolaria MCP server installed successfully') - }) - - it('install action shows error toast on failure', async () => { - mockInvoke.mockImplementation((cmd: string) => { - if (cmd === 'check_mcp_status') return Promise.resolve('not_installed') - if (cmd === 'register_mcp_tools') return Promise.reject(new Error('disk full')) - return Promise.resolve(null) - }) - - const onToast = vi.fn() - const { result } = renderHook(() => useMcpStatus('/vault', onToast)) - - await waitFor(() => { - expect(result.current.mcpStatus).toBe('not_installed') - }) - - await act(async () => { - await result.current.installMcp() - }) - - expect(result.current.mcpStatus).toBe('not_installed') - expect(onToast).toHaveBeenCalledWith(expect.stringContaining('MCP install failed')) - }) - - it('shows toast when registered for the first time', async () => { - mockInvoke.mockImplementation((cmd: string) => { - if (cmd === 'check_mcp_status') return Promise.resolve('installed') - if (cmd === 'register_mcp_tools') return Promise.resolve('registered') - return Promise.resolve(null) - }) - - const onToast = vi.fn() - renderHook(() => useMcpStatus('/vault', onToast)) - - await waitFor(() => { - expect(onToast).toHaveBeenCalledWith('Tolaria registered as MCP tool for Claude Code') - }) - }) - - it('install action shows restored toast when status was installed', async () => { - // First call: status already installed - mockInvoke.mockImplementation((cmd: string) => { - if (cmd === 'check_mcp_status') return Promise.resolve('installed') - if (cmd === 'register_mcp_tools') return Promise.resolve('updated') - return Promise.resolve(null) - }) - - const onToast = vi.fn() - const { result } = renderHook(() => useMcpStatus('/vault', onToast)) - - await waitFor(() => { - expect(result.current.mcpStatus).toBe('installed') - }) - - // Clear toasts from auto-register - onToast.mockClear() - - // Now manually trigger install (restore) - await act(async () => { - await result.current.installMcp() - }) - - expect(result.current.mcpStatus).toBe('installed') - expect(onToast).toHaveBeenCalledWith('Tolaria MCP server restored successfully') - }) - - it('does not show toast when already registered', async () => { - mockInvoke.mockImplementation((cmd: string) => { - if (cmd === 'check_mcp_status') return Promise.resolve('installed') - if (cmd === 'register_mcp_tools') return Promise.resolve('updated') - return Promise.resolve(null) - }) - - const onToast = vi.fn() - renderHook(() => useMcpStatus('/vault', onToast)) - - await waitFor(() => { - expect(mockInvoke).toHaveBeenCalledWith('register_mcp_tools', { vaultPath: '/vault' }) - }) - - // 'updated' should not trigger a toast - expect(onToast).not.toHaveBeenCalledWith('Tolaria registered as MCP tool for Claude Code') + expect(result.current.mcpStatus).toBe(expectedStatus) + expect(mockInvoke).toHaveBeenCalledWith(overrideKey, commandArgs) + expect(onToast).toHaveBeenCalledWith(expect.stringContaining(toastFragment)) }) }) diff --git a/src/hooks/useMcpStatus.ts b/src/hooks/useMcpStatus.ts index bef6b9a1..ec21a93a 100644 --- a/src/hooks/useMcpStatus.ts +++ b/src/hooks/useMcpStatus.ts @@ -2,74 +2,94 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { invoke } from '@tauri-apps/api/core' import { isTauri, mockInvoke } from '../mock-tauri' -export type McpStatus = 'checking' | 'installed' | 'not_installed' | 'no_claude_cli' +export type McpStatus = 'checking' | 'installed' | 'not_installed' function tauriCall(command: string, args?: Record): Promise { return isTauri() ? invoke(command, args) : mockInvoke(command, args) } +function normalizeMcpStatus(value: string | null | undefined): McpStatus { + return value === 'installed' ? 'installed' : 'not_installed' +} + +async function fetchMcpStatus(vaultPath: string): Promise { + try { + const result = await tauriCall('check_mcp_status', { vaultPath }) + return normalizeMcpStatus(result) + } catch { + return 'not_installed' + } +} + +function connectSuccessToast(result: string): string { + return result === 'registered' + ? 'Tolaria external AI tools connected successfully' + : 'Tolaria external AI tools setup refreshed successfully' +} + +function disconnectSuccessToast(result: string): string { + return result === 'removed' + ? 'Tolaria external AI tools disconnected successfully' + : 'Tolaria external AI tools were already disconnected' +} + /** - * Detects MCP server status on vault open and provides an install action. - * - * Combines the old `useMcpRegistration` functionality (auto-register on vault - * switch) with new status detection for the status bar indicator. + * Detects whether the active vault is explicitly connected to external MCP + * clients and exposes connect / disconnect actions. */ export function useMcpStatus( vaultPath: string, onToast: (msg: string) => void, ) { const [status, setStatus] = useState('checking') - const statusRef = useRef(status) - const registeredRef = useRef(null) const onToastRef = useRef(onToast) useEffect(() => { onToastRef.current = onToast }) - useEffect(() => { statusRef.current = status }, [status]) - // Check MCP status on vault open / vault switch + const refreshMcpStatus = useCallback(async () => { + const nextStatus = await fetchMcpStatus(vaultPath) + setStatus(nextStatus) + return nextStatus + }, [vaultPath]) + useEffect(() => { let cancelled = false setStatus('checking') // eslint-disable-line react-hooks/set-state-in-effect -- reset to checking on vault switch - tauriCall('check_mcp_status') - .then((result) => { - if (!cancelled) setStatus(result as McpStatus) - }) - .catch(() => { - if (!cancelled) setStatus('not_installed') - }) + fetchMcpStatus(vaultPath).then((nextStatus) => { + if (!cancelled) setStatus(nextStatus) + }) return () => { cancelled = true } }, [vaultPath]) - // Auto-register on vault switch (preserves old useMcpRegistration behavior) - useEffect(() => { - if (registeredRef.current === vaultPath) return - registeredRef.current = vaultPath - - tauriCall('register_mcp_tools', { vaultPath }) - .then((result) => { - if (result === 'registered') { - onToastRef.current('Tolaria registered as MCP tool for Claude Code') - } - setStatus('installed') - }) - .catch(() => { - // Non-critical — status check will show the right state - }) - }, [vaultPath]) - - const install = useCallback(async () => { - const wasInstalled = statusRef.current === 'installed' + const connectMcp = useCallback(async () => { setStatus('checking') try { - await tauriCall('register_mcp_tools', { vaultPath }) + const result = await tauriCall('register_mcp_tools', { vaultPath }) setStatus('installed') - onToastRef.current(wasInstalled ? 'Tolaria MCP server restored successfully' : 'Tolaria MCP server installed successfully') + onToastRef.current(connectSuccessToast(result)) + return true } catch (e) { setStatus('not_installed') - onToastRef.current(`MCP install failed: ${e}`) + onToastRef.current(`External AI tool setup failed: ${e}`) + return false } }, [vaultPath]) - return { mcpStatus: status, installMcp: install } + const disconnectMcp = useCallback(async () => { + setStatus('checking') + try { + const result = await tauriCall('remove_mcp_tools') + setStatus('not_installed') + onToastRef.current(disconnectSuccessToast(result)) + return true + } catch (e) { + const nextStatus = await refreshMcpStatus() + setStatus(nextStatus) + onToastRef.current(`External AI tool disconnect failed: ${e}`) + return false + } + }, [refreshMcpStatus]) + + return { mcpStatus: status, refreshMcpStatus, connectMcp, disconnectMcp } }