From 8fc581ad6bbd611cbc043558b4f8f9c6fbe7a5c1 Mon Sep 17 00:00:00 2001 From: dave Date: Wed, 29 Apr 2026 23:48:30 +0000 Subject: [PATCH] huskies: merge 878 --- server/src/chat/commands/freeze.rs | 71 ++---- server/src/http/mcp/dispatch.rs | 3 + server/src/http/mcp/story_tools/mod.rs | 6 +- .../src/http/mcp/story_tools/story/freeze.rs | 126 ++++++++++ server/src/http/mcp/story_tools/story/mod.rs | 2 + server/src/http/mcp/tools_list/mod.rs | 4 +- server/src/http/mcp/tools_list/story_tools.rs | 28 +++ server/src/service/work_item/freeze.rs | 221 ++++++++++++++++++ server/src/service/work_item/mod.rs | 3 + 9 files changed, 413 insertions(+), 51 deletions(-) create mode 100644 server/src/http/mcp/story_tools/story/freeze.rs create mode 100644 server/src/service/work_item/freeze.rs diff --git a/server/src/chat/commands/freeze.rs b/server/src/chat/commands/freeze.rs index 31b6d1ea..49a34792 100644 --- a/server/src/chat/commands/freeze.rs +++ b/server/src/chat/commands/freeze.rs @@ -38,30 +38,13 @@ pub(crate) fn freeze_by_number(project_root: &Path, story_number: &str) -> Strin } fn freeze_by_story_id(story_id: &str) -> String { - let contents = match crate::db::read_content(story_id) { - Some(c) => c, - None => return format!("Failed to read story content for **{story_id}**"), - }; + let story_name = resolve_story_name(story_id); - let meta = match parse_front_matter(&contents) { - Ok(m) => m, - Err(e) => return format!("Failed to parse front matter for **{story_id}**: {e}"), - }; - - let story_name = meta.name.as_deref().unwrap_or(story_id).to_string(); - - // Check if already frozen via the typed stage. - if crate::pipeline_state::read_typed(story_id) - .ok() - .flatten() - .map(|i| i.stage.is_frozen()) - .unwrap_or(false) - { - return format!("**{story_name}** ({story_id}) is already frozen."); - } - - match crate::pipeline_state::transition_to_frozen(story_id) { - Ok(_) => format!( + match crate::service::work_item::freeze::freeze(story_id) { + Ok(crate::service::work_item::FreezeStatus::AlreadyFrozen) => { + format!("**{story_name}** ({story_id}) is already frozen.") + } + Ok(crate::service::work_item::FreezeStatus::Frozen) => format!( "Frozen **{story_name}** ({story_id}). Pipeline advancement and auto-assign suppressed until unfrozen." ), Err(e) => format!("Failed to freeze **{story_name}** ({story_id}): {e}"), @@ -97,37 +80,31 @@ pub(crate) fn unfreeze_by_number(project_root: &Path, story_number: &str) -> Str } fn unfreeze_by_story_id(story_id: &str) -> String { - let contents = match crate::db::read_content(story_id) { - Some(c) => c, - None => return format!("Failed to read story content for **{story_id}**"), - }; + let story_name = resolve_story_name(story_id); - let meta = match parse_front_matter(&contents) { - Ok(m) => m, - Err(e) => return format!("Failed to parse front matter for **{story_id}**: {e}"), - }; - - let story_name = meta.name.as_deref().unwrap_or(story_id).to_string(); - - // Check frozen via typed stage. - let is_frozen = crate::pipeline_state::read_typed(story_id) - .ok() - .flatten() - .map(|i| i.stage.is_frozen()) - .unwrap_or(false); - - if !is_frozen { - return format!("**{story_name}** ({story_id}) is not frozen. Nothing to unfreeze."); - } - - match crate::pipeline_state::transition_to_unfrozen(story_id) { - Ok(_) => { + match crate::service::work_item::freeze::unfreeze(story_id) { + Ok(crate::service::work_item::UnfreezeStatus::NotFrozen) => { + format!("**{story_name}** ({story_id}) is not frozen. Nothing to unfreeze.") + } + Ok(crate::service::work_item::UnfreezeStatus::Unfrozen) => { format!("Unfrozen **{story_name}** ({story_id}). Normal pipeline behaviour resumed.") } Err(e) => format!("Failed to unfreeze **{story_name}** ({story_id}): {e}"), } } +/// Look up the display name for a story by reading its content store entry. +/// +/// Falls back to `story_id` if the content is missing or the front matter +/// cannot be parsed. +fn resolve_story_name(story_id: &str) -> String { + crate::db::read_content(story_id) + .as_deref() + .and_then(|c| parse_front_matter(c).ok()) + .and_then(|m| m.name) + .unwrap_or_else(|| story_id.to_string()) +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- diff --git a/server/src/http/mcp/dispatch.rs b/server/src/http/mcp/dispatch.rs index 218e681f..ad5cecf6 100644 --- a/server/src/http/mcp/dispatch.rs +++ b/server/src/http/mcp/dispatch.rs @@ -99,6 +99,9 @@ pub async fn dispatch_tool_call( "move_story" => diagnostics::tool_move_story(&args, ctx), // Unblock story "unblock_story" => story_tools::tool_unblock_story(&args, ctx), + // Freeze / unfreeze story + "freeze_story" => story_tools::tool_freeze_story(&args, ctx), + "unfreeze_story" => story_tools::tool_unfreeze_story(&args, ctx), // Shell command execution "run_command" => shell_tools::tool_run_command(&args, ctx).await, "run_tests" => shell_tools::tool_run_tests(&args, ctx).await, diff --git a/server/src/http/mcp/story_tools/mod.rs b/server/src/http/mcp/story_tools/mod.rs index 2bed7144..0ab653d9 100644 --- a/server/src/http/mcp/story_tools/mod.rs +++ b/server/src/http/mcp/story_tools/mod.rs @@ -21,7 +21,7 @@ pub(crate) use epic::{tool_create_epic, tool_list_epics, tool_show_epic}; pub(crate) use refactor::{tool_create_refactor, tool_list_refactors}; pub(crate) use spike::tool_create_spike; pub(crate) use story::{ - tool_accept_story, tool_create_story, tool_delete_story, tool_get_pipeline_status, - tool_list_upcoming, tool_purge_story, tool_unblock_story, tool_update_story, - tool_validate_stories, + tool_accept_story, tool_create_story, tool_delete_story, tool_freeze_story, + tool_get_pipeline_status, tool_list_upcoming, tool_purge_story, tool_unblock_story, + tool_unfreeze_story, tool_update_story, tool_validate_stories, }; diff --git a/server/src/http/mcp/story_tools/story/freeze.rs b/server/src/http/mcp/story_tools/story/freeze.rs new file mode 100644 index 00000000..3ee46efb --- /dev/null +++ b/server/src/http/mcp/story_tools/story/freeze.rs @@ -0,0 +1,126 @@ +//! MCP freeze and unfreeze tools — thin adapters over `service::work_item::freeze`. + +use crate::http::context::AppContext; +use crate::service::work_item::{FreezeStatus, UnfreezeStatus}; +use serde_json::Value; + +/// Freeze a work item, suppressing pipeline advancement and auto-assign. +/// +/// Accepts a `story_id` (full filename stem, e.g. `"42_story_foo"`) and +/// delegates to [`service::work_item::freeze::freeze`]. +pub(crate) fn tool_freeze_story(args: &Value, _ctx: &AppContext) -> Result { + let story_id = args + .get("story_id") + .and_then(|v| v.as_str()) + .ok_or("Missing required argument: story_id")?; + + match crate::service::work_item::freeze::freeze(story_id)? { + FreezeStatus::AlreadyFrozen => Ok(format!("Story '{story_id}' is already frozen.")), + FreezeStatus::Frozen => Ok(format!( + "Frozen story '{story_id}'. Pipeline advancement and auto-assign suppressed until unfrozen." + )), + } +} + +/// Unfreeze a work item, resuming normal pipeline behaviour. +/// +/// Accepts a `story_id` (full filename stem, e.g. `"42_story_foo"`) and +/// delegates to [`service::work_item::freeze::unfreeze`]. +pub(crate) fn tool_unfreeze_story(args: &Value, _ctx: &AppContext) -> Result { + let story_id = args + .get("story_id") + .and_then(|v| v.as_str()) + .ok_or("Missing required argument: story_id")?; + + match crate::service::work_item::freeze::unfreeze(story_id)? { + UnfreezeStatus::NotFrozen => Ok(format!( + "Story '{story_id}' is not frozen. Nothing to unfreeze." + )), + UnfreezeStatus::Unfrozen => Ok(format!( + "Unfrozen story '{story_id}'. Normal pipeline behaviour resumed." + )), + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + use crate::http::test_helpers::test_ctx; + use serde_json::json; + + #[test] + fn tool_freeze_story_freezes_item() { + crate::crdt_state::init_for_test(); + let story_id = "8790_story_mcp_freeze_tool"; + crate::db::write_item_with_content( + story_id, + "2_current", + "---\nname: MCP Freeze Tool Test\n---\n", + ); + + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + let result = tool_freeze_story(&json!({"story_id": story_id}), &ctx); + assert!(result.is_ok(), "freeze tool should succeed: {result:?}"); + assert!( + result.unwrap().contains("Frozen"), + "response should mention frozen" + ); + + let item = crate::pipeline_state::read_typed(story_id) + .expect("read_typed should succeed") + .expect("item should be present"); + assert!( + item.stage.is_frozen(), + "stage should be frozen after MCP freeze" + ); + } + + #[test] + fn tool_unfreeze_story_unfreezes_item() { + crate::crdt_state::init_for_test(); + let story_id = "8791_story_mcp_unfreeze_tool"; + crate::db::write_item_with_content( + story_id, + "2_current", + "---\nname: MCP Unfreeze Tool Test\n---\n", + ); + + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + + // Freeze first. + tool_freeze_story(&json!({"story_id": story_id}), &ctx).unwrap(); + + let result = tool_unfreeze_story(&json!({"story_id": story_id}), &ctx); + assert!(result.is_ok(), "unfreeze tool should succeed: {result:?}"); + assert!( + result.unwrap().contains("Unfrozen"), + "response should mention unfrozen" + ); + + let item = crate::pipeline_state::read_typed(story_id) + .expect("read_typed should succeed") + .expect("item should be present"); + assert!( + !item.stage.is_frozen(), + "stage should not be frozen after MCP unfreeze" + ); + } + + #[test] + fn tool_freeze_story_missing_story_id_returns_error() { + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + let result = tool_freeze_story(&json!({}), &ctx); + assert!(result.is_err(), "missing story_id should return Err"); + assert!( + result.unwrap_err().contains("story_id"), + "error should mention story_id" + ); + } +} diff --git a/server/src/http/mcp/story_tools/story/mod.rs b/server/src/http/mcp/story_tools/story/mod.rs index 5067f1e3..94156758 100644 --- a/server/src/http/mcp/story_tools/story/mod.rs +++ b/server/src/http/mcp/story_tools/story/mod.rs @@ -2,10 +2,12 @@ mod create; mod delete; +mod freeze; mod query; mod update; pub(crate) use create::{tool_create_story, tool_purge_story}; pub(crate) use delete::{tool_accept_story, tool_delete_story}; +pub(crate) use freeze::{tool_freeze_story, tool_unfreeze_story}; pub(crate) use query::{tool_get_pipeline_status, tool_list_upcoming, tool_validate_stories}; pub(crate) use update::{tool_unblock_story, tool_update_story}; diff --git a/server/src/http/mcp/tools_list/mod.rs b/server/src/http/mcp/tools_list/mod.rs index 7f1ec897..3234c9ab 100644 --- a/server/src/http/mcp/tools_list/mod.rs +++ b/server/src/http/mcp/tools_list/mod.rs @@ -104,7 +104,9 @@ mod tests { assert!(names.contains(&"create_epic")); assert!(names.contains(&"list_epics")); assert!(names.contains(&"show_epic")); - assert_eq!(tools.len(), 72); + assert!(names.contains(&"freeze_story")); + assert!(names.contains(&"unfreeze_story")); + assert_eq!(tools.len(), 74); } #[test] diff --git a/server/src/http/mcp/tools_list/story_tools.rs b/server/src/http/mcp/tools_list/story_tools.rs index f034ce0f..2529115e 100644 --- a/server/src/http/mcp/tools_list/story_tools.rs +++ b/server/src/http/mcp/tools_list/story_tools.rs @@ -641,6 +641,34 @@ pub(super) fn story_tools() -> Vec { "required": ["story_id"] } }), + json!({ + "name": "freeze_story", + "description": "Freeze a work item at its current pipeline stage, suppressing pipeline advancement and auto-assign until unfrozen.", + "inputSchema": { + "type": "object", + "properties": { + "story_id": { + "type": "string", + "description": "Work item identifier (filename stem, e.g. '42_story_my_feature')" + } + }, + "required": ["story_id"] + } + }), + json!({ + "name": "unfreeze_story", + "description": "Unfreeze a work item, resuming normal pipeline behaviour.", + "inputSchema": { + "type": "object", + "properties": { + "story_id": { + "type": "string", + "description": "Work item identifier (filename stem, e.g. '42_story_my_feature')" + } + }, + "required": ["story_id"] + } + }), json!({ "name": "status", "description": "Get a full triage dump for an in-progress story: front matter, AC checklist, active worktree/branch, git diff --stat since master, last 5 commits, and last 20 lines of the most recent agent log. Returns a clear error if the story is not in work/2_current/.", diff --git a/server/src/service/work_item/freeze.rs b/server/src/service/work_item/freeze.rs new file mode 100644 index 00000000..1970bed9 --- /dev/null +++ b/server/src/service/work_item/freeze.rs @@ -0,0 +1,221 @@ +//! Freeze and unfreeze work items — CRDT state transitions for pipeline lifecycle control. +//! +//! Both the Matrix bot commands (`freeze`/`unfreeze`) and the MCP tools +//! (`freeze_story`/`unfreeze_story`) delegate here so the CRDT mutation is +//! defined in exactly one place. + +/// Outcome of a successful [`freeze`] call. +#[derive(Debug, PartialEq, Eq)] +pub enum FreezeStatus { + /// The work item was already in the frozen stage; no state change occurred. + AlreadyFrozen, + /// The work item was successfully transitioned to the frozen stage. + Frozen, +} + +/// Outcome of a successful [`unfreeze`] call. +#[derive(Debug, PartialEq, Eq)] +pub enum UnfreezeStatus { + /// The work item was not frozen; no state change occurred. + NotFrozen, + /// The work item was successfully unfrozen and restored to its prior stage. + Unfrozen, +} + +/// Freeze a work item, suppressing pipeline advancement and auto-assign. +/// +/// Returns [`FreezeStatus::AlreadyFrozen`] if the item is already in the frozen +/// 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::pipeline_state::read_typed(story_id) + .ok() + .flatten() + .map(|i| i.stage.is_frozen()) + .unwrap_or(false); + + if already_frozen { + return Ok(FreezeStatus::AlreadyFrozen); + } + + crate::pipeline_state::transition_to_frozen(story_id) + .map(|_| FreezeStatus::Frozen) + .map_err(|e| e.to_string()) +} + +/// Unfreeze a work item, resuming normal pipeline behaviour. +/// +/// Returns [`UnfreezeStatus::NotFrozen`] if the item is not currently in the +/// frozen stage. Returns `Err` if the state transition fails. +pub fn unfreeze(story_id: &str) -> Result { + let is_frozen = crate::pipeline_state::read_typed(story_id) + .ok() + .flatten() + .map(|i| i.stage.is_frozen()) + .unwrap_or(false); + + if !is_frozen { + return Ok(UnfreezeStatus::NotFrozen); + } + + crate::pipeline_state::transition_to_unfrozen(story_id) + .map(|_| UnfreezeStatus::Unfrozen) + .map_err(|e| e.to_string()) +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn freeze_transitions_item_to_frozen_stage() { + crate::crdt_state::init_for_test(); + let story_id = "8780_story_freeze_service_basic"; + crate::db::write_item_with_content( + story_id, + "2_current", + "---\nname: Freeze Service Test\n---\n", + ); + + let result = freeze(story_id); + assert!( + matches!(result, Ok(FreezeStatus::Frozen)), + "should return Frozen: {result:?}" + ); + + let item = crate::pipeline_state::read_typed(story_id) + .expect("read_typed should succeed") + .expect("item should be present"); + assert!( + item.stage.is_frozen(), + "stage should be Frozen after freeze: {:?}", + item.stage + ); + } + + #[test] + fn freeze_already_frozen_returns_already_frozen_without_error() { + crate::crdt_state::init_for_test(); + let story_id = "8781_story_freeze_service_already"; + crate::db::write_item_with_content( + story_id, + "2_current", + "---\nname: Already Frozen\n---\n", + ); + + freeze(story_id).expect("first freeze should succeed"); + + let result = freeze(story_id); + assert!( + matches!(result, Ok(FreezeStatus::AlreadyFrozen)), + "second freeze should return AlreadyFrozen: {result:?}" + ); + } + + #[test] + fn unfreeze_transitions_item_back_from_frozen() { + crate::crdt_state::init_for_test(); + let story_id = "8782_story_unfreeze_service_basic"; + crate::db::write_item_with_content( + story_id, + "2_current", + "---\nname: Unfreeze Service Test\n---\n", + ); + + freeze(story_id).expect("freeze should succeed"); + + let result = unfreeze(story_id); + assert!( + matches!(result, Ok(UnfreezeStatus::Unfrozen)), + "should return Unfrozen: {result:?}" + ); + + let item = crate::pipeline_state::read_typed(story_id) + .expect("read_typed should succeed") + .expect("item should be present"); + assert!( + !item.stage.is_frozen(), + "stage should not be Frozen after unfreeze: {:?}", + item.stage + ); + } + + #[test] + fn unfreeze_not_frozen_item_returns_not_frozen() { + crate::crdt_state::init_for_test(); + let story_id = "8783_story_unfreeze_service_not_frozen"; + crate::db::write_item_with_content(story_id, "2_current", "---\nname: Not Frozen\n---\n"); + + let result = unfreeze(story_id); + assert!( + matches!(result, Ok(UnfreezeStatus::NotFrozen)), + "should return NotFrozen for a non-frozen item: {result:?}" + ); + } + + /// Regression: both the chat command path and the MCP tool path delegate to + /// `freeze()` / `unfreeze()`, so they must produce identical CRDT state. + /// This test proves it by calling the service directly (as both handlers do) + /// and asserting the resulting CRDT stages are both frozen. + #[test] + fn freeze_via_chat_and_mcp_paths_yields_identical_crdt_state() { + crate::crdt_state::init_for_test(); + + // Story A simulates the chat command path. + let story_a = "8784_story_freeze_regression_chat"; + crate::db::write_item_with_content( + story_a, + "2_current", + "---\nname: Regression Chat Path\n---\n", + ); + + // Story B simulates the MCP tool path. + let story_b = "8785_story_freeze_regression_mcp"; + crate::db::write_item_with_content( + story_b, + "2_current", + "---\nname: Regression MCP Path\n---\n", + ); + + // Both paths call service::work_item::freeze(). + let result_a = freeze(story_a); + let result_b = freeze(story_b); + + assert!( + matches!(result_a, Ok(FreezeStatus::Frozen)), + "chat-path freeze should succeed: {result_a:?}" + ); + assert!( + matches!(result_b, Ok(FreezeStatus::Frozen)), + "MCP-path freeze should succeed: {result_b:?}" + ); + + let state_a = crate::pipeline_state::read_typed(story_a) + .expect("read_typed should succeed") + .expect("chat-path item should be in CRDT"); + let state_b = crate::pipeline_state::read_typed(story_b) + .expect("read_typed should succeed") + .expect("MCP-path item should be in CRDT"); + + assert!( + state_a.stage.is_frozen(), + "chat-path CRDT stage must be frozen: {:?}", + state_a.stage + ); + assert!( + state_b.stage.is_frozen(), + "MCP-path CRDT stage must be frozen: {:?}", + state_b.stage + ); + // Discriminant comparison: both stages must be the same variant. + assert_eq!( + std::mem::discriminant(&state_a.stage), + std::mem::discriminant(&state_b.stage), + "chat-path and MCP-path must produce identical CRDT stage variant" + ); + } +} diff --git a/server/src/service/work_item/mod.rs b/server/src/service/work_item/mod.rs index 9bbcbbcf..9b6b0dd5 100644 --- a/server/src/service/work_item/mod.rs +++ b/server/src/service/work_item/mod.rs @@ -5,5 +5,8 @@ pub mod assign; /// Canonical delete sequence for any work item type. pub mod delete; +/// Freeze and unfreeze work items via the CRDT state machine. +pub mod freeze; pub use assign::assign_and_start; +pub use freeze::{FreezeStatus, UnfreezeStatus};