From 24165b10f5d3fe2ba6ca6f8671966ea9421e98b0 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 26 Aug 2026 07:34:06 -0400 Subject: [PATCH] Add --name to fabro mcp config and fabro mcp init Both commands always registered the MCP client entry under the fixed `mcpServers` key `fabro`, so users could not register separate Fabro servers (for example production and testing) without editing the client JSON by hand. `--name ` now selects the `mcpServers` key. It defaults to `fabro` for backward compatibility and rejects empty values. `fabro mcp init` upserts only the named entry and preserves entries with other names, so reusing a name updates that entry in place. Closes #808 Co-Authored-By: Claude Fable 5 --- docs/public/agents/mcp.mdx | 9 ++ docs/public/reference/cli.mdx | 2 + lib/apps/fabro-cli/src/args.rs | 12 +- lib/apps/fabro-cli/src/commands/mcp/mod.rs | 22 ++-- lib/apps/fabro-cli/tests/it/cmd/mcp.rs | 144 ++++++++++++++++++++- lib/apps/fabro-mcp-server/src/config.rs | 16 +-- lib/apps/fabro-mcp-server/src/lib.rs | 10 +- 7 files changed, 187 insertions(+), 28 deletions(-) diff --git a/docs/public/agents/mcp.mdx b/docs/public/agents/mcp.mdx index 3b7ec9c63..b0920090f 100644 --- a/docs/public/agents/mcp.mdx +++ b/docs/public/agents/mcp.mdx @@ -28,6 +28,15 @@ fabro mcp start Pass `--server` when the MCP client should connect to a specific Fabro server, or `--storage-dir` when it should use a non-default CLI storage directory. +Both commands register the entry under the `mcpServers` key `fabro` by default. Pass `--name` to choose a different key. Each named entry launches its own single-target `fabro mcp start` process, so you can register more than one Fabro server in the same MCP client: + +```bash +fabro mcp init claude --name fabro-production --server https://fabro.example.com +fabro mcp init claude --name fabro-testing --server https://fabro-testing.example.com +``` + +`fabro mcp init` keeps entries with other names and replaces only the entry that matches `--name`. + | Tool | Purpose | |---|---| | `fabro_run_create` | Create one or more workflow runs, optionally under a parent run, starting them by default. | diff --git a/docs/public/reference/cli.mdx b/docs/public/reference/cli.mdx index fa3b97212..85c241cd6 100644 --- a/docs/public/reference/cli.mdx +++ b/docs/public/reference/cli.mdx @@ -611,6 +611,7 @@ fabro mcp config [OPTIONS] | Option | Description | | --- | --- | +| `--name ` | Name of the mcpServers entry; use distinct names to register multiple Fabro servers (default: fabro) | | `--server ` | Fabro server target: http(s) URL or absolute Unix socket path | | `--storage-dir ` | Local storage directory (default: ~/.fabro/storage) | @@ -632,6 +633,7 @@ fabro mcp init [OPTIONS] | Option | Description | | --- | --- | +| `--name ` | Name of the mcpServers entry; use distinct names to register multiple Fabro servers (default: fabro) | | `--server ` | Fabro server target: http(s) URL or absolute Unix socket path | | `--storage-dir ` | Local storage directory (default: ~/.fabro/storage) | diff --git a/lib/apps/fabro-cli/src/args.rs b/lib/apps/fabro-cli/src/args.rs index ede9e8eb9..cd01aa98c 100644 --- a/lib/apps/fabro-cli/src/args.rs +++ b/lib/apps/fabro-cli/src/args.rs @@ -191,8 +191,13 @@ pub(crate) struct McpStartArgs { pub(crate) connection: ServerConnectionArgs, } -#[derive(Args, Debug, Clone, Default)] +#[derive(Args, Debug, Clone)] pub(crate) struct McpConfigArgs { + /// Name of the mcpServers entry; use distinct names to register multiple + /// Fabro servers + #[arg(long, value_name = "NAME", default_value = fabro_mcp_server::SERVER_NAME, value_parser = clap::builder::NonEmptyStringValueParser::new())] + pub(crate) name: String, + #[command(flatten)] pub(crate) connection: ServerConnectionArgs, } @@ -201,6 +206,11 @@ pub(crate) struct McpConfigArgs { pub(crate) struct McpInitArgs { pub(crate) agent: McpAgent, + /// Name of the mcpServers entry; use distinct names to register multiple + /// Fabro servers + #[arg(long, value_name = "NAME", default_value = fabro_mcp_server::SERVER_NAME, value_parser = clap::builder::NonEmptyStringValueParser::new())] + pub(crate) name: String, + #[command(flatten)] pub(crate) connection: ServerConnectionArgs, } diff --git a/lib/apps/fabro-cli/src/commands/mcp/mod.rs b/lib/apps/fabro-cli/src/commands/mcp/mod.rs index f6a64a0ca..b3614774a 100644 --- a/lib/apps/fabro-cli/src/commands/mcp/mod.rs +++ b/lib/apps/fabro-cli/src/commands/mcp/mod.rs @@ -2,7 +2,7 @@ use std::fmt::Write as _; use anyhow::{Context as _, Result}; -use crate::args::{McpAgent, McpCommand, McpNamespace, ServerConnectionArgs}; +use crate::args::{McpAgent, McpCommand, McpInitArgs, McpNamespace, ServerConnectionArgs}; use crate::command_context::CommandContext; use crate::server_client; @@ -12,12 +12,13 @@ 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.name, &args.connection))?; let _ = write!(base_ctx.printer().stdout_important(), "{json}"); Ok(()) } McpCommand::Init(args) => { - fabro_mcp_server::init_agent(&init_settings(args.agent, &args.connection)?)?; + fabro_mcp_server::init_agent(&init_settings(&args)?)?; Ok(()) } } @@ -56,19 +57,20 @@ fn server_settings( }) } -fn init_settings( - agent: McpAgent, - connection: &ServerConnectionArgs, -) -> Result { +fn init_settings(args: &McpInitArgs) -> Result { Ok(fabro_mcp_server::McpInitSettings { - agent: McpAgentForServer(agent).into(), - config: config_settings(connection), + agent: McpAgentForServer(args.agent).into(), + config: config_settings(&args.name, &args.connection), home_dir: home_dir()?, }) } -fn config_settings(connection: &ServerConnectionArgs) -> fabro_mcp_server::McpConfigSettings { +fn config_settings( + name: &str, + connection: &ServerConnectionArgs, +) -> fabro_mcp_server::McpConfigSettings { fabro_mcp_server::McpConfigSettings { + name: name.to_string(), server: connection.target.server.clone(), storage_dir: connection.storage_dir.clone_path(), } diff --git a/lib/apps/fabro-cli/tests/it/cmd/mcp.rs b/lib/apps/fabro-cli/tests/it/cmd/mcp.rs index aaddb71b7..67d347c00 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/mcp.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/mcp.rs @@ -122,10 +122,11 @@ fn config_help() { Options: --json Output as JSON [env: FABRO_JSON=] - --storage-dir Local storage directory (default: ~/.fabro/storage) [env: FABRO_STORAGE_DIR=] + --name Name of the mcpServers entry; use distinct names to register multiple Fabro servers [default: fabro] --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] - --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] + --storage-dir Local storage directory (default: ~/.fabro/storage) [env: FABRO_STORAGE_DIR=] --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] + --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] --quiet Suppress non-essential output [env: FABRO_QUIET=] --verbose Enable verbose output [env: FABRO_VERBOSE=] -h, --help Print help @@ -151,10 +152,11 @@ fn init_help() { Options: --json Output as JSON [env: FABRO_JSON=] - --storage-dir Local storage directory (default: ~/.fabro/storage) [env: FABRO_STORAGE_DIR=] + --name Name of the mcpServers entry; use distinct names to register multiple Fabro servers [default: fabro] --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] - --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] + --storage-dir Local storage directory (default: ~/.fabro/storage) [env: FABRO_STORAGE_DIR=] --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] + --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] --quiet Suppress non-essential output [env: FABRO_QUIET=] --verbose Enable verbose output [env: FABRO_VERBOSE=] -h, --help Print help @@ -221,6 +223,55 @@ fn config_preserves_connection_flags() { "#); } +#[test] +fn config_uses_custom_entry_name() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args([ + "mcp", + "config", + "--name", + "fabro-production", + "--server", + "https://fabro.example.test", + ]); + fabro_snapshot!(context.filters(), cmd, @r#" + success: true + exit_code: 0 + ----- stdout ----- + { + "mcpServers": { + "fabro-production": { + "command": "fabro", + "args": [ + "mcp", + "start", + "--server", + "https://fabro.example.test" + ] + } + } + } + ----- stderr ----- + "#); +} + +#[test] +fn config_rejects_empty_entry_name() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["mcp", "config", "--name", ""]); + fabro_snapshot!(context.filters(), cmd, @" + success: false + exit_code: 2 + ----- stdout ----- + ----- stderr ----- + error: a value is required for '--name ' but none was supplied + + For more information, try '--help'. + "); +} + #[test] fn init_cursor_writes_idempotent_config() { let context = test_context!(); @@ -417,6 +468,91 @@ fn init_preserves_existing_servers() { "#); } +#[test] +fn init_merges_multiple_named_fabro_entries() { + let context = test_context!(); + context + .command() + .args(["mcp", "init", "cursor"]) + .assert() + .success(); + context + .command() + .args([ + "mcp", + "init", + "cursor", + "--name", + "fabro-production", + "--server", + "https://production.example.test", + ]) + .assert() + .success(); + context + .command() + .args([ + "mcp", + "init", + "cursor", + "--name", + "fabro-testing", + "--server", + "https://testing.example.test", + ]) + .assert() + .success(); + // Reusing a name updates only that entry. + context + .command() + .args([ + "mcp", + "init", + "cursor", + "--name", + "fabro-production", + "--server", + "https://production.example.test:8443", + ]) + .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" + ] + }, + "fabro-production": { + "command": "fabro", + "args": [ + "mcp", + "start", + "--server", + "https://production.example.test:8443" + ] + }, + "fabro-testing": { + "command": "fabro", + "args": [ + "mcp", + "start", + "--server", + "https://testing.example.test" + ] + } + } + } + "#); +} + #[test] fn init_invalid_json_fails_without_overwrite() { let context = test_context!(); diff --git a/lib/apps/fabro-mcp-server/src/config.rs b/lib/apps/fabro-mcp-server/src/config.rs index ec7154348..d2b1c92ac 100644 --- a/lib/apps/fabro-mcp-server/src/config.rs +++ b/lib/apps/fabro-mcp-server/src/config.rs @@ -9,7 +9,7 @@ use anyhow::{Context as _, Result, anyhow}; use serde_json::map::Entry; use serde_json::{Map, Value, json}; -use crate::{McpAgent, McpConfigSettings, McpInitSettings, SERVER_NAME}; +use crate::{McpAgent, McpConfigSettings, McpInitSettings}; pub fn config_json(settings: &McpConfigSettings) -> Result { serde_json::to_string_pretty(&generic_config(settings)) @@ -20,17 +20,15 @@ pub fn config_json(settings: &McpConfigSettings) -> Result { pub fn init_agent(settings: &McpInitSettings) -> Result<()> { let entry = server_entry(&settings.config); for path in agent_config_paths(settings.agent, &settings.home_dir) { - merge_server_entry(&path, entry.clone())?; + merge_server_entry(&path, &settings.config.name, entry.clone())?; } Ok(()) } fn generic_config(settings: &McpConfigSettings) -> Value { - json!({ - "mcpServers": { - SERVER_NAME: server_entry(settings) - } - }) + let mut servers = Map::new(); + servers.insert(settings.name.clone(), server_entry(settings)); + json!({ "mcpServers": servers }) } fn server_entry(settings: &McpConfigSettings) -> Value { @@ -53,7 +51,7 @@ fn start_args(settings: &McpConfigSettings) -> Vec { args } -fn merge_server_entry(path: &Path, entry: Value) -> Result<()> { +fn merge_server_entry(path: &Path, name: &str, entry: Value) -> Result<()> { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent) .with_context(|| format!("failed to create {}", parent.display()))?; @@ -80,7 +78,7 @@ fn merge_server_entry(path: &Path, entry: Value) -> Result<()> { path.display() ) })?; - servers_object.insert(SERVER_NAME.to_string(), entry); + servers_object.insert(name.to_string(), entry); let rendered = serde_json::to_string_pretty(&root) .map(|json| format!("{json}\n")) diff --git a/lib/apps/fabro-mcp-server/src/lib.rs b/lib/apps/fabro-mcp-server/src/lib.rs index 068f99c14..d345ec5c1 100644 --- a/lib/apps/fabro-mcp-server/src/lib.rs +++ b/lib/apps/fabro-mcp-server/src/lib.rs @@ -13,9 +13,9 @@ pub use config::{config_json, init_agent}; use fabro_client::Client; pub use server::start; -/// The name this MCP server reports over the wire and registers under in agent -/// config files. -pub(crate) const SERVER_NAME: &str = "fabro"; +/// The name this MCP server reports over the wire. It is also the default +/// `mcpServers` key that `fabro mcp config` and `fabro mcp init` register. +pub const SERVER_NAME: &str = "fabro"; pub type FabroClientFuture = Pin> + Send>>; @@ -39,8 +39,10 @@ impl std::fmt::Debug for FabroMcpServerSettings { } } -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone)] pub struct McpConfigSettings { + /// The `mcpServers` key the generated client entry is registered under. + pub name: String, pub server: Option, pub storage_dir: Option, }