From 8b2ba1c810846d3f9b8ed7f9dcacdf8ec774d291 Mon Sep 17 00:00:00 2001 From: Timmy Date: Thu, 14 May 2026 10:18:33 +0100 Subject: [PATCH] fix: post-squash compile errors reclassify as semantic merge conflicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When deterministic-merge produces a clean git squash but the post-squash compile fails (typical when master gained a Stage payload field after the feature branch forked — e.g. story 1018 hit `error[E0063]: missing field plan` after 1010's PlanState landed), the failure is morally a merge conflict that git's diff3 missed: the conflicting literal lives in a different file from the type definition that changed on master. Routing it as GatesFailed left mergemaster idle and the story stuck. Changes: - gates.rs GateFailureKind::classify: detect rustc compile errors (`error[E\d+]`) as Build instead of falling through to Test. Clippy errors (`error[clippy::...]`) still classify as Lint. - agents/merge/mod.rs: new MergeResult::to_merge_failure_kind() method. GateFailure with failure_kind=Build maps to ConflictDetected (so the existing 998 subscriber auto-spawns mergemaster). Other gate failures stay GatesFailed. - agents/pool/pipeline/merge/runner.rs: replace the inline match with a call to the new method. Tests: 6 new unit tests covering the classifier branch and every to_merge_failure_kind arm. All 2932 tests pass. --- server/src/agents/gates.rs | 30 +++++ server/src/agents/merge/mod.rs | 118 ++++++++++++++++++ .../src/agents/pool/pipeline/merge/runner.rs | 20 +-- 3 files changed, 149 insertions(+), 19 deletions(-) diff --git a/server/src/agents/gates.rs b/server/src/agents/gates.rs index 23ada987..f51c629d 100644 --- a/server/src/agents/gates.rs +++ b/server/src/agents/gates.rs @@ -44,6 +44,13 @@ impl GateFailureKind { || output.contains("missing_doc_comments") { GateFailureKind::Lint + } else if output.contains("error[E") { + // rustc compile errors (e.g. `error[E0063]: missing field`). + // When this appears in the post-squash gate run, it almost always + // signals cross-merge breakage — master gained a field/variant the + // feature branch's code does not match. Mergemaster handles the + // recovery in its ConflictDetected path. + GateFailureKind::Build } else { GateFailureKind::Test } @@ -880,6 +887,29 @@ mod tests { ); } + #[test] + fn classify_build_from_rustc_compile_error() { + // Post-squash compile errors (typical when master drifts under a feature + // branch — e.g. story 1018 hit `error[E0063]: missing field` after + // master gained a Stage::Coding field the feature branch did not set). + assert_eq!( + GateFailureKind::classify( + "error[E0063]: missing field `plan` in initializer of `Stage`\n --> server/src/foo.rs:166:20" + ), + GateFailureKind::Build + ); + } + + #[test] + fn classify_build_does_not_misfire_on_clippy_error() { + // Clippy errors look like `error[clippy::name]` and must remain Lint, + // not Build, because the `error[E` prefix would otherwise overlap. + assert_eq!( + GateFailureKind::classify("error[clippy::unused_variable]: unused variable `x`"), + GateFailureKind::Lint + ); + } + #[test] fn classify_test_failure_for_unrecognised_output() { assert_eq!( diff --git a/server/src/agents/merge/mod.rs b/server/src/agents/merge/mod.rs index 20e5f69d..84a16273 100644 --- a/server/src/agents/merge/mod.rs +++ b/server/src/agents/merge/mod.rs @@ -52,6 +52,34 @@ impl MergeResult { | Self::Other { output, .. } => output, } } + + /// Convert a non-success outcome into the pipeline-level `MergeFailureKind` + /// used to drive recovery routing. + /// + /// A `GateFailure` whose typed `failure_kind` is `Build` is reclassified as + /// `ConflictDetected` — a post-squash compile error after a clean git merge + /// is semantically a merge conflict that git's diff3 missed (the conflicting + /// literal lives in a different file from the type definition that changed + /// on master). Routing it as a conflict ensures mergemaster auto-spawns to + /// resolve it, rather than the failure sitting as an opaque `GatesFailed`. + /// + /// Panics on `Success`; callers must guard with a success check first. + pub fn to_merge_failure_kind(&self) -> crate::pipeline_state::MergeFailureKind { + use crate::pipeline_state::MergeFailureKind; + match self { + Self::Success { .. } => { + panic!("to_merge_failure_kind called on MergeResult::Success") + } + Self::NoCommits { .. } => MergeFailureKind::NoCommits, + Self::Conflict { details, .. } => MergeFailureKind::ConflictDetected(details.clone()), + Self::GateFailure { + output, + failure_kind: Some(crate::agents::gates::GateFailureKind::Build), + } => MergeFailureKind::ConflictDetected(Some(output.clone())), + Self::GateFailure { output, .. } => MergeFailureKind::GatesFailed(output.clone()), + Self::Other { output, .. } => MergeFailureKind::Other(output.clone()), + } + } } /// Status of an async merge job. @@ -86,3 +114,93 @@ pub struct MergeReport { pub worktree_cleaned_up: bool, pub story_archived: bool, } + +#[cfg(test)] +mod tests { + use super::*; + use crate::agents::gates::GateFailureKind; + use crate::pipeline_state::MergeFailureKind; + + #[test] + fn to_failure_kind_maps_build_gate_failure_to_conflict_detected() { + // Post-squash compile error (e.g. story 1018: `error[E0063]: missing field + // plan` because master gained Stage::Coding's plan field after the + // coder's branch was committed) is a semantic merge conflict that git + // missed. Mergemaster should auto-spawn, so the kind must be + // ConflictDetected, not GatesFailed. + let output = "error[E0063]: missing field `plan` in initializer of `Stage`".to_string(); + let result = MergeResult::GateFailure { + output: output.clone(), + failure_kind: Some(GateFailureKind::Build), + }; + match result.to_merge_failure_kind() { + MergeFailureKind::ConflictDetected(details) => { + assert_eq!(details.as_deref(), Some(output.as_str())); + } + other => panic!("expected ConflictDetected, got {other:?}"), + } + } + + #[test] + fn to_failure_kind_maps_test_gate_failure_to_gates_failed() { + // Non-build gate failures (test failure, fmt drift, lint, etc.) are + // genuine gate problems, not merge conflicts. They must remain + // GatesFailed so mergemaster does NOT auto-spawn for them. + let result = MergeResult::GateFailure { + output: "test result: FAILED. 1 failed".to_string(), + failure_kind: Some(GateFailureKind::Test), + }; + assert!(matches!( + result.to_merge_failure_kind(), + MergeFailureKind::GatesFailed(_) + )); + } + + #[test] + fn to_failure_kind_maps_git_conflict_to_conflict_detected() { + let result = MergeResult::Conflict { + details: Some("conflicts in server/src/foo.rs".to_string()), + output: "merge failed".to_string(), + }; + match result.to_merge_failure_kind() { + MergeFailureKind::ConflictDetected(details) => { + assert_eq!(details.as_deref(), Some("conflicts in server/src/foo.rs")); + } + other => panic!("expected ConflictDetected, got {other:?}"), + } + } + + #[test] + fn to_failure_kind_maps_no_commits() { + let result = MergeResult::NoCommits { + output: "no commits".to_string(), + }; + assert!(matches!( + result.to_merge_failure_kind(), + MergeFailureKind::NoCommits + )); + } + + #[test] + fn to_failure_kind_maps_other() { + let result = MergeResult::Other { + output: "cherry-pick failed".to_string(), + conflict_details: None, + }; + assert!(matches!( + result.to_merge_failure_kind(), + MergeFailureKind::Other(_) + )); + } + + #[test] + #[should_panic(expected = "to_merge_failure_kind called on MergeResult::Success")] + fn to_failure_kind_panics_on_success() { + let result = MergeResult::Success { + conflicts_resolved: false, + conflict_details: None, + gate_output: String::new(), + }; + let _ = result.to_merge_failure_kind(); + } +} diff --git a/server/src/agents/pool/pipeline/merge/runner.rs b/server/src/agents/pool/pipeline/merge/runner.rs index 15b562f1..e6337759 100644 --- a/server/src/agents/pool/pipeline/merge/runner.rs +++ b/server/src/agents/pool/pipeline/merge/runner.rs @@ -138,25 +138,7 @@ impl AgentPool { // On any failure: record merge_failure in CRDT and emit notification. if !success { let kind = match &report { - Ok(r) => match &r.result { - crate::agents::merge::MergeResult::NoCommits { .. } => { - crate::pipeline_state::MergeFailureKind::NoCommits - } - crate::agents::merge::MergeResult::Conflict { details, .. } => { - crate::pipeline_state::MergeFailureKind::ConflictDetected( - details.clone(), - ) - } - crate::agents::merge::MergeResult::GateFailure { output, .. } => { - crate::pipeline_state::MergeFailureKind::GatesFailed(output.clone()) - } - crate::agents::merge::MergeResult::Other { output, .. } => { - crate::pipeline_state::MergeFailureKind::Other(output.clone()) - } - crate::agents::merge::MergeResult::Success { .. } => { - unreachable!("success branch is guarded by !success above") - } - }, + Ok(r) => r.result.to_merge_failure_kind(), Err(e) => crate::pipeline_state::MergeFailureKind::Other(e.clone()), }; let is_no_commits =