From c1e5c2d49bb1d959d829b1e7bef70ac44d66e25c Mon Sep 17 00:00:00 2001 From: ArnabChatterjee20k Date: Wed, 5 Aug 2026 13:41:08 +0530 Subject: [PATCH 1/8] feat(agent): attach MCP servers to agent runs, gated by issue source Adds config-driven MCP support so the Claude agent can query the production Appwrite Cloud (via the Appwrite MCP server) instead of a local stack when investigating user-reported issues. - New McpServerConfig under ProviderConfig.mcp, keyed by server name, with a per-server sources list (default use: helpscout only; empty means all sources). - Runner renders matched servers to a private 0600 .mcp.json temp file, passes --mcp-config/--strict-mcp-config, and auto-allowlists mcp__ tools for both fix and Q&A runs. Secrets stay in env via ${VAR} expansion; runs without an issue never attach MCP. - Threads issue source through the execute path; wires mcp through both runner build sites (lib.rs, main.rs). - Example config + unit tests (round-trip, source gating, rendered config perms/shape). --- claudear.example.toml | 13 ++ crates/claudear-config/src/config.rs | 75 ++++++++ crates/claudear-integrations/Cargo.toml | 3 + .../src/runner/claude.rs | 177 +++++++++++++++++- src/lib.rs | 1 + src/main.rs | 5 + 6 files changed, 268 insertions(+), 6 deletions(-) diff --git a/claudear.example.toml b/claudear.example.toml index e8e1b001..edd18cda 100644 --- a/claudear.example.toml +++ b/claudear.example.toml @@ -143,6 +143,19 @@ api_url = "" # Sandbox mode (e.g., "network-off" for Codex) sandbox = "" +# MCP servers attached to agent runs, keyed by server name. +# Gated per-run by `sources` against the issue source. Default here: HelpScout only. +# Add "discord" to enable there; set sources = [] to enable for all sources. +# Keep secrets out of this file: reference them via ${VAR} from the daemon/provider env. +# [agent.providers.claude.mcp.appwrite] +# command = "uvx" +# args = ["mcp-server-appwrite", "--databases", "--users", "--functions"] +# sources = ["helpscout"] +# [agent.providers.claude.mcp.appwrite.env] +# APPWRITE_ENDPOINT = "https://fra.cloud.appwrite.io/v1" +# APPWRITE_PROJECT_ID = "monitoring-fra" +# APPWRITE_API_KEY = "${APPWRITE_API_KEY}" # read-only key, set in daemon env + # A/B Experiments (optional) # Test different providers or configurations against each other. # diff --git a/crates/claudear-config/src/config.rs b/crates/claudear-config/src/config.rs index 4cd7869f..18905a97 100644 --- a/crates/claudear-config/src/config.rs +++ b/crates/claudear-config/src/config.rs @@ -168,6 +168,42 @@ pub struct ProviderConfig { /// Provider-specific extra configuration. #[serde(default)] pub extra: std::collections::HashMap, + /// MCP servers to attach to agent runs, keyed by server name. Gated per-run by sources. + #[serde(default)] + pub mcp: std::collections::HashMap, +} + +/// A single MCP server serialized into the agent's `.mcp.json` at run time. +/// Reference secrets via `${VAR}` in `env` so they stay out of the config file. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(default)] +pub struct McpServerConfig { + /// Command for a stdio server, e.g. "uvx" or "npx". + pub command: Option, + /// Arguments passed to `command`. + pub args: Vec, + /// Environment for the server process. Values may contain `${VAR}` references. + pub env: std::collections::HashMap, + /// URL for an HTTP/SSE transport server (alternative to `command`). + pub url: Option, + /// Transport type: "stdio" (default when `command` is set), "http", or "sse". + #[serde(rename = "type")] + pub transport: Option, + /// Headers for an HTTP/SSE transport server. + pub headers: std::collections::HashMap, + /// Issue sources this server attaches for. Empty means all sources. + pub sources: Vec, +} + +impl McpServerConfig { + /// Whether this server attaches for a run from `source`. Empty sources means all. + pub fn matches_source(&self, source: Option<&str>) -> bool { + match source { + Some(s) => self.sources.is_empty() || self.sources.iter().any(|allowed| allowed == s), + // Runs without an issue never attach MCP. + None => false, + } + } } /// Experiment configuration for A/B testing providers. @@ -3533,6 +3569,45 @@ mod tests { assert_eq!(cfg.reply().template_for(Some("x")), Some("be nice")); } + #[test] + fn test_mcp_config_parses_from_toml() { + let toml = r#" + [agent.providers.claude.mcp.appwrite] + command = "uvx" + args = ["mcp-server-appwrite", "--databases"] + sources = ["helpscout"] + [agent.providers.claude.mcp.appwrite.env] + APPWRITE_ENDPOINT = "https://fra.cloud.appwrite.io/v1" + APPWRITE_API_KEY = "${APPWRITE_API_KEY}" + "#; + let cfg: Config = toml::from_str(toml).expect("parse"); + let provider = cfg.agent.providers.get("claude").expect("provider"); + let appwrite = provider.mcp.get("appwrite").expect("mcp server"); + assert_eq!(appwrite.command.as_deref(), Some("uvx")); + assert_eq!(appwrite.sources, vec!["helpscout".to_string()]); + assert_eq!( + appwrite.env.get("APPWRITE_API_KEY").map(String::as_str), + Some("${APPWRITE_API_KEY}") + ); + } + + #[test] + fn test_mcp_matches_source() { + let helpscout_only = McpServerConfig { + sources: vec!["helpscout".to_string()], + ..Default::default() + }; + assert!(helpscout_only.matches_source(Some("helpscout"))); + assert!(!helpscout_only.matches_source(Some("discord"))); + assert!(!helpscout_only.matches_source(None)); + + let all_sources = McpServerConfig::default(); + assert!(all_sources.matches_source(Some("discord"))); + assert!(all_sources.matches_source(Some("sentry"))); + // Runs without an issue never attach, even when unrestricted. + assert!(!all_sources.matches_source(None)); + } + #[test] fn test_helpscout_config_parses_from_toml() { let toml = r#" diff --git a/crates/claudear-integrations/Cargo.toml b/crates/claudear-integrations/Cargo.toml index 778a0b9d..42381945 100644 --- a/crates/claudear-integrations/Cargo.toml +++ b/crates/claudear-integrations/Cargo.toml @@ -29,6 +29,9 @@ rustls-acme = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +# Temp files (rendered MCP config passed to the agent CLI) +tempfile = { workspace = true } + # Time chrono = { workspace = true } diff --git a/crates/claudear-integrations/src/runner/claude.rs b/crates/claudear-integrations/src/runner/claude.rs index 7524fadb..d1ef3321 100644 --- a/crates/claudear-integrations/src/runner/claude.rs +++ b/crates/claudear-integrations/src/runner/claude.rs @@ -2,6 +2,7 @@ use super::{AgentRunner, ProviderCapabilities}; use async_trait::async_trait; +use claudear_config::McpServerConfig; use claudear_core::error::{Error, Result}; use claudear_core::templates::{TemplateContext, TemplateLoader, TemplateRenderer}; use claudear_core::types::{ @@ -241,6 +242,9 @@ pub struct ClaudeRunnerConfig { pub binary: String, /// Extra environment variables to set when spawning the agent process. pub env: HashMap, + /// MCP servers to attach, keyed by server name. Attachment is gated per-run + /// by each server's `sources` list against the issue source. + pub mcp: HashMap, } impl Default for ClaudeRunnerConfig { @@ -254,6 +258,7 @@ impl Default for ClaudeRunnerConfig { skip_permissions: false, binary: "claude".to_string(), env: HashMap::new(), + mcp: HashMap::new(), } } } @@ -316,6 +321,7 @@ impl ClaudeAgentRunner { issue_identifier, env, project_dir, + Some("linear"), ) .await } @@ -581,7 +587,8 @@ The PR title should include the issue ID: {} project_dir: &Path, ) -> Result { let (env, label) = self.prepare_env_and_label(issue); - self.execute_with_env(prompt, label, env, project_dir).await + self.execute_with_env(prompt, label, env, project_dir, issue.map(|i| i.source.as_str())) + .await } async fn execute_with_env( @@ -590,8 +597,9 @@ The PR title should include the issue ID: {} label: &str, env: HashMap, project_dir: &Path, + source: Option<&str>, ) -> Result { - self.execute_with_env_and_attempt(prompt, label, env, None, project_dir, true) + self.execute_with_env_and_attempt(prompt, label, env, None, project_dir, true, source) .await } @@ -721,7 +729,15 @@ The PR title should include the issue ID: {} let prompt = build_verify_prompt(issue, context); let (env, _) = self.prepare_env_and_label(Some(issue)); let result = self - .execute_with_env_and_attempt(&prompt, &issue.short_id, env, None, project_dir, false) + .execute_with_env_and_attempt( + &prompt, + &issue.short_id, + env, + None, + project_dir, + false, + Some(&issue.source), + ) .await?; Ok(parse_verify_result(&result.output)) } @@ -739,7 +755,15 @@ The PR title should include the issue ID: {} let prompt = build_reply_prompt(issue, context, guideline, kind); let (env, _) = self.prepare_env_and_label(Some(issue)); let result = self - .execute_with_env_and_attempt(&prompt, &issue.short_id, env, None, project_dir, false) + .execute_with_env_and_attempt( + &prompt, + &issue.short_id, + env, + None, + project_dir, + false, + Some(&issue.source), + ) .await?; if result.success || !result.output.trim().is_empty() { Ok(result.output) @@ -752,6 +776,49 @@ The PR title should include the issue ID: {} } } + /// Render matched MCP servers into a `.mcp.json` in a private temp file (0600), + /// deleted when the returned handle drops. `${VAR}` in env is expanded by the CLI. + fn render_mcp_config( + servers: &[(&String, &McpServerConfig)], + ) -> std::io::Result { + let mut mcp_servers = serde_json::Map::new(); + for (name, cfg) in servers { + let mut entry = serde_json::Map::new(); + if let Some(ref command) = cfg.command { + // stdio transport + entry.insert("command".to_string(), json!(command)); + entry.insert("args".to_string(), json!(cfg.args)); + if !cfg.env.is_empty() { + entry.insert("env".to_string(), json!(cfg.env)); + } + if let Some(ref transport) = cfg.transport { + entry.insert("type".to_string(), json!(transport)); + } + } else if let Some(ref url) = cfg.url { + // http/sse transport + entry.insert( + "type".to_string(), + json!(cfg.transport.clone().unwrap_or_else(|| "http".to_string())), + ); + entry.insert("url".to_string(), json!(url)); + if !cfg.headers.is_empty() { + entry.insert("headers".to_string(), json!(cfg.headers)); + } + } + mcp_servers.insert((*name).clone(), serde_json::Value::Object(entry)); + } + let doc = json!({ "mcpServers": serde_json::Value::Object(mcp_servers) }); + + let file = tempfile::Builder::new() + .prefix("claudear-mcp-") + .suffix(".json") + .tempfile()?; + let bytes = serde_json::to_vec_pretty(&doc).map_err(std::io::Error::other)?; + std::fs::write(file.path(), bytes)?; + Ok(file) + } + + #[allow(clippy::too_many_arguments)] async fn execute_with_env_and_attempt( &self, prompt: &str, @@ -760,6 +827,7 @@ The PR title should include the issue ID: {} attempt_id: Option, project_dir: &Path, structured: bool, + source: Option<&str>, ) -> Result { // Create execution record for analytics let mut execution = AgentExecution::new(); @@ -796,11 +864,58 @@ The PR title should include the issue ID: {} })); self.tracker.record_activity(&activity).ok(); + // Attach MCP servers whose sources match this run; held until return so the + // temp file outlives the child, then auto-deleted. + let matched_mcp: Vec<(&String, &McpServerConfig)> = self + .config + .mcp + .iter() + .filter(|(_, cfg)| cfg.matches_source(source)) + .collect(); + let mut mcp_config_file: Option = None; + if !matched_mcp.is_empty() { + match Self::render_mcp_config(&matched_mcp) { + Ok(file) => { + tracing::info!( + component = "claude", + label = label, + source = source.unwrap_or("none"), + servers = matched_mcp.len(), + "Attaching MCP servers to run" + ); + mcp_config_file = Some(file); + } + Err(e) => { + tracing::warn!( + component = "claude", + label = label, + error = %e, + "Failed to render MCP config; continuing without MCP servers" + ); + } + } + } + // Tool globs to allowlist for the attached servers (empty when none). + let mcp_tool_globs: Vec = if mcp_config_file.is_some() { + matched_mcp + .iter() + .map(|(name, _)| format!("mcp__{}", name)) + .collect() + } else { + Vec::new() + }; + let mut args = vec![ "--verbose".to_string(), "--output-format".to_string(), "stream-json".to_string(), ]; + // Load only our rendered MCP config, ignoring any repo .mcp.json. + if let Some(ref file) = mcp_config_file { + args.push("--mcp-config".to_string()); + args.push(file.path().display().to_string()); + args.push("--strict-mcp-config".to_string()); + } // Structured (fix) runs enforce the result JSON schema. Read-only Q&A // runs return plain assistant text instead. if structured { @@ -836,6 +951,13 @@ The PR title should include the issue ID: {} args.push(perm.clone()); } } + // Allowlist the attached MCP servers' tools for both fix and Q&A runs. + for glob in &mcp_tool_globs { + if !args.iter().any(|a| a == glob) { + args.push("--allowedTools".to_string()); + args.push(glob.clone()); + } + } // Prompt is delivered via stdin (see spawn below), not as a CLI argument, // to avoid the OS argv size limit (E2BIG) on large prompts. `--print` // with no positional prompt reads it from stdin. @@ -1981,8 +2103,16 @@ impl AgentRunner for ClaudeAgentRunner { project_dir: &Path, ) -> Result { let (env, label) = self.prepare_env_and_label(issue); - self.execute_with_env_and_attempt(prompt, label, env, attempt_id, project_dir, true) - .await + self.execute_with_env_and_attempt( + prompt, + label, + env, + attempt_id, + project_dir, + true, + issue.map(|i| i.source.as_str()), + ) + .await } async fn answer_question( @@ -3616,6 +3746,41 @@ mod tests { assert!(debug.contains("events")); } + #[test] + fn test_render_mcp_config_stdio() { + let name = "appwrite".to_string(); + let mut env = HashMap::new(); + env.insert( + "APPWRITE_API_KEY".to_string(), + "${APPWRITE_API_KEY}".to_string(), + ); + let cfg = McpServerConfig { + command: Some("uvx".to_string()), + args: vec!["mcp-server-appwrite".to_string()], + env, + sources: vec!["helpscout".to_string()], + ..Default::default() + }; + let servers = vec![(&name, &cfg)]; + let file = ClaudeAgentRunner::render_mcp_config(&servers).expect("render"); + let doc: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(file.path()).unwrap()).unwrap(); + let server = &doc["mcpServers"]["appwrite"]; + assert_eq!(server["command"], "uvx"); + assert_eq!(server["args"][0], "mcp-server-appwrite"); + assert_eq!(server["env"]["APPWRITE_API_KEY"], "${APPWRITE_API_KEY}"); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(file.path()) + .unwrap() + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o600); + } + } + #[test] fn test_create_execution_log_files_produces_valid_paths() { let _guard = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); diff --git a/src/lib.rs b/src/lib.rs index d83c7a76..687fe7de 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -187,6 +187,7 @@ pub fn build_provider_runner( .and_then(|p| p.binary.clone()) .unwrap_or_else(|| "claude".to_string()), env: provider.map(|p| p.env.clone()).unwrap_or_default(), + mcp: provider.map(|p| p.mcp.clone()).unwrap_or_default(), }, tracker, ); diff --git a/src/main.rs b/src/main.rs index 9a37766b..8c07f55b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3800,6 +3800,11 @@ async fn async_main(cli: Cli) -> anyhow::Result<()> { .default_provider_config() .map(|p| p.env.clone()) .unwrap_or_default(), + mcp: config + .agent + .default_provider_config() + .map(|p| p.mcp.clone()) + .unwrap_or_default(), }, tracker.clone(), ))); From eb858231a1df0be684f0842abcd375e8eecf6a57 Mon Sep 17 00:00:00 2001 From: ArnabChatterjee20k Date: Wed, 5 Aug 2026 14:02:25 +0530 Subject: [PATCH 2/8] fix(agent): address PR review on MCP attachment - Pass issue source as &str via as_str() in verify/reply paths. - Write rendered MCP config to the open temp handle instead of reopening the path (avoids Windows exclusive-lock failures). - Validate exactly one of command/url per server; skip and warn otherwise so strict MCP loading never sees an ambiguous transport. - Add per-server tools allowlist: scope to mcp____ when set, else grant all via mcp__. Lets read-only runs be scoped to read tools; read-only API key remains the enforced boundary. - Example config shows read-only tool scoping; config test covers tools. --- claudear.example.toml | 3 ++ crates/claudear-config/src/config.rs | 6 +++ .../src/runner/claude.rs | 41 +++++++++++++++---- 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/claudear.example.toml b/claudear.example.toml index edd18cda..003d2258 100644 --- a/claudear.example.toml +++ b/claudear.example.toml @@ -151,6 +151,9 @@ sandbox = "" # command = "uvx" # args = ["mcp-server-appwrite", "--databases", "--users", "--functions"] # sources = ["helpscout"] +# Scope to read-only tools so Q&A/verify/reply runs cannot mutate resources. +# Empty/omitted grants all of the server's tools. Use a read-only API key too. +# tools = ["databases_list_documents", "databases_get_document"] # [agent.providers.claude.mcp.appwrite.env] # APPWRITE_ENDPOINT = "https://fra.cloud.appwrite.io/v1" # APPWRITE_PROJECT_ID = "monitoring-fra" diff --git a/crates/claudear-config/src/config.rs b/crates/claudear-config/src/config.rs index 18905a97..9e5ccfa1 100644 --- a/crates/claudear-config/src/config.rs +++ b/crates/claudear-config/src/config.rs @@ -193,6 +193,10 @@ pub struct McpServerConfig { pub headers: std::collections::HashMap, /// Issue sources this server attaches for. Empty means all sources. pub sources: Vec, + /// Specific tool names to allow (allowlisted as `mcp____`). + /// Empty grants all of the server's tools (`mcp__`). Scope this to + /// read-only tools to keep Q&A/verify/reply runs from mutating resources. + pub tools: Vec, } impl McpServerConfig { @@ -3576,6 +3580,7 @@ mod tests { command = "uvx" args = ["mcp-server-appwrite", "--databases"] sources = ["helpscout"] + tools = ["databases_get_document"] [agent.providers.claude.mcp.appwrite.env] APPWRITE_ENDPOINT = "https://fra.cloud.appwrite.io/v1" APPWRITE_API_KEY = "${APPWRITE_API_KEY}" @@ -3585,6 +3590,7 @@ mod tests { let appwrite = provider.mcp.get("appwrite").expect("mcp server"); assert_eq!(appwrite.command.as_deref(), Some("uvx")); assert_eq!(appwrite.sources, vec!["helpscout".to_string()]); + assert_eq!(appwrite.tools, vec!["databases_get_document".to_string()]); assert_eq!( appwrite.env.get("APPWRITE_API_KEY").map(String::as_str), Some("${APPWRITE_API_KEY}") diff --git a/crates/claudear-integrations/src/runner/claude.rs b/crates/claudear-integrations/src/runner/claude.rs index d1ef3321..8b2c1bbe 100644 --- a/crates/claudear-integrations/src/runner/claude.rs +++ b/crates/claudear-integrations/src/runner/claude.rs @@ -736,7 +736,7 @@ The PR title should include the issue ID: {} None, project_dir, false, - Some(&issue.source), + Some(issue.source.as_str()), ) .await?; Ok(parse_verify_result(&result.output)) @@ -762,7 +762,7 @@ The PR title should include the issue ID: {} None, project_dir, false, - Some(&issue.source), + Some(issue.source.as_str()), ) .await?; if result.success || !result.output.trim().is_empty() { @@ -809,12 +809,15 @@ The PR title should include the issue ID: {} } let doc = json!({ "mcpServers": serde_json::Value::Object(mcp_servers) }); - let file = tempfile::Builder::new() + let mut file = tempfile::Builder::new() .prefix("claudear-mcp-") .suffix(".json") .tempfile()?; let bytes = serde_json::to_vec_pretty(&doc).map_err(std::io::Error::other)?; - std::fs::write(file.path(), bytes)?; + // Write to the already-open handle; avoids reopening (fails under Windows locks). + use std::io::Write; + file.as_file_mut().write_all(&bytes)?; + file.as_file_mut().flush()?; Ok(file) } @@ -865,12 +868,25 @@ The PR title should include the issue ID: {} self.tracker.record_activity(&activity).ok(); // Attach MCP servers whose sources match this run; held until return so the - // temp file outlives the child, then auto-deleted. + // temp file outlives the child, then auto-deleted. Require exactly one of + // `command`/`url` so strict MCP loading never rejects an ambiguous server. let matched_mcp: Vec<(&String, &McpServerConfig)> = self .config .mcp .iter() .filter(|(_, cfg)| cfg.matches_source(source)) + .filter(|(name, cfg)| { + let valid = cfg.command.is_some() ^ cfg.url.is_some(); + if !valid { + tracing::warn!( + component = "claude", + label = label, + server = name.as_str(), + "Skipping MCP server: set exactly one of `command` or `url`" + ); + } + valid + }) .collect(); let mut mcp_config_file: Option = None; if !matched_mcp.is_empty() { @@ -895,11 +911,22 @@ The PR title should include the issue ID: {} } } } - // Tool globs to allowlist for the attached servers (empty when none). + // Tools to allowlist for the attached servers (empty when none). A server + // with an explicit `tools` list is scoped to `mcp____`; + // otherwise all of its tools are granted via `mcp__`. let mcp_tool_globs: Vec = if mcp_config_file.is_some() { matched_mcp .iter() - .map(|(name, _)| format!("mcp__{}", name)) + .flat_map(|(name, cfg)| { + if cfg.tools.is_empty() { + vec![format!("mcp__{}", name)] + } else { + cfg.tools + .iter() + .map(|tool| format!("mcp__{}__{}", name, tool)) + .collect() + } + }) .collect() } else { Vec::new() From 07acbc4c81e7e5a5401ee74c5da83043e06caa34 Mon Sep 17 00:00:00 2001 From: ArnabChatterjee20k Date: Wed, 5 Aug 2026 14:03:52 +0530 Subject: [PATCH 3/8] linting --- crates/claudear-integrations/src/runner/claude.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/claudear-integrations/src/runner/claude.rs b/crates/claudear-integrations/src/runner/claude.rs index 8b2c1bbe..90feab64 100644 --- a/crates/claudear-integrations/src/runner/claude.rs +++ b/crates/claudear-integrations/src/runner/claude.rs @@ -587,8 +587,14 @@ The PR title should include the issue ID: {} project_dir: &Path, ) -> Result { let (env, label) = self.prepare_env_and_label(issue); - self.execute_with_env(prompt, label, env, project_dir, issue.map(|i| i.source.as_str())) - .await + self.execute_with_env( + prompt, + label, + env, + project_dir, + issue.map(|i| i.source.as_str()), + ) + .await } async fn execute_with_env( @@ -3800,10 +3806,7 @@ mod tests { #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - let mode = std::fs::metadata(file.path()) - .unwrap() - .permissions() - .mode(); + let mode = std::fs::metadata(file.path()).unwrap().permissions().mode(); assert_eq!(mode & 0o777, 0o600); } } From 3f974cd13ece49e3cc3dc74c7254e275ef01f738 Mon Sep 17 00:00:00 2001 From: ArnabChatterjee20k Date: Wed, 5 Aug 2026 14:23:10 +0530 Subject: [PATCH 4/8] fix(agent): tighten MCP read-only boundary and transport validation - Read-only runs (Q&A/verify/reply) no longer get unscoped MCP tools; a server must declare an explicit `tools` allowlist to be usable there. Fix runs still default to all tools. Closes the read-only-boundary gap. - Validate transport consistency: reject `command` with a non-stdio type and `url` with stdio, not just presence, so --strict-mcp-config never sees a contradictory server. Added has_valid_transport() + tests. - Clarify render_mcp_config doc (temp filename, 0600 is Unix-only). --- claudear.example.toml | 4 +- crates/claudear-config/src/config.rs | 54 +++++++++++++++++++ .../src/runner/claude.rs | 30 +++++++---- 3 files changed, 76 insertions(+), 12 deletions(-) diff --git a/claudear.example.toml b/claudear.example.toml index 003d2258..36cf510e 100644 --- a/claudear.example.toml +++ b/claudear.example.toml @@ -151,8 +151,8 @@ sandbox = "" # command = "uvx" # args = ["mcp-server-appwrite", "--databases", "--users", "--functions"] # sources = ["helpscout"] -# Scope to read-only tools so Q&A/verify/reply runs cannot mutate resources. -# Empty/omitted grants all of the server's tools. Use a read-only API key too. +# Tools to allowlist. Required for read-only runs (Q&A/verify/reply): without it +# they attach no MCP tools. Fix runs with no list get all tools. Use a read-only key. # tools = ["databases_list_documents", "databases_get_document"] # [agent.providers.claude.mcp.appwrite.env] # APPWRITE_ENDPOINT = "https://fra.cloud.appwrite.io/v1" diff --git a/crates/claudear-config/src/config.rs b/crates/claudear-config/src/config.rs index 9e5ccfa1..e1b51410 100644 --- a/crates/claudear-config/src/config.rs +++ b/crates/claudear-config/src/config.rs @@ -208,6 +208,20 @@ impl McpServerConfig { None => false, } } + + /// Whether exactly one transport is configured and any explicit `type` agrees + /// with it. `command` implies stdio; `url` implies http/sse. Rejects neither, + /// both, and contradictions (e.g. `command` with `type = "http"`). + pub fn has_valid_transport(&self) -> bool { + match (self.command.is_some(), self.url.is_some()) { + (true, false) => self.transport.as_deref().is_none_or(|t| t == "stdio"), + (false, true) => self + .transport + .as_deref() + .is_none_or(|t| t == "http" || t == "sse"), + _ => false, + } + } } /// Experiment configuration for A/B testing providers. @@ -3614,6 +3628,46 @@ mod tests { assert!(!all_sources.matches_source(None)); } + #[test] + fn test_mcp_has_valid_transport() { + let stdio = McpServerConfig { + command: Some("uvx".to_string()), + ..Default::default() + }; + assert!(stdio.has_valid_transport()); + + let http = McpServerConfig { + url: Some("https://example/mcp".to_string()), + transport: Some("http".to_string()), + ..Default::default() + }; + assert!(http.has_valid_transport()); + + // Contradictions and ambiguity are rejected. + let command_with_http = McpServerConfig { + command: Some("uvx".to_string()), + transport: Some("http".to_string()), + ..Default::default() + }; + assert!(!command_with_http.has_valid_transport()); + + let url_with_stdio = McpServerConfig { + url: Some("https://example/mcp".to_string()), + transport: Some("stdio".to_string()), + ..Default::default() + }; + assert!(!url_with_stdio.has_valid_transport()); + + let both = McpServerConfig { + command: Some("uvx".to_string()), + url: Some("https://example/mcp".to_string()), + ..Default::default() + }; + assert!(!both.has_valid_transport()); + + assert!(!McpServerConfig::default().has_valid_transport()); + } + #[test] fn test_helpscout_config_parses_from_toml() { let toml = r#" diff --git a/crates/claudear-integrations/src/runner/claude.rs b/crates/claudear-integrations/src/runner/claude.rs index 90feab64..19fed274 100644 --- a/crates/claudear-integrations/src/runner/claude.rs +++ b/crates/claudear-integrations/src/runner/claude.rs @@ -782,8 +782,9 @@ The PR title should include the issue ID: {} } } - /// Render matched MCP servers into a `.mcp.json` in a private temp file (0600), - /// deleted when the returned handle drops. `${VAR}` in env is expanded by the CLI. + /// Render matched MCP servers into a private temp file (claudear-mcp-*.json, + /// 0600 on Unix) passed to the CLI via --mcp-config and deleted when the handle + /// drops. `${VAR}` in env is expanded by the CLI. fn render_mcp_config( servers: &[(&String, &McpServerConfig)], ) -> std::io::Result { @@ -882,13 +883,13 @@ The PR title should include the issue ID: {} .iter() .filter(|(_, cfg)| cfg.matches_source(source)) .filter(|(name, cfg)| { - let valid = cfg.command.is_some() ^ cfg.url.is_some(); + let valid = cfg.has_valid_transport(); if !valid { tracing::warn!( component = "claude", label = label, server = name.as_str(), - "Skipping MCP server: set exactly one of `command` or `url`" + "Skipping MCP server: set exactly one of `command`/`url` with a matching `type`" ); } valid @@ -917,20 +918,29 @@ The PR title should include the issue ID: {} } } } - // Tools to allowlist for the attached servers (empty when none). A server - // with an explicit `tools` list is scoped to `mcp____`; - // otherwise all of its tools are granted via `mcp__`. + // Tools to allowlist for the attached servers. An explicit `tools` list is + // scoped to `mcp____`. With no list, fix runs may use all of a + // server's tools (`mcp__`), but read-only runs (Q&A/verify/reply) + // get none: granting unscoped tools there could permit prod mutations. let mcp_tool_globs: Vec = if mcp_config_file.is_some() { matched_mcp .iter() .flat_map(|(name, cfg)| { - if cfg.tools.is_empty() { - vec![format!("mcp__{}", name)] - } else { + if !cfg.tools.is_empty() { cfg.tools .iter() .map(|tool| format!("mcp__{}__{}", name, tool)) .collect() + } else if structured { + vec![format!("mcp__{}", name)] + } else { + tracing::warn!( + component = "claude", + label = label, + server = name.as_str(), + "Read-only run: MCP server has no `tools` allowlist; not granting its tools" + ); + Vec::new() } }) .collect() From 18de5045b9d3079aae3538afc02a062c5e8aff5a Mon Sep 17 00:00:00 2001 From: ArnabChatterjee20k Date: Wed, 5 Aug 2026 14:28:50 +0530 Subject: [PATCH 5/8] fix(agent): separate read-only MCP tool allowlist from fix-run tools Read-only runs (Q&A/verify/reply) now draw tools only from a dedicated per-server readonly_tools list, never from `tools` (which may include mutating tools used by fix runs). Since a tool's capability cannot be verified at config time, the operator must explicitly list non-mutating tools for read-only use; with none listed, read-only runs get no MCP tools. Closes the remaining production-mutation path. --- claudear.example.toml | 8 +++-- crates/claudear-config/src/config.rs | 15 +++++++-- .../src/runner/claude.rs | 33 +++++++++++-------- 3 files changed, 37 insertions(+), 19 deletions(-) diff --git a/claudear.example.toml b/claudear.example.toml index 36cf510e..423578df 100644 --- a/claudear.example.toml +++ b/claudear.example.toml @@ -151,9 +151,11 @@ sandbox = "" # command = "uvx" # args = ["mcp-server-appwrite", "--databases", "--users", "--functions"] # sources = ["helpscout"] -# Tools to allowlist. Required for read-only runs (Q&A/verify/reply): without it -# they attach no MCP tools. Fix runs with no list get all tools. Use a read-only key. -# tools = ["databases_list_documents", "databases_get_document"] +# tools: allowed on fix runs (empty grants all of the server's tools). +# readonly_tools: allowed on Q&A/verify/reply runs; list only non-mutating tools. +# Read-only runs get no MCP tools unless listed here. Use a read-only API key too. +# tools = ["databases_list_documents", "databases_get_document"] +# readonly_tools = ["databases_list_documents", "databases_get_document"] # [agent.providers.claude.mcp.appwrite.env] # APPWRITE_ENDPOINT = "https://fra.cloud.appwrite.io/v1" # APPWRITE_PROJECT_ID = "monitoring-fra" diff --git a/crates/claudear-config/src/config.rs b/crates/claudear-config/src/config.rs index e1b51410..b286e709 100644 --- a/crates/claudear-config/src/config.rs +++ b/crates/claudear-config/src/config.rs @@ -193,10 +193,14 @@ pub struct McpServerConfig { pub headers: std::collections::HashMap, /// Issue sources this server attaches for. Empty means all sources. pub sources: Vec, - /// Specific tool names to allow (allowlisted as `mcp____`). - /// Empty grants all of the server's tools (`mcp__`). Scope this to - /// read-only tools to keep Q&A/verify/reply runs from mutating resources. + /// Tool names allowed on fix (structured) runs, as `mcp____`. + /// Empty grants all of the server's tools (`mcp__`). pub tools: Vec, + /// Tool names allowed on read-only runs (Q&A/verify/reply), as + /// `mcp____`. Empty grants none: read-only runs never receive + /// unscoped tools, so only tools the operator lists here (which must be + /// non-mutating) are reachable when investigating without a fix. + pub readonly_tools: Vec, } impl McpServerConfig { @@ -3595,6 +3599,7 @@ mod tests { args = ["mcp-server-appwrite", "--databases"] sources = ["helpscout"] tools = ["databases_get_document"] + readonly_tools = ["databases_list_documents"] [agent.providers.claude.mcp.appwrite.env] APPWRITE_ENDPOINT = "https://fra.cloud.appwrite.io/v1" APPWRITE_API_KEY = "${APPWRITE_API_KEY}" @@ -3605,6 +3610,10 @@ mod tests { assert_eq!(appwrite.command.as_deref(), Some("uvx")); assert_eq!(appwrite.sources, vec!["helpscout".to_string()]); assert_eq!(appwrite.tools, vec!["databases_get_document".to_string()]); + assert_eq!( + appwrite.readonly_tools, + vec!["databases_list_documents".to_string()] + ); assert_eq!( appwrite.env.get("APPWRITE_API_KEY").map(String::as_str), Some("${APPWRITE_API_KEY}") diff --git a/crates/claudear-integrations/src/runner/claude.rs b/crates/claudear-integrations/src/runner/claude.rs index 19fed274..5f0d7c1f 100644 --- a/crates/claudear-integrations/src/runner/claude.rs +++ b/crates/claudear-integrations/src/runner/claude.rs @@ -918,29 +918,36 @@ The PR title should include the issue ID: {} } } } - // Tools to allowlist for the attached servers. An explicit `tools` list is - // scoped to `mcp____`. With no list, fix runs may use all of a - // server's tools (`mcp__`), but read-only runs (Q&A/verify/reply) - // get none: granting unscoped tools there could permit prod mutations. + // Tools to allowlist for the attached servers. Fix runs draw from `tools` + // (empty = all of the server's tools via `mcp__`). Read-only runs + // draw only from the operator-declared `readonly_tools`; with none listed + // they get no MCP tools, since we cannot verify a tool is non-mutating. let mcp_tool_globs: Vec = if mcp_config_file.is_some() { matched_mcp .iter() .flat_map(|(name, cfg)| { - if !cfg.tools.is_empty() { - cfg.tools - .iter() - .map(|tool| format!("mcp__{}__{}", name, tool)) - .collect() - } else if structured { - vec![format!("mcp__{}", name)] - } else { + if structured { + if cfg.tools.is_empty() { + vec![format!("mcp__{}", name)] + } else { + cfg.tools + .iter() + .map(|tool| format!("mcp__{}__{}", name, tool)) + .collect() + } + } else if cfg.readonly_tools.is_empty() { tracing::warn!( component = "claude", label = label, server = name.as_str(), - "Read-only run: MCP server has no `tools` allowlist; not granting its tools" + "Read-only run: MCP server has no `readonly_tools`; not granting its tools" ); Vec::new() + } else { + cfg.readonly_tools + .iter() + .map(|tool| format!("mcp__{}__{}", name, tool)) + .collect() } }) .collect() From 794804e8728fb0d67a6cfaf6c5815d4f167300c7 Mon Sep 17 00:00:00 2001 From: ArnabChatterjee20k Date: Wed, 5 Aug 2026 14:46:26 +0530 Subject: [PATCH 6/8] test(agent): cover http transport in render_mcp_config Asserts the url branch emits type/url/headers and omits stdio-only fields (command/args/env). Addresses review coverage gap. --- .../src/runner/claude.rs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/crates/claudear-integrations/src/runner/claude.rs b/crates/claudear-integrations/src/runner/claude.rs index 5f0d7c1f..f3163c3d 100644 --- a/crates/claudear-integrations/src/runner/claude.rs +++ b/crates/claudear-integrations/src/runner/claude.rs @@ -3828,6 +3828,31 @@ mod tests { } } + #[test] + fn test_render_mcp_config_http() { + let name = "remote".to_string(); + let mut headers = HashMap::new(); + headers.insert("Authorization".to_string(), "Bearer ${TOKEN}".to_string()); + let cfg = McpServerConfig { + url: Some("https://example.com/mcp".to_string()), + transport: Some("http".to_string()), + headers, + ..Default::default() + }; + let servers = vec![(&name, &cfg)]; + let file = ClaudeAgentRunner::render_mcp_config(&servers).expect("render"); + let doc: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(file.path()).unwrap()).unwrap(); + let server = &doc["mcpServers"]["remote"]; + assert_eq!(server["type"], "http"); + assert_eq!(server["url"], "https://example.com/mcp"); + assert_eq!(server["headers"]["Authorization"], "Bearer ${TOKEN}"); + // stdio-only fields must be absent for an http transport. + assert!(server.get("command").is_none()); + assert!(server.get("args").is_none()); + assert!(server.get("env").is_none()); + } + #[test] fn test_create_execution_log_files_produces_valid_paths() { let _guard = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); From ec064a7b9d9a2760bd2ac19d045d609daf3e4af8 Mon Sep 17 00:00:00 2001 From: ArnabChatterjee20k Date: Wed, 5 Aug 2026 15:04:10 +0530 Subject: [PATCH 7/8] refactor(agent): accurate MCP comment; tests read via open temp handle - Reword the strict-mcp-config comment: flags are only added when a config is attached; no MCP flags when nothing matches. - Render tests read the temp file via reopen() instead of by path, matching render_mcp_config's Windows-safe handle write. --- .../claudear-integrations/src/runner/claude.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/claudear-integrations/src/runner/claude.rs b/crates/claudear-integrations/src/runner/claude.rs index f3163c3d..d8a139b9 100644 --- a/crates/claudear-integrations/src/runner/claude.rs +++ b/crates/claudear-integrations/src/runner/claude.rs @@ -960,7 +960,8 @@ The PR title should include the issue ID: {} "--output-format".to_string(), "stream-json".to_string(), ]; - // Load only our rendered MCP config, ignoring any repo .mcp.json. + // When we attach a rendered config, load only it (--strict ignores any repo + // .mcp.json). With no servers matched, no MCP flags are added at all. if let Some(ref file) = mcp_config_file { args.push("--mcp-config".to_string()); args.push(file.path().display().to_string()); @@ -3796,6 +3797,15 @@ mod tests { assert!(debug.contains("events")); } + // Read a rendered temp file via the already-open handle (avoids reopening by + // path, which can lock on Windows), mirroring render_mcp_config's own approach. + fn read_temp(file: &tempfile::NamedTempFile) -> String { + use std::io::Read; + let mut s = String::new(); + file.reopen().unwrap().read_to_string(&mut s).unwrap(); + s + } + #[test] fn test_render_mcp_config_stdio() { let name = "appwrite".to_string(); @@ -3813,8 +3823,7 @@ mod tests { }; let servers = vec![(&name, &cfg)]; let file = ClaudeAgentRunner::render_mcp_config(&servers).expect("render"); - let doc: serde_json::Value = - serde_json::from_str(&std::fs::read_to_string(file.path()).unwrap()).unwrap(); + let doc: serde_json::Value = serde_json::from_str(&read_temp(&file)).unwrap(); let server = &doc["mcpServers"]["appwrite"]; assert_eq!(server["command"], "uvx"); assert_eq!(server["args"][0], "mcp-server-appwrite"); @@ -3841,8 +3850,7 @@ mod tests { }; let servers = vec![(&name, &cfg)]; let file = ClaudeAgentRunner::render_mcp_config(&servers).expect("render"); - let doc: serde_json::Value = - serde_json::from_str(&std::fs::read_to_string(file.path()).unwrap()).unwrap(); + let doc: serde_json::Value = serde_json::from_str(&read_temp(&file)).unwrap(); let server = &doc["mcpServers"]["remote"]; assert_eq!(server["type"], "http"); assert_eq!(server["url"], "https://example.com/mcp"); From 5acc2bc41d916da614916cdfdeca0fb97deb0285 Mon Sep 17 00:00:00 2001 From: ArnabChatterjee20k Date: Wed, 5 Aug 2026 15:34:18 +0530 Subject: [PATCH 8/8] refactor(agent): single tools allowlist for all MCP runs The API key is the access boundary, so a separate read-only tool list added complexity without a real guarantee. Collapse to one `tools` array (empty = all of the server's tools) applied uniformly to fix and Q&A runs. Drop readonly_tools. --- claudear.example.toml | 7 ++--- crates/claudear-config/src/config.rs | 15 ++-------- .../src/runner/claude.rs | 28 ++++--------------- 3 files changed, 11 insertions(+), 39 deletions(-) diff --git a/claudear.example.toml b/claudear.example.toml index 423578df..31dd782a 100644 --- a/claudear.example.toml +++ b/claudear.example.toml @@ -151,11 +151,8 @@ sandbox = "" # command = "uvx" # args = ["mcp-server-appwrite", "--databases", "--users", "--functions"] # sources = ["helpscout"] -# tools: allowed on fix runs (empty grants all of the server's tools). -# readonly_tools: allowed on Q&A/verify/reply runs; list only non-mutating tools. -# Read-only runs get no MCP tools unless listed here. Use a read-only API key too. -# tools = ["databases_list_documents", "databases_get_document"] -# readonly_tools = ["databases_list_documents", "databases_get_document"] +# tools: tool names to allow (empty/omitted grants all of the server's tools). +# tools = ["databases_list_documents", "databases_get_document"] # [agent.providers.claude.mcp.appwrite.env] # APPWRITE_ENDPOINT = "https://fra.cloud.appwrite.io/v1" # APPWRITE_PROJECT_ID = "monitoring-fra" diff --git a/crates/claudear-config/src/config.rs b/crates/claudear-config/src/config.rs index b286e709..88fd3b0b 100644 --- a/crates/claudear-config/src/config.rs +++ b/crates/claudear-config/src/config.rs @@ -193,14 +193,10 @@ pub struct McpServerConfig { pub headers: std::collections::HashMap, /// Issue sources this server attaches for. Empty means all sources. pub sources: Vec, - /// Tool names allowed on fix (structured) runs, as `mcp____`. - /// Empty grants all of the server's tools (`mcp__`). + /// Tool names to allow, as `mcp____`. Empty grants all of the + /// server's tools (`mcp__`). Applies to every run that attaches this + /// server. pub tools: Vec, - /// Tool names allowed on read-only runs (Q&A/verify/reply), as - /// `mcp____`. Empty grants none: read-only runs never receive - /// unscoped tools, so only tools the operator lists here (which must be - /// non-mutating) are reachable when investigating without a fix. - pub readonly_tools: Vec, } impl McpServerConfig { @@ -3599,7 +3595,6 @@ mod tests { args = ["mcp-server-appwrite", "--databases"] sources = ["helpscout"] tools = ["databases_get_document"] - readonly_tools = ["databases_list_documents"] [agent.providers.claude.mcp.appwrite.env] APPWRITE_ENDPOINT = "https://fra.cloud.appwrite.io/v1" APPWRITE_API_KEY = "${APPWRITE_API_KEY}" @@ -3610,10 +3605,6 @@ mod tests { assert_eq!(appwrite.command.as_deref(), Some("uvx")); assert_eq!(appwrite.sources, vec!["helpscout".to_string()]); assert_eq!(appwrite.tools, vec!["databases_get_document".to_string()]); - assert_eq!( - appwrite.readonly_tools, - vec!["databases_list_documents".to_string()] - ); assert_eq!( appwrite.env.get("APPWRITE_API_KEY").map(String::as_str), Some("${APPWRITE_API_KEY}") diff --git a/crates/claudear-integrations/src/runner/claude.rs b/crates/claudear-integrations/src/runner/claude.rs index d8a139b9..3d216bb6 100644 --- a/crates/claudear-integrations/src/runner/claude.rs +++ b/crates/claudear-integrations/src/runner/claude.rs @@ -918,33 +918,17 @@ The PR title should include the issue ID: {} } } } - // Tools to allowlist for the attached servers. Fix runs draw from `tools` - // (empty = all of the server's tools via `mcp__`). Read-only runs - // draw only from the operator-declared `readonly_tools`; with none listed - // they get no MCP tools, since we cannot verify a tool is non-mutating. + // Tools to allowlist for the attached servers. An explicit `tools` list is + // scoped to `mcp____`; empty grants all of the server's tools + // via `mcp__`. Applied uniformly to fix and read-only runs. let mcp_tool_globs: Vec = if mcp_config_file.is_some() { matched_mcp .iter() .flat_map(|(name, cfg)| { - if structured { - if cfg.tools.is_empty() { - vec![format!("mcp__{}", name)] - } else { - cfg.tools - .iter() - .map(|tool| format!("mcp__{}__{}", name, tool)) - .collect() - } - } else if cfg.readonly_tools.is_empty() { - tracing::warn!( - component = "claude", - label = label, - server = name.as_str(), - "Read-only run: MCP server has no `readonly_tools`; not granting its tools" - ); - Vec::new() + if cfg.tools.is_empty() { + vec![format!("mcp__{}", name)] } else { - cfg.readonly_tools + cfg.tools .iter() .map(|tool| format!("mcp__{}__{}", name, tool)) .collect()