From 541433d96e9fb199a0a3a5b6aea166f70a3eceb3 Mon Sep 17 00:00:00 2001 From: dave Date: Tue, 12 May 2026 22:42:04 +0000 Subject: [PATCH] huskies: merge 893 --- server/src/agents/lifecycle.rs | 58 ++++++++++++++++++++++++- server/src/pipeline_state/tests.rs | 28 ++++++++++++ server/src/pipeline_state/transition.rs | 13 +++++- server/src/pipeline_state/types.rs | 3 +- 4 files changed, 98 insertions(+), 4 deletions(-) diff --git a/server/src/agents/lifecycle.rs b/server/src/agents/lifecycle.rs index 67d68966..67d2d988 100644 --- a/server/src/agents/lifecycle.rs +++ b/server/src/agents/lifecycle.rs @@ -334,7 +334,9 @@ 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), + // Story 893: MergeFailure + Unblock now goes to Coding (retry), so + // manual demotion to backlog uses Demote instead. + (Stage::MergeFailure { .. }, "backlog") => Ok(PipelineEvent::Demote), ( Stage::Archived { reason: ArchiveReason::Blocked { .. }, @@ -592,6 +594,60 @@ mod tests { ); } + // ── Story 893: MergeFailure unblock → Coding regression ───────────────── + + /// Regression test (story 893): unblocking a story in `MergeFailure` via + /// `transition_to_unblocked` transitions it to `Stage::Coding`, not `Backlog`. + /// After the unblock, the auto-assigner can pick it up normally (it looks for + /// stories in `Coding` / active stages). + #[test] + fn unblock_merge_failure_story_lands_in_coding() { + crate::db::ensure_content_store(); + crate::db::write_item_with_content( + "99893_story_merge_failure_unblock", + "merge_failure", + "---\nname: MergeFailure Unblock Test\n---\n# Story\n", + crate::db::ItemMeta::named("MergeFailure Unblock Test"), + ); + + // Verify starting state is MergeFailure. + let item = crate::pipeline_state::read_typed("99893_story_merge_failure_unblock") + .expect("CRDT read should succeed") + .expect("item should exist"); + assert!( + matches!(item.stage, Stage::MergeFailure { .. }), + "should start in MergeFailure: {:?}", + item.stage + ); + + // Unblock routes through transition_to_unblocked (same path as unblock_story MCP). + transition_to_unblocked("99893_story_merge_failure_unblock") + .expect("transition_to_unblocked should succeed for MergeFailure story"); + + // Story must land in Coding, not Backlog — the auto-assigner picks up + // Coding-stage stories without an extra DepsMet promotion step. + let item = crate::pipeline_state::read_typed("99893_story_merge_failure_unblock") + .expect("CRDT read should succeed") + .expect("item should exist after unblock"); + assert_eq!( + item.stage.dir_name(), + "coding", + "MergeFailure story should land in Coding after unblock for immediate retry: {:?}", + item.stage + ); + assert!( + matches!(item.stage, Stage::Coding), + "stage should be Stage::Coding after unblock, got: {:?}", + item.stage + ); + // auto_assign checks is_active() — Coding satisfies it. + assert!( + item.stage.is_active(), + "Coding satisfies is_active() so auto_assign can pick it up: {:?}", + item.stage + ); + } + // ── feature_branch_has_unmerged_changes tests ──────────────────────────── fn init_git_repo(repo: &std::path::Path) { diff --git a/server/src/pipeline_state/tests.rs b/server/src/pipeline_state/tests.rs index add9fe1e..aa5187d1 100644 --- a/server/src/pipeline_state/tests.rs +++ b/server/src/pipeline_state/tests.rs @@ -750,6 +750,34 @@ fn repeated_merge_failure_apply_transition_no_error_no_duplicate_notification() ); } +// ── Story 893: MergeFailure + Unblock → Coding (retry) ───────────── + +/// AC1: `MergeFailure + Unblock` transitions to `Coding` (retry), not `Backlog`. +#[test] +fn merge_failure_unblock_returns_to_coding() { + let s = Stage::MergeFailure { + reason: "conflicts in server/src/main.rs".into(), + }; + let result = transition(s, PipelineEvent::Unblock).unwrap(); + assert!( + matches!(result, Stage::Coding), + "MergeFailure + Unblock should return to Coding for immediate retry, got: {result:?}" + ); +} + +/// AC1 (complement): `MergeFailure + Demote` still goes to `Backlog` for manual parking. +#[test] +fn merge_failure_demote_returns_to_backlog() { + let s = Stage::MergeFailure { + reason: "conflicts".into(), + }; + let result = transition(s, PipelineEvent::Demote).unwrap(); + assert!( + matches!(result, Stage::Backlog), + "MergeFailure + Demote should park the story in Backlog, got: {result:?}" + ); +} + // ── Story 892: MergeFailure → Done (manual recovery) ─────────────── /// Regression test (story 892): `accept_story` on a story in `MergeFailure` diff --git a/server/src/pipeline_state/transition.rs b/server/src/pipeline_state/transition.rs index 34cb340b..42a2c836 100644 --- a/server/src/pipeline_state/transition.rs +++ b/server/src/pipeline_state/transition.rs @@ -242,8 +242,17 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result Ok(Coding), - // ── Unblock MergeFailure → Backlog ─────────────────────────────── - (MergeFailure { .. }, Unblock) => Ok(Backlog), + // ── Unblock MergeFailure → Coding (retry) ──────────────────────── + // `unblock_story` on a failed merge re-enters the coding stage so a + // coder agent can rework the branch, rather than routing back through + // the backlog (which would require an extra DepsMet promotion step). + (MergeFailure { .. }, Unblock) => Ok(Coding), + + // ── Demote MergeFailure → Backlog (manual parking) ─────────────── + // Lets operators park a failed-merge story in the backlog without an + // agent retry. Complements `Unblock` (→ Coding) which triggers an + // immediate retry by a coder agent. + (MergeFailure { .. }, Demote) => Ok(Backlog), // ── Legacy unblock: Archived(Blocked|MergeFailed) → Backlog ── ( diff --git a/server/src/pipeline_state/types.rs b/server/src/pipeline_state/types.rs index ee838f4d..427ec071 100644 --- a/server/src/pipeline_state/types.rs +++ b/server/src/pipeline_state/types.rs @@ -108,7 +108,8 @@ pub enum Stage { /// 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`. + /// this is a recoverable intermediate state — `Unblock` returns to `Coding` + /// (immediate agent retry) and `Demote` returns to `Backlog` (manual park). MergeFailure { reason: String }, }