diff --git a/server/src/agents/lifecycle.rs b/server/src/agents/lifecycle.rs index dbe4162f..17541240 100644 --- a/server/src/agents/lifecycle.rs +++ b/server/src/agents/lifecycle.rs @@ -248,6 +248,28 @@ pub fn transition_to_blocked(story_id: &str, reason: &str) -> Result<(), String> .map_err(|e| e.to_string()) } +/// Transition a story from `Stage::Merge` to `Stage::MergeFailure` via the state machine. +/// +/// Builds a `PipelineEvent::MergeFailed { reason }`, validates the transition, writes +/// the resulting `Stage::MergeFailure` to the CRDT, and persists the reason to front +/// matter so it survives server restarts. +/// Returns `Err` on `TransitionError` — callers must NOT fall back to direct register writes. +pub fn transition_to_merge_failure(story_id: &str, reason: &str) -> Result<(), String> { + let reason_owned = reason.to_string(); + let transform: Box String> = Box::new(move |content: &str| { + crate::io::story_metadata::write_merge_failure_in_content(content, &reason_owned) + }); + apply_transition( + story_id, + PipelineEvent::MergeFailed { + reason: reason.to_string(), + }, + Some(&*transform), + ) + .map(|_| ()) + .map_err(|e| e.to_string()) +} + /// Transition a story out of `Blocked` back to `Coding` via the state machine. /// /// Builds a `PipelineEvent::Unblock`, validates the transition, writes the @@ -301,6 +323,7 @@ fn map_stage_move_to_event( }), (Stage::Coding | Stage::Qa | Stage::Backlog, "done") => Ok(PipelineEvent::Close), (Stage::Blocked { .. }, "current") => Ok(PipelineEvent::Unblock), + (Stage::MergeFailure { .. }, "backlog") => Ok(PipelineEvent::Unblock), ( Stage::Archived { reason: ArchiveReason::Blocked { .. }, @@ -388,6 +411,7 @@ fn stage_to_name(s: &Stage) -> &'static str { Stage::Blocked { .. } => "blocked", Stage::Qa => "qa", Stage::Merge { .. } => "merge", + Stage::MergeFailure { .. } => "merge_failure", Stage::Done { .. } => "done", Stage::Archived { .. } => "archived", Stage::Frozen { .. } => "frozen", diff --git a/server/src/agents/pool/pipeline/merge/runner.rs b/server/src/agents/pool/pipeline/merge/runner.rs index d13dfffa..c5d3dd84 100644 --- a/server/src/agents/pool/pipeline/merge/runner.rs +++ b/server/src/agents/pool/pipeline/merge/runner.rs @@ -114,16 +114,6 @@ impl AgentPool { Err(e) => e.clone(), }; let is_no_commits = reason.contains("no commits to merge"); - if !is_no_commits { - // Write merge_failure to content for non-blocking failures. - if let Some(contents) = crate::db::read_content(&sid) { - let updated = crate::io::story_metadata::write_merge_failure_in_content( - &contents, &reason, - ); - crate::db::write_content(&sid, &updated); - crate::db::write_item_with_content(&sid, "4_merge", &updated); - } - } if is_no_commits { if let Err(e) = crate::agents::lifecycle::transition_to_blocked(&sid, &reason) { crate::slog_error!("[merge] Failed to transition '{sid}' to Blocked: {e}"); @@ -135,6 +125,14 @@ impl AgentPool { reason, }); } else { + // Transition through the state machine (Merge → MergeFailure). + if let Err(e) = + crate::agents::lifecycle::transition_to_merge_failure(&sid, &reason) + { + crate::slog_error!( + "[merge] Failed to transition '{sid}' to MergeFailure: {e}" + ); + } let _ = pool .watcher_tx .send(crate::io::watcher::WatcherEvent::MergeFailure { diff --git a/server/src/chat/transport/matrix/delete.rs b/server/src/chat/transport/matrix/delete.rs index a3d316e9..2c2f0390 100644 --- a/server/src/chat/transport/matrix/delete.rs +++ b/server/src/chat/transport/matrix/delete.rs @@ -114,6 +114,7 @@ fn stage_display_name(stage: &str) -> &str { Some(Stage::Merge { .. }) => "merge", Some(Stage::Done { .. }) => "done", Some(Stage::Archived { .. }) => "archived", + Some(Stage::MergeFailure { .. }) => "merge-failure", Some(Stage::Frozen { .. }) => "frozen", None => stage, } diff --git a/server/src/http/mcp/merge_tools.rs b/server/src/http/mcp/merge_tools.rs index d2361c3d..30bd1617 100644 --- a/server/src/http/mcp/merge_tools.rs +++ b/server/src/http/mcp/merge_tools.rs @@ -1,7 +1,6 @@ //! MCP merge tools — merge agent work to master and report merge failures. use crate::agents::move_story_to_merge; use crate::http::context::AppContext; -use crate::io::story_metadata::write_merge_failure; use crate::slog; use crate::slog_warn; use serde_json::{Value, json}; @@ -189,30 +188,14 @@ pub(super) fn tool_report_merge_failure(args: &Value, ctx: &AppContext) -> Resul reason: reason.to_string(), }); - // Persist the failure reason to the story file's front matter so it - // survives server restarts and is visible in the web UI. - if let Ok(project_root) = ctx.state.get_project_root() { - let story_file = project_root - .join(".huskies") - .join("work") - .join("4_merge") - .join(format!("{story_id}.md")); - if story_file.exists() { - if let Err(e) = write_merge_failure(&story_file, reason) { - slog_warn!( - "[mergemaster] Failed to persist merge_failure to story file for '{story_id}': {e}" - ); - } - } else { - slog_warn!( - "[mergemaster] Story file not found in 4_merge/ for '{story_id}'; \ - merge_failure not persisted to front matter" - ); - } + // Route the failure through the typed state machine (Merge → MergeFailure). + // This persists the reason in front matter and updates the CRDT stage. + if let Err(e) = crate::agents::lifecycle::transition_to_merge_failure(story_id, reason) { + slog_warn!("[mergemaster] Failed to transition '{story_id}' to MergeFailure: {e}"); } Ok(format!( - "Merge failure for '{story_id}' recorded. Story remains in work/4_merge/. Reason: {reason}" + "Merge failure for '{story_id}' recorded. Reason: {reason}" )) } @@ -451,7 +434,6 @@ mod tests { assert!(result.is_ok()); let msg = result.unwrap(); assert!(msg.contains("42_story_foo")); - assert!(msg.contains("work/4_merge/")); assert!(msg.contains("Unresolvable merge conflicts")); } } diff --git a/server/src/http/mcp/story_tools/epic.rs b/server/src/http/mcp/story_tools/epic.rs index a2540541..c2b046fc 100644 --- a/server/src/http/mcp/story_tools/epic.rs +++ b/server/src/http/mcp/story_tools/epic.rs @@ -144,6 +144,7 @@ pub(crate) fn tool_show_epic(args: &Value, _ctx: &AppContext) -> Result "merge", Stage::Done { .. } => "done", Stage::Archived { .. } => "archived", + Stage::MergeFailure { .. } => "merge_failure", Stage::Frozen { .. } => "frozen", Stage::Blocked { .. } => "blocked", }; diff --git a/server/src/http/workflow/pipeline.rs b/server/src/http/workflow/pipeline.rs index 231a6eed..beef38a8 100644 --- a/server/src/http/workflow/pipeline.rs +++ b/server/src/http/workflow/pipeline.rs @@ -149,6 +149,7 @@ pub fn load_pipeline_state(ctx: &AppContext) -> Result { Stage::Blocked { .. } => state.current.push(story), // blocked shown with current Stage::Qa => state.qa.push(story), Stage::Merge { .. } => state.merge.push(story), + Stage::MergeFailure { .. } => state.merge.push(story), // show merge failures with merge Stage::Done { .. } => state.done.push(story), Stage::Archived { .. } => {} // skip archived Stage::Frozen { .. } => state.backlog.push(story), // show frozen with backlog diff --git a/server/src/io/story_metadata/fields.rs b/server/src/io/story_metadata/fields.rs index 25ec8f3b..d055582d 100644 --- a/server/src/io/story_metadata/fields.rs +++ b/server/src/io/story_metadata/fields.rs @@ -83,27 +83,6 @@ pub fn clear_front_matter_field(path: &Path, key: &str) -> Result<(), String> { Ok(()) } -/// Write or update a `merge_failure:` field in the YAML front matter of a story file. -/// -/// The reason is stored as a quoted YAML string so that colons, hashes, and newlines -/// in the failure message do not break front-matter parsing. -/// If no front matter is present, this is a no-op (returns Ok). -pub fn write_merge_failure(path: &Path, reason: &str) -> Result<(), String> { - let contents = - fs::read_to_string(path).map_err(|e| format!("Failed to read story file: {e}"))?; - - // Produce a YAML-safe inline quoted string: collapse newlines, escape inner quotes. - let escaped = reason - .replace('"', "\\\"") - .replace('\n', " ") - .replace('\r', ""); - let yaml_value = format!("\"{escaped}\""); - - let updated = set_front_matter_field(&contents, "merge_failure", &yaml_value); - fs::write(path, &updated).map_err(|e| format!("Failed to write story file: {e}"))?; - Ok(()) -} - /// Write `review_hold: true` to the YAML front matter of a story file. /// /// Used to mark spikes that have passed QA and are waiting for human review. diff --git a/server/src/io/story_metadata/mod.rs b/server/src/io/story_metadata/mod.rs index eadef1ff..913a21d9 100644 --- a/server/src/io/story_metadata/mod.rs +++ b/server/src/io/story_metadata/mod.rs @@ -14,9 +14,9 @@ mod types; pub use deps::{check_archived_deps, check_archived_deps_from_list, check_unmet_deps}; pub use fields::{ clear_front_matter_field, clear_front_matter_field_in_content, set_front_matter_field, - write_depends_on, write_depends_on_in_content, write_merge_failure, - write_merge_failure_in_content, write_mergemaster_attempted_in_content, - write_rejection_notes_to_content, write_review_hold, write_review_hold_in_content, + write_depends_on, write_depends_on_in_content, write_merge_failure_in_content, + write_mergemaster_attempted_in_content, write_rejection_notes_to_content, write_review_hold, + write_review_hold_in_content, }; pub use parser::{ is_story_frozen_in_store, parse_front_matter, parse_unchecked_todos, resolve_qa_mode, diff --git a/server/src/io/watcher/mod.rs b/server/src/io/watcher/mod.rs index ace0e16d..c4da8209 100644 --- a/server/src/io/watcher/mod.rs +++ b/server/src/io/watcher/mod.rs @@ -55,6 +55,9 @@ pub fn stage_metadata(stage: &str, item_id: &str) -> Option<(&'static str, Strin Stage::Blocked { .. } => ("block", format!("huskies: block {item_id}")), Stage::Qa => ("qa", format!("huskies: queue {item_id} for QA")), Stage::Merge { .. } => ("merge", format!("huskies: queue {item_id} for merge")), + Stage::MergeFailure { .. } => { + ("merge_failure", format!("huskies: merge_failure {item_id}")) + } Stage::Done { .. } => ("done", format!("huskies: done {item_id}")), Stage::Archived { .. } => ("accept", format!("huskies: accept {item_id}")), Stage::Frozen { .. } => ("freeze", format!("huskies: freeze {item_id}")), diff --git a/server/src/pipeline_state/projection.rs b/server/src/pipeline_state/projection.rs index 8ed5dfcf..1f629dac 100644 --- a/server/src/pipeline_state/projection.rs +++ b/server/src/pipeline_state/projection.rs @@ -91,6 +91,14 @@ pub fn project_stage(view: &PipelineItemView) -> Result commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"), }) } + "4_merge_failure" => { + // The reason is persisted in front-matter (merge_failure: "...") but + // is not part of the raw CRDT view; the projection uses an empty + // string here. Consumers that need the reason should read content. + Ok(Stage::MergeFailure { + reason: String::new(), + }) + } "5_done" => { // Use the stored merged_at timestamp if present. Legacy items // that pre-date this field have merged_at = None, so we fall back diff --git a/server/src/pipeline_state/tests.rs b/server/src/pipeline_state/tests.rs index 3e4a4fd9..44b53170 100644 --- a/server/src/pipeline_state/tests.rs +++ b/server/src/pipeline_state/tests.rs @@ -655,4 +655,57 @@ fn regression_freeze_unfreeze_restores_crdt_stage() { ); } +// ── Story 868: MergeFailure regression ───────────────────────────── + +/// Regression test (story 868): applying `PipelineEvent::MergeFailed` to a story +/// in `Stage::Merge` transitions it to `Stage::MergeFailure` and the emitted +/// `TransitionFired` event carries the full reason string in its payload. +#[test] +fn merge_failure_transition_emits_event_with_full_reason() { + crate::crdt_state::init_for_test(); + crate::db::ensure_content_store(); + + let story_id = "99868_story_merge_failure_event"; + crate::db::write_item_with_content( + story_id, + "4_merge", + "---\nname: Merge Failure Event Test\n---\n# Story\n", + ); + + let reason = "Conflict in server/src/main.rs: both modified"; + let fired = super::apply::apply_transition( + story_id, + PipelineEvent::MergeFailed { + reason: reason.to_string(), + }, + None, + ) + .expect("MergeFailed transition should succeed"); + + // The emitted event payload carries the full reason string. + match &fired.event { + PipelineEvent::MergeFailed { reason: r } => { + assert_eq!(r, reason, "emitted event should carry the full reason"); + } + other => panic!("expected MergeFailed event, got: {other:?}"), + } + + // The story transitioned to MergeFailure. + assert!( + matches!(fired.after, Stage::MergeFailure { .. }), + "after-stage should be MergeFailure: {:?}", + fired.after + ); + + // Verify CRDT reflects the new stage. + let item = read_typed(story_id) + .expect("CRDT read should succeed") + .expect("item should exist"); + assert_eq!( + item.stage.dir_name(), + "4_merge_failure", + "CRDT stage should be 4_merge_failure" + ); +} + // ── ProjectionError Display ───────────────────────────────────────── diff --git a/server/src/pipeline_state/transition.rs b/server/src/pipeline_state/transition.rs index f9be223b..37510e4b 100644 --- a/server/src/pipeline_state/transition.rs +++ b/server/src/pipeline_state/transition.rs @@ -33,6 +33,9 @@ pub enum PipelineEvent { }, /// Mergemaster squash succeeded. MergeSucceeded { merge_commit: GitSha }, + /// Merge pipeline failed (conflicts or gate failures); story moves to + /// `Stage::MergeFailure` awaiting human intervention or retry. + MergeFailed { reason: String }, /// Mergemaster gave up after retry budget. MergeFailedFinal { reason: String }, /// Story accepted (Done → Archived). @@ -87,6 +90,7 @@ pub fn event_label(e: &PipelineEvent) -> &'static str { PipelineEvent::GatesFailed { .. } => "GatesFailed", PipelineEvent::QaSkipped { .. } => "QaSkipped", PipelineEvent::MergeSucceeded { .. } => "MergeSucceeded", + PipelineEvent::MergeFailed { .. } => "MergeFailed", PipelineEvent::MergeFailedFinal { .. } => "MergeFailedFinal", PipelineEvent::Accepted => "Accepted", PipelineEvent::Block { .. } => "Block", @@ -174,6 +178,9 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result Ok(MergeFailure { reason }), + (Merge { .. }, MergeFailedFinal { reason }) => Ok(Archived { archived_at: now, reason: ArchiveReason::MergeFailed { reason }, @@ -221,6 +228,9 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result Ok(Coding), + // ── Unblock MergeFailure → Backlog ─────────────────────────────── + (MergeFailure { .. }, Unblock) => Ok(Backlog), + // ── Legacy unblock: Archived(Blocked|MergeFailed) → Backlog ── ( Archived { diff --git a/server/src/pipeline_state/types.rs b/server/src/pipeline_state/types.rs index 195118f1..c93c04c2 100644 --- a/server/src/pipeline_state/types.rs +++ b/server/src/pipeline_state/types.rs @@ -60,9 +60,9 @@ impl fmt::Display for AgentName { /// | current | `Coding` | /// | qa_pending | `Qa` | /// | merge_pending | `Merge { .. }` | +/// | merge_failure | `MergeFailure { .. }` | /// | done | `Done { .. }` | /// | blocked | `Blocked { .. }` | -/// | merge_failure | `Archived { MergeFailed { .. } }` | /// | archived | `Archived { Completed }` | /// | superseded | `Archived { Superseded { .. } }` | /// | rejected | `Archived { Rejected { .. } }` | @@ -106,6 +106,11 @@ pub enum Stage { reason: ArchiveReason, }, + /// Merge pipeline failed (conflicts or gate failures). Story is held here + /// awaiting human intervention or retry. Unlike `Archived(MergeFailed)`, + /// this is a recoverable intermediate state — `Unblock` returns to `Backlog`. + MergeFailure { reason: String }, + /// Pipeline advancement and auto-assign are suspended. Resumes to /// `resume_to` when unfrozen. Frozen { resume_to: Box }, @@ -154,12 +159,13 @@ impl Stage { stage_dir_name(self) } - /// Returns true if this is the `Blocked` variant (or the legacy - /// `Archived(Blocked)` for backward-compatible reads). + /// Returns true if this is the `Blocked` or `MergeFailure` variant (or the + /// legacy `Archived(Blocked)` for backward-compatible reads). pub fn is_blocked(&self) -> bool { matches!( self, Stage::Blocked { .. } + | Stage::MergeFailure { .. } | Stage::Archived { reason: ArchiveReason::Blocked { .. }, .. @@ -198,6 +204,11 @@ impl Stage { }), // Frozen: stub with Coding as resume_to — rich resume_to is loaded // from front matter by the projection layer. + "4_merge_failure" => Some(Stage::MergeFailure { + reason: String::new(), + }), + // Frozen: stub with Coding as resume_to — rich resume_to is loaded + // from front matter by the projection layer. "7_frozen" => Some(Stage::Frozen { resume_to: Box::new(Stage::Coding), }), @@ -278,6 +289,7 @@ pub fn stage_label(s: &Stage) -> &'static str { Stage::Coding => "Coding", Stage::Qa => "Qa", Stage::Merge { .. } => "Merge", + Stage::MergeFailure { .. } => "MergeFailure", Stage::Done { .. } => "Done", Stage::Blocked { .. } => "Blocked", Stage::Archived { .. } => "Archived", @@ -294,6 +306,7 @@ pub fn stage_dir_name(s: &Stage) -> &'static str { Stage::Blocked { .. } => "2_blocked", Stage::Qa => "3_qa", Stage::Merge { .. } => "4_merge", + Stage::MergeFailure { .. } => "4_merge_failure", Stage::Done { .. } => "5_done", Stage::Archived { .. } => "6_archived", Stage::Frozen { .. } => "7_frozen", diff --git a/server/src/service/agents/mod.rs b/server/src/service/agents/mod.rs index 0ba0af32..3a97be69 100644 --- a/server/src/service/agents/mod.rs +++ b/server/src/service/agents/mod.rs @@ -208,6 +208,7 @@ pub fn get_work_item_content( crate::pipeline_state::Stage::Blocked { .. } => "blocked", crate::pipeline_state::Stage::Qa => "qa", crate::pipeline_state::Stage::Merge { .. } => "merge", + crate::pipeline_state::Stage::MergeFailure { .. } => "merge_failure", crate::pipeline_state::Stage::Done { .. } => "done", crate::pipeline_state::Stage::Archived { .. } => "archived", crate::pipeline_state::Stage::Frozen { .. } => "frozen", diff --git a/server/src/service/notifications/format.rs b/server/src/service/notifications/format.rs index 73a8b0f6..9b7a6494 100644 --- a/server/src/service/notifications/format.rs +++ b/server/src/service/notifications/format.rs @@ -18,6 +18,7 @@ pub fn stage_display_name(stage: &str) -> &'static str { Some(Stage::Merge { .. }) => "Merge", Some(Stage::Done { .. }) => "Done", Some(Stage::Archived { .. }) => "Archived", + Some(Stage::MergeFailure { .. }) => "MergeFailure", Some(Stage::Frozen { .. }) => "Frozen", None => "Unknown", }