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-config/src/config.rs b/crates/claudear-config/src/config.rs index 88fd3b0b..d619e9cf 100644 --- a/crates/claudear-config/src/config.rs +++ b/crates/claudear-config/src/config.rs @@ -805,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 { @@ -815,6 +818,7 @@ impl Default for ReplyConfig { default_template: None, templates: std::collections::HashMap::new(), verify_timeout_secs: 1800, + verify_fail_open: true, } } } @@ -1086,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. @@ -1108,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, @@ -8120,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..bcb0fb8e 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 = 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 { @@ -1019,6 +1239,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 { @@ -2036,7 +2273,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 +2294,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, @@ -2193,10 +2432,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 +2444,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 @@ -3244,6 +3484,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 +3512,7 @@ mod tests { review_feedback: None, existing_pr_branch: None, intent: None, + diagnosis: None, }; assert!(input.attempt_id.is_none()); @@ -4174,6 +4416,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/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());