diff --git a/server/src/agents/lifecycle.rs b/server/src/agents/lifecycle.rs index 0bd173fb..dbe4162f 100644 --- a/server/src/agents/lifecycle.rs +++ b/server/src/agents/lifecycle.rs @@ -230,6 +230,45 @@ pub fn reject_story_from_qa(story_id: &str, notes: &str) -> Result<(), String> { .map_err(|e| e.to_string()) } +/// Transition a story to the `Blocked` stage via the state machine. +/// +/// Builds a `PipelineEvent::Block { reason }`, validates the transition, and +/// writes the resulting `Stage::Blocked` to the CRDT. Returns `Err` on +/// `TransitionError` — callers must NOT fall back to direct register writes. +pub fn transition_to_blocked(story_id: &str, reason: &str) -> Result<(), String> { + let transform = fields_to_clear_transform(&["blocked"]); + apply_transition( + story_id, + PipelineEvent::Block { + reason: reason.to_string(), + }, + transform.as_ref().map(|f| f.as_ref()), + ) + .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 +/// resulting `Stage::Coding` to the CRDT, and resets `retry_count` to 0. +/// Returns `Err` on `TransitionError` — callers must NOT fall back to direct +/// register writes. +pub fn transition_to_unblocked(story_id: &str) -> Result<(), String> { + let transform = fields_to_clear_transform(&["blocked", "merge_failure", "retry_count"]); + apply_transition( + story_id, + PipelineEvent::Unblock, + transform.as_ref().map(|f| f.as_ref()), + ) + .map(|_| ()) + .map_err(|e| e.to_string())?; + + // Reset the retry counter in the CRDT so the story gets a fresh budget. + crate::crdt_state::set_retry_count(story_id, 0); + Ok(()) +} + /// Map a (current stage, target stage name) pair to the appropriate PipelineEvent. fn map_stage_move_to_event( from: &Stage, @@ -261,6 +300,7 @@ fn map_stage_move_to_event( merge_commit: GitSha("manual".to_string()), }), (Stage::Coding | Stage::Qa | Stage::Backlog, "done") => Ok(PipelineEvent::Close), + (Stage::Blocked { .. }, "current") => Ok(PipelineEvent::Unblock), ( Stage::Archived { reason: ArchiveReason::Blocked { .. }, @@ -345,6 +385,7 @@ fn stage_to_name(s: &Stage) -> &'static str { Stage::Upcoming => "upcoming", Stage::Backlog => "backlog", Stage::Coding => "current", + Stage::Blocked { .. } => "blocked", Stage::Qa => "qa", Stage::Merge { .. } => "merge", Stage::Done { .. } => "done", @@ -452,6 +493,68 @@ mod tests { assert_eq!(item_type_from_id("99999"), "story"); } + // ── Story 866: block/unblock round-trip regression test ────────────────── + + /// Regression test (story 866): block a story via the new state-machine path, + /// verify it lands in `Stage::Blocked`, then unblock and verify it returns + /// to `Stage::Coding`. + #[test] + fn block_unblock_round_trip_via_state_machine() { + crate::db::ensure_content_store(); + crate::db::write_item_with_content( + "99866_story_block_test", + "2_current", + "---\nname: Block Round Trip\n---\n# Story\n", + ); + + // Verify starting state is Coding. + let item = crate::pipeline_state::read_typed("99866_story_block_test") + .expect("read should succeed") + .expect("item should exist"); + assert_eq!( + item.stage.dir_name(), + "2_current", + "should start in 2_current" + ); + + // Block via the state machine. + transition_to_blocked("99866_story_block_test", "retry limit exceeded") + .expect("transition_to_blocked should succeed"); + + // Verify the CRDT now shows Stage::Blocked. + let item = crate::pipeline_state::read_typed("99866_story_block_test") + .expect("read should succeed") + .expect("item should exist after block"); + assert_eq!( + item.stage.dir_name(), + "2_blocked", + "should be in 2_blocked after transition_to_blocked" + ); + assert!(item.stage.is_blocked(), "is_blocked() should return true"); + assert!( + matches!(item.stage, Stage::Blocked { .. }), + "stage should be Stage::Blocked variant" + ); + + // Unblock via the state machine. + transition_to_unblocked("99866_story_block_test") + .expect("transition_to_unblocked should succeed"); + + // Verify the story returned to Coding. + let item = crate::pipeline_state::read_typed("99866_story_block_test") + .expect("read should succeed") + .expect("item should exist after unblock"); + assert_eq!( + item.stage.dir_name(), + "2_current", + "should return to 2_current after unblock" + ); + assert!( + matches!(item.stage, Stage::Coding), + "stage should be Stage::Coding after unblock" + ); + } + // ── feature_branch_has_unmerged_changes tests ──────────────────────────── fn init_git_repo(repo: &std::path::Path) { diff --git a/server/src/agents/pool/auto_assign/merge.rs b/server/src/agents/pool/auto_assign/merge.rs index 98fadb98..9f508437 100644 --- a/server/src/agents/pool/auto_assign/merge.rs +++ b/server/src/agents/pool/auto_assign/merge.rs @@ -119,34 +119,21 @@ impl AgentPool { } // AC6: Detect empty-diff stories before starting the merge pipeline. - // If the worktree has no commits on the feature branch, write a - // merge_failure and block the story immediately — no merge job needed. + // If the worktree has no commits on the feature branch, block the + // story immediately via the state machine — no merge job needed. if let Some(wt_path) = worktree::find_worktree_path(project_root, story_id) && !crate::agents::gates::worktree_has_committed_work(&wt_path) { - slog_warn!( - "[auto-assign] Story '{story_id}' in 4_merge/ has no commits \ - on feature branch. Writing merge_failure and blocking." - ); let empty_diff_reason = "Feature branch has no code changes — the coder agent \ did not produce any commits."; - if let Some(contents) = crate::db::read_content(story_id) { - let updated = crate::io::story_metadata::write_merge_failure_in_content( - &contents, - empty_diff_reason, - ); - let blocked = crate::io::story_metadata::write_blocked_in_content(&updated); - crate::db::write_content(story_id, &blocked); - crate::db::write_item_with_content(story_id, "4_merge", &blocked); - } else { - let story_path = project_root - .join(".huskies/work/4_merge") - .join(format!("{story_id}.md")); - let _ = crate::io::story_metadata::write_merge_failure( - &story_path, - empty_diff_reason, - ); - let _ = crate::io::story_metadata::write_blocked(&story_path); + slog_warn!( + "[auto-assign] Story '{story_id}' in 4_merge/ has no commits \ + on feature branch. Blocking via state machine." + ); + if let Err(e) = + crate::agents::lifecycle::transition_to_blocked(story_id, empty_diff_reason) + { + slog_error!("[auto-assign] Failed to transition '{story_id}' to Blocked: {e}"); } let _ = self .watcher_tx diff --git a/server/src/agents/pool/auto_assign/story_checks.rs b/server/src/agents/pool/auto_assign/story_checks.rs index b5a4270a..151cc9ed 100644 --- a/server/src/agents/pool/auto_assign/story_checks.rs +++ b/server/src/agents/pool/auto_assign/story_checks.rs @@ -34,8 +34,16 @@ pub(super) fn has_review_hold(project_root: &Path, _stage_dir: &str, story_id: & .unwrap_or(false) } -/// Return `true` if the story file has `blocked: true` in its front matter. +/// Return `true` if the story is blocked — either via the typed `Stage::Blocked` +/// variant or the legacy `blocked: true` front-matter field. pub(super) fn is_story_blocked(project_root: &Path, _stage_dir: &str, story_id: &str) -> bool { + // Check the typed stage first (authoritative after story 866). + if let Ok(Some(item)) = crate::pipeline_state::read_typed(story_id) + && item.stage.is_blocked() + { + return true; + } + // Legacy fallback: check front-matter field for backward compatibility. use crate::io::story_metadata::parse_front_matter; let contents = match read_story_contents(project_root, story_id) { Some(c) => c, diff --git a/server/src/agents/pool/auto_assign/watchdog/tests/limits_tests.rs b/server/src/agents/pool/auto_assign/watchdog/tests/limits_tests.rs index 55e776cc..b3aa0092 100644 --- a/server/src/agents/pool/auto_assign/watchdog/tests/limits_tests.rs +++ b/server/src/agents/pool/auto_assign/watchdog/tests/limits_tests.rs @@ -267,12 +267,13 @@ max_turns = 10 let found = pool.run_watchdog_pass(Some(root)); assert!(found >= 1, "watchdog should detect the over-limit agent"); - // With max_retries=1, the first violation blocks immediately. - let updated = crate::db::read_content(story_id) - .expect("story content must still exist after watchdog termination"); - assert!( - updated.contains("blocked: true"), - "story must be marked `blocked: true` after limit termination with max_retries=1 — got:\n{updated}" + // With max_retries=1, the first violation blocks immediately via the state machine. + let item = crate::crdt_state::read_item(story_id) + .expect("story must be in CRDT after watchdog termination"); + assert_eq!( + item.stage, "2_blocked", + "story stage must be 2_blocked after limit termination with max_retries=1 — got: {}", + item.stage ); // Sanity: the agent itself is also Failed with the right reason. @@ -407,11 +408,12 @@ max_turns = 10 let event = rx.try_recv().expect("watchdog must emit an Error event"); assert!(matches!(event, AgentEvent::Error { .. })); - // With max_retries=1, the story is blocked. - let updated = crate::db::read_content(story_id).unwrap(); - assert!( - updated.contains("blocked: true"), - "story must be blocked after per-session overrun with max_retries=1" + // With max_retries=1, the story is blocked via the state machine. + let item = crate::crdt_state::read_item(story_id) + .expect("story must be in CRDT after per-session overrun"); + assert_eq!( + item.stage, "2_blocked", + "story stage must be 2_blocked after per-session overrun with max_retries=1" ); } @@ -470,9 +472,9 @@ max_turns = 10 Some(1), "after session 1, retry_count should be 1 in CRDT" ); - let content = crate::db::read_content(story_id).unwrap(); - assert!( - !content.contains("blocked: true"), + assert_ne!( + item.stage.as_str(), + "2_blocked", "story should NOT be blocked after session 1" ); } @@ -490,9 +492,9 @@ max_turns = 10 Some(2), "after session 2, retry_count should be 2 in CRDT" ); - let content = crate::db::read_content(story_id).unwrap(); - assert!( - !content.contains("blocked: true"), + assert_ne!( + item.stage.as_str(), + "2_blocked", "story should NOT be blocked after session 2" ); } @@ -504,16 +506,18 @@ max_turns = 10 pool.inject_test_agent_with_session(story_id, "coder-1", AgentStatus::Running, "session-3"); pool.run_watchdog_pass(Some(root)); - let content = crate::db::read_content(story_id).unwrap(); - assert!( - content.contains("blocked: true"), - "story must be blocked after session 3 (retry_count=3 >= max_retries=3) — got:\n{content}" - ); let item = crate::crdt_state::read_item(story_id).expect("story must be in CRDT"); + assert_eq!( + item.stage, "2_blocked", + "story must be blocked after session 3 (retry_count=3 >= max_retries=3) — got: {}", + item.stage + ); + // retry_count resets to 0 on stage transition (Bug 780) — the fact + // that the story reached 2_blocked proves the retry limit was hit. assert_eq!( item.retry_count, - Some(3), - "retry_count should be 3 in CRDT after session 3" + Some(0), + "retry_count should reset to 0 after stage transition to blocked" ); } } diff --git a/server/src/agents/pool/pipeline/advance/helpers.rs b/server/src/agents/pool/pipeline/advance/helpers.rs index 9f65d223..e0c0933b 100644 --- a/server/src/agents/pool/pipeline/advance/helpers.rs +++ b/server/src/agents/pool/pipeline/advance/helpers.rs @@ -104,8 +104,6 @@ pub(crate) fn should_block_story( max_retries: u32, stage_label: &str, ) -> Option { - use crate::io::story_metadata::write_blocked_in_content; - if max_retries == 0 { return None; } @@ -119,23 +117,16 @@ pub(crate) fn should_block_story( } if new_count >= max_retries { + let reason = + format!("Retry limit exceeded ({new_count}/{max_retries}) at {stage_label} stage"); slog_warn!( "[pipeline] Story '{story_id}' reached retry limit ({new_count}/{max_retries}) \ at {stage_label} stage. Marking as blocked." ); - if let Some(contents) = crate::db::read_content(story_id) { - let blocked = write_blocked_in_content(&contents); - crate::db::write_content(story_id, &blocked); - let stage = crate::pipeline_state::read_typed(story_id) - .ok() - .flatten() - .map(|i| i.stage.dir_name().to_string()) - .unwrap_or_else(|| "2_current".to_string()); - crate::db::write_item_with_content(story_id, &stage, &blocked); + if let Err(e) = crate::agents::lifecycle::transition_to_blocked(story_id, &reason) { + slog_error!("[pipeline] Failed to transition '{story_id}' to Blocked: {e}"); } - Some(format!( - "Retry limit exceeded ({new_count}/{max_retries}) at {stage_label} stage" - )) + Some(reason) } else { slog!( "[pipeline] Story '{story_id}' retry {new_count}/{max_retries} at {stage_label} stage." diff --git a/server/src/agents/pool/pipeline/merge/runner.rs b/server/src/agents/pool/pipeline/merge/runner.rs index 0b847472..d13dfffa 100644 --- a/server/src/agents/pool/pipeline/merge/runner.rs +++ b/server/src/agents/pool/pipeline/merge/runner.rs @@ -114,19 +114,20 @@ impl AgentPool { Err(e) => e.clone(), }; let is_no_commits = reason.contains("no commits to merge"); - if let Some(contents) = crate::db::read_content(&sid) { - let with_failure = crate::io::story_metadata::write_merge_failure_in_content( - &contents, &reason, - ); - let updated = if is_no_commits { - crate::io::story_metadata::write_blocked_in_content(&with_failure) - } else { - with_failure - }; - crate::db::write_content(&sid, &updated); - crate::db::write_item_with_content(&sid, "4_merge", &updated); + 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}"); + } let _ = pool .watcher_tx .send(crate::io::watcher::WatcherEvent::StoryBlocked { diff --git a/server/src/chat/commands/unblock.rs b/server/src/chat/commands/unblock.rs index 8ff9a34c..58839383 100644 --- a/server/src/chat/commands/unblock.rs +++ b/server/src/chat/commands/unblock.rs @@ -44,8 +44,13 @@ pub(crate) fn unblock_by_number(project_root: &Path, story_number: &str) -> Stri unblock_by_story_id(&story_id) } -/// Unblock a story using the content store (DB-backed). +/// Unblock a story via the typed state machine. +/// +/// Checks whether the story is in `Stage::Blocked` (new path) or has legacy +/// `blocked: true` / `merge_failure` front-matter, then routes through +/// [`crate::agents::lifecycle::transition_to_unblocked`]. fn unblock_by_story_id(story_id: &str) -> String { + // Read content for the story name and legacy field checks. let contents = match crate::db::read_content(story_id) { Some(c) => c, None => return format!("Failed to read story content for **{story_id}**"), @@ -57,34 +62,51 @@ fn unblock_by_story_id(story_id: &str) -> String { }; let story_name = meta.name.as_deref().unwrap_or(story_id).to_string(); + + // Check if the story is blocked via the typed stage or legacy front-matter. + let typed_blocked = crate::pipeline_state::read_typed(story_id) + .ok() + .flatten() + .is_some_and(|item| item.stage.is_blocked()); let has_blocked = meta.blocked == Some(true); let has_merge_failure = meta.merge_failure.is_some(); - if !has_blocked && !has_merge_failure { + if !typed_blocked && !has_blocked && !has_merge_failure { return format!("**{story_name}** ({story_id}) is not blocked. Nothing to unblock."); } - let mut updated = contents; - if has_blocked { - updated = clear_front_matter_field_in_content(&updated, "blocked"); - } - if has_merge_failure { - updated = clear_front_matter_field_in_content(&updated, "merge_failure"); - } - // retry_count lives in the CRDT; clear any stale copy from front-matter. - updated = clear_front_matter_field_in_content(&updated, "retry_count"); + // Route through the state machine. + match crate::agents::lifecycle::transition_to_unblocked(story_id) { + Ok(()) => {} + Err(e) => { + // If the typed transition fails (e.g. legacy Archived item), + // fall back to clearing front-matter fields directly. + crate::slog_warn!( + "[unblock] State-machine transition failed for '{story_id}': {e}. \ + Falling back to front-matter cleanup." + ); + let mut updated = contents; + if has_blocked { + updated = clear_front_matter_field_in_content(&updated, "blocked"); + } + if has_merge_failure { + updated = clear_front_matter_field_in_content(&updated, "merge_failure"); + } + updated = clear_front_matter_field_in_content(&updated, "retry_count"); - crate::db::write_content(story_id, &updated); - let stage = crate::pipeline_state::read_typed(story_id) - .ok() - .flatten() - .map(|i| i.stage.dir_name().to_string()) - .unwrap_or_else(|| "2_current".to_string()); - crate::db::write_item_with_content(story_id, &stage, &updated); - crate::crdt_state::set_retry_count(story_id, 0); + crate::db::write_content(story_id, &updated); + let stage = crate::pipeline_state::read_typed(story_id) + .ok() + .flatten() + .map(|i| i.stage.dir_name().to_string()) + .unwrap_or_else(|| "2_current".to_string()); + crate::db::write_item_with_content(story_id, &stage, &updated); + crate::crdt_state::set_retry_count(story_id, 0); + } + } let mut cleared = Vec::new(); - if has_blocked { + if typed_blocked || has_blocked { cleared.push("blocked"); } if has_merge_failure { diff --git a/server/src/chat/transport/matrix/delete.rs b/server/src/chat/transport/matrix/delete.rs index 3778b68e..a3d316e9 100644 --- a/server/src/chat/transport/matrix/delete.rs +++ b/server/src/chat/transport/matrix/delete.rs @@ -109,6 +109,7 @@ fn stage_display_name(stage: &str) -> &str { Some(Stage::Upcoming) => "upcoming", Some(Stage::Backlog) => "backlog", Some(Stage::Coding) => "in-progress", + Some(Stage::Blocked { .. }) => "blocked", Some(Stage::Qa) => "QA", Some(Stage::Merge { .. }) => "merge", Some(Stage::Done { .. }) => "done", diff --git a/server/src/http/mcp/story_tools/epic.rs b/server/src/http/mcp/story_tools/epic.rs index eef71365..a2540541 100644 --- a/server/src/http/mcp/story_tools/epic.rs +++ b/server/src/http/mcp/story_tools/epic.rs @@ -145,6 +145,7 @@ pub(crate) fn tool_show_epic(args: &Value, _ctx: &AppContext) -> Result "done", Stage::Archived { .. } => "archived", Stage::Frozen { .. } => "frozen", + Stage::Blocked { .. } => "blocked", }; member_items.push(json!({ "story_id": sid, diff --git a/server/src/http/workflow/pipeline.rs b/server/src/http/workflow/pipeline.rs index 7a1b77cc..231a6eed 100644 --- a/server/src/http/workflow/pipeline.rs +++ b/server/src/http/workflow/pipeline.rs @@ -146,6 +146,7 @@ pub fn load_pipeline_state(ctx: &AppContext) -> Result { Stage::Upcoming => state.backlog.push(story), // upcoming shown with backlog Stage::Backlog => state.backlog.push(story), Stage::Coding => state.current.push(story), + Stage::Blocked { .. } => state.current.push(story), // blocked shown with current Stage::Qa => state.qa.push(story), Stage::Merge { .. } => state.merge.push(story), Stage::Done { .. } => state.done.push(story), diff --git a/server/src/io/story_metadata/fields.rs b/server/src/io/story_metadata/fields.rs index fa1fbc3f..25ec8f3b 100644 --- a/server/src/io/story_metadata/fields.rs +++ b/server/src/io/story_metadata/fields.rs @@ -115,18 +115,6 @@ pub fn write_review_hold(path: &Path) -> Result<(), String> { Ok(()) } -/// Write `blocked: true` to the YAML front matter of a story file. -/// -/// Used to mark stories that have exceeded the retry limit and should not -/// be auto-assigned again. -pub fn write_blocked(path: &Path) -> Result<(), String> { - let contents = - fs::read_to_string(path).map_err(|e| format!("Failed to read story file: {e}"))?; - let updated = set_front_matter_field(&contents, "blocked", "true"); - fs::write(path, &updated).map_err(|e| format!("Failed to write story file: {e}"))?; - Ok(()) -} - /// Write or update `depends_on:` field in the YAML front matter of a story file. /// /// Serialises `deps` as an inline YAML sequence, e.g. `[477, 478]`. @@ -162,11 +150,6 @@ pub fn write_rejection_notes_to_content(contents: &str, notes: &str) -> String { format!("{contents}{section}") } -/// Write `blocked: true` to story content (pure function). -pub fn write_blocked_in_content(contents: &str) -> String { - set_front_matter_field(contents, "blocked", "true") -} - /// Write or update `merge_failure` in story content (pure function). pub fn write_merge_failure_in_content(contents: &str, reason: &str) -> String { let escaped = reason diff --git a/server/src/io/story_metadata/mod.rs b/server/src/io/story_metadata/mod.rs index a1575663..eadef1ff 100644 --- a/server/src/io/story_metadata/mod.rs +++ b/server/src/io/story_metadata/mod.rs @@ -14,8 +14,8 @@ 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_blocked, write_blocked_in_content, write_depends_on, write_depends_on_in_content, - write_merge_failure, write_merge_failure_in_content, write_mergemaster_attempted_in_content, + 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, }; pub use parser::{ diff --git a/server/src/io/watcher/mod.rs b/server/src/io/watcher/mod.rs index cd849dd1..ace0e16d 100644 --- a/server/src/io/watcher/mod.rs +++ b/server/src/io/watcher/mod.rs @@ -52,6 +52,7 @@ pub fn stage_metadata(stage: &str, item_id: &str) -> Option<(&'static str, Strin Stage::Upcoming => ("create", format!("huskies: triage {item_id}")), Stage::Backlog => ("create", format!("huskies: create {item_id}")), Stage::Coding => ("start", format!("huskies: start {item_id}")), + 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::Done { .. } => ("done", format!("huskies: done {item_id}")), diff --git a/server/src/pipeline_state/projection.rs b/server/src/pipeline_state/projection.rs index f8eb96c4..8ed5dfcf 100644 --- a/server/src/pipeline_state/projection.rs +++ b/server/src/pipeline_state/projection.rs @@ -72,6 +72,9 @@ pub fn project_stage(view: &PipelineItemView) -> Result match view.stage.as_str() { "0_upcoming" => Ok(Stage::Upcoming), "1_backlog" => Ok(Stage::Backlog), + "2_blocked" => Ok(Stage::Blocked { + reason: String::new(), + }), "2_current" => Ok(Stage::Coding), "3_qa" => Ok(Stage::Qa), "4_merge" => { @@ -147,10 +150,11 @@ impl PipelineItem { let dir = stage_dir_name(&self.stage); let blocked = matches!( self.stage, - Stage::Archived { - reason: ArchiveReason::Blocked { .. }, - .. - } + Stage::Blocked { .. } + | Stage::Archived { + reason: ArchiveReason::Blocked { .. }, + .. + } ); // Frozen stories map to "7_frozen"; they are not "blocked" in the CRDT sense. (dir, blocked) @@ -302,6 +306,26 @@ mod tests { } } + #[test] + fn project_blocked_item() { + let view = PipelineItemView { + story_id: "42_story_test".to_string(), + stage: "2_blocked".to_string(), + name: Some("Test".to_string()), + agent: None, + retry_count: None, + blocked: None, + depends_on: None, + claimed_by: None, + claimed_at: None, + merged_at: None, + qa_mode: None, + mergemaster_attempted: None, + }; + let item = PipelineItem::try_from(&view).unwrap(); + assert!(matches!(item.stage, Stage::Blocked { .. })); + } + #[test] fn project_archived_blocked_item() { let view = PipelineItemView { @@ -385,6 +409,13 @@ mod tests { (Stage::Upcoming, "0_upcoming", false), (Stage::Backlog, "1_backlog", false), (Stage::Coding, "2_current", false), + ( + Stage::Blocked { + reason: "stuck".into(), + }, + "2_blocked", + true, + ), (Stage::Qa, "3_qa", false), ( Stage::Merge { diff --git a/server/src/pipeline_state/tests.rs b/server/src/pipeline_state/tests.rs index 0447b6ea..3e4a4fd9 100644 --- a/server/src/pipeline_state/tests.rs +++ b/server/src/pipeline_state/tests.rs @@ -172,13 +172,7 @@ fn block_from_any_active_stage() { reason: "stuck".into(), }, ); - assert!(matches!( - result, - Ok(Stage::Archived { - reason: ArchiveReason::Blocked { .. }, - .. - }) - )); + assert!(matches!(result, Ok(Stage::Blocked { .. }))); } let m = Stage::Merge { @@ -191,17 +185,20 @@ fn block_from_any_active_stage() { reason: "stuck".into(), }, ); - assert!(matches!( - result, - Ok(Stage::Archived { - reason: ArchiveReason::Blocked { .. }, - .. - }) - )); + assert!(matches!(result, Ok(Stage::Blocked { .. }))); } #[test] -fn unblock_returns_to_backlog() { +fn unblock_returns_to_coding() { + let s = Stage::Blocked { + reason: "test".into(), + }; + let result = transition(s, PipelineEvent::Unblock).unwrap(); + assert!(matches!(result, Stage::Coding)); +} + +#[test] +fn legacy_unblock_archived_blocked_returns_to_backlog() { let s = Stage::Archived { archived_at: chrono::Utc::now(), reason: ArchiveReason::Blocked { diff --git a/server/src/pipeline_state/transition.rs b/server/src/pipeline_state/transition.rs index a258edb6..f9be223b 100644 --- a/server/src/pipeline_state/transition.rs +++ b/server/src/pipeline_state/transition.rs @@ -160,14 +160,11 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result Ok(Archived { - archived_at: now, - reason: ArchiveReason::Blocked { reason }, - }), + | (Merge { .. }, Block { reason }) => Ok(Blocked { reason }), (Backlog, ReviewHold { reason }) | (Coding, ReviewHold { reason }) @@ -221,7 +218,10 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result Ok(Coding), + + // ── Legacy unblock: Archived(Blocked|MergeFailed) → Backlog ── ( Archived { reason: ArchiveReason::Blocked { .. }, diff --git a/server/src/pipeline_state/types.rs b/server/src/pipeline_state/types.rs index 009d6070..195118f1 100644 --- a/server/src/pipeline_state/types.rs +++ b/server/src/pipeline_state/types.rs @@ -47,7 +47,7 @@ impl fmt::Display for AgentName { /// after CRDT convergence. Notice what is NOT a field: /// - `agent` — local execution state, not pipeline state /// - `retry_count` — also local -/// - `blocked` — folded into `Archived { reason: Blocked { .. } }` +/// - `blocked` — now a first-class `Blocked { reason }` stage /// /// ## Canonical state machine (story 857) /// @@ -61,7 +61,7 @@ impl fmt::Display for AgentName { /// | qa_pending | `Qa` | /// | merge_pending | `Merge { .. }` | /// | done | `Done { .. }` | -/// | blocked | `Archived { Blocked { .. } }` | +/// | blocked | `Blocked { .. }` | /// | merge_failure | `Archived { MergeFailed { .. } }` | /// | archived | `Archived { Completed }` | /// | superseded | `Archived { Superseded { .. } }` | @@ -95,6 +95,11 @@ pub enum Stage { merge_commit: GitSha, }, + /// Story is blocked, awaiting human resolution or retry-limit review. + /// Unlike `Archived`, a blocked story is expected to return to active work + /// (via `Unblock → Coding`). + Blocked { reason: String }, + /// Out of the active flow. The reason explains why. Archived { archived_at: DateTime, @@ -149,14 +154,16 @@ impl Stage { stage_dir_name(self) } - /// Returns true if this is the Archived(Blocked) variant. + /// Returns true if this is the `Blocked` variant (or the legacy + /// `Archived(Blocked)` for backward-compatible reads). pub fn is_blocked(&self) -> bool { matches!( self, - Stage::Archived { - reason: ArchiveReason::Blocked { .. }, - .. - } + Stage::Blocked { .. } + | Stage::Archived { + reason: ArchiveReason::Blocked { .. }, + .. + } ) } @@ -172,6 +179,9 @@ impl Stage { match s { "0_upcoming" => Some(Stage::Upcoming), "1_backlog" => Some(Stage::Backlog), + "2_blocked" => Some(Stage::Blocked { + reason: String::new(), + }), "2_current" => Some(Stage::Coding), "3_qa" => Some(Stage::Qa), "4_merge" => Some(Stage::Merge { @@ -269,6 +279,7 @@ pub fn stage_label(s: &Stage) -> &'static str { Stage::Qa => "Qa", Stage::Merge { .. } => "Merge", Stage::Done { .. } => "Done", + Stage::Blocked { .. } => "Blocked", Stage::Archived { .. } => "Archived", Stage::Frozen { .. } => "Frozen", } @@ -280,6 +291,7 @@ pub fn stage_dir_name(s: &Stage) -> &'static str { Stage::Upcoming => "0_upcoming", Stage::Backlog => "1_backlog", Stage::Coding => "2_current", + Stage::Blocked { .. } => "2_blocked", Stage::Qa => "3_qa", Stage::Merge { .. } => "4_merge", Stage::Done { .. } => "5_done", diff --git a/server/src/service/agents/mod.rs b/server/src/service/agents/mod.rs index d385bea3..0ba0af32 100644 --- a/server/src/service/agents/mod.rs +++ b/server/src/service/agents/mod.rs @@ -205,6 +205,7 @@ pub fn get_work_item_content( crate::pipeline_state::Stage::Upcoming => "upcoming", crate::pipeline_state::Stage::Backlog => "backlog", crate::pipeline_state::Stage::Coding => "current", + crate::pipeline_state::Stage::Blocked { .. } => "blocked", crate::pipeline_state::Stage::Qa => "qa", crate::pipeline_state::Stage::Merge { .. } => "merge", crate::pipeline_state::Stage::Done { .. } => "done", diff --git a/server/src/service/notifications/format.rs b/server/src/service/notifications/format.rs index 93d79e78..73a8b0f6 100644 --- a/server/src/service/notifications/format.rs +++ b/server/src/service/notifications/format.rs @@ -13,6 +13,7 @@ pub fn stage_display_name(stage: &str) -> &'static str { Some(Stage::Upcoming) => "Upcoming", Some(Stage::Backlog) => "Backlog", Some(Stage::Coding) => "Current", + Some(Stage::Blocked { .. }) => "Blocked", Some(Stage::Qa) => "QA", Some(Stage::Merge { .. }) => "Merge", Some(Stage::Done { .. }) => "Done",