diff --git a/PLAN.md b/PLAN.md new file mode 100644 index 00000000..68d7d159 --- /dev/null +++ b/PLAN.md @@ -0,0 +1,22 @@ +# Plan: Story 945 — Delete WorkItem flag soup + +Goal: Fold `blocked`, `review_hold`, `frozen`, and `mergemaster_attempted` +flag fields into `Stage` / `ArchiveReason` / `ExecutionState` variants so the +typed state machine is the single source of truth. + +## ACs → implementation locations +- AC 1 (delete flag fields): `server/src/crdt_state/types.rs` (PipelineItemCrdt, WorkItem) — DONE; `server/src/pipeline_state/types.rs` (PipelineItem) — DONE. +- AC 2 (variants + `match`-based callers): New variants `Stage::MergeFailureFinal`, `Stage::Frozen { resume_to }`, `Stage::ReviewHold { resume_to, reason }` in `pipeline_state/types.rs` — DONE. Auto-assigner, watchdog, unblock/freeze/unfreeze, merge pipeline all `match` on Stage — DONE. +- AC 3 (no Option flag-poking): grep for `set_blocked|set_review_hold|set_frozen|set_mergemaster_attempted` returns no results — DONE. +- AC 4 (cargo check/clippy/test pass): `run_check` clean, `run_tests` reports 2908 passed / 0 failed — DONE. + +## Decisions +- Resume target for `Frozen`/`ReviewHold`: stored as a sibling `resume_to: LwwRegisterCrdt` on `PipelineItemCrdt` rather than encoded into the stage string. Rejected: encoding into stage register (would require parsing variants out of strings and lose round-trip cleanness). +- Reason text for `Blocked`/`MergeFailure`/`MergeFailureFinal`/`ReviewHold`: kept on the Stage variant in memory, but the wire-form stage register only carries the canonical dir name (`"merge_failure_final"`, `"review_hold"`). Reasons are reconstructed at read time from companion CRDT data (MergeJob.error) where needed. Acceptable because reason is human-text, not load-bearing for routing. +- Pre-934 `7_frozen` legacy migration: rewrites stage to `"frozen"` and sets `resume_to = "backlog"`, restoring `Stage::Frozen { resume_to: Backlog }` on read. The defensive projection fallback still maps raw `7_frozen` → `Backlog` for un-migrated reads. + +## Current state +All flag fields removed; all 47 modified files compile; 2908 tests pass; doc coverage clean. + +## What's left +- [x] Commit changes diff --git a/server/migrations/20240104000000_drop_blocked_column.sql b/server/migrations/20240104000000_drop_blocked_column.sql new file mode 100644 index 00000000..3093a28c --- /dev/null +++ b/server/migrations/20240104000000_drop_blocked_column.sql @@ -0,0 +1,4 @@ +-- Story 945: drop the `blocked` boolean column from the shadow pipeline_items +-- table. `Stage::Blocked { reason }` is now the single source of truth for +-- "blocked" — the legacy flag has been deleted from the CRDT and Rust types. +ALTER TABLE pipeline_items DROP COLUMN blocked; diff --git a/server/src/agent_mode/claim.rs b/server/src/agent_mode/claim.rs index 2d9d01cb..5471ef38 100644 --- a/server/src/agent_mode/claim.rs +++ b/server/src/agent_mode/claim.rs @@ -97,7 +97,6 @@ mod tests { None, None, None, - None, Some(stale_holder), Some(stale_time), None, diff --git a/server/src/agent_mode/loop_ops.rs b/server/src/agent_mode/loop_ops.rs index 4220dd75..033b6094 100644 --- a/server/src/agent_mode/loop_ops.rs +++ b/server/src/agent_mode/loop_ops.rs @@ -46,8 +46,8 @@ pub(super) async fn scan_and_claim( continue; } - // Skip blocked stories. - if item.blocked() { + // Skip blocked stories (story 945: `Stage::Blocked` is the source of truth). + if item.stage().is_blocked() { continue; } diff --git a/server/src/agents/lifecycle.rs b/server/src/agents/lifecycle.rs index 67d2d988..ac31d419 100644 --- a/server/src/agents/lifecycle.rs +++ b/server/src/agents/lifecycle.rs @@ -192,11 +192,12 @@ pub fn move_story_to_qa(story_id: &str) -> Result<(), String> { .map_err(|e| e.to_string()) } -/// Move a story from `work/3_qa/` back to `work/2_current/`, clearing -/// `review_hold` (story 932: CRDT register) and appending rejection notes. +/// Move a story from `work/3_qa/` back to `work/2_current/`, appending +/// rejection notes. Story 945: the legacy `review_hold` flag is gone; if a +/// story is in `Stage::ReviewHold`, the `GatesFailed` event simply fails to +/// transition, which is the correct behaviour (you cannot reject from QA a +/// story that is currently parked in review hold). pub fn reject_story_from_qa(story_id: &str, notes: &str) -> Result<(), String> { - crate::crdt_state::set_review_hold(story_id, false); - if notes.is_empty() { apply_transition( story_id, @@ -292,11 +293,9 @@ pub fn transition_to_unblocked(story_id: &str) -> Result<(), String> { .map(|_| ()) .map_err(|e| e.to_string())?; - // Reset CRDT registers so the legacy `blocked`/`retry_count` fields match - // the new typed stage. Pre-865, YAML stripping kept these in sync as a - // side-effect of the content_transform above; post-865 the content has no - // YAML, so we must clear the registers explicitly. - crate::crdt_state::set_blocked(story_id, false); + // Story 945: the legacy `blocked` boolean flag is gone — `Stage::Blocked` + // is the single source of truth. We still reset `retry_count` so a fresh + // attempt at the resumed stage starts at zero. crate::crdt_state::set_retry_count(story_id, 0); Ok(()) } @@ -428,6 +427,9 @@ fn stage_to_name(s: &Stage) -> &'static str { Stage::Qa => "qa", Stage::Merge { .. } => "merge", Stage::MergeFailure { .. } => "merge_failure", + Stage::MergeFailureFinal { .. } => "merge_failure_final", + Stage::Frozen { .. } => "frozen", + Stage::ReviewHold { .. } => "review_hold", Stage::Done { .. } => "done", Stage::Archived { .. } => "archived", } diff --git a/server/src/agents/pool/auto_assign/auto_assign.rs b/server/src/agents/pool/auto_assign/auto_assign.rs index ce2a3003..153babf6 100644 --- a/server/src/agents/pool/auto_assign/auto_assign.rs +++ b/server/src/agents/pool/auto_assign/auto_assign.rs @@ -594,13 +594,15 @@ mod tests { ) .unwrap(); crate::db::ensure_content_store(); + // Story 945: "blocked AND in 4_merge" is no longer representable as + // separate states. A blocked story lives in `Stage::Blocked` (which + // maps to wire-form "blocked"), so auto-assign won't see it in 4_merge. crate::db::write_item_with_content( "9863_story_blocked_conflict", - "4_merge", + "blocked", "CONFLICT (content): foo.rs", crate::db::ItemMeta { name: Some("Blocked conflict".to_string()), - blocked: Some(true), ..Default::default() }, ); @@ -633,13 +635,13 @@ mod tests { ) .unwrap(); crate::db::ensure_content_store(); + // Story 945: "mergemaster attempted" is now `Stage::MergeFailureFinal`. crate::db::write_item_with_content( "9862_story_attempted", - "4_merge", + "merge_failure_final", "CONFLICT (content): foo.rs", crate::db::ItemMeta::named("Already tried"), ); - crate::crdt_state::set_mergemaster_attempted("9862_story_attempted", true); let pool = AgentPool::new_test(3001); pool.auto_assign_available_work(tmp.path()).await; @@ -712,16 +714,13 @@ mod tests { .unwrap(); crate::crdt_state::init_for_test(); crate::db::ensure_content_store(); - // mergemaster_attempted is set by the exit path when genuine give-up occurs. + // Story 945: the genuine give-up state is now `Stage::MergeFailureFinal`. crate::db::write_item_with_content( "920_story_genuine", - "4_merge_failure", + "merge_failure_final", "CONFLICT (content): bar.rs", crate::db::ItemMeta::named("Genuine"), ); - // The CRDT register is the sole authority; set it explicitly as the - // spawn exit path would after report_merge_failure. - crate::crdt_state::set_mergemaster_attempted("920_story_genuine", true); let pool = AgentPool::new_test(3001); pool.auto_assign_available_work(tmp.path()).await; diff --git a/server/src/agents/pool/auto_assign/reconcile.rs b/server/src/agents/pool/auto_assign/reconcile.rs index 35e0a28f..cfc6c936 100644 --- a/server/src/agents/pool/auto_assign/reconcile.rs +++ b/server/src/agents/pool/auto_assign/reconcile.rs @@ -215,10 +215,16 @@ impl AgentPool { message: format!("Failed to advance to QA: {e}"), }); } else { - // Story 932: review_hold is a typed CRDT register. - crate::crdt_state::set_review_hold(story_id, true); + // Story 945: ReviewHold is a typed Stage variant. + let _ = crate::pipeline_state::apply_transition( + story_id, + crate::pipeline_state::PipelineEvent::ReviewHold { + reason: "qa: human — gates passed, awaiting review".to_string(), + }, + None, + ); eprintln!( - "[startup:reconcile] Moved '{story_id}' → 3_qa/ (qa: human — holding for review)." + "[startup:reconcile] Moved '{story_id}' → review_hold (qa: human — holding for review)." ); let _ = progress_tx.send(ReconciliationEvent { story_id: story_id.clone(), @@ -278,8 +284,14 @@ impl AgentPool { }; if needs_human_review { - // Story 932: review_hold is a typed CRDT register. - crate::crdt_state::set_review_hold(story_id, true); + // Story 945: ReviewHold is a typed Stage variant. + let _ = crate::pipeline_state::apply_transition( + story_id, + crate::pipeline_state::PipelineEvent::ReviewHold { + reason: "Passed QA — waiting for human review.".to_string(), + }, + None, + ); eprintln!( "[startup:reconcile] '{story_id}' passed QA — holding for human review." ); diff --git a/server/src/agents/pool/auto_assign/story_checks.rs b/server/src/agents/pool/auto_assign/story_checks.rs index eb7990fb..e2b106c0 100644 --- a/server/src/agents/pool/auto_assign/story_checks.rs +++ b/server/src/agents/pool/auto_assign/story_checks.rs @@ -21,15 +21,16 @@ pub(super) fn read_story_front_matter_agent( .filter(|s| !s.is_empty()) } -/// Return `true` if the story has its `review_hold` CRDT register set. +/// Return `true` if the story is in `Stage::ReviewHold`. /// -/// Sub-story 932: `review_hold` is now a dedicated CRDT register on -/// `PipelineItemCrdt`, distinct from `Stage::Frozen`. The auto-assigner uses -/// this to keep human-QA items / spikes parked after gates pass until a -/// reviewer explicitly clears the hold (e.g. via `tool_approve_qa`). +/// Story 945: `Stage::ReviewHold { resume_to, reason }` is the single source +/// of truth — the legacy `review_hold: bool` CRDT register has been deleted. +/// The auto-assigner uses this to keep human-QA items / spikes parked after +/// gates pass until a reviewer explicitly clears the hold (e.g. via +/// `tool_approve_qa`). pub(super) fn has_review_hold(_project_root: &Path, _stage_dir: &str, story_id: &str) -> bool { crate::crdt_state::read_item(story_id) - .map(|w| w.review_hold()) + .map(|w| w.stage().is_review_hold()) .unwrap_or(false) } @@ -80,18 +81,19 @@ pub(super) fn has_content_conflict_failure( .unwrap_or(false) } -/// Return `true` if the CRDT `mergemaster_attempted` register is set for this story. +/// Return `true` if the story is in `Stage::MergeFailureFinal`. /// +/// Story 945: `Stage::MergeFailureFinal` is the single source of truth — +/// the legacy `mergemaster_attempted: bool` CRDT register has been deleted. /// Used to prevent the auto-assigner from repeatedly spawning mergemaster for -/// the same story after a failed mergemaster session. The CRDT register is the -/// only source consulted — the legacy YAML field is no longer checked. +/// the same story after a failed mergemaster session. pub(super) fn has_mergemaster_attempted( _project_root: &Path, _stage_dir: &str, story_id: &str, ) -> bool { crate::crdt_state::read_item(story_id) - .map(|view| view.mergemaster_attempted()) + .map(|view| view.stage().is_mergemaster_attempted()) .unwrap_or(false) } @@ -116,13 +118,15 @@ pub(super) fn check_archived_dependencies( crate::crdt_state::check_archived_deps_crdt(story_id) } -/// Return `true` if the story's `frozen` CRDT flag is set (story 934, stage 4). +/// Return `true` if the story is in `Stage::Frozen`. /// -/// `frozen` is orthogonal to [`Stage`]: a frozen story keeps its current stage -/// register but is skipped by the auto-assigner. +/// Story 945: `Stage::Frozen { resume_to }` is the single source of truth — +/// the legacy `frozen: bool` CRDT register has been deleted. Frozen stories +/// are skipped by the auto-assigner until `Unfreeze` returns them to +/// `resume_to`. pub(super) fn is_story_frozen(_project_root: &Path, _stage_dir: &str, story_id: &str) -> bool { crate::crdt_state::read_item(story_id) - .map(|view| view.frozen()) + .map(|view| view.stage().is_frozen()) .unwrap_or(false) } @@ -139,9 +143,11 @@ mod tests { crate::crdt_state::init_for_test(); crate::db::ensure_content_store(); let tmp = tempfile::tempdir().unwrap(); + // Story 945: review_hold is now a typed Stage variant, seeded via + // the wire-form stage register directly. crate::crdt_state::write_item_str( "890_spike_held", - "3_qa", + "review_hold", Some("Held Spike"), None, None, @@ -149,9 +155,7 @@ mod tests { None, None, None, - None, ); - crate::crdt_state::set_review_hold("890_spike_held", true); assert!(has_review_hold(tmp.path(), "3_qa", "890_spike_held")); } @@ -170,7 +174,6 @@ mod tests { None, None, None, - None, ); assert!(!has_review_hold(tmp.path(), "3_qa", "890_spike_active_qa")); } @@ -258,7 +261,6 @@ mod tests { Some("Blocked"), None, None, - None, Some("[999]"), None, None, @@ -285,7 +287,6 @@ mod tests { None, None, None, - None, ); crate::crdt_state::write_item_str( "10_story_ok", @@ -293,7 +294,6 @@ mod tests { Some("Ok"), None, None, - None, Some("[999]"), None, None, @@ -320,7 +320,6 @@ mod tests { None, None, None, - None, ); assert!(!has_unmet_dependencies( tmp.path(), @@ -346,7 +345,6 @@ mod tests { None, None, None, - None, ); crate::crdt_state::write_item_str( "503_story_dependent", @@ -354,7 +352,6 @@ mod tests { Some("Dependent"), None, None, - None, Some("[500]"), None, None, @@ -380,7 +377,6 @@ mod tests { None, None, None, - None, ); crate::crdt_state::write_item_str( "503_story_waiting", @@ -388,7 +384,6 @@ mod tests { Some("Waiting"), None, None, - None, Some("[490]"), None, None, 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 c48e7398..52766ea5 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 @@ -253,7 +253,6 @@ max_turns = 10 None, None, None, - None, ); // 12 turns in a single session exceeds the configured max of 10. @@ -381,7 +380,6 @@ max_turns = 10 None, None, None, - None, ); // Prior session with 5 turns (under limit alone). @@ -461,7 +459,6 @@ max_turns = 10 None, None, None, - None, ); // Session 1: exceeds limit → retry_count=1 in CRDT, NOT blocked. diff --git a/server/src/agents/pool/pipeline/advance/helpers.rs b/server/src/agents/pool/pipeline/advance/helpers.rs index 2a9e811a..dc7b8ea0 100644 --- a/server/src/agents/pool/pipeline/advance/helpers.rs +++ b/server/src/agents/pool/pipeline/advance/helpers.rs @@ -66,10 +66,21 @@ pub(super) fn resolve_qa_mode_from_store( .unwrap_or(default) } -/// Mark a story as held for human review (story 932: CRDT register). +/// Mark a story as held for human review (story 945: `Stage::ReviewHold`). +/// +/// The caller has just moved the story to QA via `move_story_to_qa`, so the +/// story is in `Stage::Qa`. We transition to `Stage::ReviewHold { resume_to: +/// Qa, reason }` so the auto-assigner skips it while preserving the resume +/// target. pub(super) fn write_review_hold_to_store(story_id: &str) { - if !crate::crdt_state::set_review_hold(story_id, true) { - slog_error!("[pipeline] Cannot set review_hold for '{story_id}': no CRDT entry"); + if let Err(e) = crate::pipeline_state::apply_transition_str( + story_id, + crate::pipeline_state::PipelineEvent::ReviewHold { + reason: "Held for human review".to_string(), + }, + None, + ) { + slog_error!("[pipeline] Cannot set review_hold for '{story_id}': {e}"); } } diff --git a/server/src/agents/pool/start/spawn.rs b/server/src/agents/pool/start/spawn.rs index 525aa47c..4dc32b3b 100644 --- a/server/src/agents/pool/start/spawn.rs +++ b/server/src/agents/pool/start/spawn.rs @@ -574,7 +574,11 @@ pub(super) async fn run_agent_spawn( } }; if is_genuine { - crate::crdt_state::set_mergemaster_attempted(&sid, true); + let _ = crate::pipeline_state::apply_transition_str( + &sid, + crate::pipeline_state::PipelineEvent::MergemasterAttempted, + None, + ); } let _ = tx_done.send(AgentEvent::Done { story_id: sid.clone(), diff --git a/server/src/agents/pool/start/tests_selection.rs b/server/src/agents/pool/start/tests_selection.rs index d7439a8c..9f7f6c2a 100644 --- a/server/src/agents/pool/start/tests_selection.rs +++ b/server/src/agents/pool/start/tests_selection.rs @@ -293,7 +293,6 @@ stage = "coder" None, None, None, - None, ); let pool = AgentPool::new_test(3011); diff --git a/server/src/chat/commands/backlog.rs b/server/src/chat/commands/backlog.rs index 93d57f52..ebd89b38 100644 --- a/server/src/chat/commands/backlog.rs +++ b/server/src/chat/commands/backlog.rs @@ -71,7 +71,6 @@ mod tests { stage, depends_on: Vec::new(), retry_count: 0, - frozen: false, } } @@ -82,7 +81,6 @@ mod tests { stage, depends_on: deps.iter().map(|n| StoryId(n.to_string())).collect(), retry_count: 0, - frozen: false, } } diff --git a/server/src/chat/commands/status/tests.rs b/server/src/chat/commands/status/tests.rs index 1e16e961..c7f83ef7 100644 --- a/server/src/chat/commands/status/tests.rs +++ b/server/src/chat/commands/status/tests.rs @@ -13,7 +13,6 @@ fn make_item(id: &str, name: &str, stage: Stage) -> PipelineItem { stage, depends_on: Vec::new(), retry_count: 0, - frozen: false, } } @@ -25,7 +24,6 @@ fn make_item_with_deps(id: &str, name: &str, stage: Stage, deps: Vec) -> Pi stage, depends_on: deps.iter().map(|n| StoryId(n.to_string())).collect(), retry_count: 0, - frozen: false, } } diff --git a/server/src/chat/commands/triage.rs b/server/src/chat/commands/triage.rs index 6f244173..847907e3 100644 --- a/server/src/chat/commands/triage.rs +++ b/server/src/chat/commands/triage.rs @@ -88,7 +88,8 @@ fn build_triage_dump( // ---- CRDT metadata ---- if let Some(ref w) = crdt_item { let mut fields: Vec = Vec::new(); - if w.blocked() { + // Story 945: `Stage::Blocked` is the source of truth. + if w.stage().is_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 641920e3..8b801b32 100644 --- a/server/src/chat/commands/unblock.rs +++ b/server/src/chat/commands/unblock.rs @@ -58,7 +58,8 @@ fn unblock_by_story_id(story_id: &str) -> String { .and_then(|i| i.name().map(str::to_string)) .unwrap_or_else(|| story_id.to_string()); - // Canonical "is this story blocked?" comes from the typed pipeline state. + // 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() @@ -67,10 +68,8 @@ fn unblock_by_story_id(story_id: &str) -> String { typed_item.as_ref().map(|i| &i.stage), Some(crate::pipeline_state::Stage::MergeFailure { .. }) ); - // CRDT register fallback for items not yet projected into typed state. - let crdt_blocked = crdt_item.as_ref().is_some_and(|i| i.blocked()); - if !typed_blocked && !crdt_blocked { + if !typed_blocked { return format!("**{story_name}** ({story_id}) is not blocked. Nothing to unblock."); } @@ -211,7 +210,6 @@ mod tests { Some("Stuck Story"), None, Some(5), - Some(true), None, None, None, @@ -238,9 +236,11 @@ mod tests { 0, "retry_count should be reset to 0 in CRDT after unblock" ); + // Story 945: `Stage::Blocked` was the source of truth; after unblock + // the stage must have transitioned out of `Blocked`. assert!( - !item.blocked(), - "blocked flag should be cleared in CRDT after unblock" + !item.stage().is_blocked(), + "stage must no longer be Blocked after unblock" ); } @@ -279,7 +279,6 @@ mod tests { Some("Stuck Story"), None, Some(5), - Some(true), None, None, None, @@ -305,9 +304,11 @@ mod tests { 0, "retry_count must be reset to 0 in CRDT after unblock" ); + // Story 945: `Stage::Blocked` was the source of truth; after unblock + // the stage must have transitioned out of `Blocked`. assert!( - !item.blocked(), - "blocked flag must be cleared in CRDT after unblock" + !item.stage().is_blocked(), + "stage must no longer be Blocked after unblock" ); } @@ -321,8 +322,18 @@ mod tests { "# Story\n", Some("In QA"), ); - crate::crdt_state::set_blocked("9901_story_in_qa", true); - crate::crdt_state::set_retry_count("9901_story_in_qa", 3); + // Story 945: blocked is now `Stage::Blocked` — seed via CRDT stage. + crate::crdt_state::write_item_str( + "9901_story_in_qa", + "blocked", + Some("In QA"), + None, + Some(3), + None, + None, + None, + None, + ); let output = unblock_cmd_with_root(tmp.path(), "9901").unwrap(); assert!( diff --git a/server/src/chat/transport/matrix/assign.rs b/server/src/chat/transport/matrix/assign.rs index 152f150b..114958ed 100644 --- a/server/src/chat/transport/matrix/assign.rs +++ b/server/src/chat/transport/matrix/assign.rs @@ -323,7 +323,6 @@ mod tests { None, None, None, - None, ); let agents = std::sync::Arc::new(AgentPool::new_test(3000)); @@ -379,7 +378,6 @@ mod tests { None, None, None, - None, ); let agents = std::sync::Arc::new(AgentPool::new_test(3000)); @@ -430,7 +428,6 @@ mod tests { None, None, None, - None, ); let agents = std::sync::Arc::new(AgentPool::new_test(3000)); diff --git a/server/src/chat/transport/matrix/delete.rs b/server/src/chat/transport/matrix/delete.rs index 3261ce40..c1e1b583 100644 --- a/server/src/chat/transport/matrix/delete.rs +++ b/server/src/chat/transport/matrix/delete.rs @@ -115,6 +115,9 @@ fn stage_display_label(stage: &crate::pipeline_state::Stage) -> &'static str { Stage::Done { .. } => "done", Stage::Archived { .. } => "archived", Stage::MergeFailure { .. } => "merge-failure", + Stage::MergeFailureFinal { .. } => "merge-failure-final", + Stage::Frozen { .. } => "frozen", + Stage::ReviewHold { .. } => "review-hold", } } @@ -250,7 +253,6 @@ mod tests { None, None, None, - None, ); // Seed in content store so find_story_by_number can resolve it. diff --git a/server/src/crdt_snapshot/tests.rs b/server/src/crdt_snapshot/tests.rs index ecc26239..0a94a9ad 100644 --- a/server/src/crdt_snapshot/tests.rs +++ b/server/src/crdt_snapshot/tests.rs @@ -241,7 +241,6 @@ fn snapshot_generation_includes_manifest() { None, None, None, - None, ); crate::crdt_state::write_item_str( "636_test_b", @@ -253,7 +252,6 @@ fn snapshot_generation_includes_manifest() { None, None, None, - None, ); let snapshot = generate_snapshot(); @@ -286,7 +284,6 @@ fn attribution_query_by_story_id() { None, None, None, - None, ); let snapshot = generate_snapshot().unwrap(); @@ -324,7 +321,6 @@ fn compaction_reduces_ops() { None, None, None, - None, ); } @@ -363,7 +359,6 @@ fn latest_snapshot_available_after_compaction() { None, None, None, - None, ); let snapshot = generate_snapshot().unwrap(); @@ -636,7 +631,6 @@ fn attribution_preserved_after_compaction() { None, None, None, - None, ); crate::crdt_state::write_item_str( "636_archived_story", @@ -648,7 +642,6 @@ fn attribution_preserved_after_compaction() { None, None, None, - None, ); crate::crdt_state::write_item_str( "636_archived_story", @@ -660,7 +653,6 @@ fn attribution_preserved_after_compaction() { None, None, None, - None, ); // Generate snapshot. diff --git a/server/src/crdt_state/mod.rs b/server/src/crdt_state/mod.rs index 0e1ea71d..380ae4ef 100644 --- a/server/src/crdt_state/mod.rs +++ b/server/src/crdt_state/mod.rs @@ -53,9 +53,8 @@ pub use types::{ }; pub use write::{ bump_retry_count, migrate_legacy_stage_strings, migrate_names_from_slugs, - migrate_story_ids_to_numeric, name_from_story_id, set_agent, set_blocked, set_depends_on, - set_epic, set_frozen, set_item_type, set_mergemaster_attempted, set_name, set_qa_mode, - set_retry_count, set_review_hold, write_item, + migrate_story_ids_to_numeric, name_from_story_id, set_agent, set_depends_on, set_epic, + set_item_type, set_name, set_qa_mode, set_resume_to, set_retry_count, write_item, }; #[cfg(test)] diff --git a/server/src/crdt_state/ops.rs b/server/src/crdt_state/ops.rs index a585ab2e..5bdefd04 100644 --- a/server/src/crdt_state/ops.rs +++ b/server/src/crdt_state/ops.rs @@ -552,7 +552,6 @@ mod tests { None, None, None, - None, ); assert!( read_item(story_id).is_none(), diff --git a/server/src/crdt_state/read.rs b/server/src/crdt_state/read.rs index 99c5326b..7569672d 100644 --- a/server/src/crdt_state/read.rs +++ b/server/src/crdt_state/read.rs @@ -21,7 +21,6 @@ pub struct CrdtItemDump { pub name: Option, pub agent: Option, pub retry_count: Option, - pub blocked: Option, pub depends_on: Option>, pub claimed_by: Option, pub claimed_at: Option, @@ -135,10 +134,6 @@ pub fn dump_crdt_state(story_id_filter: Option<&str>) -> CrdtStateDump { JsonValue::Number(n) => Some(n as i64), _ => None, }; - let blocked = match item_crdt.blocked.view() { - JsonValue::Bool(b) => Some(b), - _ => None, - }; let depends_on = match item_crdt.depends_on.view() { JsonValue::String(s) if !s.is_empty() => serde_json::from_str::>(&s).ok(), _ => None, @@ -161,7 +156,6 @@ pub fn dump_crdt_state(story_id_filter: Option<&str>) -> CrdtStateDump { name, agent, retry_count, - blocked, depends_on, claimed_by, claimed_at, @@ -321,10 +315,6 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option Some(n as i64), _ => None, }; - let blocked = match item.blocked.view() { - JsonValue::Bool(b) => Some(b), - _ => None, - }; let depends_on = match item.depends_on.view() { JsonValue::String(s) if !s.is_empty() => serde_json::from_str::>(&s).ok(), _ => None, @@ -348,16 +338,6 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option None, }; - let mergemaster_attempted = match item.mergemaster_attempted.view() { - JsonValue::Bool(b) => Some(b), - _ => None, - }; - - let review_hold = match item.review_hold.view() { - JsonValue::Bool(b) => Some(b), - _ => None, - }; - let item_type = match item.item_type.view() { JsonValue::String(s) if !s.is_empty() => Some(s), _ => None, @@ -368,12 +348,12 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option None, }; - let frozen = match item.frozen.view() { - JsonValue::Bool(b) => Some(b), + let resume_to = match item.resume_to.view() { + JsonValue::String(s) if !s.is_empty() => Some(s), _ => None, }; - let stage = project_stage_for_view(&stage_str, &story_id, merged_at, blocked)?; + let stage = project_stage_for_view(&stage_str, &story_id, merged_at, resume_to.as_deref())?; Some(PipelineItemView { story_id, @@ -381,17 +361,13 @@ pub(super) fn extract_item_view(item: &PipelineItemCrdt) -> Option, - blocked: Option, + resume_to: Option<&str>, ) -> Option { use crate::pipeline_state::{ArchiveReason, BranchName, GitSha, Stage}; use chrono::{DateTime, Utc}; @@ -436,6 +412,12 @@ fn project_stage_for_view( other => other, }; + // Story 945: resume target for `Frozen` / `ReviewHold` variants is stored + // in the sibling `resume_to` register. Fall back to `Coding` when the + // register is empty or holds an unrecognised value. + let resume_target = + || -> Box { Box::new(resume_to.and_then(Stage::from_dir).unwrap_or(Stage::Coding)) }; + match clean { "upcoming" => Some(Stage::Upcoming), "backlog" => Some(Stage::Backlog), @@ -451,6 +433,16 @@ fn project_stage_for_view( "merge_failure" => Some(Stage::MergeFailure { reason: String::new(), }), + "merge_failure_final" => Some(Stage::MergeFailureFinal { + reason: String::new(), + }), + "frozen" => Some(Stage::Frozen { + resume_to: resume_target(), + }), + "review_hold" => Some(Stage::ReviewHold { + resume_to: resume_target(), + reason: String::new(), + }), "done" => { let merged_at = merged_at .map(|ts| { @@ -462,19 +454,10 @@ fn project_stage_for_view( merge_commit: GitSha("legacy".to_string()), }) } - "archived" => { - let reason = if blocked.unwrap_or(false) { - ArchiveReason::Blocked { - reason: "migrated from legacy blocked field".to_string(), - } - } else { - ArchiveReason::Completed - }; - Some(Stage::Archived { - archived_at: Utc::now(), - reason, - }) - } + "archived" => Some(Stage::Archived { + archived_at: Utc::now(), + reason: ArchiveReason::Completed, + }), _ => None, } } @@ -589,7 +572,6 @@ mod tests { assert_eq!(view.name.as_deref(), Some("View Test")); assert_eq!(view.agent.as_deref(), Some("coder-1")); assert_eq!(view.retry_count, Some(2)); - assert_eq!(view.blocked, Some(true)); assert_eq!(view.depends_on, Some(vec![10, 20])); } @@ -647,7 +629,6 @@ mod tests { None, None, None, - None, ); // The story is live on this node. @@ -718,7 +699,6 @@ mod tests { None, None, None, - None, ); assert!( read_item(story_id).is_none(), diff --git a/server/src/crdt_state/state/tests.rs b/server/src/crdt_state/state/tests.rs index 59d0c580..0ae6b4a2 100644 --- a/server/src/crdt_state/state/tests.rs +++ b/server/src/crdt_state/state/tests.rs @@ -119,7 +119,6 @@ async fn subscribe_receives_stage_transition_events() { None, None, None, - None, ); let evt: CrdtEvent = rx.try_recv().expect("expected CrdtEvent on insert"); @@ -138,7 +137,6 @@ async fn subscribe_receives_stage_transition_events() { None, None, None, - None, ); let evt: CrdtEvent = rx.try_recv().expect("expected CrdtEvent on stage change"); diff --git a/server/src/crdt_state/types.rs b/server/src/crdt_state/types.rs index f42d4eef..c5e105a8 100644 --- a/server/src/crdt_state/types.rs +++ b/server/src/crdt_state/types.rs @@ -49,6 +49,13 @@ pub struct PipelineDoc { } /// CRDT sub-document representing a single pipeline work item with LWW fields for stage, agent, etc. +/// +/// Story 945: the `blocked`, `review_hold`, `frozen`, and +/// `mergemaster_attempted` boolean flags have been deleted. Each of the +/// states those flags encoded is represented as a `Stage` variant, so the +/// stage register is the single source of truth. A sibling `resume_to` +/// register stores the `Stage::Frozen { resume_to }` / `Stage::ReviewHold +/// { resume_to, .. }` resume target as a clean wire-form stage name. #[add_crdt_fields] #[derive(Clone, CrdtNode, Debug)] pub struct PipelineItemCrdt { @@ -57,7 +64,6 @@ pub struct PipelineItemCrdt { pub name: LwwRegisterCrdt, pub agent: LwwRegisterCrdt, pub retry_count: LwwRegisterCrdt, - pub blocked: LwwRegisterCrdt, pub depends_on: LwwRegisterCrdt, /// Node ID (hex-encoded Ed25519 pubkey) of the node that claimed this item. /// Used for distributed work claiming — the LWW register resolves conflicts @@ -74,18 +80,6 @@ pub struct PipelineItemCrdt { /// QA mode override for this item: `"server"`, `"agent"`, or `"human"`. /// Empty string means "use the project default". pub qa_mode: LwwRegisterCrdt, - /// Set to `true` when the auto-assigner has already spawned a mergemaster - /// session for a content-conflict failure. Prevents repeated auto-spawns - /// across restarts. Written as `false` (not removed) when cleared. - pub mergemaster_attempted: LwwRegisterCrdt, - /// Set to `true` when a story is held for human review at a pipeline stage - /// boundary (e.g. spikes after QA passes; human-QA items after gates pass). - /// The auto-assigner skips items with this flag so a human can inspect the - /// state before the pipeline continues. Written as `false` (not removed) - /// when explicitly cleared (e.g. by `tool_approve_qa`). Sub-story 932 of - /// the 929 CRDT-only migration; replaces the legacy `review_hold: true` - /// YAML front-matter field. - pub review_hold: LwwRegisterCrdt, /// Item type: `"story"`, `"bug"`, `"spike"`, `"refactor"`, or `"epic"`. /// Empty string means "infer from story_id slug or default to story". /// Sub-story 933; replaces the legacy `type:` YAML front-matter field. @@ -94,12 +88,11 @@ pub struct PipelineItemCrdt { /// member of any epic. Sub-story 933; replaces the legacy `epic:` YAML /// front-matter field that linked member work items to their epic. pub epic: LwwRegisterCrdt, - /// Set to `true` when a story is frozen. Frozen stories stay at their - /// current `Stage` but are skipped by the auto-assigner until explicitly - /// unfrozen. Orthogonal to `Stage` (story 934, stage 4); replaces the - /// pre-934 `Stage::Frozen { resume_to }` variant whose resume payload was - /// just "the stage you were in when you froze". - pub frozen: LwwRegisterCrdt, + /// Story 945: resume-target stage for `Stage::Frozen` and + /// `Stage::ReviewHold` variants. Stored as a clean wire-form stage + /// name (e.g. `"coding"`, `"qa"`). Empty string means "no resume target + /// stored" (defaults to `Coding` on read). + pub resume_to: LwwRegisterCrdt, } /// CRDT node that holds a single peer's presence entry. @@ -140,7 +133,6 @@ pub struct WorkItem { pub(super) name: Option, pub(super) agent: Option, pub(super) retry_count: Option, - pub(super) blocked: Option, pub(super) depends_on: Option>, /// Node ID of the node that claimed this item (hex-encoded Ed25519 pubkey). pub(super) claimed_by: Option, @@ -150,16 +142,10 @@ pub struct WorkItem { pub(super) merged_at: Option, /// QA mode override: `"server"`, `"agent"`, or `"human"`. pub(super) qa_mode: Option, - /// Whether the auto-assigner has already attempted a mergemaster spawn. - pub(super) mergemaster_attempted: Option, - /// Whether the item is held for human review (sub-story 932). - pub(super) review_hold: Option, /// Item type (sub-story 933): `"story"`, `"bug"`, `"spike"`, `"refactor"`, `"epic"`. pub(super) item_type: Option, /// Epic ID this item belongs to, or `None` (sub-story 933). pub(super) epic: Option, - /// Whether the item is frozen (story 934, stage 4). Orthogonal to `Stage`. - pub(super) frozen: Option, } impl WorkItem { @@ -183,11 +169,6 @@ impl WorkItem { self.agent.as_deref() } - /// Whether the item is blocked. Returns `false` when the register is unset. - pub fn blocked(&self) -> bool { - self.blocked.unwrap_or(false) - } - /// Retry counter. Returns `0` when the register is unset. pub fn retry_count(&self) -> u32 { self.retry_count.unwrap_or(0).max(0) as u32 @@ -218,16 +199,6 @@ impl WorkItem { self.qa_mode.as_deref() } - /// Whether a mergemaster spawn has already been attempted. Returns `false` when unset. - pub fn mergemaster_attempted(&self) -> bool { - self.mergemaster_attempted.unwrap_or(false) - } - - /// Whether the item is held for human review. Returns `false` when unset. - pub fn review_hold(&self) -> bool { - self.review_hold.unwrap_or(false) - } - /// Item type (`"story"`, `"bug"`, `"spike"`, `"refactor"`, `"epic"`), or /// `None` when the register is unset. pub fn item_type(&self) -> Option<&str> { @@ -239,13 +210,6 @@ impl WorkItem { self.epic.as_deref() } - /// Whether the item is frozen (story 934, stage 4). Returns `false` when - /// the register is unset. Orthogonal to [`Self::stage`]: a frozen story - /// stays at its current stage but is skipped by the auto-assigner. - pub fn frozen(&self) -> bool { - self.frozen.unwrap_or(false) - } - /// Construct a `WorkItem` for use in tests outside `crdt_state::*`. /// /// Within `crdt_state` use a struct literal directly (fields are `pub(super)`). @@ -258,17 +222,13 @@ impl WorkItem { name: Option, agent: Option, retry_count: Option, - blocked: Option, depends_on: Option>, claimed_by: Option, claimed_at: Option, merged_at: Option, qa_mode: Option, - mergemaster_attempted: Option, - review_hold: Option, item_type: Option, epic: Option, - frozen: Option, ) -> Self { Self { story_id: story_id.into(), @@ -276,17 +236,13 @@ impl WorkItem { name, agent, retry_count, - blocked, depends_on, claimed_by, claimed_at, merged_at, qa_mode, - mergemaster_attempted, - review_hold, item_type, epic, - frozen, } } } diff --git a/server/src/crdt_state/write/item.rs b/server/src/crdt_state/write/item.rs index 3015300f..eea5c1e9 100644 --- a/server/src/crdt_state/write/item.rs +++ b/server/src/crdt_state/write/item.rs @@ -82,15 +82,15 @@ pub fn set_epic(story_id: &str, epic_id: Option<&str>) -> bool { true } -/// Set the `review_hold` CRDT flag for a pipeline item (sub-story 932). +/// Set the `resume_to` CRDT register for a pipeline item (story 945). /// -/// `true` marks the item as held for human review at a pipeline-stage boundary; -/// the auto-assigner skips items with this flag. `false` clears the hold and -/// is written explicitly (the register is not removed) so the cleared state -/// survives CRDT replay correctly. +/// This sibling register stores the wire-form stage name (`"coding"`, `"qa"`, +/// etc.) that a `Stage::Frozen` or `Stage::ReviewHold` variant should resume +/// to. Empty string means "no resume target stored" (defaults to `Coding` +/// on read). /// /// Returns `true` if the item was found and the op was applied, `false` otherwise. -pub fn set_review_hold(story_id: &str, value: bool) -> bool { +pub fn set_resume_to(story_id: &str, stage: &Stage) -> bool { let Some(state_mutex) = get_crdt() else { return false; }; @@ -100,54 +100,8 @@ pub fn set_review_hold(story_id: &str, value: bool) -> bool { let Some(&idx) = state.index.get(story_id) else { return false; }; - apply_and_persist(&mut state, |s| s.crdt.doc.items[idx].review_hold.set(value)); - true -} - -/// Set the `frozen` CRDT flag for a pipeline item (story 934, stage 4). -/// -/// `true` freezes the story at its current `Stage` — the auto-assigner skips -/// it but the stage register is untouched. `false` unfreezes; the story -/// remains at its current stage and resumes auto-assignment. Both writes -/// are explicit (not removals) so the cleared state survives CRDT replay. -/// -/// Returns `true` if the item was found and the op was applied, `false` otherwise. -pub fn set_frozen(story_id: &str, value: bool) -> bool { - let Some(state_mutex) = get_crdt() else { - return false; - }; - let Ok(mut state) = state_mutex.lock() else { - return false; - }; - let Some(&idx) = state.index.get(story_id) else { - return false; - }; - apply_and_persist(&mut state, |s| s.crdt.doc.items[idx].frozen.set(value)); - true -} - -/// Set the `mergemaster_attempted` CRDT flag for a pipeline item. -/// -/// Passing `true` records that a mergemaster session has been spawned for this -/// item, preventing repeated auto-spawns across restarts. -/// Passing `false` explicitly writes `false` (does not remove the register) so -/// the cleared state is distinguishable from an unset register and survives -/// CRDT replay correctly. -/// -/// Returns `true` if the item was found and the op was applied, `false` otherwise. -pub fn set_mergemaster_attempted(story_id: &str, value: bool) -> bool { - let Some(state_mutex) = get_crdt() else { - return false; - }; - let Ok(mut state) = state_mutex.lock() else { - return false; - }; - let Some(&idx) = state.index.get(story_id) else { - return false; - }; - apply_and_persist(&mut state, |s| { - s.crdt.doc.items[idx].mergemaster_attempted.set(value) - }); + let value = stage_dir_name(stage).to_string(); + apply_and_persist(&mut state, |s| s.crdt.doc.items[idx].resume_to.set(value)); true } @@ -244,7 +198,6 @@ pub fn write_item( name: Option<&str>, agent: Option<&str>, retry_count: Option, - blocked: Option, depends_on: Option<&str>, claimed_by: Option<&str>, claimed_at: Option, @@ -292,9 +245,6 @@ pub fn write_item( s.crdt.doc.items[idx].retry_count.set(rc as f64) }); } - if let Some(b) = blocked { - apply_and_persist(&mut state, |s| s.crdt.doc.items[idx].blocked.set(b)); - } if let Some(d) = depends_on { apply_and_persist(&mut state, |s| { s.crdt.doc.items[idx].depends_on.set(d.to_string()) @@ -335,17 +285,14 @@ pub fn write_item( "name": name.unwrap_or(""), "agent": agent.unwrap_or(""), "retry_count": retry_count.unwrap_or(0) as f64, - "blocked": blocked.unwrap_or(false), "depends_on": depends_on.unwrap_or(""), "claimed_by": claimed_by.unwrap_or(""), "claimed_at": claimed_at.unwrap_or(0.0), "merged_at": merged_at.unwrap_or(0.0), "qa_mode": "", - "mergemaster_attempted": false, - "review_hold": false, "item_type": "", "epic": "", - "frozen": false, + "resume_to": "", }) .into(); @@ -366,17 +313,14 @@ pub fn write_item( item.name.advance_seq(floor); item.agent.advance_seq(floor); item.retry_count.advance_seq(floor); - item.blocked.advance_seq(floor); item.depends_on.advance_seq(floor); item.claimed_by.advance_seq(floor); item.claimed_at.advance_seq(floor); item.merged_at.advance_seq(floor); item.qa_mode.advance_seq(floor); - item.mergemaster_attempted.advance_seq(floor); - item.review_hold.advance_seq(floor); item.item_type.advance_seq(floor); item.epic.advance_seq(floor); - item.frozen.advance_seq(floor); + item.resume_to.advance_seq(floor); } // Broadcast a CrdtEvent for the new item. @@ -404,7 +348,6 @@ pub fn write_item_str( name: Option<&str>, agent: Option<&str>, retry_count: Option, - blocked: Option, depends_on: Option<&str>, claimed_by: Option<&str>, claimed_at: Option, @@ -436,7 +379,6 @@ pub fn write_item_str( name, agent, retry_count, - blocked, depends_on, claimed_by, claimed_at, @@ -463,25 +405,6 @@ pub fn set_retry_count(story_id: &str, count: i64) { } } -/// Set the `blocked` register on a story to the given value. -/// -/// Pure metadata operation — the item's stage is not changed. -/// Use this alongside a state-machine transition out of `Blocked` / -/// `MergeFailure` to keep the legacy `blocked` register in sync with the -/// typed stage post-865 (where YAML side-effects no longer clear the -/// register on their own). -pub fn set_blocked(story_id: &str, blocked: bool) { - let Some(state_mutex) = get_crdt() else { - return; - }; - let Ok(mut state) = state_mutex.lock() else { - return; - }; - if let Some(&idx) = state.index.get(story_id) { - apply_and_persist(&mut state, |s| s.crdt.doc.items[idx].blocked.set(blocked)); - } -} - /// Increment `retry_count` by 1 and return the new value. /// /// Pure metadata operation — the item's stage is not changed. diff --git a/server/src/crdt_state/write/migrations.rs b/server/src/crdt_state/write/migrations.rs index 5806b76f..49fd69ef 100644 --- a/server/src/crdt_state/write/migrations.rs +++ b/server/src/crdt_state/write/migrations.rs @@ -212,8 +212,8 @@ fn legacy_stage_to_clean(s: &str) -> Option<&'static str> { /// vocabulary (`"coding"`, `"merge"`, etc.). /// /// Items that were at `"7_frozen"` additionally get the new `frozen` flag set -/// — the stage variant `Frozen` was dropped in story 934 stage 4 in favour of -/// an orthogonal CRDT register. +/// Story 945: the `Frozen` variant returns as `Stage::Frozen { resume_to }`, +/// replacing the orthogonal CRDT register that briefly existed in 934 stage 4. /// /// One-time startup migration: items that have transitioned at least once /// since story 934 stage 1 (which made writes emit clean form) are no-ops. @@ -222,8 +222,10 @@ pub fn migrate_legacy_stage_strings() { return; }; - // First pass: collect (index, clean_stage, set_frozen) for items that - // still carry legacy stage strings. + // First pass: collect (index, clean_stage, was_frozen) for items that + // still carry legacy stage strings. `was_frozen` triggers writing + // `resume_to = "backlog"` so the post-945 typed projection reads back as + // `Stage::Frozen { resume_to: Stage::Backlog }`. let migrations: Vec<(usize, &'static str, bool)> = { let Ok(state) = state_mutex.lock() else { return; @@ -239,7 +241,11 @@ pub fn migrate_legacy_stage_strings() { }; let clean = legacy_stage_to_clean(¤t)?; let was_frozen = current == "7_frozen"; - Some((idx, clean, was_frozen)) + // For legacy frozen items, store the post-945 stage as + // "frozen" rather than "backlog" so the typed projection + // produces `Stage::Frozen` again. + let stage_out = if was_frozen { "frozen" } else { clean }; + Some((idx, stage_out, was_frozen)) }) .collect() }; @@ -258,12 +264,14 @@ pub fn migrate_legacy_stage_strings() { s.crdt.doc.items[idx].stage.set(clean.to_string()) }); if was_frozen { - apply_and_persist(&mut state, |s| s.crdt.doc.items[idx].frozen.set(true)); + apply_and_persist(&mut state, |s| { + s.crdt.doc.items[idx].resume_to.set("backlog".to_string()) + }); } } slog!( "[crdt] Migrated {count} legacy stage strings to clean wire form \ - ({frozen_count} of which were '7_frozen' → backlog + frozen=true)" + ({frozen_count} of which were '7_frozen' → frozen + resume_to=backlog)" ); } @@ -292,7 +300,6 @@ mod stage_migration_tests { None, None, None, - None, ); // Then overwrite the stage register with the raw legacy string, // bypassing `db::normalise_stage_str` / `write_item_str`'s mapping. @@ -353,27 +360,33 @@ mod stage_migration_tests { } #[test] - fn migrate_collapses_7_frozen_to_backlog_and_sets_frozen_flag() { + fn migrate_promotes_7_frozen_to_typed_frozen_variant() { init_for_test(); let story_id = "9510_legacy_frozen"; seed_with_raw_stage(story_id, "7_frozen"); - // Sanity: before migration, the frozen flag is false. - let before = read_item(story_id).expect("seeded item exists"); - assert!(!before.frozen(), "frozen flag should start false"); + // Sanity: before migration, the projection's legacy fallback maps + // raw `"7_frozen"` → `Stage::Backlog` (frozen state is lost without the + // migration's resume_to write). + let before = read_item(story_id).expect("legacy 7_frozen should still project"); + assert!( + matches!(before.stage(), crate::pipeline_state::Stage::Backlog), + "raw 7_frozen should fall back to Backlog before migration; got {:?}", + before.stage() + ); migrate_legacy_stage_strings(); let after = read_item(story_id).expect("item must still exist after migration"); - assert!( - matches!(after.stage(), crate::pipeline_state::Stage::Backlog), - "7_frozen should collapse to Backlog: got {:?}", - after.stage() - ); - assert!( - after.frozen(), - "frozen flag should be set after 7_frozen migration" - ); + match after.stage() { + crate::pipeline_state::Stage::Frozen { resume_to } => { + assert!( + matches!(**resume_to, crate::pipeline_state::Stage::Backlog), + "resume_to should default to Backlog for migrated 7_frozen items" + ); + } + other => panic!("7_frozen should migrate to Stage::Frozen; got {other:?}"), + } } #[test] @@ -390,7 +403,6 @@ mod stage_migration_tests { None, None, None, - None, ); seed_with_raw_stage("9521_needs_migration", "2_current"); diff --git a/server/src/crdt_state/write/mod.rs b/server/src/crdt_state/write/mod.rs index e45ad42a..34272b05 100644 --- a/server/src/crdt_state/write/mod.rs +++ b/server/src/crdt_state/write/mod.rs @@ -10,8 +10,8 @@ mod migrations; mod tests; pub use item::{ - bump_retry_count, set_agent, set_blocked, set_depends_on, set_epic, set_frozen, set_item_type, - set_mergemaster_attempted, set_name, set_qa_mode, set_retry_count, set_review_hold, write_item, + bump_retry_count, set_agent, set_depends_on, set_epic, set_item_type, set_name, set_qa_mode, + set_resume_to, set_retry_count, write_item, }; #[cfg(test)] diff --git a/server/src/crdt_state/write/tests.rs b/server/src/crdt_state/write/tests.rs index 87f564eb..6f4d2a20 100644 --- a/server/src/crdt_state/write/tests.rs +++ b/server/src/crdt_state/write/tests.rs @@ -100,7 +100,6 @@ fn migrate_story_ids_to_numeric_rewrites_slug_ids() { None, None, None, - None, ); let result = migrate_story_ids_to_numeric(); @@ -133,7 +132,6 @@ fn migrate_story_ids_to_numeric_is_idempotent() { None, None, None, - None, ); // First call — nothing to migrate. @@ -163,20 +161,8 @@ fn migrate_story_ids_to_numeric_skips_conflict() { None, None, None, - None, - ); - write_item_str( - "44", - "2_current", - None, - None, - None, - None, - None, - None, - None, - None, ); + write_item_str("44", "2_current", None, None, None, None, None, None, None); let result = migrate_story_ids_to_numeric(); // The slug entry must NOT be migrated because "44" is already occupied. @@ -210,7 +196,6 @@ fn migrate_story_ids_to_numeric_preserves_stage_and_name() { None, None, None, - None, ); migrate_story_ids_to_numeric(); @@ -236,7 +221,6 @@ fn migrate_names_from_slugs_fills_empty_names() { None, None, None, - None, ); // Before migration the name should be empty. @@ -271,7 +255,6 @@ fn migrate_names_from_slugs_leaves_existing_names_unchanged() { None, None, None, - None, ); migrate_names_from_slugs(); @@ -309,7 +292,6 @@ fn set_depends_on_round_trip_and_clear() { None, None, None, - None, ); // Set depends_on to [837] and verify CRDT register holds the list. @@ -349,62 +331,6 @@ fn set_depends_on_returns_false_for_unknown_story() { ); } -// ── set_mergemaster_attempted regression tests ─────────────────────────── - -#[test] -fn set_mergemaster_attempted_true_then_false_flips_register() { - init_for_test(); - - write_item_str( - "873_story_mergemaster_flip", - "4_merge", - None, - None, - None, - None, - None, - None, - None, - None, - ); - - // Set true — register must read back as true. - let ok = set_mergemaster_attempted("873_story_mergemaster_flip", true); - assert!( - ok, - "set_mergemaster_attempted should return true for known item" - ); - let view = read_item("873_story_mergemaster_flip").unwrap(); - assert_eq!( - view.mergemaster_attempted, - Some(true), - "CRDT register should hold true after setting true" - ); - - // Set false — register must flip back to false (not unset). - let ok = set_mergemaster_attempted("873_story_mergemaster_flip", false); - assert!( - ok, - "set_mergemaster_attempted(false) should return true for known item" - ); - let view = read_item("873_story_mergemaster_flip").unwrap(); - assert_eq!( - view.mergemaster_attempted, - Some(false), - "CRDT register should hold false after explicit clear" - ); -} - -#[test] -fn set_mergemaster_attempted_returns_false_for_unknown_story() { - init_for_test(); - let ok = set_mergemaster_attempted("nonexistent_story_mm", true); - assert!( - !ok, - "set_mergemaster_attempted should return false for unknown story_id" - ); -} - // ── set_agent tests ────────────────────────────────────────────────────── #[test] @@ -421,7 +347,6 @@ fn set_agent_some_writes_name() { None, None, None, - None, ); let found = set_agent("871_story_set_agent_write", Some("coder-1")); @@ -449,7 +374,6 @@ fn set_agent_none_clears_register() { None, None, None, - None, ); // Confirm agent is set. @@ -495,7 +419,6 @@ fn set_qa_mode_round_trip_server_then_human() { None, None, None, - None, ); // Set qa=server via typed path and assert CRDT register reflects it. @@ -551,7 +474,6 @@ fn bump_retry_count_increments_by_one() { None, None, None, - None, ); let v1 = bump_retry_count("9001_story_bump_test"); @@ -581,7 +503,6 @@ fn set_retry_count_resets_to_zero() { None, None, None, - None, ); set_retry_count("9002_story_set_test", 0); @@ -765,7 +686,6 @@ async fn tombstone_survives_concurrent_writes() { None, None, None, - None, ); assert!( read_item(story_id).is_some(), @@ -787,7 +707,6 @@ async fn tombstone_survives_concurrent_writes() { None, None, None, - None, ); tokio::time::sleep(tokio::time::Duration::from_millis(10)).await; } diff --git a/server/src/db/mod.rs b/server/src/db/mod.rs index ed3bac7a..74e7c600 100644 --- a/server/src/db/mod.rs +++ b/server/src/db/mod.rs @@ -57,21 +57,20 @@ mod tests { write_story( &stage_dir, "10_story_shadow_test.md", - "---\nname: Shadow Test\nagent: coder-opus\nretry_count: 2\nblocked: false\n---\n# Story\n", + "---\nname: Shadow Test\nagent: coder-opus\nretry_count: 2\n---\n# Story\n", ); // Perform the upsert directly (bypass the global singleton). let now = chrono::Utc::now().to_rfc3339(); sqlx::query( "INSERT INTO pipeline_items \ - (id, name, stage, agent, retry_count, blocked, depends_on, content, created_at, updated_at) \ - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?9) \ + (id, name, stage, agent, retry_count, depends_on, content, created_at, updated_at) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?8) \ ON CONFLICT(id) DO UPDATE SET \ name = excluded.name, \ stage = excluded.stage, \ agent = excluded.agent, \ retry_count = excluded.retry_count, \ - blocked = excluded.blocked, \ depends_on = excluded.depends_on, \ content = COALESCE(excluded.content, pipeline_items.content), \ updated_at = excluded.updated_at", @@ -81,7 +80,6 @@ mod tests { .bind("2_current") .bind("coder-opus") .bind(2_i64) - .bind(0_i64) .bind(Option::::None) .bind("---\nname: Shadow Test\n---\n# Story\n") .bind(&now) @@ -122,15 +120,14 @@ mod tests { let content = "---\nname: Test\n---\n# Story\n"; sqlx::query( "INSERT INTO pipeline_items \ - (id, name, stage, agent, retry_count, blocked, depends_on, content, created_at, updated_at) \ - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?9)", + (id, name, stage, agent, retry_count, depends_on, content, created_at, updated_at) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?8)", ) .bind("99_story_col_test") .bind(Option::::None) .bind("1_backlog") .bind(Option::::None) .bind(Option::::None) - .bind(Option::::None) .bind(Option::::None) .bind(content) .bind(&now) @@ -162,15 +159,14 @@ mod tests { // Insert initial row in backlog. sqlx::query( "INSERT INTO pipeline_items \ - (id, name, stage, agent, retry_count, blocked, depends_on, content, created_at, updated_at) \ - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?9)", + (id, name, stage, agent, retry_count, depends_on, content, created_at, updated_at) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?8)", ) .bind("5_story_move") .bind("Move Me") .bind("1_backlog") .bind(Option::::None) .bind(Option::::None) - .bind(Option::::None) .bind(Option::::None) .bind("---\nname: Move Me\n---\n") .bind(&now) @@ -181,14 +177,13 @@ mod tests { // Upsert with new stage (simulating move to current). sqlx::query( "INSERT INTO pipeline_items \ - (id, name, stage, agent, retry_count, blocked, depends_on, content, created_at, updated_at) \ - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?9) \ + (id, name, stage, agent, retry_count, depends_on, content, created_at, updated_at) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?8) \ ON CONFLICT(id) DO UPDATE SET \ name = excluded.name, \ stage = excluded.stage, \ agent = excluded.agent, \ retry_count = excluded.retry_count, \ - blocked = excluded.blocked, \ depends_on = excluded.depends_on, \ content = COALESCE(excluded.content, pipeline_items.content), \ updated_at = excluded.updated_at", @@ -198,7 +193,6 @@ mod tests { .bind("2_current") .bind(Option::::None) .bind(Option::::None) - .bind(Option::::None) .bind(Option::::None) .bind(Option::::None) // content NULL → COALESCE preserves existing .bind(&now) @@ -266,15 +260,14 @@ mod tests { // Insert a live row. sqlx::query( "INSERT INTO pipeline_items \ - (id, name, stage, agent, retry_count, blocked, depends_on, content, created_at, updated_at) \ - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?9)", + (id, name, stage, agent, retry_count, depends_on, content, created_at, updated_at) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?8)", ) .bind("42_story_to_delete") .bind("Delete Me") .bind("2_current") .bind(Option::::None) .bind(Option::::None) - .bind(Option::::None) .bind(Option::::None) .bind("---\nname: Delete Me\n---\n") .bind(&now) @@ -322,7 +315,6 @@ mod tests { name: Some("Typed Name".into()), agent: Some("coder-1".into()), retry_count: Some(2), - blocked: Some(true), depends_on: Some(vec![100, 200]), }; write_item_with_content(story_id, "2_current", content, meta); @@ -332,7 +324,6 @@ mod tests { assert_eq!(view.name(), Some("Typed Name")); assert_eq!(view.agent(), Some("coder-1")); assert_eq!(view.retry_count(), 2); - assert!(view.blocked()); assert_eq!(view.depends_on(), &[100, 200]); // Content is stored verbatim (no parsing, no rewrite). @@ -416,7 +407,6 @@ mod tests { None, None, None, - None, ); write_content( story_id, diff --git a/server/src/db/ops.rs b/server/src/db/ops.rs index 4e56c557..545af3b5 100644 --- a/server/src/db/ops.rs +++ b/server/src/db/ops.rs @@ -18,7 +18,6 @@ pub struct ItemMeta { pub name: Option, pub agent: Option, pub retry_count: Option, - pub blocked: Option, pub depends_on: Option>, } @@ -50,12 +49,11 @@ fn normalise_stage_str(stage: &str) -> &str { "3_qa" => "qa", "4_merge" => "merge", "4_merge_failure" => "merge_failure", + "4_merge_failure_final" => "merge_failure_final", "5_done" => "done", "6_archived" => "archived", - // `7_frozen` has no direct clean equivalent (the variant was - // removed in story 934 stage 4). Returning the unmapped string - // makes `Stage::from_dir` return None, so the write is logged and - // skipped — frozen items should be seeded via the `frozen` flag. + "7_frozen" => "frozen", + "7_review_hold" => "review_hold", other => other, } } @@ -94,7 +92,6 @@ pub fn write_item_with_content(story_id: &str, stage: &str, content: &str, meta: meta.name.as_deref(), meta.agent.as_deref(), meta.retry_count, - meta.blocked, depends_on_json.as_deref(), None, None, @@ -109,7 +106,6 @@ pub fn write_item_with_content(story_id: &str, stage: &str, content: &str, meta: name: meta.name, agent: meta.agent, retry_count: meta.retry_count, - blocked: meta.blocked, depends_on: depends_on_json, content: Some(content.to_string()), }; @@ -139,10 +135,10 @@ pub fn move_item_stage( _ => None, }; - // Story 929: metadata (name/agent/blocked/depends_on) is owned by the - // CRDT typed registers — no need to re-derive it from the content body's - // YAML front matter on every stage transition. Pass `None` for those - // fields so write_item leaves the existing registers untouched. + // Story 929: metadata (name/agent/depends_on) is owned by the CRDT typed + // registers — no need to re-derive it from the content body's YAML front + // matter on every stage transition. Pass `None` for those fields so + // write_item leaves the existing registers untouched. let new_stage = normalise_stage_str(new_stage); let Some(typed_stage) = crate::pipeline_state::Stage::from_dir(new_stage) else { crate::slog!( @@ -161,7 +157,6 @@ pub fn move_item_stage( None, None, None, - None, merged_at_ts, ); // Bug 780: stage transitions reset retry_count to 0. retry_count tracks @@ -177,7 +172,6 @@ pub fn move_item_stage( let view = crate::crdt_state::read_item(story_id); let name = view.as_ref().and_then(|v| v.name().map(str::to_string)); let agent = view.as_ref().and_then(|v| v.agent().map(str::to_string)); - let blocked = view.as_ref().map(|v| v.blocked()); let depends_on = view .as_ref() .map(|v| v.depends_on()) @@ -189,7 +183,6 @@ pub fn move_item_stage( name, agent, retry_count: Some(0), - blocked, depends_on, content, }; @@ -212,7 +205,6 @@ pub fn delete_item(story_id: &str) { name: None, agent: None, retry_count: None, - blocked: None, depends_on: None, content: None, }; diff --git a/server/src/db/shadow_write.rs b/server/src/db/shadow_write.rs index 77a8f7a2..d45a8382 100644 --- a/server/src/db/shadow_write.rs +++ b/server/src/db/shadow_write.rs @@ -18,7 +18,6 @@ pub(super) struct PipelineWriteMsg { pub(super) name: Option, pub(super) agent: Option, pub(super) retry_count: Option, - pub(super) blocked: Option, pub(super) depends_on: Option, pub(super) content: Option, } @@ -84,14 +83,13 @@ pub async fn init(db_path: &Path) -> Result<(), sqlx::Error> { let now = chrono::Utc::now().to_rfc3339(); let result = sqlx::query( "INSERT INTO pipeline_items \ - (id, name, stage, agent, retry_count, blocked, depends_on, content, created_at, updated_at) \ - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?9) \ + (id, name, stage, agent, retry_count, depends_on, content, created_at, updated_at) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?8) \ ON CONFLICT(id) DO UPDATE SET \ name = excluded.name, \ stage = excluded.stage, \ agent = excluded.agent, \ retry_count = excluded.retry_count, \ - blocked = excluded.blocked, \ depends_on = excluded.depends_on, \ content = COALESCE(excluded.content, pipeline_items.content), \ updated_at = excluded.updated_at", @@ -101,7 +99,6 @@ pub async fn init(db_path: &Path) -> Result<(), sqlx::Error> { .bind(&msg.stage) .bind(&msg.agent) .bind(msg.retry_count) - .bind(msg.blocked.map(|b| b as i64)) .bind(&msg.depends_on) .bind(&msg.content) .bind(&now) diff --git a/server/src/http/agents/tests.rs b/server/src/http/agents/tests.rs index e1fcf885..2391fce3 100644 --- a/server/src/http/agents/tests.rs +++ b/server/src/http/agents/tests.rs @@ -285,7 +285,6 @@ async fn get_work_item_content_returns_content_from_backlog() { None, None, None, - None, ); let ctx = AppContext::new_test(root.to_path_buf()); let api = AgentsApi { ctx: Arc::new(ctx) }; @@ -320,7 +319,6 @@ async fn get_work_item_content_returns_content_from_current() { None, None, None, - None, ); let ctx = AppContext::new_test(root.to_path_buf()); let api = AgentsApi { ctx: Arc::new(ctx) }; diff --git a/server/src/http/mcp/diagnostics/mod.rs b/server/src/http/mcp/diagnostics/mod.rs index 2892b77c..ad013c38 100644 --- a/server/src/http/mcp/diagnostics/mod.rs +++ b/server/src/http/mcp/diagnostics/mod.rs @@ -92,7 +92,6 @@ pub(crate) fn tool_dump_crdt(args: &Value) -> Result { "name": item.name, "agent": item.agent, "retry_count": item.retry_count, - "blocked": item.blocked, "depends_on": item.depends_on, "claimed_by": item.claimed_by, "claimed_at": item.claimed_at, diff --git a/server/src/http/mcp/merge_tools.rs b/server/src/http/mcp/merge_tools.rs index 5021e944..d180e3c1 100644 --- a/server/src/http/mcp/merge_tools.rs +++ b/server/src/http/mcp/merge_tools.rs @@ -291,7 +291,6 @@ mod tests { None, None, None, - None, ); let tmp = tempfile::tempdir().unwrap(); let ctx = test_ctx(tmp.path()); @@ -318,7 +317,6 @@ mod tests { None, None, None, - None, ); let tmp = tempfile::tempdir().unwrap(); let ctx = test_ctx(tmp.path()); diff --git a/server/src/http/mcp/qa_tools.rs b/server/src/http/mcp/qa_tools.rs index 79f86e2e..56a4abd2 100644 --- a/server/src/http/mcp/qa_tools.rs +++ b/server/src/http/mcp/qa_tools.rs @@ -54,8 +54,16 @@ pub(super) async fn tool_approve_qa(args: &Value, ctx: &AppContext) -> Result Result Result = parse_ac_items(&contents) .into_iter() @@ -357,7 +346,7 @@ mod tests { } #[tokio::test] - async fn tool_status_returns_blocked_retry_count_and_depends_on() { + async fn tool_status_returns_retry_count_and_depends_on() { let tmp = tempdir().unwrap(); crate::crdt_state::init_for_test(); @@ -369,7 +358,6 @@ mod tests { story_content, crate::db::ItemMeta::named("Blocked Story"), ); - crate::crdt_state::set_blocked("9887_story_blocked_test", true); crate::crdt_state::set_retry_count("9887_story_blocked_test", 3); crate::crdt_state::set_depends_on("9887_story_blocked_test", &[100, 200]); @@ -379,7 +367,6 @@ mod tests { .unwrap(); let parsed: serde_json::Value = serde_json::from_str(&result).unwrap(); - assert_eq!(parsed["front_matter"]["blocked"], true); assert_eq!(parsed["front_matter"]["retry_count"], 3); let depends_on = parsed["front_matter"]["depends_on"].as_array().unwrap(); assert_eq!(depends_on.len(), 2); diff --git a/server/src/http/mcp/story_tools/epic.rs b/server/src/http/mcp/story_tools/epic.rs index aa82ce0c..932108f1 100644 --- a/server/src/http/mcp/story_tools/epic.rs +++ b/server/src/http/mcp/story_tools/epic.rs @@ -127,20 +127,20 @@ pub(crate) fn tool_show_epic(args: &Value, _ctx: &AppContext) -> Result "backlog", - Stage::Coding => "current", - Stage::Qa => "qa", - Stage::Merge { .. } => "merge", - Stage::Done { .. } => "done", - Stage::Archived { .. } => "archived", - Stage::MergeFailure { .. } => "merge_failure", - Stage::Blocked { .. } => "blocked", - } + // Story 945: Frozen / ReviewHold / MergeFailureFinal are first-class + // Stage variants — no more orthogonal boolean flags. + let stage_name = match &item.stage { + Stage::Upcoming | Stage::Backlog => "backlog", + Stage::Coding => "current", + Stage::Qa => "qa", + Stage::Merge { .. } => "merge", + Stage::Done { .. } => "done", + Stage::Archived { .. } => "archived", + Stage::MergeFailure { .. } => "merge_failure", + Stage::MergeFailureFinal { .. } => "merge_failure_final", + Stage::Blocked { .. } => "blocked", + Stage::Frozen { .. } => "frozen", + Stage::ReviewHold { .. } => "review_hold", }; if matches!(item.stage, Stage::Done { .. }) { done += 1; diff --git a/server/src/http/mcp/story_tools/story/update.rs b/server/src/http/mcp/story_tools/story/update.rs index 8f3a447c..ad0f1a12 100644 --- a/server/src/http/mcp/story_tools/story/update.rs +++ b/server/src/http/mcp/story_tools/story/update.rs @@ -91,12 +91,30 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result { - if let Some(b) = value.as_bool() { - crate::crdt_state::set_review_hold(story_id, b); - } else if value.as_str() == Some("true") { - crate::crdt_state::set_review_hold(story_id, true); - } else if value.as_str() == Some("false") { - crate::crdt_state::set_review_hold(story_id, false); + let want_hold = match value { + Value::Bool(b) => Some(*b), + Value::String(s) if s == "true" => Some(true), + Value::String(s) if s == "false" => Some(false), + _ => None, + }; + match want_hold { + Some(true) => { + crate::pipeline_state::apply_transition_str( + story_id, + crate::pipeline_state::PipelineEvent::ReviewHold { + reason: "Set via update_story".to_string(), + }, + None, + )?; + } + Some(false) => { + crate::pipeline_state::apply_transition_str( + story_id, + crate::pipeline_state::PipelineEvent::ReviewHoldCleared, + None, + )?; + } + None => {} } } "frozen" => { @@ -126,8 +144,12 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result { - if let Some(b) = value.as_bool() { - crate::crdt_state::set_mergemaster_attempted(story_id, b); + if let Some(true) = value.as_bool() { + crate::pipeline_state::apply_transition_str( + story_id, + crate::pipeline_state::PipelineEvent::MergemasterAttempted, + None, + )?; } } other => { diff --git a/server/src/http/mod.rs b/server/src/http/mod.rs index d4b89fb2..a6db6065 100644 --- a/server/src/http/mod.rs +++ b/server/src/http/mod.rs @@ -213,7 +213,6 @@ pub fn debug_crdt_handler(req: &poem::Request) -> poem::Response { "name": item.name, "agent": item.agent, "retry_count": item.retry_count, - "blocked": item.blocked, "depends_on": item.depends_on, "claimed_by": item.claimed_by, "claimed_at": item.claimed_at, diff --git a/server/src/http/workflow/pipeline.rs b/server/src/http/workflow/pipeline.rs index 152152ab..e3006a29 100644 --- a/server/src/http/workflow/pipeline.rs +++ b/server/src/http/workflow/pipeline.rs @@ -94,11 +94,15 @@ pub fn load_pipeline_state(ctx: &AppContext) -> Result { let sid = &item.story_id.0; let agent = agent_map.get(sid).cloned(); - // Stories 929/932/933: review_hold, qa_mode, epic_id come from typed - // CRDT registers. merge_failure detail lives on the MergeJob CRDT - // entry (same as status_tools.rs). + // Story 945: review_hold is `Stage::ReviewHold`; qa_mode and epic_id + // 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 = view.as_ref().map(|v| v.review_hold()).filter(|b| *b); + let review_hold = if item.stage.is_review_hold() { + Some(true) + } else { + None + }; let qa = view.as_ref().and_then(|v| v.qa_mode().map(str::to_string)); let epic_id = view.as_ref().and_then(|v| v.epic().map(str::to_string)); let merge_failure = crate::crdt_state::read_merge_job(sid).and_then(|j| j.error); @@ -151,6 +155,9 @@ pub fn load_pipeline_state(ctx: &AppContext) -> Result { Stage::Qa => state.qa.push(story), Stage::Merge { .. } => state.merge.push(story), Stage::MergeFailure { .. } => state.merge.push(story), // show merge failures with merge + Stage::MergeFailureFinal { .. } => state.merge.push(story), // mergemaster gave up + Stage::ReviewHold { .. } => state.qa.push(story), // review-held shown with QA + Stage::Frozen { .. } => state.backlog.push(story), // paused, route to backlog Stage::Done { .. } => state.done.push(story), Stage::Archived { .. } => {} // skip archived } diff --git a/server/src/io/story_metadata/parser.rs b/server/src/io/story_metadata/parser.rs index 21b32ddb..7c66010a 100644 --- a/server/src/io/story_metadata/parser.rs +++ b/server/src/io/story_metadata/parser.rs @@ -29,14 +29,16 @@ pub fn resolve_qa_mode(story_id: &str, default: QaMode) -> QaMode { .unwrap_or(default) } -/// Return `true` if the story's `frozen` CRDT flag is set (story 934, stage 4). +/// Return `true` if the story is currently in `Stage::Frozen` +/// (story 945: frozen is a typed stage variant, not a flag). /// /// Used by the pipeline advance code to suppress stage transitions for frozen -/// stories. `frozen` is orthogonal to [`Stage`]: a frozen story still has its -/// stage register intact but is paused until unfrozen. +/// stories. pub fn is_story_frozen_in_store(story_id: &str) -> bool { - crate::crdt_state::read_item(story_id) - .map(|view| view.frozen()) + crate::pipeline_state::read_typed(story_id) + .ok() + .flatten() + .map(|item| item.is_frozen()) .unwrap_or(false) } diff --git a/server/src/io/watcher/mod.rs b/server/src/io/watcher/mod.rs index 96bbcd2d..0d11a47c 100644 --- a/server/src/io/watcher/mod.rs +++ b/server/src/io/watcher/mod.rs @@ -58,6 +58,12 @@ pub fn stage_metadata(stage: &str, item_id: &str) -> Option<(&'static str, Strin Stage::MergeFailure { .. } => { ("merge_failure", format!("huskies: merge_failure {item_id}")) } + Stage::MergeFailureFinal { .. } => ( + "merge_failure_final", + format!("huskies: merge_failure_final {item_id}"), + ), + Stage::Frozen { .. } => ("freeze", format!("huskies: freeze {item_id}")), + Stage::ReviewHold { .. } => ("review_hold", format!("huskies: review_hold {item_id}")), Stage::Done { .. } => ("done", format!("huskies: done {item_id}")), Stage::Archived { .. } => ("accept", format!("huskies: accept {item_id}")), }; diff --git a/server/src/io/watcher/tests.rs b/server/src/io/watcher/tests.rs index 644674ca..a6f77a3e 100644 --- a/server/src/io/watcher/tests.rs +++ b/server/src/io/watcher/tests.rs @@ -166,7 +166,6 @@ fn sweep_uses_crdt_merged_at_not_utc_now() { None, None, None, - None, Some(ten_seconds_ago), ); @@ -199,7 +198,6 @@ fn sweep_keeps_item_newer_than_retention() { None, None, None, - None, Some(one_second_ago), ); diff --git a/server/src/pipeline_state/apply.rs b/server/src/pipeline_state/apply.rs index 4ebbbe48..4a968b6f 100644 --- a/server/src/pipeline_state/apply.rs +++ b/server/src/pipeline_state/apply.rs @@ -100,31 +100,17 @@ pub fn apply_transition_str( /// Freeze a story. /// -/// Story 934, stage 4: `frozen` is now a CRDT flag orthogonal to [`Stage`], -/// so the story stays at its current stage and only the boolean register -/// changes. Returns `Err(NotFound)` if no item exists for `story_id`. +/// Story 945: `Stage::Frozen { resume_to }` is the single source of truth; +/// the previous `frozen: bool` flag has been removed. Transitions any +/// non-terminal stage to `Stage::Frozen { resume_to: }`. pub fn transition_to_frozen(story_id: &str) -> Result<(), ApplyError> { - if read_typed(story_id)?.is_none() { - return Err(ApplyError::NotFound(story_id.to_string())); - } - crate::crdt_state::set_frozen(story_id, true); - crate::slog!("[pipeline/transition] #{}: Freeze (flag set)", story_id); - Ok(()) + apply_transition(story_id, PipelineEvent::Freeze, None).map(|_| ()) } /// Unfreeze a story. /// -/// Story 934, stage 4: paired with [`transition_to_frozen`]; clears the -/// CRDT `frozen` flag without touching the stage register. Returns -/// `Err(NotFound)` if no item exists for `story_id`. +/// Story 945: returns the story to the `resume_to` stage stored on +/// `Stage::Frozen`. pub fn transition_to_unfrozen(story_id: &str) -> Result<(), ApplyError> { - if read_typed(story_id)?.is_none() { - return Err(ApplyError::NotFound(story_id.to_string())); - } - crate::crdt_state::set_frozen(story_id, false); - crate::slog!( - "[pipeline/transition] #{}: Unfreeze (flag cleared)", - story_id - ); - Ok(()) + apply_transition(story_id, PipelineEvent::Unfreeze, None).map(|_| ()) } diff --git a/server/src/pipeline_state/projection.rs b/server/src/pipeline_state/projection.rs index 3b591a02..66622cde 100644 --- a/server/src/pipeline_state/projection.rs +++ b/server/src/pipeline_state/projection.rs @@ -10,7 +10,7 @@ use std::fmt; use crate::crdt_state::PipelineItemView; -use super::{ArchiveReason, PipelineItem, Stage, StoryId, stage_dir_name}; +use super::{PipelineItem, StoryId}; /// Errors from projecting loose CRDT data into typed enums. #[derive(Debug, Clone, PartialEq, Eq)] @@ -60,30 +60,10 @@ impl TryFrom<&PipelineItemView> for PipelineItem { stage: view.stage().clone(), depends_on, retry_count, - frozen: view.frozen(), }) } } -// ── Reverse projection: PipelineItem → stage dir string ───────────────────── - -impl PipelineItem { - /// Convert back to the loose fields that the CRDT write path expects. - /// Returns `(stage_dir, blocked)`. - pub fn to_crdt_fields(&self) -> (&'static str, bool) { - let dir = stage_dir_name(&self.stage); - let blocked = matches!( - self.stage, - Stage::Blocked { .. } - | Stage::Archived { - reason: ArchiveReason::Blocked { .. }, - .. - } - ); - (dir, blocked) - } -} - // ── Bridge to existing CRDT reads ─────────────────────────────────────────── /// Read all pipeline items from the CRDT and project them into typed enums. @@ -120,7 +100,7 @@ pub fn read_typed(story_id: &str) -> Result, ProjectionErro #[cfg(test)] mod tests { use super::*; - use crate::pipeline_state::{BranchName, GitSha}; + use crate::pipeline_state::{ArchiveReason, BranchName, GitSha, Stage}; use chrono::Utc; use std::num::NonZeroU32; @@ -130,9 +110,6 @@ mod tests { fn fb(name: &str) -> BranchName { BranchName(name.to_string()) } - fn sha(s: &str) -> GitSha { - GitSha(s.to_string()) - } fn make_view(story_id: &str, stage: Stage, name: Option<&str>) -> PipelineItemView { PipelineItemView::for_test( @@ -148,10 +125,6 @@ mod tests { None, None, None, - None, - None, - None, - None, ) } @@ -170,7 +143,6 @@ mod tests { Some("Test Story".to_string()), None, None, - None, Some(vec![10, 20]), None, None, @@ -178,9 +150,6 @@ mod tests { None, None, None, - None, - None, - None, ); let item = PipelineItem::try_from(&view).unwrap(); assert_eq!(item.story_id, StoryId("42_story_test".to_string())); @@ -205,10 +174,6 @@ mod tests { None, None, None, - None, - None, - None, - None, ); let item = PipelineItem::try_from(&view).unwrap(); assert!(matches!(item.stage, Stage::Coding)); @@ -263,10 +228,6 @@ mod tests { Some("Test".to_string()), None, None, - Some(true), - None, - None, - None, None, None, None, @@ -296,10 +257,6 @@ mod tests { Some("Test".to_string()), None, None, - Some(false), - None, - None, - None, None, None, None, @@ -318,71 +275,17 @@ mod tests { )); } - // ── Reverse projection tests ──────────────────────────────────────── - #[test] - fn reverse_projection_stage_dirs() { - let cases: Vec<(Stage, &str, bool)> = vec![ - (Stage::Upcoming, "upcoming", false), - (Stage::Backlog, "backlog", false), - (Stage::Coding, "coding", false), - ( - Stage::Blocked { - reason: "stuck".into(), - }, - "blocked", - true, - ), - (Stage::Qa, "qa", false), - ( - Stage::Merge { - feature_branch: fb("f"), - commits_ahead: nz(1), - }, - "merge", - false, - ), - ( - Stage::Done { - merged_at: Utc::now(), - merge_commit: sha("abc"), - }, - "done", - false, - ), - ( - Stage::Archived { - archived_at: Utc::now(), - reason: ArchiveReason::Completed, - }, - "archived", - false, - ), - ( - Stage::Archived { - archived_at: Utc::now(), - reason: ArchiveReason::Blocked { - reason: "stuck".into(), - }, - }, - "archived", - true, - ), - ]; - - for (stage, expected_dir, expected_blocked) in cases { - let item = PipelineItem { - story_id: StoryId("test".into()), - name: "test".into(), - stage, - depends_on: vec![], - retry_count: 0, - frozen: false, - }; - let (dir, blocked) = item.to_crdt_fields(); - assert_eq!(dir, expected_dir); - assert_eq!(blocked, expected_blocked); - } + fn project_frozen_item() { + let view = make_view( + "42_story_test", + Stage::Frozen { + resume_to: Box::new(Stage::Coding), + }, + Some("Frozen Story"), + ); + let item = PipelineItem::try_from(&view).unwrap(); + assert!(item.is_frozen()); } // ── Event bus tests ───────────────────────────────────────────────── @@ -395,4 +298,11 @@ mod tests { let err = ProjectionError::MissingField("story_id"); assert_eq!(err.to_string(), "missing required field: story_id"); } + + // Compile-time check that GitSha is reachable from the test imports + // (mirrors the previous reverse_projection test that used it). + #[test] + fn git_sha_constructs() { + let _ = GitSha("abc".to_string()); + } } diff --git a/server/src/pipeline_state/tests.rs b/server/src/pipeline_state/tests.rs index aa5187d1..9fb6dfbf 100644 --- a/server/src/pipeline_state/tests.rs +++ b/server/src/pipeline_state/tests.rs @@ -300,6 +300,9 @@ fn supersede_from_any_active_or_done() { #[test] fn review_hold_from_active_stages() { + // Story 945: `ReviewHold` transitions to `Stage::ReviewHold { resume_to }` + // with the resume_to set to the originating stage, replacing the legacy + // boolean flag. for s in [Stage::Backlog, Stage::Coding, Stage::Qa] { let result = transition( s.clone(), @@ -307,13 +310,14 @@ fn review_hold_from_active_stages() { reason: "review".into(), }, ); - assert!(matches!( - result, - Ok(Stage::Archived { - reason: ArchiveReason::ReviewHeld { .. }, - .. - }) - )); + let resumed = match result { + Ok(Stage::ReviewHold { resume_to, .. }) => *resume_to, + other => panic!("ReviewHold should produce Stage::ReviewHold; got {other:?}"), + }; + assert_eq!( + resumed, s, + "resume_to should preserve the originating stage" + ); } } @@ -578,7 +582,9 @@ fn cannot_reject_from_archived() { /// to "restore". Tests the freeze/unfreeze API on the apply layer, since /// freeze/unfreeze are no longer pure stage transitions. #[test] -fn freeze_sets_flag_without_changing_stage() { +fn freeze_transitions_to_frozen_variant_with_resume_to() { + // Story 945: freeze/unfreeze move the typed stage to `Stage::Frozen + // { resume_to }` and back, replacing the orthogonal boolean flag. crate::crdt_state::init_for_test(); crate::db::ensure_content_store(); @@ -597,24 +603,26 @@ fn freeze_sets_flag_without_changing_stage() { super::apply::transition_to_frozen(story_id).expect("freeze should succeed"); let item = read_typed(story_id).unwrap().unwrap(); - assert!( - matches!(item.stage, Stage::Coding), - "stage register stays at Coding after freeze: {:?}", - item.stage - ); - assert!(item.is_frozen(), "frozen flag should be set after freeze"); + match &item.stage { + Stage::Frozen { resume_to } => assert!( + matches!(**resume_to, Stage::Coding), + "resume_to should preserve the previous stage; got {resume_to:?}" + ), + other => panic!("stage should be Stage::Frozen after freeze; got {other:?}"), + } + assert!(item.is_frozen(), "is_frozen() should be true after freeze"); super::apply::transition_to_unfrozen(story_id).expect("unfreeze should succeed"); let item = read_typed(story_id).unwrap().unwrap(); assert!( matches!(item.stage, Stage::Coding), - "stage register still at Coding after unfreeze: {:?}", + "stage should return to Coding after unfreeze: {:?}", item.stage ); assert!( !item.is_frozen(), - "frozen flag should be cleared after unfreeze" + "is_frozen() should be false after unfreeze" ); } diff --git a/server/src/pipeline_state/transition.rs b/server/src/pipeline_state/transition.rs index 42a2c836..51fdfb18 100644 --- a/server/src/pipeline_state/transition.rs +++ b/server/src/pipeline_state/transition.rs @@ -59,6 +59,15 @@ pub enum PipelineEvent { Close, /// Manual demotion back to backlog from an active stage. Demote, + /// Story 945: freeze a story at its current stage. + Freeze, + /// Story 945: unfreeze a frozen story, returning to `resume_to`. + Unfreeze, + /// Story 945: clear a `ReviewHold`, returning to `resume_to`. + ReviewHoldCleared, + /// Story 945: mergemaster has been auto-spawned and gave up; transitions + /// `Stage::MergeFailure` → `Stage::MergeFailureFinal`. + MergemasterAttempted, } // ── Per-node execution events ─────────────────────────────────────────────── @@ -98,6 +107,10 @@ pub fn event_label(e: &PipelineEvent) -> &'static str { PipelineEvent::Triage => "Triage", PipelineEvent::Close => "Close", PipelineEvent::Demote => "Demote", + PipelineEvent::Freeze => "Freeze", + PipelineEvent::Unfreeze => "Unfreeze", + PipelineEvent::ReviewHoldCleared => "ReviewHoldCleared", + PipelineEvent::MergemasterAttempted => "MergemasterAttempted", } } @@ -172,13 +185,16 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result Ok(Blocked { reason }), - (Backlog, ReviewHold { reason }) - | (Coding, ReviewHold { reason }) - | (Qa, ReviewHold { reason }) - | (Merge { .. }, ReviewHold { reason }) => Ok(Archived { - archived_at: now, - reason: ArchiveReason::ReviewHeld { reason }, - }), + // Story 945: ReviewHold no longer auto-archives. It transitions the + // story to `Stage::ReviewHold { resume_to, reason }`, preserving the + // current stage as the resume target so a reviewer can clear the + // hold and continue. + (s @ (Backlog | Coding | Qa | Merge { .. }), PipelineEvent::ReviewHold { reason }) => { + Ok(Stage::ReviewHold { + resume_to: Box::new(s), + reason, + }) + } // ── MergeFailed: Merge → MergeFailure (recoverable intermediate) ── (Merge { .. }, MergeFailed { reason }) => Ok(MergeFailure { reason }), @@ -239,6 +255,36 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result Ok(Frozen { + resume_to: Box::new(s), + }), + + // ── Unfreeze: Frozen → resume_to ─────────────────────────────── + (Frozen { resume_to }, Unfreeze) => Ok(*resume_to), + + // ── ReviewHoldCleared: ReviewHold → resume_to ────────────────── + (Stage::ReviewHold { resume_to, .. }, ReviewHoldCleared) => Ok(*resume_to), + + // ── MergemasterAttempted: MergeFailure → MergeFailureFinal ───── + (MergeFailure { reason }, MergemasterAttempted) => Ok(MergeFailureFinal { reason }), + (MergeFailureFinal { reason }, MergemasterAttempted) => Ok(MergeFailureFinal { reason }), + + // ── Unblock: from Frozen/ReviewHold → resume_to ──────────────── + (Frozen { resume_to }, Unblock) => Ok(*resume_to), + (Stage::ReviewHold { resume_to, .. }, Unblock) => Ok(*resume_to), + // ── Unblock: Blocked → Coding ───────────────────────────────── (Blocked { .. }, Unblock) => Ok(Coding), diff --git a/server/src/pipeline_state/types.rs b/server/src/pipeline_state/types.rs index 427ec071..904728b0 100644 --- a/server/src/pipeline_state/types.rs +++ b/server/src/pipeline_state/types.rs @@ -111,6 +111,29 @@ pub enum Stage { /// this is a recoverable intermediate state — `Unblock` returns to `Coding` /// (immediate agent retry) and `Demote` returns to `Backlog` (manual park). MergeFailure { reason: String }, + + /// Merge pipeline failed AND mergemaster has already been auto-spawned to + /// recover; the agent gave up. The story stays here awaiting human + /// intervention — the auto-assigner will NOT spawn mergemaster again. + /// Replaces the legacy `mergemaster_attempted: true` boolean flag. + MergeFailureFinal { reason: String }, + + /// Story is frozen — kept at this stage as a snapshot of its previous + /// stage. Replaces the legacy `frozen: true` boolean flag: there is no + /// "frozen AND coding" combinatorial state; a story is either Frozen + /// (with the stage it would resume to) or not. The auto-assigner skips + /// frozen stories; `Unfreeze` transitions back to `resume_to`. + Frozen { resume_to: Box }, + + /// Story is held for human review at a pipeline-stage boundary (e.g. + /// spikes after QA passes, human-QA items after gates pass). Replaces + /// the legacy `review_hold: true` boolean flag: the held story has a + /// definite resume target stored on the variant. The auto-assigner + /// skips review-held stories; clearing the hold returns to `resume_to`. + ReviewHold { + resume_to: Box, + reason: String, + }, } /// Why a story was archived. Subsumes the old `blocked`, `merge_failure`, @@ -151,13 +174,15 @@ impl Stage { stage_dir_name(self) } - /// Returns true if this is the `Blocked` or `MergeFailure` variant (or the - /// legacy `Archived(Blocked)` for backward-compatible reads). + /// 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 { .. }, .. @@ -165,6 +190,27 @@ impl Stage { ) } + /// 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 @@ -189,6 +235,16 @@ impl Stage { "merge_failure" => Some(Stage::MergeFailure { reason: String::new(), }), + "merge_failure_final" => Some(Stage::MergeFailureFinal { + reason: String::new(), + }), + "frozen" => Some(Stage::Frozen { + resume_to: Box::new(Stage::Coding), + }), + "review_hold" => Some(Stage::ReviewHold { + resume_to: Box::new(Stage::Coding), + reason: String::new(), + }), "done" => Some(Stage::Done { merged_at: DateTime::::UNIX_EPOCH, merge_commit: GitSha(String::new()), @@ -242,17 +298,13 @@ pub struct PipelineItem { pub stage: Stage, pub depends_on: Vec, pub retry_count: u32, - /// Whether the item is frozen — orthogonal to [`Self::stage`]. - /// Frozen items remain at their current stage but are skipped by the - /// auto-assigner until explicitly unfrozen (story 934, stage 4). - pub frozen: bool, } impl PipelineItem { - /// Whether the item is frozen. Frozen items stay at their current - /// [`Stage`] but are skipped by the auto-assigner until unfrozen. + /// 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.frozen + self.stage.is_frozen() } } @@ -287,8 +339,11 @@ pub fn stage_label(s: &Stage) -> &'static str { Stage::Qa => "Qa", Stage::Merge { .. } => "Merge", Stage::MergeFailure { .. } => "MergeFailure", + Stage::MergeFailureFinal { .. } => "MergeFailureFinal", Stage::Done { .. } => "Done", Stage::Blocked { .. } => "Blocked", + Stage::Frozen { .. } => "Frozen", + Stage::ReviewHold { .. } => "ReviewHold", Stage::Archived { .. } => "Archived", } } @@ -307,6 +362,9 @@ pub fn stage_dir_name(s: &Stage) -> &'static str { Stage::Qa => "qa", Stage::Merge { .. } => "merge", Stage::MergeFailure { .. } => "merge_failure", + Stage::MergeFailureFinal { .. } => "merge_failure_final", + Stage::Frozen { .. } => "frozen", + Stage::ReviewHold { .. } => "review_hold", Stage::Done { .. } => "done", Stage::Archived { .. } => "archived", } diff --git a/server/src/service/agents/mod.rs b/server/src/service/agents/mod.rs index b87c86c0..9d51b841 100644 --- a/server/src/service/agents/mod.rs +++ b/server/src/service/agents/mod.rs @@ -357,7 +357,6 @@ max_budget_usd = 5.0 None, None, None, - None, ); let item = get_work_item_content(tmp.path(), "42_story_foo").unwrap(); assert!(item.content.contains("Some content.")); diff --git a/server/src/service/notifications/format.rs b/server/src/service/notifications/format.rs index f9dbf26c..26798e62 100644 --- a/server/src/service/notifications/format.rs +++ b/server/src/service/notifications/format.rs @@ -19,6 +19,9 @@ pub fn stage_display_name(stage: &Stage) -> &'static str { Stage::Done { .. } => "Done", Stage::Archived { .. } => "Archived", Stage::MergeFailure { .. } => "MergeFailure", + Stage::MergeFailureFinal { .. } => "MergeFailureFinal", + Stage::Frozen { .. } => "Frozen", + Stage::ReviewHold { .. } => "ReviewHold", } } diff --git a/server/src/service/work_item/assign.rs b/server/src/service/work_item/assign.rs index e586e998..be1be4e5 100644 --- a/server/src/service/work_item/assign.rs +++ b/server/src/service/work_item/assign.rs @@ -66,7 +66,6 @@ mod tests { None, None, None, - None, ); let tmp = tempfile::tempdir().unwrap(); @@ -108,7 +107,6 @@ mod tests { None, None, None, - None, ); } diff --git a/server/src/service/work_item/delete.rs b/server/src/service/work_item/delete.rs index 29bee7d7..5533eef2 100644 --- a/server/src/service/work_item/delete.rs +++ b/server/src/service/work_item/delete.rs @@ -205,7 +205,6 @@ mod tests { None, None, None, - None, ); // Seed content store. diff --git a/server/src/service/work_item/freeze.rs b/server/src/service/work_item/freeze.rs index 3d804342..8f13c697 100644 --- a/server/src/service/work_item/freeze.rs +++ b/server/src/service/work_item/freeze.rs @@ -28,8 +28,10 @@ pub enum UnfreezeStatus { /// stage without making any CRDT writes. Returns `Err` if the state transition /// fails (e.g. the item is not found or is in a terminal stage). pub fn freeze(story_id: &str) -> Result { - let already_frozen = crate::crdt_state::read_item(story_id) - .map(|view| view.frozen()) + let already_frozen = crate::pipeline_state::read_typed(story_id) + .ok() + .flatten() + .map(|item| item.is_frozen()) .unwrap_or(false); if already_frozen { @@ -46,8 +48,10 @@ pub fn freeze(story_id: &str) -> Result { /// Returns [`UnfreezeStatus::NotFrozen`] if the item is not currently frozen. /// Returns `Err` if the state transition fails. pub fn unfreeze(story_id: &str) -> Result { - let is_frozen = crate::crdt_state::read_item(story_id) - .map(|view| view.frozen()) + let is_frozen = crate::pipeline_state::read_typed(story_id) + .ok() + .flatten() + .map(|item| item.is_frozen()) .unwrap_or(false); if !is_frozen {