diff --git a/claudear.example.toml b/claudear.example.toml index e8e1b001..6f1847cd 100644 --- a/claudear.example.toml +++ b/claudear.example.toml @@ -143,6 +143,38 @@ 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 + +# Grafana (mcp-grafana) — query dashboards, datasources, and metrics so the +# agent can pull live telemetry when triaging an issue. +# [agent.providers.claude.mcp.grafana] +# command = "uvx" +# args = ["mcp-grafana"] +# sources = ["sentry"] +# tools: tool names to allow (empty/omitted grants all of the server's tools). +# tools = ["search_dashboards", "query_prometheus", "list_datasources"] +# [agent.providers.claude.mcp.grafana.env] +# GRAFANA_URL = "https://telemetry.example.com/" +# Env values are written to .mcp.json verbatim, so you can inline literals here +# or reference ${VAR} from the daemon env (Claude Code expands ${VAR}). +# GRAFANA_SERVICE_ACCOUNT_TOKEN = "glsa_xxxxxxxxxxxx" # or "${GRAFANA_SERVICE_ACCOUNT_TOKEN}" +# When Grafana sits behind Cloudflare Access, pass the service-token headers as a +# JSON string (inline literals or ${VAR} both work). +# GRAFANA_EXTRA_HEADERS = "{\"CF-Access-Client-Id\": \"your-cf-access-client-id\", \"CF-Access-Client-Secret\": \"your-cf-access-client-secret\"}" + # A/B Experiments (optional) # Test different providers or configurations against each other. # diff --git a/crates/claudear-analysis/src/evaluation/types.rs b/crates/claudear-analysis/src/evaluation/types.rs index c43bfe08..d64f01a8 100644 --- a/crates/claudear-analysis/src/evaluation/types.rs +++ b/crates/claudear-analysis/src/evaluation/types.rs @@ -29,6 +29,20 @@ impl EvaluationResult { } } + /// Whether any test tool gained new failures vs the baseline. + pub fn has_new_test_failures(&self) -> bool { + self.deltas + .iter() + .any(|d| d.after.category == EvalCategory::Test && d.new_failures > 0) + } + + /// Whether the fix introduced new failures or regressions in any tool. + pub fn has_regressions(&self) -> bool { + self.deltas + .iter() + .any(|d| d.new_failures > 0 || !d.regressions.is_empty()) + } + fn build_summary(deltas: &[EvalDelta]) -> String { if deltas.is_empty() { return "No evaluation tools ran.".to_string(); @@ -225,6 +239,42 @@ mod tests { assert!(!result.summary.is_empty()); } + #[test] + fn test_has_new_test_failures() { + // A newly-added failing test (red) shows up as a new test failure. + let red = EvaluationResult::new( + 1, + "org/repo".into(), + vec![EvalDelta::compute( + make_snapshot(EvalCategory::Test, "cargo test", 10, 0), + make_snapshot(EvalCategory::Test, "cargo test", 10, 1), + )], + ); + assert!(red.has_new_test_failures()); + + // Once the fix lands, the test passes again (green) — no new test failures. + let green = EvaluationResult::new( + 1, + "org/repo".into(), + vec![EvalDelta::compute( + make_snapshot(EvalCategory::Test, "cargo test", 10, 0), + make_snapshot(EvalCategory::Test, "cargo test", 11, 0), + )], + ); + assert!(!green.has_new_test_failures()); + + // A lint regression is not a test failure. + let lint = EvaluationResult::new( + 1, + "org/repo".into(), + vec![EvalDelta::compute( + make_snapshot(EvalCategory::Lint, "clippy", 10, 0), + make_snapshot(EvalCategory::Lint, "clippy", 10, 1), + )], + ); + assert!(!lint.has_new_test_failures()); + } + #[test] fn test_evaluation_result_pr_comment() { let before = make_snapshot(EvalCategory::Test, "cargo test", 10, 2); diff --git a/crates/claudear-analysis/src/knowledgebase/discord/mod.rs b/crates/claudear-analysis/src/knowledgebase/discord/mod.rs index 894eef39..a6980c0e 100644 --- a/crates/claudear-analysis/src/knowledgebase/discord/mod.rs +++ b/crates/claudear-analysis/src/knowledgebase/discord/mod.rs @@ -319,18 +319,11 @@ pub fn format_discord_search_context(results: &[DiscordSearchResult]) -> String for (i, result) in results.iter().enumerate() { let chunk = &result.chunk; - let label = match chunk.guild_id.as_deref().filter(|g| !g.is_empty()) { - Some(guild_id) => format!( - "[Channel `{}`](https://discord.com/channels/{}/{}/{})", - chunk.channel_id, guild_id, chunk.channel_id, chunk.start_message_id, - ), - None => format!("Channel `{}`", chunk.channel_id), - }; let _ = writeln!( context, "### {}. {} (Similarity: {:.0}%)", i + 1, - label, + discord_span_links(chunk), result.score * 100.0, ); @@ -361,6 +354,61 @@ pub fn format_discord_search_context(results: &[DiscordSearchResult]) -> String context } +/// Build a Discord jump URL to a single message. Falls back to the `@me` +/// (DM) path when the guild id is missing so we always emit a clickable link. +fn discord_jump_url(guild_id: Option<&str>, channel_id: &str, message_id: &str) -> String { + match guild_id.filter(|g| !g.is_empty()) { + Some(guild_id) => format!( + "https://discord.com/channels/{}/{}/{}", + guild_id, channel_id, message_id + ), + None => format!( + "https://discord.com/channels/@me/{}/{}", + channel_id, message_id + ), + } +} + +/// Render a chunk's channel label with jump links spanning its window: `from` +/// (first message) and `to` (last message). Collapses to a single link when the +/// window is one message. +fn discord_span_links(chunk: &DiscordMessageChunk) -> String { + let guild = chunk.guild_id.as_deref(); + let start = discord_jump_url(guild, &chunk.channel_id, &chunk.start_message_id); + if chunk.end_message_id == chunk.start_message_id { + format!("[Channel `{}`]({})", chunk.channel_id, start) + } else { + let end = discord_jump_url(guild, &chunk.channel_id, &chunk.end_message_id); + format!( + "Channel `{}` — [from]({}) → [to]({})", + chunk.channel_id, start, end + ) + } +} + +/// Build a compact Discord-markdown block of jump links to the retrieved +/// discussions, for appending to an outgoing notification so readers can open +/// the referenced conversations. Each entry links the start and end of the +/// conversation window. Returns an empty string when there are no results. +pub fn format_discord_reference_links(results: &[DiscordSearchResult]) -> String { + use std::fmt::Write; + + if results.is_empty() { + return String::new(); + } + + let mut out = String::from("\n\n\u{1F4CE} **Referenced Discord discussions**\n"); + for result in results { + let _ = writeln!( + out, + "- {} ({:.0}%)", + discord_span_links(&result.chunk), + result.score * 100.0, + ); + } + out +} + fn sha256_hex(text: &str) -> String { let mut hasher = Sha256::new(); hasher.update(text.as_bytes()); @@ -450,19 +498,20 @@ mod tests { assert!(out.contains("95%")); assert!(out.contains("alice, bob")); assert!(out.contains("alice: hi")); - // Channel heading is a clickable deep link to the first message. - assert!(out.contains("[Channel `chan1`](https://discord.com/channels/g/chan1/1)")); + // A multi-message window links both the first and last message. + assert!(out.contains("[from](https://discord.com/channels/g/chan1/1)")); + assert!(out.contains("[to](https://discord.com/channels/g/chan1/2)")); } #[test] - fn test_format_context_without_guild_is_not_linked() { + fn test_format_context_without_guild_uses_me_fallback() { let chunk = DiscordMessageChunk { id: Some(1), guild_id: None, channel_id: "chan1".to_string(), channel_kind: DiscordChannelKind::Channel, start_message_id: "1".to_string(), - end_message_id: "2".to_string(), + end_message_id: "1".to_string(), participant_ids: None, start_message_time: "2024-01-01T10:00:00Z".to_string(), end_message_time: "2024-01-01T10:05:00Z".to_string(), @@ -472,9 +521,38 @@ mod tests { }; let out = format_discord_search_context(&[DiscordSearchResult { chunk, score: 0.5 }]); - assert!(out.contains("Channel `chan1`")); - // No guild id => no permalink. - assert!(!out.contains("https://discord.com/channels")); + // Missing guild id => still linkable via the @me path; single message + // collapses to one link. + assert!(out.contains("[Channel `chan1`](https://discord.com/channels/@me/chan1/1)")); + } + + #[test] + fn test_reference_links_empty_is_empty_string() { + assert!(format_discord_reference_links(&[]).is_empty()); + } + + #[test] + fn test_reference_links_span_and_score() { + let chunk = DiscordMessageChunk { + id: Some(1), + guild_id: Some("g".to_string()), + channel_id: "chan1".to_string(), + channel_kind: DiscordChannelKind::Channel, + start_message_id: "10".to_string(), + end_message_id: "20".to_string(), + participant_ids: None, + start_message_time: "2024-01-01T10:00:00Z".to_string(), + end_message_time: "2024-01-01T10:05:00Z".to_string(), + chunk_text: "hi".to_string(), + context_text: "ctx".to_string(), + content_hash: Some("h".to_string()), + }; + let out = format_discord_reference_links(&[DiscordSearchResult { chunk, score: 0.87 }]); + + assert!(out.contains("Referenced Discord discussions")); + assert!(out.contains("[from](https://discord.com/channels/g/chan1/10)")); + assert!(out.contains("[to](https://discord.com/channels/g/chan1/20)")); + assert!(out.contains("87%")); } // ---- end-to-end index (needs embedding model + sqlite) -------------- diff --git a/crates/claudear-analysis/src/knowledgebase/mod.rs b/crates/claudear-analysis/src/knowledgebase/mod.rs index 252dbd8f..238f310f 100644 --- a/crates/claudear-analysis/src/knowledgebase/mod.rs +++ b/crates/claudear-analysis/src/knowledgebase/mod.rs @@ -1,6 +1,6 @@ pub mod discord; pub use discord::{ - format_discord_search_context, DiscordIndexer, DiscordMessageInput, DiscordSearchService, - DISCORD_INDEX_VERSION, + format_discord_reference_links, format_discord_search_context, DiscordIndexer, + DiscordMessageInput, DiscordSearchService, DISCORD_INDEX_VERSION, }; diff --git a/crates/claudear-config/src/config.rs b/crates/claudear-config/src/config.rs index 4cd7869f..d619e9cf 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. @@ -751,6 +805,9 @@ pub struct ReplyConfig { pub templates: std::collections::HashMap, /// Timeout for verifying (reproducing) a reported bug, in seconds (default: 1800). pub verify_timeout_secs: u64, + /// When verify can't run (timeout/error/unsupported), assume reproduced and fix + /// anyway (default: true). Set false to ask the reporter for repro steps instead. + pub verify_fail_open: bool, } impl Default for ReplyConfig { @@ -761,6 +818,7 @@ impl Default for ReplyConfig { default_template: None, templates: std::collections::HashMap::new(), verify_timeout_secs: 1800, + verify_fail_open: true, } } } @@ -1032,6 +1090,9 @@ pub struct EvaluationConfig { pub post_pr_comment: bool, /// Fail the fix attempt on regression. pub fail_on_regression: bool, + /// Enforce red->green: author a failing test first (must fail on the unfixed + /// code), then fix, then require it to pass. Needs test_delta enabled. + pub require_red_green: bool, /// Custom test command override. pub custom_test_cmd: Option, /// Custom lint command override. @@ -1054,6 +1115,7 @@ impl Default for EvaluationConfig { total_timeout_secs: 900, post_pr_comment: true, fail_on_regression: false, + require_red_green: false, custom_test_cmd: None, custom_lint_cmd: None, custom_analysis_cmd: None, @@ -3533,6 +3595,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#" @@ -7985,6 +8128,7 @@ instructions_file = "my-instructions.md" total_timeout_secs: 1800, post_pr_comment: false, fail_on_regression: true, + require_red_green: false, custom_test_cmd: Some("npm test".to_string()), custom_lint_cmd: None, custom_analysis_cmd: Some("sonar".to_string()), diff --git a/crates/claudear-core/src/types.rs b/crates/claudear-core/src/types.rs index 8ea4e0d2..6ba512f3 100644 --- a/crates/claudear-core/src/types.rs +++ b/crates/claudear-core/src/types.rs @@ -3232,6 +3232,18 @@ pub enum TimelineEventStatus { #[serde(rename = "verify_completed")] VerifyCompleted, + /// Red-green: the failing-test (red) phase began. + #[serde(rename = "red_green_started")] + RedGreenStarted, + + /// Red-green: the authored test failed on the unfixed code (red confirmed). + #[serde(rename = "red_confirmed")] + RedConfirmed, + + /// Red-green: the authored test passed after the fix (green confirmed). + #[serde(rename = "green_confirmed")] + GreenConfirmed, + #[serde(rename = "reply_started")] ReplyStarted, @@ -3292,6 +3304,9 @@ impl TimelineEventStatus { Self::FixStarted => "fix_started", Self::VerifyStarted => "verify_started", Self::VerifyCompleted => "verify_completed", + Self::RedGreenStarted => "red_green_started", + Self::RedConfirmed => "red_confirmed", + Self::GreenConfirmed => "green_confirmed", Self::ReplyStarted => "reply_started", Self::ReplySent => "reply_sent", Self::FixSucceeded => "fix_succeeded", diff --git a/crates/claudear-e2e/src/config.rs b/crates/claudear-e2e/src/config.rs index 6874c7b0..aabb660a 100644 --- a/crates/claudear-e2e/src/config.rs +++ b/crates/claudear-e2e/src/config.rs @@ -235,6 +235,7 @@ impl ConfigBuilder { template.to_string(), )]), verify_timeout_secs, + verify_fail_open: true, }; self } diff --git a/crates/claudear-engine/src/processing.rs b/crates/claudear-engine/src/processing.rs index ead047cb..55d185ec 100644 --- a/crates/claudear-engine/src/processing.rs +++ b/crates/claudear-engine/src/processing.rs @@ -155,6 +155,56 @@ fn build_verification_note( out } +/// Prompt for the red phase: write a failing test only, no fix. +fn build_failing_test_prompt(issue: &Issue, context: &str) -> String { + format!( + "You are reproducing a bug from {source} by writing a FAILING test. Do NOT fix it yet.\n\n\ + {context}\n\n\ + Issue: {short_id} - {title}\n\n\ + Instructions:\n\ + 1. Analyze the issue and locate the relevant code.\n\ + 2. Add a single new test that reproduces the bug. It MUST fail against the current, \ + unfixed code.\n\ + 3. Do NOT modify any application/source code — only add the test.\n\ + 4. Do NOT open a PR, commit, or push.\n\ + Stop once the failing test is written.", + source = issue.source, + context = context, + short_id = issue.short_id, + title = issue.title, + ) +} + +/// Whether a verify verdict has real findings (the conservative fallbacks don't). +fn diagnosis_has_details(verdict: &VerifyResult) -> bool { + !verdict.impact.trim().is_empty() + || !verdict.root_cause.trim().is_empty() + || !verdict.suggested_fix.trim().is_empty() + || !verdict.evidence.trim().is_empty() +} + +/// Build the diagnosis block prepended to the fix prompt context. +fn build_diagnosis_context(verdict: &VerifyResult) -> String { + let mut out = String::from( + "## Verified diagnosis (from the reproduce/verify stage)\n\nThis issue was \ + independently reproduced before this fix run. Treat the findings below as the \ + starting point: confirm them in code, then implement the minimal fix. Do not \ + re-litigate whether the bug exists.\n", + ); + let mut section = |label: &str, body: &str| { + let body = body.trim(); + if !body.is_empty() { + out.push_str(&format!("\n{label}: {body}\n")); + } + }; + section("Summary", &verdict.summary); + section("Why it's an issue", &verdict.impact); + section("Root cause", &verdict.root_cause); + section("Suggested fix direction", &verdict.suggested_fix); + section("Evidence", &verdict.evidence); + out +} + /// Heuristic bug/security detection used as a fallback when the LLM classifier is /// unavailable. Mirrors `FixAttempt::is_bug`: Sentry issues are always bugs, and /// any label containing a known bug word counts. @@ -221,6 +271,8 @@ pub struct ProcessingInput { pub review_feedback: Option, pub existing_pr_branch: Option, pub intent: Option, + /// Diagnosis carried forward from the reproduce/verify stage into the fix run. + pub diagnosis: Option, } /// What happened during processing. @@ -317,6 +369,7 @@ impl IssueProcessor { attempt_id, ref review_feedback, ref existing_pr_branch, + ref diagnosis, .. } = input; @@ -479,6 +532,108 @@ impl IssueProcessor { Vec::new() }; + // Red phase: author a failing test and require it to fail before fixing. + if self.config.evaluation.require_red_green { + let has_test_baseline = eval_before_snapshots + .iter() + .any(|s| s.category == claudear_core::types::EvalCategory::Test); + if !has_test_baseline { + tracing::warn!( + short_id = %issue.short_id, + "require_red_green set but no test tool detected; skipping red-green" + ); + } else { + self.record_timeline_event( + issue, + TimelineEventStatus::RedGreenStarted, + format!("Writing failing test for {}", issue.short_id), + json!({}), + ); + self.record_issue_decision( + issue, + "red_green_started", + format!("Red phase: authoring failing test for {}", issue.short_id), + json!({}), + ); + let (context, _discord_refs) = self.build_rag_context(issue, attempt_id).await; + let prompt = build_failing_test_prompt(issue, &context); + match self + .agent + .execute_with_attempt( + &prompt, + Some(&*issue), + attempt_id, + &effective_project_dir, + ) + .await + { + Ok(_) => { + let repo = resolution.repo_name().unwrap_or("unknown").to_string(); + let red = claudear_analysis::evaluation::CodeQualityEvaluator::run_after_and_compute_deltas( + &effective_project_dir, + &self.config.evaluation, + eval_before_snapshots.clone(), + attempt_id.unwrap_or(0), + &repo, + ) + .await; + let is_red = matches!(&red, Ok(r) if r.has_new_test_failures()); + if !is_red { + let error = "Red-green: authored test did not fail on the unfixed code; could not confirm reproduction".to_string(); + tracing::warn!(short_id = %issue.short_id, "{}", error); + let _ = self.tracker.record_action_run( + source_name, + &issue.id, + &issue.short_id, + "red_green", + "not_reproduced", + &error, + ); + self.record_issue_decision( + issue, + "red_green_not_reproduced", + error.clone(), + json!({}), + ); + self.tracker + .mark_failed(source_name, &issue.id, &error) + .ok(); + self.cleanup_worktree(resolution, issue, &project_dir).await; + return Ok(ProcessingOutcome::Failed { error }); + } + tracing::info!(short_id = %issue.short_id, "Red-green: failing test confirmed (red)"); + let _ = self.tracker.record_action_run( + source_name, + &issue.id, + &issue.short_id, + "red_green", + "red_confirmed", + "Authored test fails on unfixed code", + ); + self.record_timeline_event( + issue, + TimelineEventStatus::RedConfirmed, + format!("Failing test confirmed (red) for {}", issue.short_id), + json!({}), + ); + self.record_issue_decision( + issue, + "red_confirmed", + format!( + "Red confirmed: test fails on unfixed code for {}", + issue.short_id + ), + json!({}), + ); + } + Err(e) => { + // Agent infra error: skip the red gate rather than fail the attempt. + tracing::warn!(short_id = %issue.short_id, error = %e, "Red phase agent run failed; skipping red-green"); + } + } + } + } + // Resolve issue assignee to a configured user if let Some(assignee) = issue.get_metadata::("assignee") { if let Some(resolved) = self.user_registry.resolve(&issue.source, &assignee) { @@ -528,7 +683,7 @@ impl IssueProcessor { let mut current_project_dir = project_dir.clone(); let mut current_effective_dir = effective_project_dir.clone(); - let result = loop { + let mut result = loop { let pipeline_result = self .execute_pipeline( issue, @@ -537,6 +692,7 @@ impl IssueProcessor { attempt_id, review_feedback.as_deref(), existing_pr_branch.as_deref(), + diagnosis.as_ref(), ¤t_effective_dir, context_provider, ) @@ -710,6 +866,8 @@ impl IssueProcessor { self.tracker.record_metric(&processing_time_metric).ok(); // Run code quality evaluation (AFTER hook) + let mut regression_gate_tripped = false; + let mut regression_reason = String::new(); if !eval_before_snapshots.is_empty() { let eval_attempt_id = attempt_id.unwrap_or(0); let eval_repo = current_resolution.repo_name().unwrap_or("unknown"); @@ -731,6 +889,51 @@ impl IssueProcessor { "Evaluation complete" ); + // Gate the fix on regressions (and, in red-green mode, on a + // test that still fails after the fix). + let gate = self.config.evaluation.fail_on_regression + || self.config.evaluation.require_red_green; + regression_gate_tripped = gate && eval_result.has_regressions(); + if self.config.evaluation.require_red_green { + if regression_gate_tripped && eval_result.has_new_test_failures() { + regression_reason = + "Red-green: authored test still fails after the fix (not green)" + .to_string(); + let _ = self.tracker.record_action_run( + source_name, + &issue.id, + &issue.short_id, + "red_green", + "not_green", + ®ression_reason, + ); + } else if !eval_result.has_new_test_failures() { + let _ = self.tracker.record_action_run( + source_name, + &issue.id, + &issue.short_id, + "red_green", + "green_confirmed", + "Authored test passes after the fix", + ); + self.record_timeline_event( + issue, + TimelineEventStatus::GreenConfirmed, + format!("Test passes after fix (green) for {}", issue.short_id), + json!({}), + ); + self.record_issue_decision( + issue, + "green_confirmed", + format!( + "Green confirmed: test passes after fix for {}", + issue.short_id + ), + json!({}), + ); + } + } + // Post evaluation comment on PR if self.config.evaluation.post_pr_comment { let pr_url = match &result { @@ -767,6 +970,22 @@ impl IssueProcessor { } } + // Fail a successful attempt whose fix introduced regressions (triggers retry). + if regression_gate_tripped { + if let Ok(ProcessingOutcome::Success { pr_url }) = &result { + let error = if regression_reason.is_empty() { + "Fix introduced quality regressions (fail_on_regression)".to_string() + } else { + regression_reason.clone() + }; + tracing::warn!(short_id = %issue.short_id, pr_url = %pr_url, "{}", error); + self.tracker + .mark_failed(source_name, &issue.id, &error) + .ok(); + result = Ok(ProcessingOutcome::Failed { error }); + } + } + // Cleanup worktree self.cleanup_worktree(¤t_resolution, issue, ¤t_project_dir) .await; @@ -799,6 +1018,7 @@ impl IssueProcessor { attempt_id: Option, review_feedback: Option<&str>, existing_pr_branch: Option<&str>, + diagnosis: Option<&VerifyResult>, effective_project_dir: &std::path::Path, context_provider: &dyn ContextProvider, ) -> Result { @@ -925,7 +1145,8 @@ impl IssueProcessor { } // Enrich context with indexed Discord discussions (independent of code index). - let (discord_ctx, discord_items) = + // Reference links are for user-facing notifications, not this agent context. + let (discord_ctx, discord_items, _discord_refs) = self.discord_grounding_context(issue, 5, attempt_id).await; if !discord_ctx.is_empty() { let metric = ProcessingMetric::new("discord_search_context_added", 1.0) @@ -1019,6 +1240,23 @@ impl IssueProcessor { // Ground the fix in the reply thread when this issue is a reply. context = self.with_reply_chain(issue, context).await; + // Start the fix from the verify stage's diagnosis when it has real findings. + if let Some(verdict) = diagnosis { + if diagnosis_has_details(verdict) { + context = format!("{}\n{}", build_diagnosis_context(verdict), context); + } + } + + // In red-green mode a failing test is already in the tree from the red phase. + if self.config.evaluation.require_red_green { + context = format!( + "## Failing test already present\n\nA test reproducing this bug has already been \ + written to the working tree and currently fails. Implement the minimal fix to make \ + it pass. Do not delete or weaken it, and do not add another reproducing test.\n\n{}", + context + ); + } + // Claude execution + ask loop let mut rounds: u8 = 0; let claude_result = loop { @@ -1863,7 +2101,7 @@ impl IssueProcessor { } else { String::new() }; - let (discord_ctx, discord_items) = self + let (discord_ctx, discord_items, discord_refs) = self .discord_grounding_context(issue, self.config.qa.max_context_chunks, attempt_id) .await; if !discord_ctx.is_empty() { @@ -1904,7 +2142,14 @@ impl IssueProcessor { match answer_result { Ok(Ok(answer)) => { - match self.notifier.notify_answer(issue, &answer).await { + // Append jump links to the referenced discussions for delivery only; + // the stored answer (for reply-chain grounding) stays clean. + let answer_to_send = if discord_refs.is_empty() { + answer.clone() + } else { + format!("{}{}", answer, discord_refs) + }; + match self.notifier.notify_answer(issue, &answer_to_send).await { Ok(sent_ids) => { if !sent_ids.is_empty() { if let Err(e) = self.tracker.record_answer_message_ids( @@ -2036,7 +2281,7 @@ impl IssueProcessor { /// if confirmed, resolved via the fix pipeline; everything else gets a reply. async fn run_action_pipeline( &self, - input: ProcessingInput, + mut input: ProcessingInput, context_provider: &dyn ContextProvider, ) -> ProcessingOutcome { // Prefer the intent decided upstream (carried on the input); only classify @@ -2057,6 +2302,8 @@ impl IssueProcessor { ) .await; if verdict.reproduced { + // Carry the diagnosis into the fix run. + input.diagnosis = Some(verdict); return match self.run_inner(input, context_provider).await { Ok(ProcessingOutcome::WrongRepo { original_repo, @@ -2170,7 +2417,8 @@ impl IssueProcessor { attempt_id: Option, ) -> VerifyResult { let project_dir = self.action_project_dir(resolution); - let context = self.build_rag_context(issue, attempt_id).await; + // Verify is read-only and posts no user-facing message, so drop the refs. + let (context, _discord_refs) = self.build_rag_context(issue, attempt_id).await; self.record_issue_decision( issue, @@ -2193,10 +2441,11 @@ impl IssueProcessor { ) .await; + let fail_open = self.config.reply().verify_fail_open; let verdict = match result { Ok(Ok(v)) => v, Ok(Err(e)) => VerifyResult { - reproduced: true, + reproduced: fail_open, summary: "Verification unsupported/failed; proceeding to resolve".to_string(), impact: String::new(), root_cause: String::new(), @@ -2204,7 +2453,7 @@ impl IssueProcessor { evidence: e.to_string(), }, Err(_) => VerifyResult { - reproduced: true, + reproduced: fail_open, summary: format!( "Verification timed out after {}s; proceeding to resolve", self.config.reply().verify_timeout_secs @@ -2289,7 +2538,7 @@ impl IssueProcessor { attempt_id: Option, ) -> ProcessingOutcome { let project_dir = self.action_project_dir(resolution); - let context = self.build_rag_context(issue, attempt_id).await; + let (context, discord_refs) = self.build_rag_context(issue, attempt_id).await; // The inbox key is the HelpScout mailbox id when present, else the source. let inbox_key = issue @@ -2326,9 +2575,16 @@ impl IssueProcessor { // Deliver: conversational sources go via the notifier; tracker // sources post a comment on the ticket (falling back to notifier). // Capture any sent message ids so a later reply maps back here. + // Referenced-discussion jump links are appended to notifier + // deliveries only; ticket comments keep the plain reply. + let reply_notify = if discord_refs.is_empty() { + reply.clone() + } else { + format!("{}{}", reply, discord_refs) + }; let mut answer_ids: Vec = Vec::new(); let delivered: Result<()> = if qa_eligible_source(source_name) { - match self.notifier.notify_answer(issue, &reply).await { + match self.notifier.notify_answer(issue, &reply_notify).await { Ok(ids) => { answer_ids = ids; Ok(()) @@ -2340,7 +2596,7 @@ impl IssueProcessor { Ok(()) => Ok(()), Err(e) => { tracing::warn!(short_id = %issue.short_id, error = %e, "post_reply failed; falling back to notifier"); - match self.notifier.notify_answer(issue, &reply).await { + match self.notifier.notify_answer(issue, &reply_notify).await { Ok(ids) => { answer_ids = ids; Ok(()) @@ -2422,7 +2678,10 @@ impl IssueProcessor { /// Retrieve RAG grounding context for an issue from the code index, plus any /// indexed Discord discussions. /// Build the RAG grounding context for the action pipeline (verify/reply). - async fn build_rag_context(&self, issue: &Issue, attempt_id: Option) -> String { + /// Returns the agent-facing context plus a Discord-markdown block of jump + /// links to any referenced discussions, for appending to the outgoing + /// notification (empty when there are none). + async fn build_rag_context(&self, issue: &Issue, attempt_id: Option) -> (String, String) { let mut retrieved_items: Vec = Vec::new(); let mut context = String::new(); if let Some(ref code_search) = self.code_search_service { @@ -2463,7 +2722,7 @@ impl IssueProcessor { } } } - let (discord_ctx, discord_items) = self + let (discord_ctx, discord_items, discord_refs) = self .discord_grounding_context(issue, self.config.qa.max_context_chunks, attempt_id) .await; if !discord_ctx.is_empty() { @@ -2483,7 +2742,7 @@ impl IssueProcessor { self.spawn_retrieval_judge(id, issue, &retrieved_items); } - context + (context, discord_refs) } /// Debug-gated confirmation that retrieval rows were persisted. Only emitted @@ -2643,17 +2902,19 @@ impl IssueProcessor { /// Retrieve grounding context from the indexed Discord knowledge source. /// Returns the formatted context (empty when the source is disabled or yields - /// no results) plus the retrieved chunks as [`RetrievedItem`]s so the caller - /// can feed them to the relevance judge. When `attempt_id` is set, also - /// records the retrieved chunks for quality assessment. + /// no results), the retrieved chunks as [`RetrievedItem`]s so the caller can + /// feed them to the relevance judge, and a Discord-markdown block of jump + /// links to the referenced discussions for appending to the outgoing + /// notification. When `attempt_id` is set, also records the retrieved chunks + /// for quality assessment. async fn discord_grounding_context( &self, issue: &Issue, limit: usize, attempt_id: Option, - ) -> (String, Vec) { + ) -> (String, Vec, String) { let Some(ref discord_search) = self.discord_search_service else { - return (String::new(), Vec::new()); + return (String::new(), Vec::new(), String::new()); }; let query = claudear_analysis::repo::code_index::build_code_search_query(issue); match discord_search.search(&query, None, limit).await { @@ -2689,9 +2950,11 @@ impl IssueProcessor { } let context = claudear_analysis::knowledgebase::format_discord_search_context(&results); - (context, items) + let refs = + claudear_analysis::knowledgebase::format_discord_reference_links(&results); + (context, items, refs) } - _ => (String::new(), Vec::new()), + _ => (String::new(), Vec::new(), String::new()), } } @@ -3244,6 +3507,7 @@ mod tests { review_feedback: Some("Fix the tests".to_string()), existing_pr_branch: Some("claudear/fix-123".to_string()), intent: None, + diagnosis: None, }; assert_eq!(input.source_name, "linear"); @@ -3271,6 +3535,7 @@ mod tests { review_feedback: None, existing_pr_branch: None, intent: None, + diagnosis: None, }; assert!(input.attempt_id.is_none()); @@ -4174,6 +4439,7 @@ mod tests { review_feedback: None, existing_pr_branch: None, intent: None, + diagnosis: None, }; // Use a dummy context provider diff --git a/crates/claudear-engine/src/watcher.rs b/crates/claudear-engine/src/watcher.rs index b67ee0f8..3efcaafe 100644 --- a/crates/claudear-engine/src/watcher.rs +++ b/crates/claudear-engine/src/watcher.rs @@ -3870,6 +3870,7 @@ Create a PR with your changes.{custom_instructions}"#, review_feedback, existing_pr_branch, intent, + diagnosis: None, }; let context_provider = crate::processing::SourceContext(source.as_ref()); @@ -4568,6 +4569,7 @@ Create a PR with your changes.{custom_instructions}"#, review_feedback: None, existing_pr_branch: None, intent: None, + diagnosis: None, }; let context_provider = crate::processing::SourceContext(source.as_ref()); 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..cf719d30 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,12 @@ 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, + /// Mirrors the global `debug_logging` flag. When true, the rendered MCP + /// config is logged (with secret values redacted) on each attach. + pub debug_logging: bool, } impl Default for ClaudeRunnerConfig { @@ -254,6 +261,8 @@ impl Default for ClaudeRunnerConfig { skip_permissions: false, binary: "claude".to_string(), env: HashMap::new(), + mcp: HashMap::new(), + debug_logging: false, } } } @@ -316,6 +325,7 @@ impl ClaudeAgentRunner { issue_identifier, env, project_dir, + Some("linear"), ) .await } @@ -581,7 +591,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 +607,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 +739,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 +765,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 +786,97 @@ 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) + } + + /// A redacted JSON view of the matched MCP servers for debug logging. + /// Mirrors what `render_mcp_config` writes, but masks every `env`/`headers` + /// value that could hold a secret. Pure `${VAR}` references are shown as-is + /// (they name a variable, not a secret); anything else becomes "***". + fn redact_mcp_for_log(servers: &[(&String, &McpServerConfig)]) -> serde_json::Value { + let mask = |v: &str| -> String { + let t = v.trim(); + if t.starts_with("${") && t.ends_with('}') && !t[2..].contains("${") { + v.to_string() + } else { + "***".to_string() + } + }; + let redact_map = |m: &std::collections::HashMap| -> serde_json::Value { + json!(m + .iter() + .map(|(k, v)| (k.clone(), mask(v))) + .collect::>()) + }; + let mut out = serde_json::Map::new(); + for (name, cfg) in servers { + let mut entry = serde_json::Map::new(); + if let Some(ref command) = cfg.command { + 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(), redact_map(&cfg.env)); + } + } else if let Some(ref url) = cfg.url { + entry.insert("url".to_string(), json!(url)); + if !cfg.headers.is_empty() { + entry.insert("headers".to_string(), redact_map(&cfg.headers)); + } + } + if let Some(ref transport) = cfg.transport { + entry.insert("type".to_string(), json!(transport)); + } + entry.insert("sources".to_string(), json!(cfg.sources)); + entry.insert("tools".to_string(), json!(cfg.tools)); + out.insert((*name).clone(), serde_json::Value::Object(entry)); + } + json!({ "mcpServers": serde_json::Value::Object(out) }) + } + + #[allow(clippy::too_many_arguments)] async fn execute_with_env_and_attempt( &self, prompt: &str, @@ -760,6 +885,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 +922,93 @@ 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" + ); + if self.config.debug_logging { + let redacted = Self::redact_mcp_for_log(&matched_mcp); + tracing::info!( + component = "claude", + label = label, + path = %file.path().display(), + config = %redacted, + "Rendered MCP config (secret values redacted)" + ); + } + 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 +1044,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 +2196,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 +3839,158 @@ 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_redact_mcp_for_log_masks_secrets() { + let name = "grafana".to_string(); + let mut env = HashMap::new(); + // Pure ${VAR} ref -> shown; literal secret and JSON blob -> masked. + env.insert( + "GRAFANA_URL".to_string(), + "https://tel.example.com/".to_string(), + ); + env.insert( + "GRAFANA_SERVICE_ACCOUNT_TOKEN".to_string(), + "${GRAFANA_SERVICE_ACCOUNT_TOKEN}".to_string(), + ); + env.insert( + "GRAFANA_TOKEN_LITERAL".to_string(), + "glsa_realsecret".to_string(), + ); + env.insert( + "GRAFANA_EXTRA_HEADERS".to_string(), + "{\"CF-Access-Client-Id\": \"${CF_ID}\"}".to_string(), + ); + let cfg = McpServerConfig { + command: Some("uvx".to_string()), + args: vec!["mcp-grafana".to_string()], + env, + sources: vec!["sentry".to_string()], + tools: vec!["list_datasources".to_string()], + ..Default::default() + }; + let servers = vec![(&name, &cfg)]; + let doc = ClaudeAgentRunner::redact_mcp_for_log(&servers); + let env_out = &doc["mcpServers"]["grafana"]["env"]; + // Non-secret-looking URL is still masked (we mask all non-${VAR} values). + assert_eq!(env_out["GRAFANA_URL"], "***"); + // Pure var reference passes through unmasked. + assert_eq!( + env_out["GRAFANA_SERVICE_ACCOUNT_TOKEN"], + "${GRAFANA_SERVICE_ACCOUNT_TOKEN}" + ); + // Literal secret is masked. + assert_eq!(env_out["GRAFANA_TOKEN_LITERAL"], "***"); + // JSON blob with an embedded ${VAR} is not a pure ref -> masked. + assert_eq!(env_out["GRAFANA_EXTRA_HEADERS"], "***"); + // Non-secret structure is preserved. + assert_eq!(doc["mcpServers"]["grafana"]["command"], "uvx"); + assert_eq!(doc["mcpServers"]["grafana"]["args"][0], "mcp-grafana"); + assert_eq!(doc["mcpServers"]["grafana"]["sources"][0], "sentry"); + assert_eq!(doc["mcpServers"]["grafana"]["tools"][0], "list_datasources"); + } + + #[test] + fn test_render_mcp_config_stdio_json_headers_env() { + // A stdio server whose env carries a JSON blob with ${VAR} references + // (e.g. Grafana behind Cloudflare Access). The value is written verbatim; + // Claude Code expands the ${VAR}s at runtime, including inside the string. + let name = "grafana".to_string(); + let mut env = HashMap::new(); + env.insert( + "GRAFANA_SERVICE_ACCOUNT_TOKEN".to_string(), + "${GRAFANA_SERVICE_ACCOUNT_TOKEN}".to_string(), + ); + let headers_json = + "{\"CF-Access-Client-Id\": \"${CF_ACCESS_CLIENT_ID}\", \"CF-Access-Client-Secret\": \"${CF_ACCESS_CLIENT_SECRET}\"}"; + env.insert( + "GRAFANA_EXTRA_HEADERS".to_string(), + headers_json.to_string(), + ); + let cfg = McpServerConfig { + command: Some("uvx".to_string()), + args: vec!["mcp-grafana".to_string()], + env, + sources: vec!["sentry".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"]["grafana"]; + assert_eq!(server["command"], "uvx"); + assert_eq!(server["args"][0], "mcp-grafana"); + assert_eq!( + server["env"]["GRAFANA_SERVICE_ACCOUNT_TOKEN"], + "${GRAFANA_SERVICE_ACCOUNT_TOKEN}" + ); + // The JSON blob survives round-trip unescaped and unexpanded. + assert_eq!(server["env"]["GRAFANA_EXTRA_HEADERS"], headers_json); + } + + #[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 d83c7a76..07e86aa0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -187,6 +187,8 @@ 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(), + debug_logging: config.debug_logging, }, tracker, ); diff --git a/src/main.rs b/src/main.rs index 9a37766b..e1b99ea7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3800,6 +3800,12 @@ 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(), + debug_logging: config.debug_logging, }, tracker.clone(), ))); diff --git a/src/webhook/server.rs b/src/webhook/server.rs index 7eaae420..6765d9ac 100644 --- a/src/webhook/server.rs +++ b/src/webhook/server.rs @@ -1036,6 +1036,7 @@ async fn process_issue( // classification falls back to the heuristic; `None` keeps it on the fix pipeline // (behaviour-preserving). intent: None, + diagnosis: None, }; let context_provider = WebhookContext(handler.as_ref());