diff --git a/Cargo.lock b/Cargo.lock index bb7c02a..8c2bd9c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2554,7 +2554,7 @@ dependencies = [ [[package]] name = "kit" -version = "0.1.112" +version = "0.1.113" dependencies = [ "a2a-protocol-client", "a2a-protocol-server", diff --git a/Cargo.toml b/Cargo.toml index b543417..645d57c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "kit" -version = "0.1.112" +version = "0.1.113" edition = "2024" rust-version = "1.94.0" publish = false diff --git a/README.md b/README.md index 7881aea..2121cfd 100644 --- a/README.md +++ b/README.md @@ -170,7 +170,7 @@ While the agent works, press `Enter` to add a message to the *current* turn thro ### Live MCP configuration -You do not need to restart Kit after you add a server. Kit reloads `mcp.json` before each `tool_search` and `auth` call. Kit waits for new servers to finish initialization. It then ranks tools from all configured servers. +You do not need to restart Kit after you add a server. Kit merges plugins, configured `mcp_config`, project-root `.mcp.json`, and `--mcp-config` in that order, then reloads every named file before each `tool_search` and `auth` call. Kit waits for new servers to finish initialization before ranking tools. Kit detects OAuth from the server's Bearer challenge. An `auth` block is not necessary. The model calls `auth({ name })` and gives you a URL. After you complete the browser flow, Kit resumes the session. If an access token expires, Kit refreshes the token and repeats the rejected call once. diff --git a/docs/user/agent-plugins.md b/docs/user/agent-plugins.md index 85c1491..89aa011 100644 --- a/docs/user/agent-plugins.md +++ b/docs/user/agent-plugins.md @@ -44,7 +44,7 @@ For `stdio`, Kit materializes the validated portable declaration as follows: The location follows the loaded Kit configuration directory, so a config loaded from another directory uses that directory's `plugin-data`. Streamable HTTP uses the validated URL and headers as declared; stdio placeholders are not expanded in HTTP URLs or headers. Plugin HTTP declarations do not add the explicit MCP file's `description`, bearer-token, or OAuth fields. -Supported MCP server names must be unique across plugins. If two plugins declare the same supported server name, startup fails and identifies both aliases. A same-named entry in an explicit MCP JSON file is different: it intentionally overrides the plugin server. Kit live-reloads the explicit file before `tool_search` and `auth`; changing an override replaces it, and removing it restores the original plugin server without restarting Kit. An invalid file edit fails the current call and preserves the last valid combined configuration. +Supported MCP server names must be unique across plugins. If two plugins declare the same supported server name, startup fails and identifies both aliases. A same-named entry in configured, project-local, or command-line MCP JSON intentionally overrides the plugin server. Kit live-reloads every named file before `tool_search` and `auth`; changing an override replaces it, and removing it restores the next lower configured or plugin server without restarting Kit. An invalid file edit fails the current call and preserves the last valid combined configuration. ## Cache and startup behavior @@ -60,4 +60,4 @@ Resolution or package-validation failures stop startup. Non-fatal package diagno Skill collision precedence is project skills, then user skills, then plugins in lexical alias order. Only immediate valid plugin skill directories approved by the package validator are exposed; nested `SKILL.md` files are not recursively added. -`serve`, `acp`, and `prompt` resolve plugins directly. `tui` validates them before launch, and its built-in Kit server reloads the same global configuration, cache, and plugin MCP declarations. Nested built-in `acp.kit` children receive Kit's explicit MCP path and credential settings and reload plugins from the same global Kit configuration, so plugin-only MCP also works in those children. External ACP profile processes receive standard ACP traffic but do not inherit Kit plugin declarations or Kit MCP configuration unless that external program implements and configures its own equivalent behavior. +`serve`, `acp`, and `prompt` resolve plugins directly. `tui` validates them before launch, and its built-in Kit server reloads the same global configuration, cache, and plugin MCP declarations. Nested built-in `acp.kit` children receive Kit's configured and explicit MCP paths, project root, and credential settings. They rediscover project `.mcp.json` and reload plugins from the same global Kit configuration, preserving the full MCP precedence order. External ACP profile processes receive standard ACP traffic but do not inherit Kit plugin declarations or Kit MCP configuration unless that external program implements and configures its own equivalent behavior. diff --git a/docs/user/getting-started-and-configuration.md b/docs/user/getting-started-and-configuration.md index d1d2ff4..d543dae 100644 --- a/docs/user/getting-started-and-configuration.md +++ b/docs/user/getting-started-and-configuration.md @@ -162,7 +162,7 @@ otel_capture_message_content = false otel_message_content_max_messages = 64 otel_message_content_max_bytes = 16384 -# Optional: explicit MCP servers overlay any same-named plugin servers. +# Optional: user MCP servers below project .mcp.json and --mcp-config. mcp_config = "/path/to/mcp.json" credential_store = "file" # "memory", "keychain", or "file" credential_dir = "/path/to/private/credentials" @@ -193,7 +193,7 @@ sha256 = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" `root`, `provider`, `model`, and credential settings apply to all four runtime commands. Subagent model aliases and explicit-override allowlists are scoped by fully qualified harness under `[subagent.harnesses."acp.name"]`. Omitting `allow_model_overrides` permits all explicit model selections accepted by that harness; an empty list disables explicit model overrides. This policy does not restrict the harness's inherited or default model. -`a2a` applies to `serve` and `tui`. Configured plugins can provide MCP servers without `mcp_config`; supported `stdio` and `streamable-http` declarations are registered, while `sse` declarations are skipped with a stderr diagnostic. If `mcp_config` is also set, its same-named entries override plugin servers, and live removal of an override restores the plugin server. Plugin data is stored under `/plugin-data/`. See [Agent Plugins](agent-plugins.md) for placeholders, collision rules, and ACP child behavior. `otel_endpoint` enables OTLP/gRPC export of AgentKit's GenAI trace spans. Use a collector endpoint such as `http://localhost:4317` without a `/v1/traces` suffix. `credential_store` selects one backend for OpenAI, Speakeasy, and MCP and defaults to `memory`; selecting `file` requires `credential_dir`, while a credential directory is invalid with `memory` or `keychain`. Memory credentials are process-local and are not shared with the TUI server process or nested Kit children. Standalone OpenAI and Speakeasy login requires persistent `keychain` or `file` storage. ACP profiles are direct executable-and-argument configurations, not shell command strings. `[subagent].harness` must name an available fully qualified profile such as `acp.review`; otherwise startup reports `unknown subagent ACP harness`. When no subagent harness is selected, the built-in `acp.kit` profile is used. +`a2a` applies to `serve` and `tui`. Configured plugins can provide MCP servers without `mcp_config`; supported `stdio` and `streamable-http` declarations are registered, while `sse` declarations are skipped with a stderr diagnostic. MCP servers merge by name in this order: plugins, configured `mcp_config`, `/.mcp.json`, then `--mcp-config`. Higher layers replace whole conflicts while preserving non-conflicting lower entries; live removal reveals the next lower layer. Plugin data is stored under `/plugin-data/`. See [Agent Plugins](agent-plugins.md) for placeholders, collision rules, and ACP child behavior. `otel_endpoint` enables OTLP/gRPC export of AgentKit's GenAI trace spans. Use a collector endpoint such as `http://localhost:4317` without a `/v1/traces` suffix. `credential_store` selects one backend for OpenAI, Speakeasy, and MCP and defaults to `memory`; selecting `file` requires `credential_dir`, while a credential directory is invalid with `memory` or `keychain`. Memory credentials are process-local and are not shared with the TUI server process or nested Kit children. Standalone OpenAI and Speakeasy login requires persistent `keychain` or `file` storage. ACP profiles are direct executable-and-argument configurations, not shell command strings. `[subagent].harness` must name an available fully qualified profile such as `acp.review`; otherwise startup reports `unknown subagent ACP harness`. When no subagent harness is selected, the built-in `acp.kit` profile is used. ### Configuration precedence and built-in defaults diff --git a/docs/user/mcp.md b/docs/user/mcp.md index d772c42..1bfdbe6 100644 --- a/docs/user/mcp.md +++ b/docs/user/mcp.md @@ -1,6 +1,6 @@ # Configure and Use MCP Servers -Kit connects to Model Context Protocol (MCP) servers supplied by configured Agent Plugins, an explicit JSON file, or both. Plugin-only operation does not require an MCP JSON file. Supply an explicit file with `--mcp-config` or set `mcp_config` in `~/.kit/config.toml`; command-line values override TOML values. Kit does not scan for MCP configuration from other locations. Run `kit --help` and `kit --help` for the exhaustive CLI reference. +Kit merges Model Context Protocol (MCP) servers from Agent Plugins, `mcp_config` in `~/.kit/config.toml`, `.mcp.json` in the canonical runtime root, and `--mcp-config`, in that precedence order. A higher layer replaces a whole same-named server; non-conflicting lower-layer servers remain. The project file is optional, while configured and command-line files are required when specified. Relative configured and command-line paths retain launch-directory resolution. Run `kit --help` and `kit --help` for the exhaustive CLI reference. ## Agent Plugin MCP configuration @@ -8,16 +8,17 @@ Validated Agent Plugins can contribute `stdio` and `streamable-http` servers. De Plugin stdio declarations use the canonical plugin package as `PLUGIN_ROOT` and a persistent `/plugin-data/` directory as `PLUGIN_DATA`. Kit injects both environment variables and replaces every occurrence of `${PLUGIN_ROOT}` and `${PLUGIN_DATA}` in stdio arguments and plugin-supplied environment values. A `./` command is resolved beneath the plugin root. An omitted `cwd` leaves the transport default unchanged and inherits Kit's working directory; `./...` is plugin-root-relative; and `${PLUGIN_ROOT}` or `${PLUGIN_DATA}`, optionally with a validated contained suffix, selects that directory. Kit creates missing data-rooted working directories. These stdio placeholders are not expanded in streamable HTTP URLs or headers. -When an explicit file contains the same server name as a plugin, the explicit entry wins. Kit reloads that file before each `tool_search` and `auth`; removing the explicit entry restores the plugin baseline in the live runtime. An invalid edit fails that call while retaining the last valid combined configuration. See [Agent Plugins](agent-plugins.md) for package configuration and ACP inheritance details. +Higher-precedence JSON entries override same-named plugin servers. Kit reloads every named JSON layer before each `tool_search` and `auth`; removing an override restores the next lower layer in the live runtime. Creating or deleting the optional project file is detected the same way. An invalid edit fails that call while retaining the last valid combined configuration. See [Agent Plugins](agent-plugins.md) for package configuration and ACP inheritance details. ## MCP JSON configuration -The top-level key is `mcpServers`; each key beneath it is the server name shown by `tool_search` and accepted by `auth`. The JSON schema is strict: unknown fields make the configuration invalid. Kit validates and registers this configuration at startup, then reloads it before each `tool_search` and `auth` call so live sessions see added, changed, and removed explicit servers—and restored plugin baselines—without a restart. An invalid edit makes the current call fail but retains the last valid configuration so it becomes usable again when the file is repaired. Every configured server begins connecting in the background when Kit starts, using stored credentials only; interactive OAuth is never started implicitly. Each `tool_search` call waits for servers that are still initializing—including servers just added by a reload—before searching, so results always reflect a settled configuration. Give every server a specific `description`: it appears in listings and search results, and it is how an agent recognizes a server that still needs authentication. Use the exact query `mcp` (case-insensitive) for a compact configured-server status list. If the response cap omits tail entries, `total_servers`, `returned_servers`, and `truncated` report the omission. +The top-level key is `mcpServers`; each key beneath it is the server name shown by `tool_search` and accepted by `auth`. The JSON schema is strict: unknown fields, mixed `command`/`url` transports, and mismatched transport types are invalid. The optional `type` is `stdio` for a command server and `streamable-http` or `http` for a URL server; legacy entries without `type` remain valid. Kit validates every named layer before changing runtime state, then reloads them before each `tool_search` and `auth` call so live sessions see added, changed, and removed servers—and restored lower layers—without a restart. An invalid edit makes the current call fail but retains the last valid combined configuration so it becomes usable again when the file is repaired. Every configured server begins connecting in the background when Kit starts, using stored credentials only; interactive OAuth is never started implicitly. Each `tool_search` call waits for servers that are still initializing—including servers just added by a reload—before searching, so results always reflect a settled configuration. Give every server a specific `description`: it appears in listings and search results, and it is how an agent recognizes a server that still needs authentication. Use the exact query `mcp` (case-insensitive) for a compact configured-server status list. If the response cap omits tail entries, `total_servers`, `returned_servers`, and `truncated` report the omission. ```json { "mcpServers": { "local-files": { + "type": "stdio", "command": "my-mcp-server", "args": ["--stdio"], "cwd": "/path/to/project", @@ -25,6 +26,7 @@ The top-level key is `mcpServers`; each key beneath it is the server name shown "description": "Local file tools" }, "projects": { + "type": "streamable-http", "url": "https://mcp.example.com/mcp", "description": "Issues and project management", "auth": { @@ -36,6 +38,8 @@ The top-level key is `mcpServers`; each key beneath it is the server name shown } ``` +For a project-local stdio server, an omitted `cwd` defaults to the directory containing the root `.mcp.json`, and a relative `cwd` resolves from that directory. An absolute `cwd` is unchanged. Other layers preserve the existing transport behavior. Kit reads these files but never rewrites them. + Start Kit with the installed binary: ```sh @@ -112,7 +116,7 @@ Interactive browser authentication is enabled in the long-lived `kit tui`, `kit ## ACP child behavior -Nested built-in `acp.kit` children receive the explicit MCP path and credential settings from their parent and reload configured plugins from the same global Kit configuration. They therefore see plugin-only servers and the same explicit-over-plugin precedence. External ACP profiles are separate programs: Kit sends them standard ACP initialization and prompt traffic, but does not inject Kit plugin declarations or Kit MCP configuration. Configure MCP separately in an external agent if it supports that behavior. +Nested built-in `acp.kit` children receive the configured and explicit MCP paths, credential settings, and effective project root from their parent. They rediscover the project `.mcp.json` and reload plugins from the same global Kit configuration, preserving the plugin → configured → project → explicit precedence. External ACP profiles are separate programs: Kit sends them standard ACP initialization and prompt traffic, but does not inject Kit plugin declarations or Kit MCP configuration. Configure MCP separately in an external agent if it supports that behavior. ## OAuth credential stores @@ -165,7 +169,7 @@ Persistent stores restore OpenAI credentials when Kit starts and restore MCP OAu ### Configuration file errors -- **`could not read MCP config ...`**: verify the explicitly selected `--mcp-config`/`mcp_config` path and file permissions. The file is optional when plugins provide all required servers; Kit does not scan for other MCP files. +- **`could not read MCP config ...`**: verify the configured `mcp_config` or `--mcp-config` path and permissions; named files are required. Root `.mcp.json` is optional, and its absence is tracked for live creation. - **`invalid MCP config ...`**: validate JSON syntax, the exact `mcpServers` spelling, field types, and field names such as `bearerToken`, `clientId`, and `clientMetadataUrl`. Unknown fields are rejected. - **`MCP server names must not be empty`**, **`has an empty command`**, or **`has an empty URL`**: give every entry a non-blank name and its transport a non-blank `command` or `url`. - **`MCP server ... is declared by both plugins ...`**: rename one plugin server or disable one of the colliding plugins. Explicit-file entries may override plugin servers, but plugin/plugin collisions are errors. diff --git a/docs/user/security-limits-and-troubleshooting.md b/docs/user/security-limits-and-troubleshooting.md index 3e5ef07..ad81f89 100644 --- a/docs/user/security-limits-and-troubleshooting.md +++ b/docs/user/security-limits-and-troubleshooting.md @@ -121,12 +121,12 @@ Provider context windows, model token limits, child-agent turn limits, remote ra ### MCP tools are missing or authentication fails -1. Confirm that the server comes from a configured, valid Agent Plugin or from the selected explicit MCP file. Plugin-only operation needs no file, but Kit does not scan for unconfigured plugins or other MCP files. An SSE plugin server is skipped with a diagnostic; use `streamable-http` instead. A duplicate supported server name across two plugins stops startup, while an explicit same-named entry intentionally overrides a plugin server. +1. Confirm that the server comes from a configured, valid Agent Plugin, configured `mcp_config`, root `.mcp.json`, or `--mcp-config`. Plugin-only operation needs no file; project discovery checks only the canonical runtime root. An SSE plugin server is skipped with a diagnostic; use `streamable-http` instead. A duplicate supported server name across two plugins stops startup, while an explicit same-named entry intentionally overrides a plugin server. 2. Search with `tool_search`; it waits for background server initialization to settle, and the exact query `mcp` compactly lists configured servers and reports any cap-driven tail omission. Inspect statuses: `authenticated`, `authentication_required`, `pending`, or `error`. An `error` result includes the initialization diagnostic. Invoke only tool names returned by the search. 3. For `authentication_required`, call `auth` with the exact server name in a `kit tui`, `kit serve`, or `kit acp` session, open its URL, and complete the browser flow within 10 minutes. Kit connects the server and resumes the originating ACP session automatically. If a configured `bearerToken` or `Authorization` header is rejected, Kit reports an error instead and does not replace that static credential with inferred OAuth; update or remove it first. 4. In one-shot `kit prompt`, use a long-lived command to authenticate or configure persistent credentials there for later one-shot use. 5. For file-store errors such as `OAuth credential directory must be a real directory, not a symlink`, `OAuth credential path must be a regular file`, or `OAuth credential file is accessible by other users`, correct ownership, path type, and permissions rather than weakening the checks. -6. If an explicit override was removed, the plugin server with that name is restored on the next `tool_search` or `auth`. If the file is invalid, repair it first; Kit retains the last valid combined configuration. +6. If an override was removed, the next lower configured or plugin server with that name is restored on the next `tool_search` or `auth`. If the file is invalid, repair it first; Kit retains the last valid combined configuration. 7. If a server has status `error`, relay its diagnostic to the user, test the configured stdio command or remote URL independently, and check the 20-second connection limit. Treat server diagnostics as potentially sensitive. ### A session cannot be resumed diff --git a/src/acp_child.rs b/src/acp_child.rs index d8579cb..217148f 100644 --- a/src/acp_child.rs +++ b/src/acp_child.rs @@ -280,6 +280,13 @@ impl AcpHarnesses { if resume { command.arg("--resume"); } + if config.legacy_mcp_config { + command.arg("--internal-mcp-legacy"); + } else if let Some(path) = &config.configured_mcp_config { + command.arg("--internal-mcp-config").arg(path); + } else if config.configured_mcp_config_inherited { + command.arg("--internal-no-mcp-config"); + } if let Some(path) = &config.mcp_config { command.arg("--mcp-config").arg(path); } @@ -353,6 +360,9 @@ pub(crate) struct ChildConfig { pub provider: crate::ProviderKind, pub reasoning_effort: Option, pub openrouter_api_key: Option, + pub configured_mcp_config: Option, + pub configured_mcp_config_inherited: bool, + pub legacy_mcp_config: bool, pub mcp_config: Option, pub credential_storage: CredentialStorage, pub telemetry: crate::telemetry::Settings, @@ -1227,6 +1237,9 @@ mod tests { provider: crate::ProviderKind::OpenRouter, reasoning_effort: None, openrouter_api_key, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(), @@ -1419,6 +1432,9 @@ mod tests { provider: Default::default(), reasoning_effort: None, openrouter_api_key: None, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(), @@ -1479,6 +1495,9 @@ mod tests { provider: Default::default(), reasoning_effort: None, openrouter_api_key: None, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(), @@ -1534,6 +1553,9 @@ mod tests { provider: Default::default(), reasoning_effort: None, openrouter_api_key: None, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(), @@ -1567,12 +1589,19 @@ mod tests { assert!(harnesses.contains("acp.kit")); assert!(harnesses.is_kit("acp.kit")); let root = tempfile::tempdir().unwrap(); + let configured_directory = tempfile::tempdir().unwrap(); + let configured_mcp = configured_directory.path().join("configured.json"); + assert!(configured_mcp.is_absolute()); + assert!(!configured_mcp.starts_with(root.path())); let config = ChildConfig { root: root.path().to_path_buf(), model: "test-model".into(), provider: crate::ProviderKind::OpenRouter, reasoning_effort: Some(crate::ReasoningEffort::High), openrouter_api_key: Some(crate::provider::OpenRouterApiKey::new("child-secret")), + configured_mcp_config: Some(configured_mcp.clone()), + configured_mcp_config_inherited: true, + legacy_mcp_config: false, mcp_config: None, credential_storage: CredentialStorage::Filesystem(root.path().join("credentials")), telemetry: crate::telemetry::Settings::try_new( @@ -1619,6 +1648,10 @@ mod tests { ); assert!(args.iter().any(|arg| arg == "--root")); assert!(args.iter().any(|arg| arg == "--resume")); + assert!(args.windows(2).any(|pair| { + pair[0] == "--internal-mcp-config" && pair[1] == configured_mcp.to_string_lossy() + })); + assert!(args.iter().all(|arg| arg != "--mcp-config")); assert!( args.windows(2) .any(|pair| pair == ["--credential-store", "file"]) @@ -1648,6 +1681,39 @@ mod tests { assert!(command.as_std().get_envs().any(|(name, value)| { name == "OPENROUTER_API_KEY" && value == Some(std::ffi::OsStr::new("child-secret")) })); + + let mut no_configured = config.clone(); + no_configured.configured_mcp_config = None; + let command = harnesses + .spawn("acp.kit", &no_configured, Some(("session", false)), 2) + .unwrap(); + let args = command + .as_std() + .get_args() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect::>(); + assert!(args.iter().any(|arg| arg == "--internal-no-mcp-config")); + assert!(args.iter().all(|arg| arg != "--internal-mcp-config")); + + let mut legacy = config; + legacy.configured_mcp_config = None; + legacy.configured_mcp_config_inherited = false; + legacy.legacy_mcp_config = true; + legacy.mcp_config = Some(PathBuf::from("/legacy/explicit.json")); + let command = harnesses + .spawn("acp.kit", &legacy, Some(("session", false)), 2) + .unwrap(); + let args = command + .as_std() + .get_args() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect::>(); + assert!(args.iter().any(|arg| arg == "--internal-mcp-legacy")); + assert!( + args.windows(2) + .any(|pair| { pair == ["--mcp-config", "/legacy/explicit.json"] }) + ); + assert!(args.iter().all(|arg| arg != "--internal-no-mcp-config")); } #[tokio::test(flavor = "multi_thread", worker_threads = 4)] @@ -1668,6 +1734,9 @@ mod tests { provider: Default::default(), reasoning_effort: None, openrouter_api_key: None, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(), @@ -1732,6 +1801,9 @@ mod tests { provider: Default::default(), reasoning_effort: None, openrouter_api_key: None, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(), @@ -1994,6 +2066,9 @@ mod tests { provider: Default::default(), reasoning_effort: None, openrouter_api_key: None, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(), diff --git a/src/main.rs b/src/main.rs index 3889428..494b4f6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -106,16 +106,51 @@ impl CredentialArgs { #[derive(Args)] struct McpArgs { - /// Explicit MCP server configuration (never discovered automatically). + /// Highest-precedence MCP server configuration. #[arg(long)] mcp_config: Option, + /// Resolved config.toml MCP path inherited by built-in Kit children. + #[arg(long = "internal-mcp-config", hide = true)] + configured_mcp_config: Option, + /// Preserve inherited layered configuration without a configured source. + #[arg(long = "internal-no-mcp-config", hide = true)] + no_configured_mcp_config: bool, + /// Preserve legacy single-file MCP behavior in built-in Kit children. + #[arg(long = "internal-mcp-legacy", hide = true)] + legacy_mcp_config: bool, #[command(flatten)] credentials: CredentialArgs, } impl McpArgs { - fn config_path<'a>(&'a self, config: &'a Config) -> Option<&'a Path> { - self.mcp_config.as_deref().or(config.mcp_config.as_deref()) + fn config_paths(&self, config: &Config) -> io::Result<(Option, Option)> { + fn launch_path(path: &Path) -> io::Result { + if path.is_absolute() { + Ok(path.to_path_buf()) + } else { + Ok(env::current_dir()?.join(path)) + } + } + + if self.no_configured_mcp_config && self.configured_mcp_config.is_some() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "inherited MCP configuration cannot be both present and absent", + )); + } + let configured = if self.no_configured_mcp_config { + None + } else { + self.configured_mcp_config + .as_deref() + .or(config.mcp_config.as_deref()) + .map(launch_path) + .transpose()? + }; + Ok(( + configured, + self.mcp_config.as_deref().map(launch_path).transpose()?, + )) } } @@ -835,6 +870,7 @@ async fn main() -> Result<(), Box> { let reasoning_effort = config.reasoning_effort(reasoning_effort); let a2a = config.a2a(a2a); let credential_storage = mcp.credentials.storage(&config)?; + let (configured_mcp, explicit_mcp) = mcp.config_paths(&config)?; let plugins = config.resolve_plugins(&root).await?; let runtime = match session_id { Some(id) => { @@ -865,14 +901,26 @@ async fn main() -> Result<(), Box> { let runtime = kit::Runtime::with_telemetry(runtime, telemetry_settings.clone())?; let (harnesses, default_harness) = config.harnesses()?; let runtime = kit::Runtime::with_acp_harnesses(runtime, harnesses, default_harness)?; - let runtime = kit::Runtime::with_mcp_config( - runtime, - mcp.config_path(&config), - plugins.mcp_plugins, - true, - credential_storage, - ) - .await?; + let runtime = if mcp.legacy_mcp_config { + kit::Runtime::with_mcp_config( + runtime, + explicit_mcp.as_deref(), + plugins.mcp_plugins, + true, + credential_storage, + ) + .await? + } else { + kit::Runtime::with_mcp_sources( + runtime, + configured_mcp.as_deref(), + explicit_mcp.as_deref(), + plugins.mcp_plugins, + true, + credential_storage, + ) + .await? + }; let address = a2a.unwrap_or_else(|| "127.0.0.1:0".into()); let serve_a2a = !no_a2a; let sessions = kit::protocols::acp::SessionRegistry::new(); @@ -914,6 +962,7 @@ async fn main() -> Result<(), Box> { let provider = config.provider(provider); let reasoning_effort = config.reasoning_effort(reasoning_effort); let credential_storage = mcp.credentials.storage(&config)?; + let (configured_mcp, explicit_mcp) = mcp.config_paths(&config)?; let plugins = config.resolve_plugins(&root).await?; let runtime = match session_id { Some(id) => { @@ -949,14 +998,26 @@ async fn main() -> Result<(), Box> { )?; let (harnesses, default_harness) = config.harnesses()?; let runtime = kit::Runtime::with_acp_harnesses(runtime, harnesses, default_harness)?; - let runtime = kit::Runtime::with_mcp_config( - runtime, - mcp.config_path(&config), - plugins.mcp_plugins, - true, - credential_storage, - ) - .await?; + let runtime = if mcp.legacy_mcp_config { + kit::Runtime::with_mcp_config( + runtime, + explicit_mcp.as_deref(), + plugins.mcp_plugins, + true, + credential_storage, + ) + .await? + } else { + kit::Runtime::with_mcp_sources( + runtime, + configured_mcp.as_deref(), + explicit_mcp.as_deref(), + plugins.mcp_plugins, + true, + credential_storage, + ) + .await? + }; match protocol_version { AcpProtocolVersion::V1 => kit::protocols::acp::serve(runtime).await?, AcpProtocolVersion::V2 => kit::protocols::acp::v2::serve(runtime).await?, @@ -977,6 +1038,7 @@ async fn main() -> Result<(), Box> { let provider = config.provider(provider); let reasoning_effort = config.reasoning_effort(reasoning_effort); let credential_storage = mcp.credentials.storage(&config)?; + let (configured_mcp, explicit_mcp) = mcp.config_paths(&config)?; let plugins = config.resolve_plugins(&root).await?; let session_id = resume.clone().unwrap_or_else(kit::session::new_id); let runtime = @@ -1001,14 +1063,26 @@ async fn main() -> Result<(), Box> { let runtime = kit::Runtime::with_telemetry(runtime, telemetry_settings.clone())?; let (harnesses, default_harness) = config.harnesses()?; let runtime = kit::Runtime::with_acp_harnesses(runtime, harnesses, default_harness)?; - let runtime = kit::Runtime::with_mcp_config( - runtime, - mcp.config_path(&config), - plugins.mcp_plugins, - false, - credential_storage, - ) - .await?; + let runtime = if mcp.legacy_mcp_config { + kit::Runtime::with_mcp_config( + runtime, + explicit_mcp.as_deref(), + plugins.mcp_plugins, + false, + credential_storage, + ) + .await? + } else { + kit::Runtime::with_mcp_sources( + runtime, + configured_mcp.as_deref(), + explicit_mcp.as_deref(), + plugins.mcp_plugins, + false, + credential_storage, + ) + .await? + }; let output = runtime.run_persistent(prompt).await?; println!("{output}"); println!("session_id: {session_id}"); @@ -1032,6 +1106,7 @@ async fn main() -> Result<(), Box> { let reasoning_effort = config.reasoning_effort(reasoning_effort); let a2a = config.a2a(a2a); let credential_storage = mcp.credentials.storage(&config)?; + let (_, explicit_mcp) = mcp.config_paths(&config)?; config.resolve_plugins(&root).await?; kit::tui::run_with_reasoning_effort_and_openrouter_key( &root, @@ -1039,7 +1114,7 @@ async fn main() -> Result<(), Box> { provider, reasoning_effort, a2a.as_deref(), - mcp.config_path(&config), + explicit_mcp.as_deref(), &credential_storage, &telemetry_settings, openrouter_api_key.as_ref().map(|(key, _)| key), @@ -1169,15 +1244,76 @@ credential_dir = "/configured/credentials" let mcp = McpArgs { mcp_config: None, + configured_mcp_config: None, + no_configured_mcp_config: false, + legacy_mcp_config: false, credentials: CredentialArgs { credential_store: None, credential_dir: None, }, }; assert_eq!( - mcp.config_path(&config), - Some(std::path::Path::new("/configured/mcp.json")) + mcp.config_paths(&config).unwrap(), + (Some(PathBuf::from("/configured/mcp.json")), None) ); + let cli_mcp = McpArgs { + mcp_config: Some(PathBuf::from("/cli/mcp.json")), + configured_mcp_config: None, + no_configured_mcp_config: false, + legacy_mcp_config: false, + credentials: CredentialArgs { + credential_store: None, + credential_dir: None, + }, + }; + assert_eq!( + cli_mcp.config_paths(&config).unwrap(), + ( + Some(PathBuf::from("/configured/mcp.json")), + Some(PathBuf::from("/cli/mcp.json")), + ) + ); + + let inherited_mcp = McpArgs { + mcp_config: None, + configured_mcp_config: Some(PathBuf::from("/parent/configured.json")), + no_configured_mcp_config: false, + legacy_mcp_config: false, + credentials: CredentialArgs { + credential_store: None, + credential_dir: None, + }, + }; + assert_eq!( + inherited_mcp.config_paths(&config).unwrap(), + (Some(PathBuf::from("/parent/configured.json")), None) + ); + let inherited_without_config = McpArgs { + mcp_config: None, + configured_mcp_config: None, + no_configured_mcp_config: true, + legacy_mcp_config: false, + credentials: CredentialArgs { + credential_store: None, + credential_dir: None, + }, + }; + assert_eq!( + inherited_without_config.config_paths(&config).unwrap(), + (None, None) + ); + let conflicting_inherited_mcp = McpArgs { + configured_mcp_config: Some(PathBuf::from("/parent/configured.json")), + ..inherited_without_config + }; + assert_eq!( + conflicting_inherited_mcp + .config_paths(&config) + .unwrap_err() + .to_string(), + "inherited MCP configuration cannot be both present and absent" + ); + let storage = mcp.credentials.storage(&config).unwrap(); assert_eq!(storage.cli_name(), "file"); assert_eq!( @@ -1187,6 +1323,9 @@ credential_dir = "/configured/credentials" let override_mcp = McpArgs { mcp_config: None, + configured_mcp_config: None, + no_configured_mcp_config: false, + legacy_mcp_config: false, credentials: CredentialArgs { credential_store: Some(CredentialStoreKind::Memory), credential_dir: None, @@ -1373,6 +1512,9 @@ subdir = "packages/plugin" fn file_credentials_require_an_explicit_directory() { let missing = McpArgs { mcp_config: None, + configured_mcp_config: None, + no_configured_mcp_config: false, + legacy_mcp_config: false, credentials: CredentialArgs { credential_store: Some(CredentialStoreKind::File), credential_dir: None, @@ -1382,6 +1524,9 @@ subdir = "packages/plugin" let stray = McpArgs { mcp_config: None, + configured_mcp_config: None, + no_configured_mcp_config: false, + legacy_mcp_config: false, credentials: CredentialArgs { credential_store: Some(CredentialStoreKind::Memory), credential_dir: Some("credentials".into()), @@ -1727,10 +1872,12 @@ review = "opus" } #[test] - fn acp_accepts_hidden_immediate_subagent_parent_context() { + fn acp_accepts_hidden_inherited_context() { let cli = Cli::try_parse_from([ "kit", "acp", + "--internal-mcp-config", + "/resolved/configured.json", "--subagent-parent-id", "s-parent", "--subagent-parent-name", @@ -1738,6 +1885,7 @@ review = "opus" ]) .unwrap(); let Command::Acp { + mcp, subagent_parent_id, subagent_parent_name, .. @@ -1745,8 +1893,44 @@ review = "opus" else { panic!("expected acp command"); }; + assert_eq!( + mcp.configured_mcp_config.as_deref(), + Some(std::path::Path::new("/resolved/configured.json")) + ); assert_eq!(subagent_parent_id.as_deref(), Some("s-parent")); assert_eq!(subagent_parent_name.as_deref(), Some("偵察 🦀")); + + let cli = Cli::try_parse_from(["kit", "acp", "--internal-no-mcp-config"]).unwrap(); + let Command::Acp { mcp, .. } = cli.command else { + panic!("expected acp command"); + }; + assert!(mcp.no_configured_mcp_config); + assert!( + mcp.config_paths(&Config { + mcp_config: Some("/child/configured.json".into()), + ..Config::default() + }) + .unwrap() + .0 + .is_none() + ); + + let cli = Cli::try_parse_from([ + "kit", + "acp", + "--internal-mcp-legacy", + "--mcp-config", + "/legacy/explicit.json", + ]) + .unwrap(); + let Command::Acp { mcp, .. } = cli.command else { + panic!("expected acp command"); + }; + assert!(mcp.legacy_mcp_config); + assert_eq!( + mcp.mcp_config.as_deref(), + Some(std::path::Path::new("/legacy/explicit.json")) + ); } #[test] diff --git a/src/runtime.rs b/src/runtime.rs index 95faea4..30907ec 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -224,6 +224,14 @@ pub(crate) struct AcpDriver { pub canonical_transcript: Vec, } +struct McpInstallSources { + sources: Vec, + configured_path: Option, + explicit_path: Option, + legacy: bool, + configured_inherited: bool, +} + pub struct Runtime { root: PathBuf, adapter: SelectableAdapter, @@ -326,6 +334,9 @@ impl Runtime { provider, reasoning_effort, openrouter_api_key: openrouter_api_key.clone(), + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: true, mcp_config: None, credential_storage: credential_storage.clone(), telemetry: Default::default(), @@ -542,8 +553,8 @@ impl Runtime { Ok(Arc::new(runtime)) } - /// Registers validated plugin MCP servers and overlays an optional explicit - /// MCP file, then starts connecting all servers in the background. + /// Preserves the original single-file behavior. `path` is the explicit MCP + /// source and project-local configuration is not discovered. pub async fn with_mcp_config( runtime: Arc, path: Option<&Path>, @@ -554,8 +565,105 @@ impl Runtime { if path.is_none() && plugin_mcps.is_empty() { return Ok(runtime); } + let sources = path + .map(|path| crate::tools::mcp::ConfigSource::required(path.to_path_buf())) + .into_iter() + .collect(); + Self::install_mcp( + runtime, + McpInstallSources { + sources, + configured_path: None, + explicit_path: path.map(Path::to_path_buf), + legacy: true, + configured_inherited: false, + }, + plugin_mcps, + interactive_oauth_enabled, + credential_storage, + ) + .await + } + + /// Registers plugin MCP servers and ordered configured, project-local, and + /// explicit MCP files, then starts connecting all servers in the background. + pub async fn with_mcp_sources( + runtime: Arc, + configured_path: Option<&Path>, + explicit_path: Option<&Path>, + plugin_mcps: Vec, + interactive_oauth_enabled: bool, + credential_storage: crate::tools::mcp::CredentialStorage, + ) -> Result, String> { + let launch_cwd = std::env::current_dir().map_err(|error| { + format!("could not resolve MCP paths from launch directory: {error}") + })?; + Self::with_mcp_sources_from_cwd( + runtime, + configured_path, + explicit_path, + &launch_cwd, + plugin_mcps, + interactive_oauth_enabled, + credential_storage, + ) + .await + } + + async fn with_mcp_sources_from_cwd( + runtime: Arc, + configured_path: Option<&Path>, + explicit_path: Option<&Path>, + launch_cwd: &Path, + plugin_mcps: Vec, + interactive_oauth_enabled: bool, + credential_storage: crate::tools::mcp::CredentialStorage, + ) -> Result, String> { + let resolve = |path: &Path| { + if path.is_absolute() { + path.to_path_buf() + } else { + launch_cwd.join(path) + } + }; + let configured_path = configured_path.map(resolve); + let explicit_path = explicit_path.map(resolve); + let mut sources = Vec::new(); + if let Some(path) = &configured_path { + sources.push(crate::tools::mcp::ConfigSource::required(path.clone())); + } + sources.push(crate::tools::mcp::ConfigSource::optional_project( + runtime.root.join(".mcp.json"), + runtime.root.clone(), + )); + if let Some(path) = &explicit_path { + sources.push(crate::tools::mcp::ConfigSource::required(path.clone())); + } + Self::install_mcp( + runtime, + McpInstallSources { + sources, + configured_path, + explicit_path, + legacy: false, + configured_inherited: true, + }, + plugin_mcps, + interactive_oauth_enabled, + credential_storage, + ) + .await + } + + async fn install_mcp( + runtime: Arc, + install: McpInstallSources, + plugin_mcps: Vec, + interactive_oauth_enabled: bool, + credential_storage: crate::tools::mcp::CredentialStorage, + ) -> Result, String> { let mcp = crate::tools::mcp::connect( - path, + install.sources, &plugin_mcps, interactive_oauth_enabled, credential_storage.clone(), @@ -573,7 +681,10 @@ impl Runtime { provider: runtime.provider, reasoning_effort: runtime.reasoning_effort, openrouter_api_key: runtime.openrouter_api_key.clone(), - mcp_config: path.map(Path::to_path_buf), + configured_mcp_config: install.configured_path, + configured_mcp_config_inherited: install.configured_inherited, + legacy_mcp_config: install.legacy, + mcp_config: install.explicit_path, credential_storage, telemetry: previous.telemetry, harnesses: previous.harnesses, diff --git a/src/runtime/tests.rs b/src/runtime/tests.rs index c35fea2..5d58915 100644 --- a/src/runtime/tests.rs +++ b/src/runtime/tests.rs @@ -26,20 +26,180 @@ async fn subagent_manager_survives_runtime_reconstruction() { Runtime::with_acp_harnesses(runtime, harnesses, crate::acp_child::BUILTIN_HARNESS.into()) .unwrap(); - let mcp_path = root.path().join("mcp.json"); + let configured_directory = tempfile::tempdir().unwrap(); + let mcp_path = configured_directory.path().join("mcp.json"); std::fs::write(&mcp_path, r#"{"mcpServers":{}}"#).unwrap(); - let runtime = Runtime::with_mcp_config( + let runtime = Runtime::with_mcp_sources( runtime, Some(&mcp_path), + None, Vec::new(), false, crate::credentials::CredentialStorage::Memory, ) .await .unwrap(); + assert_eq!( + runtime + .subagents + .child_config() + .configured_mcp_config + .as_deref(), + Some(mcp_path.as_path()) + ); + assert!( + runtime + .subagents + .child_config() + .configured_mcp_config_inherited + ); + assert!(runtime.subagents.child_config().mcp_config.is_none()); let _fresh = runtime.subagents.fresh(); } +#[tokio::test] +async fn legacy_with_mcp_config_remains_an_explicit_layer() { + let root = tempfile::tempdir().unwrap(); + let explicit = root.path().join("explicit.json"); + std::fs::write(&explicit, r#"{"mcpServers":{}}"#).unwrap(); + std::fs::write( + root.path().join(".mcp.json"), + r#"{"mcpServers":{"project-only":{"command":"missing"}}}"#, + ) + .unwrap(); + let runtime = Runtime::new(root.path(), "gpt-5.4").unwrap(); + let runtime = Runtime::with_mcp_config( + runtime, + Some(&explicit), + Vec::new(), + false, + crate::credentials::CredentialStorage::Memory, + ) + .await + .unwrap(); + + assert!( + runtime + .subagents + .child_config() + .configured_mcp_config + .is_none() + ); + assert_eq!( + runtime.subagents.child_config().mcp_config.as_deref(), + Some(explicit.as_path()) + ); + assert!(runtime.subagents.child_config().legacy_mcp_config); + assert!( + !runtime + .subagents + .child_config() + .configured_mcp_config_inherited + ); + assert_eq!( + runtime.mcp.config_source_states().await, + vec![(explicit, true, true)] + ); +} + +#[tokio::test] +async fn legacy_with_no_config_still_propagates_without_project_discovery() { + let root = tempfile::tempdir().unwrap(); + std::fs::write( + root.path().join(".mcp.json"), + r#"{"mcpServers":{"project-only":{"command":"missing"}}}"#, + ) + .unwrap(); + let runtime = Runtime::new(root.path(), "gpt-5.4").unwrap(); + let shared = Arc::clone(&runtime); + let runtime = Runtime::with_mcp_config( + runtime, + None, + Vec::new(), + false, + crate::credentials::CredentialStorage::Memory, + ) + .await + .unwrap(); + assert!(Arc::ptr_eq(&runtime, &shared)); + let child = runtime.subagents.child_config(); + + assert!(child.configured_mcp_config.is_none()); + assert!(!child.configured_mcp_config_inherited); + assert!(child.legacy_mcp_config); + assert!(child.mcp_config.is_none()); + assert!(runtime.mcp.config_source_states().await.is_empty()); +} + +#[tokio::test] +async fn layered_relative_paths_use_captured_launch_cwd() { + let root = tempfile::tempdir().unwrap(); + let launch = tempfile::tempdir().unwrap(); + let configured = launch.path().join("configured.json"); + let explicit = launch.path().join("explicit.json"); + std::fs::write(&configured, r#"{"mcpServers":{}}"#).unwrap(); + std::fs::write(&explicit, r#"{"mcpServers":{}}"#).unwrap(); + let runtime = Runtime::new(root.path(), "gpt-5.4").unwrap(); + let runtime = Runtime::with_mcp_sources_from_cwd( + runtime, + Some(std::path::Path::new("configured.json")), + Some(std::path::Path::new("explicit.json")), + launch.path(), + Vec::new(), + false, + crate::credentials::CredentialStorage::Memory, + ) + .await + .unwrap(); + let child = runtime.subagents.child_config(); + + assert_eq!( + child.configured_mcp_config.as_deref(), + Some(configured.as_path()) + ); + assert!(child.configured_mcp_config_inherited); + assert!(!child.legacy_mcp_config); + assert_eq!(child.mcp_config.as_deref(), Some(explicit.as_path())); + assert_eq!( + runtime.mcp.config_source_states().await, + vec![ + (configured, true, true), + ( + root.path().canonicalize().unwrap().join(".mcp.json"), + false, + false + ), + (explicit, true, true), + ] + ); +} + +#[tokio::test] +async fn project_mcp_source_is_canonical_root_and_tracks_absence() { + let root = tempfile::tempdir().unwrap(); + let canonical_root = root.path().canonicalize().unwrap(); + let runtime = Runtime::new(root.path(), "gpt-5.4").unwrap(); + let runtime = Runtime::with_mcp_sources( + runtime, + None, + None, + Vec::new(), + false, + crate::credentials::CredentialStorage::Memory, + ) + .await + .unwrap(); + + assert_eq!( + runtime.mcp.config_source_states().await, + vec![(canonical_root.join(".mcp.json"), false, false)] + ); + let child = runtime.subagents.child_config(); + assert!(child.configured_mcp_config.is_none()); + assert!(child.configured_mcp_config_inherited); + assert!(!child.legacy_mcp_config); +} + #[test] fn resolved_reasoning_effort_reaches_root_adapter_and_kit_children() { let root = tempfile::tempdir().unwrap(); diff --git a/src/tools/mcp.rs b/src/tools/mcp.rs index e9af3da..c1ee10a 100644 --- a/src/tools/mcp.rs +++ b/src/tools/mcp.rs @@ -26,7 +26,7 @@ use agentkit_tools_core::{ }; use async_trait::async_trait; use rmcp::transport::auth::AuthorizationManager; -use serde::Deserialize; +use serde::{Deserialize, Deserializer}; use serde_json::{Value, json}; use tokio::sync::{Mutex, OwnedMutexGuard, RwLock, Semaphore, mpsc, oneshot}; @@ -64,9 +64,37 @@ enum Server { Http(Http), } +fn deserialize_transport_type<'de, D, T>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, + T: Deserialize<'de>, +{ + T::deserialize(deserializer).map(Some) +} + +#[derive(Deserialize)] +enum StdioType { + #[serde(rename = "stdio")] + Stdio, +} + +#[derive(Deserialize)] +enum HttpType { + #[serde(rename = "streamable-http")] + StreamableHttp, + #[serde(rename = "http")] + Http, +} + #[derive(Deserialize)] #[serde(deny_unknown_fields)] struct Stdio { + #[serde( + rename = "type", + default, + deserialize_with = "deserialize_transport_type" + )] + _transport_type: Option, command: String, description: Option, #[serde(default)] @@ -79,6 +107,12 @@ struct Stdio { #[derive(Deserialize)] #[serde(deny_unknown_fields)] struct Http { + #[serde( + rename = "type", + default, + deserialize_with = "deserialize_transport_type" + )] + _transport_type: Option, url: String, description: Option, #[serde(rename = "bearerToken")] @@ -123,13 +157,61 @@ struct ServerRecord { } struct ReloadState { - path: Option, - raw: Vec, + sources: Vec, entries: BTreeMap>, plugins: BTreeMap, plugin_entries: BTreeMap>, } +#[derive(Clone)] +pub(crate) struct ConfigSource { + path: PathBuf, + required: bool, + default_stdio_cwd: Option, +} + +impl ConfigSource { + pub(crate) fn required(path: PathBuf) -> Self { + Self { + path, + required: true, + default_stdio_cwd: None, + } + } + + pub(crate) fn optional_project(path: PathBuf, cwd: PathBuf) -> Self { + Self { + path, + required: false, + default_stdio_cwd: Some(cwd), + } + } +} + +pub(crate) trait IntoConfigSources { + fn into_config_sources(self) -> Vec; +} + +impl IntoConfigSources for Vec { + fn into_config_sources(self) -> Vec { + self + } +} + +impl> IntoConfigSources for Option

{ + fn into_config_sources(self) -> Vec { + self.map(|path| ConfigSource::required(path.as_ref().to_path_buf())) + .into_iter() + .collect() + } +} + +#[derive(Clone)] +struct SourceState { + source: ConfigSource, + raw: Option>, +} + #[derive(Clone)] struct PreparedServer { config: McpServerConfig, @@ -490,7 +572,21 @@ fn can_opportunistically_refresh(request: &AuthRequest, record: &ServerRecord) - && !challenge_requires_interactive_authorization(&request.challenge) } -fn prepare_config(bytes: &[u8], path: &Path) -> Result { +fn resolve_stdio_cwd(cwd: Option<&Path>, default: Option<&Path>) -> Option { + match cwd { + Some(cwd) if cwd.is_relative() => default + .map(|base| base.join(cwd)) + .or_else(|| Some(cwd.to_path_buf())), + Some(cwd) => Some(cwd.to_path_buf()), + None => default.map(Path::to_path_buf), + } +} + +fn prepare_config( + bytes: &[u8], + path: &Path, + default_stdio_cwd: Option<&Path>, +) -> Result { let config: Config = serde_json::from_slice(bytes) .map_err(|error| format!("invalid MCP config {}: {error}", path.display()))?; let value: Value = serde_json::from_slice(bytes) @@ -499,7 +595,7 @@ fn prepare_config(bytes: &[u8], path: &Path) -> Result Result resolve_stdio_cwd(server.cwd.as_deref(), default_stdio_cwd), + Server::Http(_) => None, + }; + if let Some(cwd) = &effective_cwd { + fingerprint.push(0); + fingerprint.extend_from_slice(cwd.as_os_str().as_encoded_bytes()); + } + entries.insert(id.clone(), fingerprint.clone()); + let (binding, record) = prepare_server(&id, server, fingerprint, effective_cwd.as_deref())?; prepared.insert( id.clone(), PreparedServer { @@ -544,6 +649,7 @@ fn prepare_server( id: &str, server: Server, fingerprint: Vec, + effective_stdio_cwd: Option<&Path>, ) -> Result<(McpTransportBinding, ServerRecord), String> { match server { Server::Stdio(server) => { @@ -553,7 +659,7 @@ fn prepare_server( let mut transport = StdioTransportConfig::new(server.command); transport.args = server.args; transport.env = server.env.into_iter().collect(); - transport.cwd = server.cwd; + transport.cwd = effective_stdio_cwd.map(Path::to_path_buf); Ok(( McpTransportBinding::Stdio(transport), ServerRecord { @@ -817,27 +923,53 @@ fn prepare_plugins( Ok((prepared, entries)) } -pub async fn connect( - path: Option<&Path>, +async fn read_source(source: &ConfigSource) -> Result>, String> { + match tokio::fs::read(&source.path).await { + Ok(bytes) => Ok(Some(bytes)), + Err(error) if !source.required && error.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(error) => Err(format!( + "could not read MCP config {}: {error}", + source.path.display() + )), + } +} + +fn prepare_sources( + sources: &[SourceState], + plugin_prepared: &PreparedServers, + plugin_entries: &ServerFingerprints, +) -> Result { + let mut prepared = plugin_prepared.clone(); + let mut entries = plugin_entries.clone(); + for state in sources { + let Some(bytes) = &state.raw else { + continue; + }; + let (source_prepared, source_entries) = prepare_config( + bytes, + &state.source.path, + state.source.default_stdio_cwd.as_deref(), + )?; + prepared.extend(source_prepared); + entries.extend(source_entries); + } + Ok((prepared, entries)) +} + +pub(crate) async fn connect( + sources: impl IntoConfigSources, plugins: &[crate::plugins::ResolvedPluginMcp], interactive_oauth_enabled: bool, credential_storage: CredentialStorage, ) -> Result { + let sources = sources.into_config_sources(); let (plugin_prepared, plugin_entries) = prepare_plugins(plugins)?; - let (bytes, explicit_prepared, explicit_entries) = match path { - Some(path) => { - let bytes = tokio::fs::read(path).await.map_err(|error| { - format!("could not read MCP config {}: {error}", path.display()) - })?; - let (prepared, entries) = prepare_config(&bytes, path)?; - (bytes, prepared, entries) - } - None => (Vec::new(), BTreeMap::new(), BTreeMap::new()), - }; - let mut prepared = plugin_prepared.clone(); - prepared.extend(explicit_prepared); - let mut entries = plugin_entries.clone(); - entries.extend(explicit_entries); + let mut source_states = Vec::with_capacity(sources.len()); + for source in sources { + let raw = read_source(&source).await?; + source_states.push(SourceState { source, raw }); + } + let (prepared, entries) = prepare_sources(&source_states, &plugin_prepared, &plugin_entries)?; validate_server_names(prepared.keys())?; let challenges = Arc::new(Mutex::new(BTreeMap::new())); let mut manager = McpServerManager::new(); @@ -871,8 +1003,7 @@ pub async fn connect( (oauth_sessions, active_replays), credential_storage, ReloadState { - path: path.map(Path::to_path_buf), - raw: bytes, + sources: source_states, entries, plugins: plugin_prepared, plugin_entries, @@ -1038,6 +1169,24 @@ impl McpRuntime { self.publish_to(session_id, self.event_generation(session_id), event); } + #[cfg(test)] + pub(crate) async fn config_source_states(&self) -> Vec<(PathBuf, bool, bool)> { + self.inner + .reload + .lock() + .await + .sources + .iter() + .map(|state| { + ( + state.source.path.clone(), + state.source.required, + state.raw.is_some(), + ) + }) + .collect() + } + async fn reload_config(&self) -> Result<(), String> { let permit = Arc::clone(&self.inner.reload_flight) .acquire_owned() @@ -1054,23 +1203,25 @@ impl McpRuntime { async fn reload_config_inner(&self) -> Result<(), String> { let mut state = self.inner.reload.lock().await; - let Some(path) = state.path.clone() else { - return Ok(()); - }; - let bytes = tokio::fs::read(&path) - .await - .map_err(|error| format!("could not read MCP config {}: {error}", path.display()))?; - if bytes == state.raw { + let mut next_sources = Vec::with_capacity(state.sources.len()); + for current in &state.sources { + next_sources.push(SourceState { + source: current.source.clone(), + raw: read_source(¤t.source).await?, + }); + } + if next_sources + .iter() + .zip(&state.sources) + .all(|(next, current)| next.raw == current.raw) + { return Ok(()); } - // Validate explicit transports before changing live state, then layer - // them over the immutable plugin baseline. - let (explicit_prepared, explicit_entries) = prepare_config(&bytes, &path)?; - let mut prepared = state.plugins.clone(); - prepared.extend(explicit_prepared); - let mut entries = state.plugin_entries.clone(); - entries.extend(explicit_entries); + // Validate every source before changing live state, then layer them in + // declared precedence order over the immutable plugin baseline. + let (mut prepared, entries) = + prepare_sources(&next_sources, &state.plugins, &state.plugin_entries)?; validate_server_names(prepared.keys())?; let changed = state .entries @@ -1080,7 +1231,7 @@ impl McpRuntime { .cloned() .collect::>(); if changed.is_empty() { - state.raw = bytes; + state.sources = next_sources; return Ok(()); } drop(state); @@ -1106,11 +1257,12 @@ impl McpRuntime { } let mut state = self.inner.reload.lock().await; let _initialization = self.inner.initialization.lock().await; - let current_bytes = tokio::fs::read(&path) - .await - .map_err(|error| format!("could not reread MCP config {}: {error}", path.display()))?; - if current_bytes != bytes { - return Err("MCP config changed while reload was waiting; retry the operation".into()); + for expected in &next_sources { + if read_source(&expected.source).await? != expected.raw { + return Err( + "MCP config changed while reload was waiting; retry the operation".into(), + ); + } } { @@ -1150,7 +1302,7 @@ impl McpRuntime { active_replays.remove(name); } } - state.raw = bytes; + state.sources = next_sources; state.entries = entries; Ok(()) } @@ -2078,8 +2230,7 @@ pub fn empty() -> McpRuntime { (oauth_sessions, active_replays), credential_storage, ReloadState { - path: None, - raw: Vec::new(), + sources: Vec::new(), entries: BTreeMap::new(), plugins: BTreeMap::new(), plugin_entries: BTreeMap::new(), @@ -2476,7 +2627,7 @@ fn render_search( #[cfg(test)] mod tests { - use std::{collections::BTreeMap, sync::Arc, time::Duration}; + use std::{collections::BTreeMap, path::Path, sync::Arc, time::Duration}; use agentkit_core::MetadataMap; use agentkit_mcp::{ @@ -2495,11 +2646,12 @@ mod tests { }; use super::{ - AuthRecorder, Config, CredentialStorage, McpTool, OAuthSession, OpportunisticRefresh, - PreparedQuery, PreparedSpec, ReplayCleanup, ServerRecord, ServerStatus, - agentkit_auth_not_applied, agentkit_replay_rejected, can_opportunistically_refresh, - challenge_requires_interactive_authorization, matched_score, prepare_plugins, - regular_term_score, serializes_tool_calls, validate_server_names, + AuthRecorder, Config, ConfigSource, CredentialStorage, McpTool, OAuthSession, + OpportunisticRefresh, PreparedQuery, PreparedSpec, ReplayCleanup, ServerRecord, + ServerStatus, agentkit_auth_not_applied, agentkit_replay_rejected, + can_opportunistically_refresh, challenge_requires_interactive_authorization, matched_score, + prepare_config, prepare_plugins, regular_term_score, serializes_tool_calls, + validate_server_names, }; use crate::plugins::ResolvedPluginMcp; @@ -3303,21 +3455,72 @@ mod tests { } #[test] - fn config_is_strict_and_accepts_oauth_servers() { - assert!( - serde_json::from_str::( - r#"{"mcpServers":{"ok":{"command":"server","args":["--stdio"],"env":{"A":"B"}}}}"# - ) - .is_ok() - ); - assert!(serde_json::from_str::(r#"{"mcpServers":{"linear":{"url":"https://mcp.example/mcp","description":"Issue tracking","auth":{"type":"oauth","scopes":[]}}}}"#).is_ok()); - assert!(serde_json::from_str::(r#"{"mcpServers":{},"extra":true}"#).is_err()); - assert!( - serde_json::from_str::( - r#"{"mcpServers":{"bad":{"command":"x","url":"http://localhost"}}}"# - ) - .is_err() + fn config_is_strict_and_accepts_compatible_transport_types() { + for config in [ + r#"{"mcpServers":{"legacy":{"command":"server"}}}"#, + r#"{"mcpServers":{"typed":{"type":"stdio","command":"server"}}}"#, + r#"{"mcpServers":{"legacy":{"url":"https://mcp.example/mcp"}}}"#, + r#"{"mcpServers":{"typed":{"type":"streamable-http","url":"https://mcp.example/mcp"}}}"#, + r#"{"mcpServers":{"typed":{"type":"http","url":"https://mcp.example/mcp"}}}"#, + ] { + assert!(serde_json::from_str::(config).is_ok(), "{config}"); + } + for config in [ + r#"{"mcpServers":{},"extra":true}"#, + r#"{"mcpServers":{"bad":{"command":"x","url":"http://localhost"}}}"#, + r#"{"mcpServers":{"bad":{"type":"http","command":"x"}}}"#, + r#"{"mcpServers":{"bad":{"type":"stdio","url":"http://localhost"}}}"#, + r#"{"mcpServers":{"bad":{"type":"sse","url":"http://localhost"}}}"#, + r#"{"mcpServers":{"bad":{"type":1,"command":"x"}}}"#, + r#"{"mcpServers":{"bad":{"type":null,"command":"x"}}}"#, + r#"{"mcpServers":{"bad":{"type":null,"url":"http://localhost"}}}"#, + ] { + assert!(serde_json::from_str::(config).is_err(), "{config}"); + } + } + + #[test] + fn project_stdio_resolves_cwd_before_transport_and_fingerprinting() { + let first = tempfile::tempdir().unwrap(); + let second = tempfile::tempdir().unwrap(); + let path = first.path().join(".mcp.json"); + + let missing = br#"{"mcpServers":{"local":{"command":"server"}}}"#; + let (prepared, _) = prepare_config(missing, &path, Some(first.path())).unwrap(); + let McpTransportBinding::Stdio(transport) = &prepared["local"].config.transport else { + panic!("expected stdio transport"); + }; + assert_eq!(transport.cwd.as_deref(), Some(first.path())); + + let relative = br#"{"mcpServers":{"local":{"command":"server","cwd":"tools"}}}"#; + let (prepared, first_entries) = + prepare_config(relative, &path, Some(first.path())).unwrap(); + let McpTransportBinding::Stdio(transport) = &prepared["local"].config.transport else { + panic!("expected stdio transport"); + }; + assert_eq!( + transport.cwd.as_deref(), + Some(first.path().join("tools").as_path()) ); + let (_, second_entries) = prepare_config(relative, &path, Some(second.path())).unwrap(); + assert_ne!(first_entries["local"], second_entries["local"]); + + let absolute = first.path().join("absolute"); + let absolute_config = serde_json::to_vec(&json!({ + "mcpServers": { + "local": {"command": "server", "cwd": absolute} + } + })) + .unwrap(); + let (prepared, first_entries) = + prepare_config(&absolute_config, &path, Some(first.path())).unwrap(); + let McpTransportBinding::Stdio(transport) = &prepared["local"].config.transport else { + panic!("expected stdio transport"); + }; + assert_eq!(transport.cwd.as_deref(), Some(absolute.as_path())); + let (_, second_entries) = + prepare_config(&absolute_config, &path, Some(second.path())).unwrap(); + assert_eq!(first_entries["local"], second_entries["local"]); } async fn bearer_challenge_server( @@ -3485,6 +3688,220 @@ mod tests { ); } + #[tokio::test] + async fn ordered_sources_merge_and_optional_deletion_reveals_lower_layers() { + let directory = tempfile::tempdir().unwrap(); + let configured = directory.path().join("configured.json"); + let project = directory.path().join(".mcp.json"); + let explicit = directory.path().join("explicit.json"); + std::fs::write( + &configured, + r#"{"mcpServers":{"shared":{"command":"missing-configured","description":"configured"},"configured-only":{"command":"missing"}}}"#, + ) + .unwrap(); + std::fs::write( + &project, + r#"{"mcpServers":{"shared":{"type":"stdio","command":"missing-project","description":"project"},"project-only":{"command":"missing"}}}"#, + ) + .unwrap(); + std::fs::write( + &explicit, + r#"{"mcpServers":{"shared":{"command":"missing-explicit","description":"explicit"},"explicit-only":{"command":"missing"}}}"#, + ) + .unwrap(); + let plugin = plugin( + "tools", + directory.path(), + vec![PluginMcpServer { + name: "shared".into(), + transport: PluginMcpTransport::Stdio { + command: "missing-plugin".into(), + args: Vec::new(), + env: Default::default(), + cwd: None, + }, + }], + ); + let sources = vec![ + ConfigSource::required(configured.clone()), + ConfigSource::optional_project(project.clone(), directory.path().to_path_buf()), + ConfigSource::required(explicit.clone()), + ]; + let runtime = super::connect(sources, &[plugin], true, CredentialStorage::Memory) + .await + .unwrap(); + let servers = runtime.inner.servers.read().await; + assert_eq!(servers["shared"].description, "explicit"); + for name in ["configured-only", "project-only", "explicit-only"] { + assert!(servers.contains_key(name)); + } + drop(servers); + + std::fs::write(&explicit, r#"{"mcpServers":{}}"#).unwrap(); + runtime.reload_config().await.unwrap(); + assert_eq!( + runtime.inner.servers.read().await["shared"].description, + "project" + ); + std::fs::remove_file(&project).unwrap(); + runtime.reload_config().await.unwrap(); + assert_eq!( + runtime.inner.servers.read().await["shared"].description, + "configured" + ); + std::fs::write(&configured, r#"{"mcpServers":{}}"#).unwrap(); + runtime.reload_config().await.unwrap(); + assert_eq!( + runtime.inner.servers.read().await["shared"].description, + "tools-manifest plugin MCP server" + ); + } + + #[tokio::test] + async fn missing_required_source_fails_startup() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("missing.json"); + let error = match super::connect( + vec![ConfigSource::required(path.clone())], + &[], + true, + CredentialStorage::Memory, + ) + .await + { + Ok(_) => panic!("missing required source was accepted"), + Err(error) => error, + }; + assert!(error.contains(&path.display().to_string())); + } + + #[tokio::test] + async fn absent_optional_project_source_is_tracked_for_live_creation() { + let directory = tempfile::tempdir().unwrap(); + let project = directory.path().join(".mcp.json"); + let runtime = super::connect( + vec![ConfigSource::optional_project( + project.clone(), + directory.path().to_path_buf(), + )], + &[], + true, + CredentialStorage::Memory, + ) + .await + .unwrap(); + assert!(runtime.inner.servers.read().await.is_empty()); + + std::fs::write( + &project, + r#"{"mcpServers":{"local":{"command":"missing"}}}"#, + ) + .unwrap(); + runtime.reload_config().await.unwrap(); + assert!(runtime.inner.servers.read().await.contains_key("local")); + } + + #[tokio::test] + async fn invalid_project_reload_preserves_state_until_optional_deletion() { + let directory = tempfile::tempdir().unwrap(); + let configured = directory.path().join("configured.json"); + let project = directory.path().join(".mcp.json"); + std::fs::write( + &configured, + r#"{"mcpServers":{"shared":{"command":"missing","description":"configured"}}}"#, + ) + .unwrap(); + std::fs::write( + &project, + r#"{"mcpServers":{"shared":{"command":"missing","description":"project"}}}"#, + ) + .unwrap(); + let runtime = super::connect( + vec![ + ConfigSource::required(configured), + ConfigSource::optional_project(project.clone(), directory.path().to_path_buf()), + ], + &[], + true, + CredentialStorage::Memory, + ) + .await + .unwrap(); + std::fs::write( + &project, + r#"{"mcpServers":{"broken":{"type":"http","command":"x"}}}"#, + ) + .unwrap(); + assert!( + runtime + .reload_config() + .await + .unwrap_err() + .contains("invalid MCP config") + ); + assert_eq!( + runtime.inner.servers.read().await["shared"].description, + "project" + ); + + std::fs::remove_file(&project).unwrap(); + runtime.reload_config().await.unwrap(); + assert_eq!( + runtime.inner.servers.read().await["shared"].description, + "configured" + ); + } + + #[tokio::test] + async fn shadowed_source_edits_do_not_reconnect_effectively_unchanged_servers() { + let directory = tempfile::tempdir().unwrap(); + let configured = directory.path().join("configured.json"); + let project = directory.path().join(".mcp.json"); + std::fs::write( + &configured, + r#"{"mcpServers":{"shared":{"command":"lower-one"}}}"#, + ) + .unwrap(); + std::fs::write( + &project, + r#"{"mcpServers":{"shared":{"command":"winner"}}}"#, + ) + .unwrap(); + let runtime = super::connect( + vec![ + ConfigSource::required(configured.clone()), + ConfigSource::optional_project(project, directory.path().to_path_buf()), + ], + &[], + true, + CredentialStorage::Memory, + ) + .await + .unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(25)).await; + runtime + .inner + .servers + .write() + .await + .get_mut("shared") + .unwrap() + .status = ServerStatus::Connected; + let fingerprint = runtime.inner.servers.read().await["shared"] + .fingerprint + .clone(); + + std::fs::write( + &configured, + r#"{"mcpServers":{"shared":{"command":"lower-two"}}}"#, + ) + .unwrap(); + runtime.reload_config().await.unwrap(); + let record = runtime.inner.servers.read().await["shared"].clone(); + assert_eq!(record.fingerprint, fingerprint); + assert!(matches!(record.status, ServerStatus::Connected)); + } + #[tokio::test] async fn explicit_reload_removal_restores_plugin_baseline() { let directory = tempfile::tempdir().unwrap(); @@ -3534,7 +3951,7 @@ mod tests { }, }], ); - let runtime = super::connect(None, &[plugin], true, CredentialStorage::Memory) + let runtime = super::connect(None::<&Path>, &[plugin], true, CredentialStorage::Memory) .await .unwrap(); assert!(runtime.inner.servers.read().await.contains_key("remote")); @@ -3762,6 +4179,7 @@ mod tests { let error = match super::prepare_config( br#"{"mcpServers":{"remote":{"url":"https://example.com/mcp","headers":{"AUTHORIZATION":"Bearer static"},"auth":{"type":"oauth"}}}}"#, path, + None, ) { Ok(_) => panic!("OAuth and static authorization were accepted together"), Err(error) => error, diff --git a/src/tools/subagent/tests.rs b/src/tools/subagent/tests.rs index 180c36a..05b0c80 100644 --- a/src/tools/subagent/tests.rs +++ b/src/tools/subagent/tests.rs @@ -312,6 +312,9 @@ fn manager_with_disconnected_session( provider: Default::default(), reasoning_effort: None, openrouter_api_key: None, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(), @@ -379,6 +382,9 @@ async fn close_does_not_block_listings_or_allow_stale_reuse() { provider: Default::default(), reasoning_effort: None, openrouter_api_key: None, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(), @@ -538,6 +544,9 @@ fn manager_with_generic_harness(root: &Path, args: Vec) -> Subagents { provider: Default::default(), reasoning_effort: None, openrouter_api_key: None, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(), @@ -1554,6 +1563,9 @@ async fn reusable_prompt_failure_remains_failed_idle_and_can_be_retried() { provider: Default::default(), reasoning_effort: None, openrouter_api_key: None, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(), @@ -1648,6 +1660,9 @@ async fn listing_includes_named_starting_and_idle_subagents() { provider: Default::default(), reasoning_effort: None, openrouter_api_key: None, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(), @@ -1797,6 +1812,9 @@ async fn generic_harness_without_native_fork_returns_unsupported() { provider: Default::default(), reasoning_effort: None, openrouter_api_key: None, + configured_mcp_config: None, + configured_mcp_config_inherited: false, + legacy_mcp_config: false, mcp_config: None, credential_storage: Default::default(), telemetry: Default::default(),