diff --git a/server/src/agent_mode/loop_ops.rs b/server/src/agent_mode/loop_ops.rs index b7595072..6e18e005 100644 --- a/server/src/agent_mode/loop_ops.rs +++ b/server/src/agent_mode/loop_ops.rs @@ -52,7 +52,16 @@ pub(super) async fn scan_and_claim( } // Skip blocked stories (story 945: `Stage::Blocked` is the source of truth). - if item.stage().is_blocked() { + if matches!( + item.stage(), + crate::pipeline_state::Stage::Blocked { .. } + | crate::pipeline_state::Stage::MergeFailure { .. } + | crate::pipeline_state::Stage::MergeFailureFinal { .. } + | crate::pipeline_state::Stage::Archived { + reason: crate::pipeline_state::ArchiveReason::Blocked { .. }, + .. + } + ) { continue; } diff --git a/server/src/agents/lifecycle.rs b/server/src/agents/lifecycle.rs index ca054704..93a08a49 100644 --- a/server/src/agents/lifecycle.rs +++ b/server/src/agents/lifecycle.rs @@ -679,7 +679,19 @@ mod tests { "blocked", "should be in blocked after transition_to_blocked" ); - assert!(item.stage.is_blocked(), "is_blocked() should return true"); + assert!( + matches!( + item.stage, + Stage::Blocked { .. } + | Stage::MergeFailure { .. } + | Stage::MergeFailureFinal { .. } + | Stage::Archived { + reason: ArchiveReason::Blocked { .. }, + .. + } + ), + "stage should be a blocked variant" + ); assert!( matches!(item.stage, Stage::Blocked { .. }), "stage should be Stage::Blocked variant" diff --git a/server/src/agents/pool/auto_assign/story_checks.rs b/server/src/agents/pool/auto_assign/story_checks.rs index 0b912b74..ed57afd8 100644 --- a/server/src/agents/pool/auto_assign/story_checks.rs +++ b/server/src/agents/pool/auto_assign/story_checks.rs @@ -22,7 +22,7 @@ pub(super) fn read_story_front_matter_agent(story_id: &str) -> Option { /// `tool_approve_qa`). pub(super) fn has_review_hold(story_id: &str) -> bool { crate::crdt_state::read_item(story_id) - .map(|w| w.stage().is_review_hold()) + .map(|w| matches!(w.stage(), crate::pipeline_state::Stage::ReviewHold { .. })) .unwrap_or(false) } @@ -35,7 +35,18 @@ pub(super) fn is_story_blocked(story_id: &str) -> bool { crate::pipeline_state::read_typed(story_id) .ok() .flatten() - .map(|item| item.stage.is_blocked()) + .map(|item| { + matches!( + item.stage, + crate::pipeline_state::Stage::Blocked { .. } + | crate::pipeline_state::Stage::MergeFailure { .. } + | crate::pipeline_state::Stage::MergeFailureFinal { .. } + | crate::pipeline_state::Stage::Archived { + reason: crate::pipeline_state::ArchiveReason::Blocked { .. }, + .. + } + ) + }) .unwrap_or(false) } @@ -70,7 +81,12 @@ pub(super) fn has_content_conflict_failure(story_id: &str) -> bool { /// the same story after a failed mergemaster session. pub(super) fn has_mergemaster_attempted(story_id: &str) -> bool { crate::crdt_state::read_item(story_id) - .map(|view| view.stage().is_mergemaster_attempted()) + .map(|view| { + matches!( + view.stage(), + crate::pipeline_state::Stage::MergeFailureFinal { .. } + ) + }) .unwrap_or(false) } @@ -95,7 +111,7 @@ pub(super) fn check_archived_dependencies(story_id: &str) -> Vec { /// `resume_to`. pub(super) fn is_story_frozen(story_id: &str) -> bool { crate::crdt_state::read_item(story_id) - .map(|view| view.stage().is_frozen()) + .map(|view| matches!(view.stage(), crate::pipeline_state::Stage::Frozen { .. })) .unwrap_or(false) } diff --git a/server/src/chat/commands/freeze.rs b/server/src/chat/commands/freeze.rs index 0e561a43..02b801be 100644 --- a/server/src/chat/commands/freeze.rs +++ b/server/src/chat/commands/freeze.rs @@ -205,7 +205,7 @@ mod tests { .expect("read_typed should succeed") .expect("item should be present"); assert!( - item.is_frozen(), + matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }), "stage should be Frozen after freeze: {:?}", item.stage ); diff --git a/server/src/chat/commands/status/render.rs b/server/src/chat/commands/status/render.rs index 55ea8b77..1800b5a6 100644 --- a/server/src/chat/commands/status/render.rs +++ b/server/src/chat/commands/status/render.rs @@ -218,7 +218,7 @@ fn render_item_line( Some(item.name.as_str()) }; // Use the typed CRDT stage as the sole source of truth (story 945). - let frozen = item.stage.is_frozen(); + let frozen = matches!(item.stage, Stage::Frozen { .. }); let base_label = super::story_short_label(story_id, name_opt); let display = if frozen { format!("\u{2744}\u{FE0F} {base_label}") // ❄️ prefix @@ -310,7 +310,16 @@ fn render_item_line( } } - let blocked = item.stage.is_blocked(); + let blocked = matches!( + item.stage, + Stage::Blocked { .. } + | Stage::MergeFailure { .. } + | Stage::MergeFailureFinal { .. } + | Stage::Archived { + reason: ArchiveReason::Blocked { .. }, + .. + } + ); // Blocked items with a recovery agent get differentiated indicators. if blocked { return match agent.map(|a| &a.status) { diff --git a/server/src/chat/commands/status/tests.rs b/server/src/chat/commands/status/tests.rs index 285cdbfe..5b7632e6 100644 --- a/server/src/chat/commands/status/tests.rs +++ b/server/src/chat/commands/status/tests.rs @@ -362,12 +362,30 @@ fn stage_is_blocked_returns_true_for_archived_blocked() { reason: "too many retries".to_string(), }, }; - assert!(stage.is_blocked()); + assert!(matches!( + stage, + Stage::Blocked { .. } + | Stage::MergeFailure { .. } + | Stage::MergeFailureFinal { .. } + | Stage::Archived { + reason: ArchiveReason::Blocked { .. }, + .. + } + )); } #[test] fn stage_is_blocked_returns_false_for_coding() { - assert!(!Stage::Coding.is_blocked()); + assert!(!matches!( + Stage::Coding, + Stage::Blocked { .. } + | Stage::MergeFailure { .. } + | Stage::MergeFailureFinal { .. } + | Stage::Archived { + reason: ArchiveReason::Blocked { .. }, + .. + } + )); } #[test] @@ -376,7 +394,16 @@ fn stage_is_blocked_returns_false_for_archived_completed() { archived_at: Utc::now(), reason: ArchiveReason::Completed, }; - assert!(!stage.is_blocked()); + assert!(!matches!( + stage, + Stage::Blocked { .. } + | Stage::MergeFailure { .. } + | Stage::MergeFailureFinal { .. } + | Stage::Archived { + reason: ArchiveReason::Blocked { .. }, + .. + } + )); } // -- status output shows idle dot for items with no active agent -------- diff --git a/server/src/chat/commands/triage.rs b/server/src/chat/commands/triage.rs index 903cdb0e..52a6b124 100644 --- a/server/src/chat/commands/triage.rs +++ b/server/src/chat/commands/triage.rs @@ -86,7 +86,16 @@ fn build_triage_dump( if let Some(ref w) = crdt_item { let mut fields: Vec = Vec::new(); // Story 945: `Stage::Blocked` is the source of truth. - if w.stage().is_blocked() { + if matches!( + w.stage(), + crate::pipeline_state::Stage::Blocked { .. } + | crate::pipeline_state::Stage::MergeFailure { .. } + | crate::pipeline_state::Stage::MergeFailureFinal { .. } + | crate::pipeline_state::Stage::Archived { + reason: crate::pipeline_state::ArchiveReason::Blocked { .. }, + .. + } + ) { fields.push("**blocked:** true".to_string()); } if let Some(agent) = w.agent() { diff --git a/server/src/chat/commands/unblock.rs b/server/src/chat/commands/unblock.rs index 0ad04796..b8940a4c 100644 --- a/server/src/chat/commands/unblock.rs +++ b/server/src/chat/commands/unblock.rs @@ -61,9 +61,18 @@ fn unblock_by_story_id(story_id: &str) -> String { // Story 945: `Stage::Blocked` / `Stage::MergeFailure` are the single // source of truth — the legacy `blocked` boolean flag is gone. let typed_item = crate::pipeline_state::read_typed(story_id).ok().flatten(); - let typed_blocked = typed_item - .as_ref() - .is_some_and(|item| item.stage.is_blocked()); + let typed_blocked = typed_item.as_ref().is_some_and(|item| { + matches!( + item.stage, + crate::pipeline_state::Stage::Blocked { .. } + | crate::pipeline_state::Stage::MergeFailure { .. } + | crate::pipeline_state::Stage::MergeFailureFinal { .. } + | crate::pipeline_state::Stage::Archived { + reason: crate::pipeline_state::ArchiveReason::Blocked { .. }, + .. + } + ) + }); let typed_merge_failure = matches!( typed_item.as_ref().map(|i| &i.stage), Some(crate::pipeline_state::Stage::MergeFailure { .. }) @@ -239,7 +248,16 @@ mod tests { // Story 945: `Stage::Blocked` was the source of truth; after unblock // the stage must have transitioned out of `Blocked`. assert!( - !item.stage().is_blocked(), + !matches!( + item.stage(), + crate::pipeline_state::Stage::Blocked { .. } + | crate::pipeline_state::Stage::MergeFailure { .. } + | crate::pipeline_state::Stage::MergeFailureFinal { .. } + | crate::pipeline_state::Stage::Archived { + reason: crate::pipeline_state::ArchiveReason::Blocked { .. }, + .. + } + ), "stage must no longer be Blocked after unblock" ); } @@ -307,7 +325,16 @@ mod tests { // Story 945: `Stage::Blocked` was the source of truth; after unblock // the stage must have transitioned out of `Blocked`. assert!( - !item.stage().is_blocked(), + !matches!( + item.stage(), + crate::pipeline_state::Stage::Blocked { .. } + | crate::pipeline_state::Stage::MergeFailure { .. } + | crate::pipeline_state::Stage::MergeFailureFinal { .. } + | crate::pipeline_state::Stage::Archived { + reason: crate::pipeline_state::ArchiveReason::Blocked { .. }, + .. + } + ), "stage must no longer be Blocked after unblock" ); } diff --git a/server/src/http/mcp/story_tools/story/freeze.rs b/server/src/http/mcp/story_tools/story/freeze.rs index db295f06..a7ef5eba 100644 --- a/server/src/http/mcp/story_tools/story/freeze.rs +++ b/server/src/http/mcp/story_tools/story/freeze.rs @@ -75,7 +75,10 @@ mod tests { let item = crate::pipeline_state::read_typed(story_id) .expect("read_typed should succeed") .expect("item should be present"); - assert!(item.is_frozen(), "stage should be frozen after MCP freeze"); + assert!( + matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }), + "stage should be frozen after MCP freeze" + ); } #[test] @@ -106,7 +109,7 @@ mod tests { .expect("read_typed should succeed") .expect("item should be present"); assert!( - !item.is_frozen(), + !matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }), "stage should not be frozen after MCP unfreeze" ); } diff --git a/server/src/http/workflow/pipeline.rs b/server/src/http/workflow/pipeline.rs index fe8e2ab9..6a8c1ade 100644 --- a/server/src/http/workflow/pipeline.rs +++ b/server/src/http/workflow/pipeline.rs @@ -115,7 +115,7 @@ pub fn load_pipeline_state(ctx: &AppContext) -> Result { // come from typed CRDT registers. merge_failure detail lives on the // MergeJob CRDT entry (same as status_tools.rs). let view = crate::crdt_state::read_item(sid); - let review_hold = if item.stage.is_review_hold() { + let review_hold = if matches!(item.stage, Stage::ReviewHold { .. }) { Some(true) } else { None @@ -137,12 +137,21 @@ pub fn load_pipeline_state(ctx: &AppContext) -> Result { } else { None }, - blocked: if item.stage.is_blocked() { + blocked: if matches!( + item.stage, + Stage::Blocked { .. } + | Stage::MergeFailure { .. } + | Stage::MergeFailureFinal { .. } + | Stage::Archived { + reason: crate::pipeline_state::ArchiveReason::Blocked { .. }, + .. + } + ) { Some(true) } else { None }, - frozen: if item.stage.is_frozen() { + frozen: if matches!(item.stage, Stage::Frozen { .. }) { Some(true) } else { None @@ -262,12 +271,21 @@ pub fn load_upcoming_stories(_ctx: &AppContext) -> Result, St } else { None }, - blocked: if item.stage.is_blocked() { + blocked: if matches!( + item.stage, + Stage::Blocked { .. } + | Stage::MergeFailure { .. } + | Stage::MergeFailureFinal { .. } + | Stage::Archived { + reason: crate::pipeline_state::ArchiveReason::Blocked { .. }, + .. + } + ) { Some(true) } else { None }, - frozen: if item.stage.is_frozen() { + frozen: if matches!(item.stage, Stage::Frozen { .. }) { Some(true) } else { None diff --git a/server/src/io/story_metadata/parser.rs b/server/src/io/story_metadata/parser.rs index e9bffa86..d3ec0b4b 100644 --- a/server/src/io/story_metadata/parser.rs +++ b/server/src/io/story_metadata/parser.rs @@ -36,7 +36,7 @@ pub fn is_story_frozen_in_store(story_id: &str) -> bool { crate::pipeline_state::read_typed(story_id) .ok() .flatten() - .map(|item| item.is_frozen()) + .map(|item| matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. })) .unwrap_or(false) } diff --git a/server/src/pipeline_state/projection.rs b/server/src/pipeline_state/projection.rs index 5264d5f8..d5b20dc1 100644 --- a/server/src/pipeline_state/projection.rs +++ b/server/src/pipeline_state/projection.rs @@ -275,7 +275,7 @@ mod tests { Some("Frozen Story"), ); let item = PipelineItem::try_from(&view).unwrap(); - assert!(item.is_frozen()); + assert!(matches!(item.stage, Stage::Frozen { .. })); } // ── Event bus tests ───────────────────────────────────────────────── diff --git a/server/src/pipeline_state/tests.rs b/server/src/pipeline_state/tests.rs index 56e35a7d..956cb359 100644 --- a/server/src/pipeline_state/tests.rs +++ b/server/src/pipeline_state/tests.rs @@ -562,7 +562,7 @@ fn freeze_transitions_to_frozen_variant_with_resume_to() { let item = read_typed(story_id).unwrap().unwrap(); assert!(matches!(item.stage, Stage::Coding)); - assert!(!item.is_frozen()); + assert!(!matches!(item.stage, Stage::Frozen { .. })); super::apply::transition_to_frozen(story_id).expect("freeze should succeed"); @@ -574,7 +574,10 @@ fn freeze_transitions_to_frozen_variant_with_resume_to() { ), other => panic!("stage should be Stage::Frozen after freeze; got {other:?}"), } - assert!(item.is_frozen(), "is_frozen() should be true after freeze"); + assert!( + matches!(item.stage, Stage::Frozen { .. }), + "is_frozen() should be true after freeze" + ); super::apply::transition_to_unfrozen(story_id).expect("unfreeze should succeed"); @@ -585,7 +588,7 @@ fn freeze_transitions_to_frozen_variant_with_resume_to() { item.stage ); assert!( - !item.is_frozen(), + !matches!(item.stage, Stage::Frozen { .. }), "is_frozen() should be false after unfreeze" ); } diff --git a/server/src/pipeline_state/types.rs b/server/src/pipeline_state/types.rs index a25ab058..b50ea0cb 100644 --- a/server/src/pipeline_state/types.rs +++ b/server/src/pipeline_state/types.rs @@ -262,43 +262,6 @@ impl Stage { stage_dir_name(self) } - /// Returns true if this is the `Blocked`, `MergeFailure`, or - /// `MergeFailureFinal` variant (or the legacy `Archived(Blocked)` for - /// backward-compatible reads). - pub fn is_blocked(&self) -> bool { - matches!( - self, - Stage::Blocked { .. } - | Stage::MergeFailure { .. } - | Stage::MergeFailureFinal { .. } - | Stage::Archived { - reason: ArchiveReason::Blocked { .. }, - .. - } - ) - } - - /// Returns true if this is the `Frozen` variant. Story 945: replaces - /// the legacy `frozen: true` boolean flag. The auto-assigner skips - /// frozen stories. - pub fn is_frozen(&self) -> bool { - matches!(self, Stage::Frozen { .. }) - } - - /// Returns true if this is the `ReviewHold` variant. Story 945: - /// replaces the legacy `review_hold: true` boolean flag. The - /// auto-assigner skips review-held stories. - pub fn is_review_hold(&self) -> bool { - matches!(self, Stage::ReviewHold { .. }) - } - - /// Returns true if mergemaster has already been auto-spawned for this - /// story (`MergeFailureFinal`). Story 945: replaces the legacy - /// `mergemaster_attempted: true` boolean flag. - pub fn is_mergemaster_attempted(&self) -> bool { - matches!(self, Stage::MergeFailureFinal { .. }) - } - /// Parse a stage from its filesystem directory name. /// /// This is the single canonical conversion boundary for turning a loose @@ -401,14 +364,6 @@ pub struct PipelineItem { pub retry_count: u32, } -impl PipelineItem { - /// Whether the item is frozen — story 945: `Stage::Frozen` is now the - /// single source of truth, replacing the legacy boolean flag. - pub fn is_frozen(&self) -> bool { - self.stage.is_frozen() - } -} - // ── Transition errors ─────────────────────────────────────────────────────── /// Error returned when a pipeline event is not valid for the current stage. diff --git a/server/src/service/work_item/freeze.rs b/server/src/service/work_item/freeze.rs index 8f13c697..a4aa9872 100644 --- a/server/src/service/work_item/freeze.rs +++ b/server/src/service/work_item/freeze.rs @@ -31,7 +31,7 @@ pub fn freeze(story_id: &str) -> Result { let already_frozen = crate::pipeline_state::read_typed(story_id) .ok() .flatten() - .map(|item| item.is_frozen()) + .map(|item| matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. })) .unwrap_or(false); if already_frozen { @@ -51,7 +51,7 @@ pub fn unfreeze(story_id: &str) -> Result { let is_frozen = crate::pipeline_state::read_typed(story_id) .ok() .flatten() - .map(|item| item.is_frozen()) + .map(|item| matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. })) .unwrap_or(false); if !is_frozen { @@ -92,7 +92,7 @@ mod tests { .expect("read_typed should succeed") .expect("item should be present"); assert!( - item.is_frozen(), + matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }), "stage should be Frozen after freeze: {:?}", item.stage ); @@ -141,7 +141,7 @@ mod tests { .expect("read_typed should succeed") .expect("item should be present"); assert!( - !item.is_frozen(), + !matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }), "stage should not be Frozen after unfreeze: {:?}", item.stage ); @@ -212,12 +212,12 @@ mod tests { .expect("MCP-path item should be in CRDT"); assert!( - state_a.is_frozen(), + matches!(state_a.stage, crate::pipeline_state::Stage::Frozen { .. }), "chat-path CRDT stage must be frozen: {:?}", state_a.stage ); assert!( - state_b.is_frozen(), + matches!(state_b.stage, crate::pipeline_state::Stage::Frozen { .. }), "MCP-path CRDT stage must be frozen: {:?}", state_b.stage );