huskies: merge 982

This commit is contained in:
dave
2026-05-13 15:30:03 +00:00
parent e6d051d016
commit 91fbad568a
15 changed files with 357 additions and 117 deletions
+21 -18
View File
@@ -11,8 +11,8 @@ use std::path::Path;
use std::process::Command; use std::process::Command;
use crate::pipeline_state::{ use crate::pipeline_state::{
ApplyError, ArchiveReason, BranchName, GitSha, PipelineEvent, Stage, TransitionFired, ApplyError, ArchiveReason, BranchName, GitSha, MergeFailureKind, PipelineEvent, Stage,
apply_transition, stage_label, TransitionFired, apply_transition, stage_label,
}; };
use crate::slog; use crate::slog;
@@ -246,10 +246,12 @@ pub fn transition_to_blocked(story_id: &str, reason: &str) -> Result<(), String>
/// Transition a story from `Stage::Merge` (or `Stage::MergeFailure`) to /// Transition a story from `Stage::Merge` (or `Stage::MergeFailure`) to
/// `Stage::MergeFailure` via the state machine. /// `Stage::MergeFailure` via the state machine.
/// ///
/// Builds a `PipelineEvent::MergeFailed { reason }`, validates the transition, /// Builds a `PipelineEvent::MergeFailed { kind }`, validates the transition,
/// writes the resulting `Stage::MergeFailure` to the CRDT, and persists the /// writes the resulting `Stage::MergeFailure` to the CRDT, and persists two
/// reason to the typed `MergeJob.error` CRDT register so it survives server /// display-only copies for status tools:
/// restarts (story 929: the legacy YAML write of `merge_failure: "..."` is gone). /// - `ContentKey::GateOutput`: the kind's gate-output string so the CRDT
/// projection layer can reconstruct the kind after a server restart.
/// - `MergeJob.error`: human-readable description for status renderers.
/// ///
/// When the story is already in `MergeFailure`, this is a silent self-loop: the /// When the story is already in `MergeFailure`, this is a silent self-loop: the
/// returned `TransitionFired::before` will be `Stage::MergeFailure`. Callers /// returned `TransitionFired::before` will be `Stage::MergeFailure`. Callers
@@ -258,26 +260,27 @@ pub fn transition_to_blocked(story_id: &str, reason: &str) -> Result<(), String>
/// Returns `Err` on `TransitionError` — callers must NOT fall back to direct register writes. /// Returns `Err` on `TransitionError` — callers must NOT fall back to direct register writes.
pub fn transition_to_merge_failure( pub fn transition_to_merge_failure(
story_id: &str, story_id: &str,
reason: &str, kind: MergeFailureKind,
) -> Result<TransitionFired, String> { ) -> Result<TransitionFired, String> {
let fired = apply_transition( let display = kind.display_reason();
story_id, let gate_output = kind.to_gate_output();
PipelineEvent::MergeFailed {
reason: reason.to_string(), let fired = apply_transition(story_id, PipelineEvent::MergeFailed { kind }, None)
},
None,
)
.map_err(|e| e.to_string())?; .map_err(|e| e.to_string())?;
// Persist the failure reason on the MergeJob CRDT entry so display tools // Persist gate-output string so the CRDT projection can reconstruct the
// (status_tools, chat status renderer, pipeline.rs::load_pipeline_state) // MergeFailureKind on server restart (display-only; scheduling uses the
// can surface it without re-parsing YAML. // typed kind from the Stage variant).
crate::db::write_content(crate::db::ContentKey::GateOutput(story_id), &gate_output);
// Persist human-readable description on the MergeJob CRDT entry so display
// tools (status renderer, pipeline state view) can surface it.
crate::crdt_state::write_merge_job( crate::crdt_state::write_merge_job(
story_id, story_id,
"failed", "failed",
chrono::Utc::now().timestamp() as f64, chrono::Utc::now().timestamp() as f64,
None, None,
Some(reason), Some(&display),
); );
Ok(fired) Ok(fired)
@@ -901,4 +901,72 @@ mod tests {
through the watcher bridge (story 958 regression)" through the watcher bridge (story 958 regression)"
); );
} }
/// AC5 (story 982): a merge failure with content conflicts — seeded via the
/// typed `transition_to_merge_failure(ConflictDetected)` path without any
/// direct content-store or MergeJob writes in the test — produces
/// `Stage::MergeFailure { kind: ConflictDetected(_), .. }` and
/// auto-spawn-mergemaster fires within one `auto_assign_available_work` call.
#[tokio::test]
async fn auto_spawn_mergemaster_for_conflict_detected_kind_without_content_store_writes() {
let tmp = tempfile::tempdir().unwrap();
let sk = tmp.path().join(".huskies");
std::fs::create_dir_all(&sk).unwrap();
std::fs::write(
sk.join("project.toml"),
"[[agent]]\nname = \"mergemaster\"\nstage = \"mergemaster\"\n",
)
.unwrap();
crate::crdt_state::init_for_test();
crate::db::ensure_content_store();
let story_id = "982_ac5_conflict_auto_spawn";
// Seed at Merge stage so the transition is valid.
crate::db::write_item_with_content(
story_id,
"4_merge",
"---\nname: AC5 auto-spawn test\n---\n",
crate::db::ItemMeta::named("AC5 auto-spawn test"),
);
// Transition to MergeFailure(ConflictDetected) via lifecycle — no direct
// content-store writes in this test body.
crate::agents::lifecycle::transition_to_merge_failure(
story_id,
crate::pipeline_state::MergeFailureKind::ConflictDetected(Some(
"CONFLICT (content): server/src/lib.rs".to_string(),
)),
)
.expect("transition to MergeFailure(ConflictDetected) should succeed");
// Verify the stage kind before triggering auto-assign.
let item = crate::pipeline_state::read_typed(story_id)
.unwrap()
.unwrap();
assert!(
matches!(
item.stage,
crate::pipeline_state::Stage::MergeFailure {
kind: crate::pipeline_state::MergeFailureKind::ConflictDetected(_),
..
}
),
"stage must be MergeFailure(ConflictDetected) before auto-assign: {:?}",
item.stage
);
// One auto-assign cycle should spawn mergemaster.
let pool = AgentPool::new_test(3001);
pool.auto_assign_available_work(tmp.path()).await;
let agents = pool.agents.lock().unwrap();
let mergemaster_spawned = agents.iter().any(|(key, a)| {
key.contains(story_id)
&& a.agent_name == "mergemaster"
&& matches!(a.status, AgentStatus::Pending | AgentStatus::Running)
});
assert!(
mergemaster_spawned,
"mergemaster must be auto-spawned for ConflictDetected kind in one auto-assign cycle"
);
}
} }
+1 -1
View File
@@ -120,7 +120,7 @@ impl AgentPool {
// Content conflicts get one automatic mergemaster attempt; other failures // Content conflicts get one automatic mergemaster attempt; other failures
// require human intervention. // require human intervention.
let merge_failure_stage = Stage::MergeFailure { let merge_failure_stage = Stage::MergeFailure {
reason: String::new(), kind: crate::pipeline_state::MergeFailureKind::Other(String::new()),
feature_branch: BranchName(String::new()), feature_branch: BranchName(String::new()),
commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"), commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"),
}; };
@@ -39,33 +39,25 @@ pub(super) fn is_story_blocked(story_id: &str) -> bool {
.unwrap_or(false) .unwrap_or(false)
} }
/// Return `true` if the story's merge failure contains a git content-conflict /// Return `true` if the story's merge failure is a git content-conflict
/// marker (`"Merge conflict"` or `"CONFLICT (content):"`). /// (`Stage::MergeFailure { kind: ConflictDetected(_), .. }`).
/// ///
/// Used by the auto-assigner to decide whether to spawn mergemaster automatically. /// Used by the auto-assigner to decide whether to spawn mergemaster automatically.
/// The typed stage register is consulted first; the CRDT content store is then /// The typed kind is carried by the CRDT projection layer (which reads
/// scanned for conflict markers (the projection layer does not carry the reason /// `ContentKey::GateOutput` on projection to reconstruct the kind on restart),
/// string). No YAML front-matter parsing is performed. /// so no direct content-store access is needed here (story 982).
pub(super) fn has_content_conflict_failure(story_id: &str) -> bool { pub(super) fn has_content_conflict_failure(story_id: &str) -> bool {
let is_merge_failure = crate::pipeline_state::read_typed(story_id) crate::pipeline_state::read_typed(story_id)
.ok() .ok()
.flatten() .flatten()
.map(|item| { .map(|item| {
matches!( matches!(
item.stage, item.stage,
crate::pipeline_state::Stage::MergeFailure { .. } crate::pipeline_state::Stage::MergeFailure {
) kind: crate::pipeline_state::MergeFailureKind::ConflictDetected(_),
}) ..
.unwrap_or(false);
if !is_merge_failure {
return false;
} }
// The projection does not carry the reason string; read the gate output )
// (where the merge runner persists the failure message) and scan for
// conflict markers.
crate::db::read_content(crate::db::ContentKey::GateOutput(story_id))
.map(|content| {
content.contains("Merge conflict") || content.contains("CONFLICT (content):")
}) })
.unwrap_or(false) .unwrap_or(false)
} }
@@ -337,4 +329,81 @@ mod tests {
let archived_deps = check_archived_dependencies("503_story_waiting"); let archived_deps = check_archived_dependencies("503_story_waiting");
assert!(archived_deps.is_empty()); assert!(archived_deps.is_empty());
} }
// ── Story 982: typed MergeFailureKind — has_content_conflict_failure ──────
/// AC2 (story 982): `has_content_conflict_failure` returns `true` when the
/// story is in `Stage::MergeFailure { kind: ConflictDetected(_), .. }`.
/// The test seeds the stage via `transition_to_merge_failure` (no direct
/// content-store or MergeJob writes in the test body).
#[test]
fn has_content_conflict_failure_true_for_conflict_detected_kind() {
crate::crdt_state::init_for_test();
crate::db::ensure_content_store();
let story_id = "982_ac2_conflict_detected";
// Seed at Merge stage so the transition is valid.
crate::db::write_item_with_content(
story_id,
"4_merge",
"---\nname: AC2 conflict test\n---\n",
crate::db::ItemMeta::named("AC2 conflict test"),
);
// Transition via the lifecycle helper — internally writes ContentKey::GateOutput
// so the CRDT projection can reconstruct the kind; no content-store writes here.
crate::agents::lifecycle::transition_to_merge_failure(
story_id,
crate::pipeline_state::MergeFailureKind::ConflictDetected(Some(
"CONFLICT (content): server/src/lib.rs".to_string(),
)),
)
.expect("transition should succeed");
// The typed match now drives the predicate — no substring scan.
assert!(
has_content_conflict_failure(story_id),
"has_content_conflict_failure must be true for ConflictDetected kind"
);
// Verify the projected stage carries the typed kind.
let item = crate::pipeline_state::read_typed(story_id)
.unwrap()
.unwrap();
assert!(
matches!(
item.stage,
crate::pipeline_state::Stage::MergeFailure {
kind: crate::pipeline_state::MergeFailureKind::ConflictDetected(_),
..
}
),
"stage must be MergeFailure(ConflictDetected): {:?}",
item.stage
);
}
/// AC2 (story 982): `has_content_conflict_failure` returns `false` when the
/// kind is `GatesFailed` — no mergemaster spawn for gate-only failures.
#[test]
fn has_content_conflict_failure_false_for_gates_failed_kind() {
crate::crdt_state::init_for_test();
crate::db::ensure_content_store();
let story_id = "982_ac2_gates_failed";
crate::db::write_item_with_content(
story_id,
"4_merge",
"---\nname: AC2 gates test\n---\n",
crate::db::ItemMeta::named("AC2 gates test"),
);
crate::agents::lifecycle::transition_to_merge_failure(
story_id,
crate::pipeline_state::MergeFailureKind::GatesFailed(
"error[clippy::unused_variable]".to_string(),
),
)
.expect("transition should succeed");
assert!(
!has_content_conflict_failure(story_id),
"has_content_conflict_failure must be false for GatesFailed kind"
);
}
} }
@@ -186,12 +186,13 @@ impl AgentPool {
fixup failure: {e}" fixup failure: {e}"
); );
} }
let reason = format!( let kind = crate::pipeline_state::MergeFailureKind::GatesFailed(format!(
"Merge fixup coder could not resolve gate failures: {}", "Merge fixup coder could not resolve gate failures: {}",
truncate_gate_output(&completion.gate_output) truncate_gate_output(&completion.gate_output)
); ));
let display = kind.display_reason();
if let Err(e) = if let Err(e) =
crate::agents::lifecycle::transition_to_merge_failure(story_id, &reason) crate::agents::lifecycle::transition_to_merge_failure(story_id, kind)
{ {
slog_error!( slog_error!(
"[pipeline] Failed to transition '{story_id}' to MergeFailure \ "[pipeline] Failed to transition '{story_id}' to MergeFailure \
@@ -200,7 +201,7 @@ impl AgentPool {
} }
let _ = self.watcher_tx.send(WatcherEvent::MergeFailure { let _ = self.watcher_tx.send(WatcherEvent::MergeFailure {
story_id: story_id.to_string(), story_id: story_id.to_string(),
reason, reason: display,
}); });
} }
} else if completion.gates_passed { } else if completion.gates_passed {
+31 -25
View File
@@ -129,32 +129,35 @@ 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 reason = match &report { let kind = match &report {
Ok(r) => { Ok(r) => {
if r.had_conflicts { if r.had_conflicts {
format!( crate::pipeline_state::MergeFailureKind::ConflictDetected(
"Merge conflict: {}", r.conflict_details.clone(),
r.conflict_details
.as_deref()
.unwrap_or("conflicts detected")
) )
} else { } else {
format!("Quality gates failed: {}", r.gate_output) crate::pipeline_state::MergeFailureKind::GatesFailed(
r.gate_output.clone(),
)
} }
} }
Err(e) => e.clone(), Err(e) => crate::pipeline_state::MergeFailureKind::Other(e.clone()),
}; };
let is_no_commits = reason.contains("no commits to merge"); let is_no_commits = matches!(
// Self-evident fix: gate-only failure (no conflicts) whose output matches &kind,
// a pattern a fixup coder can resolve in one short session (story 981). crate::pipeline_state::MergeFailureKind::Other(r) if r.contains("no commits to merge")
let gate_output = match &report { );
Ok(r) if !r.had_conflicts => r.gate_output.clone(), // Self-evident fix: gate-only failure whose output matches a pattern
_ => String::new(), // a fixup coder can resolve in one short session (story 981).
let fixup_output = match &kind {
crate::pipeline_state::MergeFailureKind::GatesFailed(o) => o.as_str(),
_ => "",
}; };
let is_fixup = let is_fixup =
!is_no_commits && !gate_output.is_empty() && is_self_evident_fix(&gate_output); !is_no_commits && !fixup_output.is_empty() && is_self_evident_fix(fixup_output);
if is_no_commits { if is_no_commits {
let reason = kind.display_reason();
if let Err(e) = crate::agents::lifecycle::transition_to_blocked(&sid, &reason) { if let Err(e) = crate::agents::lifecycle::transition_to_blocked(&sid, &reason) {
slog_error!("[merge] Failed to transition '{sid}' to Blocked: {e}"); slog_error!("[merge] Failed to transition '{sid}' to Blocked: {e}");
} }
@@ -165,15 +168,16 @@ impl AgentPool {
reason, reason,
}); });
} else if is_fixup { } else if is_fixup {
// Save gate output and mark fixup pending before any state transition // Mark fixup pending before any state transition so a concurrent
// so that a concurrent auto-assign that fires after the state change // auto-assign that fires after the state change sees the key set.
// sees the keys already set.
crate::db::write_content(crate::db::ContentKey::GateOutput(&sid), &gate_output);
crate::db::write_content(crate::db::ContentKey::MergeFixupPending(&sid), "1"); crate::db::write_content(crate::db::ContentKey::MergeFixupPending(&sid), "1");
// Merge → MergeFailure → Coding. FixupRequested also sets // Merge → MergeFailure → Coding. FixupRequested also sets
// retry_count=1 so maybe_inject_gate_failure injects the gate // retry_count=1 so maybe_inject_gate_failure injects gate output
// output into --append-system-prompt on the fixup spawn. // into --append-system-prompt on the fixup spawn.
let _ = crate::agents::lifecycle::transition_to_merge_failure(&sid, &reason); // transition_to_merge_failure also writes ContentKey::GateOutput.
let display = kind.display_reason();
let _ =
crate::agents::lifecycle::transition_to_merge_failure(sid.as_str(), kind);
match crate::agents::lifecycle::move_story_to_stage(&sid, "current") { match crate::agents::lifecycle::move_story_to_stage(&sid, "current") {
Ok(_) => { Ok(_) => {
slog!( slog!(
@@ -203,7 +207,7 @@ impl AgentPool {
let _ = pool.watcher_tx.send( let _ = pool.watcher_tx.send(
crate::io::watcher::WatcherEvent::MergeFailure { crate::io::watcher::WatcherEvent::MergeFailure {
story_id: sid.clone(), story_id: sid.clone(),
reason, reason: display,
}, },
); );
} }
@@ -212,8 +216,10 @@ impl AgentPool {
// Transition through the state machine (Merge → MergeFailure). // Transition through the state machine (Merge → MergeFailure).
// Only send the notification when the stage actually changed; if the // Only send the notification when the stage actually changed; if the
// story was already in MergeFailure (self-loop), suppress the duplicate. // story was already in MergeFailure (self-loop), suppress the duplicate.
let display = kind.display_reason();
let should_notify = match crate::agents::lifecycle::transition_to_merge_failure( let should_notify = match crate::agents::lifecycle::transition_to_merge_failure(
&sid, &reason, sid.as_str(),
kind,
) { ) {
Ok(fired) => !matches!( Ok(fired) => !matches!(
fired.before, fired.before,
@@ -231,7 +237,7 @@ impl AgentPool {
pool.watcher_tx pool.watcher_tx
.send(crate::io::watcher::WatcherEvent::MergeFailure { .send(crate::io::watcher::WatcherEvent::MergeFailure {
story_id: sid.clone(), story_id: sid.clone(),
reason, reason: display,
}); });
} }
} }
+4 -4
View File
@@ -235,12 +235,12 @@ fn render_item_line(
) { ) {
match &item.stage { match &item.stage {
// MergeFailureFinal: mergemaster already tried and gave up — always ⛔. // MergeFailureFinal: mergemaster already tried and gave up — always ⛔.
Stage::MergeFailureFinal { reason } => { Stage::MergeFailureFinal { kind } => {
let snippet = first_non_empty_snippet(reason, 120); let snippet = first_non_empty_snippet(&kind.display_reason(), 120);
return format!(" \u{26D4} {display}{cost_suffix}{dep_suffix}{snippet}\n"); return format!(" \u{26D4} {display}{cost_suffix}{dep_suffix}{snippet}\n");
} }
// MergeFailure: a recovery agent may be running or queued. // MergeFailure: a recovery agent may be running or queued.
Stage::MergeFailure { reason, .. } => { Stage::MergeFailure { kind, .. } => {
return match agent.map(|a| &a.status) { return match agent.map(|a| &a.status) {
Some(AgentStatus::Running) => format!( Some(AgentStatus::Running) => format!(
" \u{1F916} {display}{cost_suffix}{dep_suffix} — mergemaster running\n" " \u{1F916} {display}{cost_suffix}{dep_suffix} — mergemaster running\n"
@@ -249,7 +249,7 @@ fn render_item_line(
" \u{23F3} {display}{cost_suffix}{dep_suffix} — mergemaster queued\n" " \u{23F3} {display}{cost_suffix}{dep_suffix} — mergemaster queued\n"
), ),
_ => { _ => {
let snippet = first_non_empty_snippet(reason, 120); let snippet = first_non_empty_snippet(&kind.display_reason(), 120);
format!(" \u{26D4} {display}{cost_suffix}{dep_suffix}{snippet}\n") format!(" \u{26D4} {display}{cost_suffix}{dep_suffix}{snippet}\n")
} }
}; };
+3 -1
View File
@@ -901,7 +901,9 @@ fn merge_failure_item_appears_in_merge_section_not_blocked() {
"100_story_merge_fail", "100_story_merge_fail",
"Merge Failure Story", "Merge Failure Story",
Stage::MergeFailure { Stage::MergeFailure {
reason: "conflict in lib.rs".to_string(), kind: crate::pipeline_state::MergeFailureKind::ConflictDetected(Some(
"conflict in lib.rs".to_string(),
)),
feature_branch: BranchName("feature/100".to_string()), feature_branch: BranchName("feature/100".to_string()),
commits_ahead: std::num::NonZeroU32::new(1).unwrap(), commits_ahead: std::num::NonZeroU32::new(1).unwrap(),
}, },
+12 -4
View File
@@ -442,13 +442,21 @@ fn project_stage_for_view(
feature_branch: BranchName(format!("feature/story-{story_id}")), feature_branch: BranchName(format!("feature/story-{story_id}")),
commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"), commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"),
}), }),
"merge_failure" => Some(Stage::MergeFailure { "merge_failure" => {
reason: String::new(), // Reconstruct the typed kind from ContentKey::GateOutput so the
// auto-assigner can match on the variant after a server restart.
// This is the sole persistence backing for MergeFailureKind.
let kind = crate::db::read_content(crate::db::ContentKey::GateOutput(story_id))
.map(|s| crate::pipeline_state::MergeFailureKind::infer_from_gate_output(&s))
.unwrap_or(crate::pipeline_state::MergeFailureKind::Other(String::new()));
Some(Stage::MergeFailure {
kind,
feature_branch: BranchName(format!("feature/story-{story_id}")), feature_branch: BranchName(format!("feature/story-{story_id}")),
commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"), commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"),
}), })
}
"merge_failure_final" => Some(Stage::MergeFailureFinal { "merge_failure_final" => Some(Stage::MergeFailureFinal {
reason: String::new(), kind: crate::pipeline_state::MergeFailureKind::Other(String::new()),
}), }),
"frozen" => Some(Stage::Frozen { "frozen" => Some(Stage::Frozen {
resume_to: resume_target(), resume_to: resume_target(),
+1 -1
View File
@@ -338,7 +338,7 @@ mod stage_migration_tests {
"9507_legacy_merge_failure", "9507_legacy_merge_failure",
"4_merge_failure", "4_merge_failure",
Stage::MergeFailure { Stage::MergeFailure {
reason: String::new(), kind: crate::pipeline_state::MergeFailureKind::Other(String::new()),
feature_branch: crate::pipeline_state::BranchName(String::new()), feature_branch: crate::pipeline_state::BranchName(String::new()),
commits_ahead: NonZeroU32::new(1).unwrap(), commits_ahead: NonZeroU32::new(1).unwrap(),
}, },
+8 -4
View File
@@ -177,12 +177,16 @@ pub(super) fn tool_report_merge_failure(args: &Value, ctx: &AppContext) -> Resul
slog!("[mergemaster] Merge failure reported for '{story_id}': {reason}"); slog!("[mergemaster] Merge failure reported for '{story_id}': {reason}");
ctx.services.agents.set_merge_failure_reported(story_id); ctx.services.agents.set_merge_failure_reported(story_id);
// The mergemaster provides a freeform reason string; use Other so the
// auto-assigner does not re-spawn another mergemaster after this one fails.
let kind = crate::pipeline_state::MergeFailureKind::Other(reason.to_string());
let display = kind.display_reason();
// Route the failure through the typed state machine (Merge → MergeFailure). // Route the failure through the typed state machine (Merge → MergeFailure).
// This persists the reason in front matter and updates the CRDT stage.
// Only broadcast the notification when the stage actually changed; if the // Only broadcast the notification when the stage actually changed; if the
// story was already in MergeFailure (self-loop), suppress the duplicate. // story was already in MergeFailure (self-loop), suppress the duplicate.
let should_notify = let should_notify = match crate::agents::lifecycle::transition_to_merge_failure(story_id, kind)
match crate::agents::lifecycle::transition_to_merge_failure(story_id, reason) { {
Ok(fired) => !matches!( Ok(fired) => !matches!(
fired.before, fired.before,
crate::pipeline_state::Stage::MergeFailure { .. } crate::pipeline_state::Stage::MergeFailure { .. }
@@ -197,7 +201,7 @@ pub(super) fn tool_report_merge_failure(args: &Value, ctx: &AppContext) -> Resul
.watcher_tx .watcher_tx
.send(crate::io::watcher::WatcherEvent::MergeFailure { .send(crate::io::watcher::WatcherEvent::MergeFailure {
story_id: story_id.to_string(), story_id: story_id.to_string(),
reason: reason.to_string(), reason: display,
}); });
} }
+2 -2
View File
@@ -40,8 +40,8 @@ mod tests;
#[allow(unused_imports)] #[allow(unused_imports)]
pub use types::{ pub use types::{
AgentName, ArchiveReason, BranchName, ExecutionState, GitSha, NodePubkey, PipelineItem, Stage, AgentName, ArchiveReason, BranchName, ExecutionState, GitSha, MergeFailureKind, NodePubkey,
StoryId, TransitionError, stage_dir_name, stage_label, PipelineItem, Stage, StoryId, TransitionError, stage_dir_name, stage_label,
}; };
#[allow(unused_imports)] #[allow(unused_imports)]
+14 -10
View File
@@ -648,16 +648,20 @@ fn merge_failure_transition_emits_event_with_full_reason() {
let fired = super::apply::apply_transition( let fired = super::apply::apply_transition(
story_id, story_id,
PipelineEvent::MergeFailed { PipelineEvent::MergeFailed {
reason: reason.to_string(), kind: MergeFailureKind::Other(reason.to_string()),
}, },
None, None,
) )
.expect("MergeFailed transition should succeed"); .expect("MergeFailed transition should succeed");
// The emitted event payload carries the full reason string. // The emitted event payload carries the full reason via display_reason().
match &fired.event { match &fired.event {
PipelineEvent::MergeFailed { reason: r } => { PipelineEvent::MergeFailed { kind } => {
assert_eq!(r, reason, "emitted event should carry the full reason"); assert_eq!(
kind.display_reason(),
reason,
"emitted event should carry the full reason"
);
} }
other => panic!("expected MergeFailed event, got: {other:?}"), other => panic!("expected MergeFailed event, got: {other:?}"),
} }
@@ -686,14 +690,14 @@ fn merge_failure_transition_emits_event_with_full_reason() {
#[test] #[test]
fn merge_failure_plus_merge_failed_is_self_loop() { fn merge_failure_plus_merge_failed_is_self_loop() {
let s = Stage::MergeFailure { let s = Stage::MergeFailure {
reason: "initial failure".into(), kind: MergeFailureKind::Other("initial failure".into()),
feature_branch: fb("feature/story-1"), feature_branch: fb("feature/story-1"),
commits_ahead: nz(1), commits_ahead: nz(1),
}; };
let result = transition( let result = transition(
s, s,
PipelineEvent::MergeFailed { PipelineEvent::MergeFailed {
reason: "second failure".into(), kind: MergeFailureKind::Other("second failure".into()),
}, },
); );
assert!( assert!(
@@ -722,7 +726,7 @@ fn repeated_merge_failure_apply_transition_no_error_no_duplicate_notification()
let fired = super::apply::apply_transition( let fired = super::apply::apply_transition(
story_id, story_id,
PipelineEvent::MergeFailed { PipelineEvent::MergeFailed {
reason: "duplicate failure".into(), kind: MergeFailureKind::Other("duplicate failure".into()),
}, },
None, None,
) )
@@ -766,7 +770,7 @@ fn repeated_merge_failure_apply_transition_no_error_no_duplicate_notification()
#[test] #[test]
fn merge_failure_unblock_returns_to_merge() { fn merge_failure_unblock_returns_to_merge() {
let s = Stage::MergeFailure { let s = Stage::MergeFailure {
reason: "conflicts in server/src/main.rs".into(), kind: MergeFailureKind::ConflictDetected(Some("conflicts in server/src/main.rs".into())),
feature_branch: fb("feature/story-42"), feature_branch: fb("feature/story-42"),
commits_ahead: nz(3), commits_ahead: nz(3),
}; };
@@ -781,7 +785,7 @@ fn merge_failure_unblock_returns_to_merge() {
#[test] #[test]
fn merge_failure_demote_returns_to_backlog() { fn merge_failure_demote_returns_to_backlog() {
let s = Stage::MergeFailure { let s = Stage::MergeFailure {
reason: "conflicts".into(), kind: MergeFailureKind::Other("conflicts".into()),
feature_branch: fb("feature/story-1"), feature_branch: fb("feature/story-1"),
commits_ahead: nz(1), commits_ahead: nz(1),
}; };
@@ -799,7 +803,7 @@ fn merge_failure_demote_returns_to_backlog() {
#[test] #[test]
fn merge_failure_accept_pure_transition() { fn merge_failure_accept_pure_transition() {
let s = Stage::MergeFailure { let s = Stage::MergeFailure {
reason: "conflicts unresolvable".into(), kind: MergeFailureKind::ConflictDetected(Some("conflicts unresolvable".into())),
feature_branch: fb("feature/story-1"), feature_branch: fb("feature/story-1"),
commits_ahead: nz(1), commits_ahead: nz(1),
}; };
+9 -9
View File
@@ -4,8 +4,8 @@ use chrono::Utc;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use super::{ use super::{
AgentName, ArchiveReason, BranchName, ExecutionState, GitSha, Stage, StoryId, TransitionError, AgentName, ArchiveReason, BranchName, ExecutionState, GitSha, MergeFailureKind, Stage, StoryId,
stage_label, TransitionError, stage_label,
}; };
// ── Pipeline events ───────────────────────────────────────────────────────── // ── Pipeline events ─────────────────────────────────────────────────────────
@@ -35,7 +35,7 @@ pub enum PipelineEvent {
MergeSucceeded { merge_commit: GitSha }, MergeSucceeded { merge_commit: GitSha },
/// Merge pipeline failed (conflicts or gate failures); story moves to /// Merge pipeline failed (conflicts or gate failures); story moves to
/// `Stage::MergeFailure` awaiting human intervention or retry. /// `Stage::MergeFailure` awaiting human intervention or retry.
MergeFailed { reason: String }, MergeFailed { kind: MergeFailureKind },
/// Mergemaster gave up after retry budget. /// Mergemaster gave up after retry budget.
MergeFailedFinal { reason: String }, MergeFailedFinal { reason: String },
/// Story accepted (Done → Archived). /// Story accepted (Done → Archived).
@@ -214,9 +214,9 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result<Stage, Transitio
feature_branch, feature_branch,
commits_ahead, commits_ahead,
}, },
MergeFailed { reason }, MergeFailed { kind },
) => Ok(MergeFailure { ) => Ok(MergeFailure {
reason, kind,
feature_branch, feature_branch,
commits_ahead, commits_ahead,
}), }),
@@ -231,9 +231,9 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result<Stage, Transitio
commits_ahead, commits_ahead,
.. ..
}, },
MergeFailed { reason }, MergeFailed { kind },
) => Ok(MergeFailure { ) => Ok(MergeFailure {
reason, kind,
feature_branch, feature_branch,
commits_ahead, commits_ahead,
}), }),
@@ -326,8 +326,8 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result<Stage, Transitio
(Done { .. }, HotfixRequested) => Ok(Coding), (Done { .. }, HotfixRequested) => Ok(Coding),
// ── MergemasterAttempted: MergeFailure → MergeFailureFinal ───── // ── MergemasterAttempted: MergeFailure → MergeFailureFinal ─────
(MergeFailure { reason, .. }, MergemasterAttempted) => Ok(MergeFailureFinal { reason }), (MergeFailure { kind, .. }, MergemasterAttempted) => Ok(MergeFailureFinal { kind }),
(MergeFailureFinal { reason }, MergemasterAttempted) => Ok(MergeFailureFinal { reason }), (MergeFailureFinal { kind }, MergemasterAttempted) => Ok(MergeFailureFinal { kind }),
// ── Unblock: from Frozen/ReviewHold → resume_to ──────────────── // ── Unblock: from Frozen/ReviewHold → resume_to ────────────────
(Frozen { resume_to }, Unblock) => Ok(*resume_to), (Frozen { resume_to }, Unblock) => Ok(*resume_to),
+79 -4
View File
@@ -39,6 +39,76 @@ impl fmt::Display for AgentName {
} }
} }
// ── Merge failure kind ──────────────────────────────────────────────────────
/// Typed reason for a merge pipeline failure.
///
/// Replaces the freeform `reason: String` that was previously stored on
/// `Stage::MergeFailure` (story 982). Consumers match on the variant instead
/// of scanning substrings of an opaque string.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub enum MergeFailureKind {
/// Git content conflicts detected during the squash-rebase. The payload
/// is the raw conflict output (e.g. the git CONFLICT lines), if available.
ConflictDetected(Option<String>),
/// Quality gates (fmt, clippy, tests) failed after the squash merge.
/// The payload is the gate runner output.
GatesFailed(String),
/// Feature branch has no code changes ahead of master (empty diff).
EmptyDiff,
/// Squash merge produced no commits (e.g. empty-commit history).
NoCommits,
/// Unclassified merge failure; the payload carries the original message.
Other(String),
}
impl MergeFailureKind {
/// Human-readable description for display tools and `MergeJob.error`.
pub fn display_reason(&self) -> String {
match self {
MergeFailureKind::ConflictDetected(details) => format!(
"Merge conflict: {}",
details.as_deref().unwrap_or("conflicts detected")
),
MergeFailureKind::GatesFailed(output) => format!("Quality gates failed: {output}"),
MergeFailureKind::EmptyDiff => "Feature branch has no code changes".to_string(),
MergeFailureKind::NoCommits => "No commits to merge".to_string(),
MergeFailureKind::Other(reason) => reason.clone(),
}
}
/// String to persist in `ContentKey::GateOutput` so the CRDT projection
/// layer can reconstruct the kind after a server restart.
pub fn to_gate_output(&self) -> String {
match self {
// Always prefix with "Merge conflict:" so `infer_from_gate_output`
// can identify ConflictDetected on reload.
MergeFailureKind::ConflictDetected(details) => format!(
"Merge conflict: {}",
details.as_deref().unwrap_or("conflicts detected")
),
// Raw gate output is the natural identifier for a GatesFailed kind.
MergeFailureKind::GatesFailed(output) => output.clone(),
MergeFailureKind::EmptyDiff => "empty diff".to_string(),
MergeFailureKind::NoCommits => "no commits to merge".to_string(),
MergeFailureKind::Other(reason) => reason.clone(),
}
}
/// Infer a kind from a `ContentKey::GateOutput` string persisted by a
/// previous call to [`Self::to_gate_output`] or by legacy code that wrote
/// conflict markers directly. Used by the CRDT projection layer.
pub fn infer_from_gate_output(gate_output: &str) -> Self {
if gate_output.contains("CONFLICT (content):") || gate_output.contains("Merge conflict") {
MergeFailureKind::ConflictDetected(Some(gate_output.to_string()))
} else if gate_output.is_empty() {
MergeFailureKind::Other(String::new())
} else {
MergeFailureKind::GatesFailed(gate_output.to_string())
}
}
}
// ── Synced pipeline stage (lives in CRDT, converges across nodes) ─────────── // ── Synced pipeline stage (lives in CRDT, converges across nodes) ───────────
/// The pipeline stage for a work item. /// The pipeline stage for a work item.
@@ -111,7 +181,9 @@ pub enum Stage {
/// this is a recoverable intermediate state — `Unblock` returns to `Merge` /// this is a recoverable intermediate state — `Unblock` returns to `Merge`
/// (re-queues the merge) and `Demote` returns to `Backlog` (manual park). /// (re-queues the merge) and `Demote` returns to `Backlog` (manual park).
MergeFailure { MergeFailure {
reason: String, /// Typed failure reason — callers match on the variant instead of
/// substring-scanning a freeform string (story 982).
kind: MergeFailureKind,
/// Branch and commit count preserved from the preceding `Merge` state /// Branch and commit count preserved from the preceding `Merge` state
/// so `Unblock` can reconstruct the exact `Merge` variant. /// so `Unblock` can reconstruct the exact `Merge` variant.
feature_branch: BranchName, feature_branch: BranchName,
@@ -122,7 +194,10 @@ pub enum Stage {
/// recover; the agent gave up. The story stays here awaiting human /// recover; the agent gave up. The story stays here awaiting human
/// intervention — the auto-assigner will NOT spawn mergemaster again. /// intervention — the auto-assigner will NOT spawn mergemaster again.
/// Replaces the legacy `mergemaster_attempted: true` boolean flag. /// Replaces the legacy `mergemaster_attempted: true` boolean flag.
MergeFailureFinal { reason: String }, MergeFailureFinal {
/// Typed failure reason carried through from the preceding `MergeFailure`.
kind: MergeFailureKind,
},
/// Story is frozen — kept at this stage as a snapshot of its previous /// Story is frozen — kept at this stage as a snapshot of its previous
/// stage. Replaces the legacy `frozen: true` boolean flag: there is no /// stage. Replaces the legacy `frozen: true` boolean flag: there is no
@@ -229,12 +304,12 @@ impl Stage {
commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"), commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"),
}), }),
"merge_failure" => Some(Stage::MergeFailure { "merge_failure" => Some(Stage::MergeFailure {
reason: String::new(), kind: MergeFailureKind::Other(String::new()),
feature_branch: BranchName(String::new()), feature_branch: BranchName(String::new()),
commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"), commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"),
}), }),
"merge_failure_final" => Some(Stage::MergeFailureFinal { "merge_failure_final" => Some(Stage::MergeFailureFinal {
reason: String::new(), kind: MergeFailureKind::Other(String::new()),
}), }),
"frozen" => Some(Stage::Frozen { "frozen" => Some(Stage::Frozen {
resume_to: Box::new(Stage::Coding), resume_to: Box::new(Stage::Coding),