diff --git a/lib/crates/fabro-cli/src/commands/mcp/mod.rs b/lib/crates/fabro-cli/src/commands/mcp/mod.rs index 84fde4583..a7df70073 100644 --- a/lib/crates/fabro-cli/src/commands/mcp/mod.rs +++ b/lib/crates/fabro-cli/src/commands/mcp/mod.rs @@ -9,12 +9,12 @@ pub(crate) async fn dispatch(ns: McpNamespace, base_ctx: &CommandContext) -> Res fabro_mcp_server::start(server_settings(base_ctx, &args.connection)?).await } McpCommand::Config(args) => { - let json = fabro_mcp_server::config_json(config_settings(&args.connection)); + let json = fabro_mcp_server::config_json(&config_settings(&args.connection))?; print!("{json}"); Ok(()) } McpCommand::Init(args) => { - fabro_mcp_server::init_agent(init_settings(args.agent, &args.connection)?)?; + fabro_mcp_server::init_agent(&init_settings(args.agent, &args.connection)?)?; Ok(()) } } diff --git a/lib/crates/fabro-cli/tests/it/cmd/mcp.rs b/lib/crates/fabro-cli/tests/it/cmd/mcp.rs index 73070d929..7f9081553 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/mcp.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/mcp.rs @@ -1,4 +1,11 @@ -use fabro_test::{fabro_snapshot, test_context}; +#![expect( + clippy::disallowed_methods, + reason = "integration tests stage MCP config files with sync std::fs" +)] + +use std::path::{Path, PathBuf}; + +use fabro_test::{fabro_json_snapshot, fabro_snapshot, test_context}; #[test] fn help() { @@ -110,3 +117,247 @@ fn init_help() { ----- stderr ----- "); } + +#[test] +fn config_prints_generic_mcp_json() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["mcp", "config"]); + fabro_snapshot!(context.filters(), cmd, @r#" + success: true + exit_code: 0 + ----- stdout ----- + { + "mcpServers": { + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start" + ] + } + } + } + ----- stderr ----- + "#); +} + +#[test] +fn config_preserves_connection_flags() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args([ + "mcp", + "config", + "--server", + "https://example.test/api/v1", + "--storage-dir", + "/tmp/fabro-mcp-storage", + ]); + fabro_snapshot!(context.filters(), cmd, @r#" + success: true + exit_code: 0 + ----- stdout ----- + { + "mcpServers": { + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start", + "--server", + "https://example.test/api/v1", + "--storage-dir", + "/tmp/fabro-mcp-storage" + ] + } + } + } + ----- stderr ----- + "#); +} + +#[test] +fn init_cursor_writes_idempotent_config() { + let context = test_context!(); + context + .command() + .args(["mcp", "init", "cursor"]) + .assert() + .success(); + context + .command() + .args(["mcp", "init", "cursor"]) + .assert() + .success(); + + let config_path = context.home_dir.join(".cursor").join("mcp.json"); + let config: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(config_path).unwrap()).unwrap(); + fabro_json_snapshot!(context, config, @r#" + { + "mcpServers": { + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start" + ] + } + } + } + "#); +} + +#[test] +fn init_claude_writes_platform_config() { + let context = test_context!(); + context + .command() + .args(["mcp", "init", "claude"]) + .assert() + .success(); + + let config_path = expected_claude_config_path(&context.home_dir); + let config: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(config_path).unwrap()).unwrap(); + fabro_json_snapshot!(context, config, @r#" + { + "mcpServers": { + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start" + ] + } + } + } + "#); +} + +#[test] +fn init_windsurf_writes_config() { + let context = test_context!(); + context + .command() + .args(["mcp", "init", "windsurf"]) + .assert() + .success(); + + let config_path = context + .home_dir + .join(".codeium") + .join("windsurf") + .join("mcp_config.json"); + let config: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(config_path).unwrap()).unwrap(); + fabro_json_snapshot!(context, config, @r#" + { + "mcpServers": { + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start" + ] + } + } + } + "#); +} + +#[test] +fn init_preserves_existing_servers() { + let context = test_context!(); + let config_path = context.home_dir.join(".cursor").join("mcp.json"); + std::fs::create_dir_all(config_path.parent().unwrap()).unwrap(); + std::fs::write( + &config_path, + r#"{"mcpServers":{"other":{"command":"other","args":["serve"]}},"theme":"dark"}"#, + ) + .unwrap(); + + context + .command() + .args([ + "mcp", + "init", + "cursor", + "--server", + "https://example.test/api/v1", + ]) + .assert() + .success(); + + let config: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(config_path).unwrap()).unwrap(); + fabro_json_snapshot!(context, config, @r#" + { + "mcpServers": { + "other": { + "command": "other", + "args": [ + "serve" + ] + }, + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start", + "--server", + "https://example.test/api/v1" + ] + } + }, + "theme": "dark" + } + "#); +} + +#[test] +fn init_invalid_json_fails_without_overwrite() { + let context = test_context!(); + let config_path = context.home_dir.join(".cursor").join("mcp.json"); + std::fs::create_dir_all(config_path.parent().unwrap()).unwrap(); + std::fs::write(&config_path, "{not json").unwrap(); + + let mut cmd = context.command(); + cmd.args(["mcp", "init", "cursor"]); + fabro_snapshot!(context.filters(), cmd, @" + success: false + exit_code: 1 + ----- stdout ----- + ----- stderr ----- + × failed to parse MCP config [HOME_DIR]/.cursor/mcp.json + ╰─▶ key must be a string at line 1 column 2 + "); + assert_eq!(std::fs::read_to_string(config_path).unwrap(), "{not json"); +} + +fn expected_claude_config_path(home_dir: &Path) -> PathBuf { + #[cfg(target_os = "macos")] + { + home_dir + .join("Library") + .join("Application Support") + .join("Claude") + .join("claude_desktop_config.json") + } + #[cfg(target_os = "linux")] + { + home_dir + .join(".config") + .join("Claude") + .join("claude_desktop_config.json") + } + #[cfg(target_os = "windows")] + { + home_dir + .join("AppData") + .join("Roaming") + .join("Claude") + .join("claude_desktop_config.json") + } +} diff --git a/lib/crates/fabro-mcp-server/src/config.rs b/lib/crates/fabro-mcp-server/src/config.rs index 42a042b74..f0318ae7f 100644 --- a/lib/crates/fabro-mcp-server/src/config.rs +++ b/lib/crates/fabro-mcp-server/src/config.rs @@ -1,21 +1,138 @@ -use anyhow::Result; -use serde_json::json; +#![expect( + clippy::disallowed_methods, + reason = "MCP client config setup intentionally performs small synchronous JSON file reads/writes from a CLI command." +)] -use crate::{McpConfigSettings, McpInitSettings}; +use std::path::{Path, PathBuf}; -pub fn config_json(_settings: McpConfigSettings) -> String { - serde_json::to_string_pretty(&json!({ - "mcpServers": { - "fabro": { - "command": "fabro", - "args": ["mcp", "start"] - } - } - })) - .expect("static MCP config should serialize") - + "\n" +use anyhow::{Context as _, Result, anyhow}; +use serde_json::map::Entry; +use serde_json::{Map, Value, json}; + +use crate::{McpAgent, McpConfigSettings, McpInitSettings}; + +const SERVER_NAME: &str = "fabro"; + +pub fn config_json(settings: &McpConfigSettings) -> Result { + serde_json::to_string_pretty(&generic_config(settings)) + .map(|json| format!("{json}\n")) + .context("failed to render Fabro MCP client config") } -pub fn init_agent(_settings: McpInitSettings) -> Result<()> { +pub fn init_agent(settings: &McpInitSettings) -> Result<()> { + let path = agent_config_path(settings.agent, &settings.home_dir); + let entry = server_entry(&settings.config); + merge_server_entry(&path, entry)?; Ok(()) } + +fn generic_config(settings: &McpConfigSettings) -> Value { + json!({ + "mcpServers": { + SERVER_NAME: server_entry(settings) + } + }) +} + +fn server_entry(settings: &McpConfigSettings) -> Value { + json!({ + "command": "fabro", + "args": start_args(settings), + }) +} + +fn start_args(settings: &McpConfigSettings) -> Vec { + let mut args = vec!["mcp".to_string(), "start".to_string()]; + if let Some(server) = settings.server.as_ref() { + args.push("--server".to_string()); + args.push(server.clone()); + } + if let Some(storage_dir) = settings.storage_dir.as_deref() { + args.push("--storage-dir".to_string()); + args.push(storage_dir.display().to_string()); + } + args +} + +fn merge_server_entry(path: &Path, entry: Value) -> Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .with_context(|| format!("failed to create {}", parent.display()))?; + } + + let mut root = if path.exists() { + let contents = std::fs::read_to_string(path) + .with_context(|| format!("failed to read {}", path.display()))?; + serde_json::from_str::(&contents) + .with_context(|| format!("failed to parse MCP config {}", path.display()))? + } else { + Value::Object(Map::new()) + }; + + let root_object = root + .as_object_mut() + .ok_or_else(|| anyhow!("MCP config {} must contain a JSON object", path.display()))?; + + let servers = match root_object.entry("mcpServers") { + Entry::Vacant(entry) => entry.insert(Value::Object(Map::new())), + Entry::Occupied(entry) => entry.into_mut(), + }; + let servers_object = servers.as_object_mut().ok_or_else(|| { + anyhow!( + "MCP config {} field mcpServers must contain a JSON object", + path.display() + ) + })?; + servers_object.insert(SERVER_NAME.to_string(), entry); + + let rendered = serde_json::to_string_pretty(&root) + .map(|json| format!("{json}\n")) + .with_context(|| format!("failed to render MCP config {}", path.display()))?; + std::fs::write(path, rendered).with_context(|| format!("failed to write {}", path.display())) +} + +fn agent_config_path(agent: McpAgent, home_dir: &Path) -> PathBuf { + match agent { + McpAgent::Claude => claude_config_path(home_dir), + McpAgent::Cursor => home_dir.join(".cursor").join("mcp.json"), + McpAgent::Windsurf => home_dir + .join(".codeium") + .join("windsurf") + .join("mcp_config.json"), + } +} + +fn claude_config_path(home_dir: &Path) -> PathBuf { + #[cfg(target_os = "macos")] + { + home_dir + .join("Library") + .join("Application Support") + .join("Claude") + .join("claude_desktop_config.json") + } + + #[cfg(target_os = "linux")] + { + home_dir + .join(".config") + .join("Claude") + .join("claude_desktop_config.json") + } + + #[cfg(target_os = "windows")] + { + let app_data = std::env::var_os("APPDATA") + .map(PathBuf::from) + .unwrap_or_else(|| home_dir.join("AppData").join("Roaming")); + app_data.join("Claude").join("claude_desktop_config.json") + } + + #[cfg(not(any(target_os = "macos", target_os = "linux", target_os = "windows")))] + { + home_dir + .join(".config") + .join("Claude") + .join("claude_desktop_config.json") + } +}