diff --git a/claudear.example.toml b/claudear.example.toml index e8e1b00..31dd782 100644 --- a/claudear.example.toml +++ b/claudear.example.toml @@ -143,6 +143,21 @@ 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"] +# 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" +# 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 4cd7869..88fd3b0 100644 --- a/crates/claudear-config/src/config.rs +++ b/crates/claudear-config/src/config.rs @@ -168,6 +168,60 @@ 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, + /// 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, +} + +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, + } + } + + /// 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. @@ -3533,6 +3587,87 @@ 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"] + tools = ["databases_get_document"] + [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.tools, vec!["databases_get_document".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_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/Cargo.toml b/crates/claudear-integrations/Cargo.toml index 778a0b9..4238194 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 7524fad..3d216bb 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,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).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 +603,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 +735,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.as_str()), + ) .await?; Ok(parse_verify_result(&result.output)) } @@ -739,7 +761,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.as_str()), + ) .await?; if result.success || !result.output.trim().is_empty() { Ok(result.output) @@ -752,6 +782,53 @@ The PR title should include the issue ID: {} } } + /// 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 { + 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 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)?; + // 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) + } + + #[allow(clippy::too_many_arguments)] async fn execute_with_env_and_attempt( &self, prompt: &str, @@ -760,6 +837,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 +874,83 @@ 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. 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.has_valid_transport(); + if !valid { + tracing::warn!( + component = "claude", + label = label, + server = name.as_str(), + "Skipping MCP server: set exactly one of `command`/`url` with a matching `type`" + ); + } + valid + }) + .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" + ); + } + } + } + // 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 cfg.tools.is_empty() { + vec![format!("mcp__{}", name)] + } else { + cfg.tools + .iter() + .map(|tool| format!("mcp__{}__{}", name, tool)) + .collect() + } + }) + .collect() + } else { + Vec::new() + }; + let mut args = vec![ "--verbose".to_string(), "--output-format".to_string(), "stream-json".to_string(), ]; + // 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()); + 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 +986,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 +2138,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 +3781,70 @@ 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(); + 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(&read_temp(&file)).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_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(&read_temp(&file)).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()); diff --git a/src/lib.rs b/src/lib.rs index d83c7a7..687fe7d 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 9a37766..8c07f55 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(), )));