fix: post-squash compile errors reclassify as semantic merge conflicts
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.
This commit is contained in:
@@ -44,6 +44,13 @@ impl GateFailureKind {
|
|||||||
|| output.contains("missing_doc_comments")
|
|| output.contains("missing_doc_comments")
|
||||||
{
|
{
|
||||||
GateFailureKind::Lint
|
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 {
|
} else {
|
||||||
GateFailureKind::Test
|
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]
|
#[test]
|
||||||
fn classify_test_failure_for_unrecognised_output() {
|
fn classify_test_failure_for_unrecognised_output() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|||||||
@@ -52,6 +52,34 @@ impl MergeResult {
|
|||||||
| Self::Other { output, .. } => output,
|
| 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.
|
/// Status of an async merge job.
|
||||||
@@ -86,3 +114,93 @@ pub struct MergeReport {
|
|||||||
pub worktree_cleaned_up: bool,
|
pub worktree_cleaned_up: bool,
|
||||||
pub story_archived: 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();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -138,25 +138,7 @@ impl AgentPool {
|
|||||||
// On any failure: record merge_failure in CRDT and emit notification.
|
// On any failure: record merge_failure in CRDT and emit notification.
|
||||||
if !success {
|
if !success {
|
||||||
let kind = match &report {
|
let kind = match &report {
|
||||||
Ok(r) => match &r.result {
|
Ok(r) => r.result.to_merge_failure_kind(),
|
||||||
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")
|
|
||||||
}
|
|
||||||
},
|
|
||||||
Err(e) => crate::pipeline_state::MergeFailureKind::Other(e.clone()),
|
Err(e) => crate::pipeline_state::MergeFailureKind::Other(e.clone()),
|
||||||
};
|
};
|
||||||
let is_no_commits =
|
let is_no_commits =
|
||||||
|
|||||||
Reference in New Issue
Block a user