diff --git a/server/src/agents/lifecycle.rs b/server/src/agents/lifecycle.rs index f44f4e3a..46c0f35a 100644 --- a/server/src/agents/lifecycle.rs +++ b/server/src/agents/lifecycle.rs @@ -496,6 +496,25 @@ pub fn move_story_to_stage(story_id: &str, target_stage: &str) -> Result<(String Ok((from_name.to_string(), target_stage.to_string())) } +/// Transition a story from `Stage::MergeFailure` to `Stage::Merge` when the +/// deterministic-merge pipeline starts a retry attempt. +/// +/// Preserves `feature_branch` and `commits_ahead` from the preceding +/// `MergeFailure` state so the retry continues with the exact same branch. +/// If the story is already in `Stage::Merge` or any other stage, this is a +/// silent no-op — callers do not need to pre-check the current stage. +/// +/// Called by `start_merge_agent_work` before the squash-merge pipeline starts +/// so the UI shows an active merge state rather than a stale failure indicator +/// (story 1051, AC 1 and AC 4). +pub fn transition_merge_failure_to_retry(story_id: &str) -> Result<(), String> { + match apply_transition(story_id, PipelineEvent::MergeRetryStarted, None) { + Ok(_) => Ok(()), + Err(ApplyError::InvalidTransition(_)) | Err(ApplyError::NotFound(_)) => Ok(()), + Err(e) => Err(e.to_string()), + } +} + /// Move a bug from `work/2_current/` or `work/1_backlog/` to `work/5_done/`. /// /// Idempotent if already in `5_done/`. Errors if not found in `2_current/` or `1_backlog/`. @@ -1028,6 +1047,113 @@ mod tests { ); } + // ── Story 1051: MergeFailure → Merge retry transition ─────────────────── + + /// AC1 (story 1051): when the deterministic-merge pipeline starts on a + /// MergeFailure story, transition_merge_failure_to_retry moves it to Merge. + #[test] + fn transition_merge_failure_to_retry_moves_to_merge() { + crate::crdt_state::init_for_test(); + crate::db::ensure_content_store(); + + let story_id = "99105_story_merge_retry_1051"; + crate::db::write_item_with_content( + story_id, + "merge_failure", + "---\nname: Merge Retry Test\n---\n", + crate::db::ItemMeta::named("Merge Retry Test"), + ); + + transition_merge_failure_to_retry(story_id).expect("must succeed for MergeFailure story"); + + let item = crate::pipeline_state::read_typed(story_id) + .expect("CRDT read must succeed") + .expect("item must exist"); + assert_eq!( + item.stage.dir_name(), + "merge", + "MergeFailure story must be in Merge after retry transition: {:?}", + item.stage + ); + assert!( + matches!(item.stage, Stage::Merge { .. }), + "stage must be Stage::Merge variant: {:?}", + item.stage + ); + } + + /// AC1 (story 1051): transition_merge_failure_to_retry is a no-op when the + /// story is already in Stage::Merge — callers do not need to pre-check. + #[test] + fn transition_merge_failure_to_retry_noop_when_already_merge() { + crate::crdt_state::init_for_test(); + crate::db::ensure_content_store(); + + let story_id = "99106_story_already_merge_1051"; + crate::db::write_item_with_content( + story_id, + "4_merge", + "---\nname: Already Merge\n---\n", + crate::db::ItemMeta::named("Already Merge"), + ); + + // Must not error even though story is in Merge, not MergeFailure. + assert!( + transition_merge_failure_to_retry(story_id).is_ok(), + "must be a no-op when story is in Merge" + ); + + let item = crate::pipeline_state::read_typed(story_id) + .expect("CRDT read must succeed") + .expect("item must exist"); + assert_eq!( + item.stage.dir_name(), + "merge", + "story must still be in merge after no-op: {:?}", + item.stage + ); + } + + /// AC3 (story 1051): after a MergeRetryStarted transition (MergeFailure → + /// Merge), a subsequent MergeFailed event sets a fresh failure — the + /// transition's `before` stage is Merge (not MergeFailure), confirming the + /// failure flag is not carried over from the previous attempt. + #[test] + fn merge_retry_followed_by_failure_sets_fresh_merge_failure() { + crate::crdt_state::init_for_test(); + crate::db::ensure_content_store(); + + let story_id = "99107_story_fresh_failure_1051"; + crate::db::write_item_with_content( + story_id, + "merge_failure", + "---\nname: Fresh Failure Test\n---\n", + crate::db::ItemMeta::named("Fresh Failure Test"), + ); + + // Retry start: MergeFailure → Merge. + transition_merge_failure_to_retry(story_id).expect("retry start must succeed"); + + // Pipeline fails again: Merge → MergeFailure. + let fired = transition_to_merge_failure( + story_id, + MergeFailureKind::GatesFailed("new gate output".to_string()), + ) + .expect("transition_to_merge_failure must succeed after retry"); + + // 'before' must be Merge (fresh failure, not a self-loop from MergeFailure). + assert!( + matches!(fired.before, Stage::Merge { .. }), + "before must be Merge, proving this is a fresh failure not a self-loop: {:?}", + fired.before + ); + assert!( + matches!(fired.after, Stage::MergeFailure { .. }), + "after must be MergeFailure: {:?}", + fired.after + ); + } + /// Bug 226: feature_branch_has_unmerged_changes returns false when no /// feature branch exists. #[test] diff --git a/server/src/agents/pool/pipeline/merge/runner.rs b/server/src/agents/pool/pipeline/merge/runner.rs index e6337759..616067d5 100644 --- a/server/src/agents/pool/pipeline/merge/runner.rs +++ b/server/src/agents/pool/pipeline/merge/runner.rs @@ -100,6 +100,16 @@ impl AgentPool { } } + // Story 1051: if the story is in MergeFailure, transition it to Merge + // before starting the pipeline so the UI shows an active retry state + // rather than the stale failure indicator (AC 1, AC 4). + if let Err(e) = crate::agents::lifecycle::transition_merge_failure_to_retry(story_id) { + slog!( + "[merge] Could not transition '{story_id}' from MergeFailure to Merge: {e} \ + (merge will proceed regardless)" + ); + } + // Insert Running job into CRDT. let started_at = unix_now(); crate::crdt_state::write_merge_job( diff --git a/server/src/pipeline_state/transition.rs b/server/src/pipeline_state/transition.rs index c64812e7..07a4883f 100644 --- a/server/src/pipeline_state/transition.rs +++ b/server/src/pipeline_state/transition.rs @@ -76,6 +76,10 @@ pub enum PipelineEvent { MergeAborted, /// Story 974: user re-opens a Done story for a post-merge hotfix, sending it back to Coding. HotfixRequested, + /// Story 1051: deterministic-merge pipeline started a retry on a story in + /// `Stage::MergeFailure`; transitions back to `Stage::Merge` so the UI + /// shows the active retry rather than the stale failure state. + MergeRetryStarted, } // ── Per-node execution events ─────────────────────────────────────────────── @@ -123,6 +127,7 @@ pub fn event_label(e: &PipelineEvent) -> &'static str { PipelineEvent::ReQueuedForQa => "ReQueuedForQa", PipelineEvent::MergeAborted => "MergeAborted", PipelineEvent::HotfixRequested => "HotfixRequested", + PipelineEvent::MergeRetryStarted => "MergeRetryStarted", } } @@ -365,6 +370,26 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result Ok(Merge { + feature_branch, + commits_ahead, + claim: None, + retries: 0, + server_start_time: None, + }), + // ── MergemasterAttempted: MergeFailure → MergeFailureFinal ───── (MergeFailure { kind, .. }, MergemasterAttempted) => Ok(MergeFailureFinal { kind }), (MergeFailureFinal { kind }, MergemasterAttempted) => Ok(MergeFailureFinal { kind }),