diff --git a/server/src/agents/gates.rs b/server/src/agents/gates.rs index f51c629d..bbdb8ff4 100644 --- a/server/src/agents/gates.rs +++ b/server/src/agents/gates.rs @@ -33,16 +33,28 @@ impl GateFailureKind { /// Called once when a gate fails to produce a typed kind. Downstream code /// matches on the variant and must not call this on subsequent reads. pub fn classify(output: &str) -> Self { + // Strip `test ... ok` lines before checking lint-trigger keywords so + // a passing test whose name contains e.g. `missing_doc_comments` or `clippy::` + // does not produce a false-positive Lint classification (story 1101). + let stripped_for_lint: String = output + .lines() + .filter(|l| { + let t = l.trim(); + !(t.starts_with("test ") && t.ends_with("... ok")) + }) + .collect::>() + .join("\n"); + let is_lint = stripped_for_lint.contains("error[clippy::") + || stripped_for_lint.contains("warning[clippy::") + || stripped_for_lint.contains("missing_doc_comments"); + if output.contains("CONFLICT (content):") || output.contains("Merge conflict:") { GateFailureKind::ContentConflict } else if output.contains("Diff in ") || output.contains("would reformat") { GateFailureKind::Fmt } else if output.contains("missing-docs direction") { GateFailureKind::SourceMapCheck - } else if output.contains("error[clippy::") - || output.contains("warning[clippy::") - || output.contains("missing_doc_comments") - { + } else if is_lint { GateFailureKind::Lint } else if output.contains("error[E") { // rustc compile errors (e.g. `error[E0063]: missing field`). @@ -871,6 +883,19 @@ mod tests { ); } + /// Story 1101: a passing test whose name contains a lint trigger keyword + /// must NOT produce a Lint classification. + #[test] + fn classify_does_not_false_positive_on_test_name_substring() { + let output = "test agents::gates::tests::classify_lint_from_missing_doc_comments ... ok\n\ + test result: ok. 1 passed; 0 failed"; + assert_ne!( + GateFailureKind::classify(output), + GateFailureKind::Lint, + "passing test name containing 'missing_doc_comments' must not classify as Lint" + ); + } + #[test] fn classify_source_map_check_from_missing_docs_direction() { assert_eq!( diff --git a/server/src/agents/pool/pipeline/merge/runner.rs b/server/src/agents/pool/pipeline/merge/runner.rs index a950cb68..f04026a2 100644 --- a/server/src/agents/pool/pipeline/merge/runner.rs +++ b/server/src/agents/pool/pipeline/merge/runner.rs @@ -186,50 +186,6 @@ impl AgentPool { .map(|k| k.is_self_evident_fix()) .unwrap_or(false); - // Bug 1101 diagnostic: log the classified failure_kind and the - // matched classifier-trigger substring with surrounding context, - // so we can confirm whether classify() is incorrectly matching - // a passing-step stdout substring (e.g. "Diff in " inside a - // failing test's panic message) and bouncing the story to a - // fixup coder. Remove once the fix lands. - if let Ok(r) = report.as_ref() - && let crate::agents::merge::MergeResult::GateFailure { - output: gate_output, - failure_kind: Some(k), - } = &r.result - { - const TRIGGERS: &[&str] = &[ - "CONFLICT (content):", - "Merge conflict:", - "Diff in ", - "would reformat", - "missing-docs direction", - "error[clippy::", - "warning[clippy::", - "missing_doc_comments", - "error[E", - ]; - let matched = TRIGGERS - .iter() - .find_map(|t| gate_output.find(t).map(|i| (*t, i))); - let (trigger, context) = match matched { - Some((t, i)) => { - let start = i.saturating_sub(30); - let end = (i + t.len() + 60).min(gate_output.len()); - let ctx = gate_output - .get(start..end) - .unwrap_or("") - .replace('\n', " "); - (Some(t), ctx) - } - None => (None, String::from("")), - }; - slog!( - "[merge] classify diagnostic for '{sid}': failure_kind={k:?} \ - is_fixup={is_fixup} trigger={trigger:?} context='{context}'" - ); - } - if is_no_commits { let reason = kind.display_reason(); if let Err(e) = crate::agents::lifecycle::transition_to_blocked(&sid, &reason) {