From 95c0aafb68fe205accb9933e3658456d76ff3c8c Mon Sep 17 00:00:00 2001 From: dave Date: Mon, 18 May 2026 14:43:54 +0000 Subject: [PATCH] =?UTF-8?q?huskies:=20merge=201141=20story=20Convert=20wor?= =?UTF-8?q?k-item=20type=20between=20spike/story/bug/refactor=20(or=20at?= =?UTF-8?q?=20least=20spike=E2=86=92story)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- server/src/chat/commands/convert.rs | 188 ++++++++++++++++++ server/src/chat/commands/mod.rs | 6 + server/src/crdt_state/read.rs | 7 + server/src/http/mcp/diagnostics/mod.rs | 1 + server/src/http/mcp/dispatch.rs | 2 + server/src/http/mcp/story_tools/mod.rs | 6 +- .../src/http/mcp/story_tools/story/convert.rs | 178 +++++++++++++++++ server/src/http/mcp/story_tools/story/mod.rs | 2 + server/src/http/mcp/tools_list/mod.rs | 3 +- server/src/http/mcp/tools_list/story_tools.rs | 19 ++ server/src/validation/mod.rs | 7 +- server/src/validation/requests.rs | 75 +++++++ 12 files changed, 487 insertions(+), 7 deletions(-) create mode 100644 server/src/chat/commands/convert.rs create mode 100644 server/src/http/mcp/story_tools/story/convert.rs diff --git a/server/src/chat/commands/convert.rs b/server/src/chat/commands/convert.rs new file mode 100644 index 00000000..980e486d --- /dev/null +++ b/server/src/chat/commands/convert.rs @@ -0,0 +1,188 @@ +//! Handler for the `convert` chat command (story 1141). +//! +//! `convert ` changes the item-type register of a work item +//! in place. All other CRDT registers (ACs, epic, name, stage, …) are +//! untouched. Rejected for archived items. + +use super::CommandContext; + +/// Handle the `convert` command. +/// +/// Parses ` ` from `ctx.args` and delegates to +/// [`convert_by_number`]. Returns `None` (route to LLM) when args do not +/// look like a numeric ID followed by a type keyword. +pub(super) fn handle_convert(ctx: &CommandContext) -> Option { + let args = ctx.args.trim(); + let (num_str, type_str) = args.split_once(char::is_whitespace)?; + let num_str = num_str.trim(); + let type_str = type_str.trim(); + + // Route to LLM if the first token is not a bare number. + if num_str.is_empty() || !num_str.chars().all(|c| c.is_ascii_digit()) { + return None; + } + // Route to LLM if the type looks like natural language (contains spaces). + if type_str.is_empty() || type_str.contains(char::is_whitespace) { + return None; + } + + Some(convert_by_number(ctx.effective_root(), num_str, type_str)) +} + +/// Core convert logic: find item by numeric prefix and change its type. +/// +/// Returns a Markdown-formatted response suitable for all chat transports. +pub(crate) fn convert_by_number( + project_root: &std::path::Path, + story_number: &str, + new_type_str: &str, +) -> String { + let Some(new_type) = crate::io::story_metadata::ItemType::from_str(new_type_str) else { + return format!( + "Unknown type **{new_type_str}**. Accepted types: story, bug, spike, refactor, epic." + ); + }; + + let (story_id, _, _, _) = + match crate::chat::lookup::find_story_by_number(project_root, story_number) { + Some(found) => found, + None => { + return format!( + "No story, bug, spike, or refactor with number **{story_number}** found." + ); + } + }; + + let item = match crate::crdt_state::read_item(&story_id) { + Some(i) => i, + None => { + return format!("Work item **{story_number}** ({story_id}) not found in CRDT."); + } + }; + + if matches!(item.stage(), crate::pipeline_state::Stage::Archived { .. }) { + return format!( + "Cannot convert **{story_id}**: type change on an archived item is not allowed." + ); + } + + let old_type = item.item_type().map(|t| t.as_str()).unwrap_or("(inferred)"); + let story_name = item.name().to_string(); + let new_type_s = new_type.as_str(); + + if !crate::crdt_state::set_item_type(&story_id, Some(new_type)) { + return format!("Failed to convert **{story_id}**: CRDT write rejected."); + } + + format!("Converted **{story_name}** ({story_id}) from type `{old_type}` to `{new_type_s}`.") +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::super::{CommandDispatch, try_handle_command}; + + fn convert_cmd(root: &std::path::Path, args: &str) -> Option { + let services = crate::services::Services::new_test(root.to_path_buf(), "Timmy".to_string()); + let room_id = "!test:example.com".to_string(); + let dispatch = CommandDispatch { + services: &services, + project_root: &services.project_root, + bot_user_id: "@timmy:homeserver.local", + room_id: &room_id, + }; + try_handle_command(&dispatch, &format!("@timmy convert {args}")) + } + + #[test] + fn convert_command_is_registered() { + use super::super::commands; + assert!( + commands().iter().any(|c| c.name == "convert"), + "convert command must be in the registry" + ); + } + + #[test] + fn convert_no_args_routes_to_llm() { + let tmp = tempfile::TempDir::new().unwrap(); + let result = convert_cmd(tmp.path(), ""); + assert!(result.is_none(), "no args should route to LLM: {result:?}"); + } + + #[test] + fn convert_natural_language_routes_to_llm() { + let tmp = tempfile::TempDir::new().unwrap(); + let result = convert_cmd(tmp.path(), "the login bug to a story"); + assert!( + result.is_none(), + "natural-language args should route to LLM: {result:?}" + ); + } + + #[test] + fn convert_well_formed_runs_handler() { + let tmp = tempfile::TempDir::new().unwrap(); + let result = convert_cmd(tmp.path(), "999 story"); + assert!( + result.is_some(), + "well-formed args should run the handler: {result:?}" + ); + } + + #[test] + fn convert_invalid_type_returns_error() { + let tmp = tempfile::TempDir::new().unwrap(); + let result = convert_cmd(tmp.path(), "999 banana").unwrap(); + assert!( + result.contains("Unknown type") || result.contains("banana"), + "unknown type should show error: {result}" + ); + } + + #[test] + fn convert_not_found_returns_error() { + let tmp = tempfile::TempDir::new().unwrap(); + let result = convert_cmd(tmp.path(), "9988 story").unwrap(); + assert!( + result.contains("9988") && result.contains("found"), + "not-found message should include number and 'found': {result}" + ); + } + + #[test] + fn convert_changes_item_type_in_crdt() { + let tmp = tempfile::TempDir::new().unwrap(); + crate::crdt_state::init_for_test(); + crate::db::ensure_content_store(); + crate::chat::test_helpers::write_story_file( + tmp.path(), + "backlog", + "9120_spike_convert_chat.md", + "# Spike\n", + Some("Convert Chat Test"), + ); + crate::crdt_state::set_item_type( + "9120_spike_convert_chat", + Some(crate::io::story_metadata::ItemType::Spike), + ); + + let result = convert_cmd(tmp.path(), "9120 story").unwrap(); + assert!( + result.contains("story") || result.contains("Converted"), + "should confirm conversion: {result}" + ); + + let item = + crate::crdt_state::read_item("9120_spike_convert_chat").expect("item should exist"); + assert_eq!( + item.item_type(), + Some(crate::io::story_metadata::ItemType::Story), + "item_type should be Story after conversion: {:?}", + item.item_type() + ); + } +} diff --git a/server/src/chat/commands/mod.rs b/server/src/chat/commands/mod.rs index 5b6bd249..80fc2ea7 100644 --- a/server/src/chat/commands/mod.rs +++ b/server/src/chat/commands/mod.rs @@ -9,6 +9,7 @@ mod ambient; mod assign; mod backlog; mod cleanup_worktrees; +mod convert; mod cost; mod coverage; mod depends; @@ -233,6 +234,11 @@ pub fn commands() -> &'static [BotCommand] { description: "Schedule a deferred agent start: `timer `, `timer list`, `timer cancel `", handler: timer::handle_timer, }, + BotCommand { + name: "convert", + description: "Convert a work item's type: `convert ` (types: story, bug, spike, refactor, epic)", + handler: convert::handle_convert, + }, BotCommand { name: "unblock", description: "Reset a blocked story: `unblock ` (clears blocked flag and resets retry count)", diff --git a/server/src/crdt_state/read.rs b/server/src/crdt_state/read.rs index 71473024..3d62f20a 100644 --- a/server/src/crdt_state/read.rs +++ b/server/src/crdt_state/read.rs @@ -33,6 +33,8 @@ pub struct CrdtItemDump { pub is_deleted: bool, /// Origin JSON string, or `None` for items that pre-date story 1088. pub origin: Option, + /// Explicit item type register, or `None` when unset (infer from story_id prefix). + pub item_type: Option, } /// Top-level debug dump of the in-memory CRDT state. @@ -162,6 +164,10 @@ pub fn dump_crdt_state(story_id_filter: Option<&str>) -> CrdtStateDump { JsonValue::String(s) if !s.is_empty() => Some(s), _ => None, }; + let item_type = match item_crdt.item_type.view() { + JsonValue::String(s) if !s.is_empty() => Some(s), + _ => None, + }; let content_index = op.id.iter().map(|b| format!("{b:02x}")).collect::(); @@ -177,6 +183,7 @@ pub fn dump_crdt_state(story_id_filter: Option<&str>) -> CrdtStateDump { content_index, is_deleted: op.is_deleted, origin, + item_type, }); } diff --git a/server/src/http/mcp/diagnostics/mod.rs b/server/src/http/mcp/diagnostics/mod.rs index b15da0e4..ef42766a 100644 --- a/server/src/http/mcp/diagnostics/mod.rs +++ b/server/src/http/mcp/diagnostics/mod.rs @@ -115,6 +115,7 @@ pub(crate) fn tool_dump_crdt(args: &Value) -> Result { "content_index": item.content_index, "is_deleted": item.is_deleted, "origin": item.origin, + "item_type": item.item_type, }) }) .collect(); diff --git a/server/src/http/mcp/dispatch.rs b/server/src/http/mcp/dispatch.rs index 0346b0c3..a5458edb 100644 --- a/server/src/http/mcp/dispatch.rs +++ b/server/src/http/mcp/dispatch.rs @@ -102,6 +102,8 @@ 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), + // Convert work-item type in place (story 1141) + "convert_item_type" => story_tools::tool_convert_item_type(&args, ctx), // Freeze / unfreeze story "freeze_story" => story_tools::tool_freeze_story(&args, ctx), "unfreeze_story" => story_tools::tool_unfreeze_story(&args, ctx), diff --git a/server/src/http/mcp/story_tools/mod.rs b/server/src/http/mcp/story_tools/mod.rs index b4e2d990..2ad80782 100644 --- a/server/src/http/mcp/story_tools/mod.rs +++ b/server/src/http/mcp/story_tools/mod.rs @@ -69,7 +69,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_freeze_story, - tool_get_pipeline_status, tool_list_upcoming, tool_purge_story, tool_unblock_story, - tool_unfreeze_story, tool_update_story, tool_validate_stories, + tool_accept_story, tool_convert_item_type, 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/convert.rs b/server/src/http/mcp/story_tools/story/convert.rs new file mode 100644 index 00000000..f3b26b0e --- /dev/null +++ b/server/src/http/mcp/story_tools/story/convert.rs @@ -0,0 +1,178 @@ +//! MCP tool for converting a work item's type in place (story 1141). +//! +//! `convert_item_type` changes the type register of an existing CRDT item +//! from any value to another (story ↔ bug ↔ spike ↔ refactor) without +//! touching the story_id, ACs, epic association, or any other register. + +use crate::http::context::AppContext; +use crate::pipeline_state::Stage; +use serde_json::Value; + +/// Convert a work item's type in the CRDT. +/// +/// Accepts `story_id` (full filename stem, e.g. `"42_spike_my_spike"`) and +/// `new_type` (one of `"story"`, `"bug"`, `"spike"`, `"refactor"`, `"epic"`). +/// Returns an error when the item does not exist or is in the `Archived` stage. +pub(crate) fn tool_convert_item_type(args: &Value, _ctx: &AppContext) -> Result { + let req = crate::validation::ConvertItemTypeRequest::from_json(args)?; + let story_id = req.story_id.as_str(); + + let item = crate::crdt_state::read_item(story_id) + .ok_or_else(|| format!("Work item '{story_id}' not found in CRDT."))?; + + if matches!(item.stage(), Stage::Archived { .. }) { + return Err(format!( + "Cannot convert '{story_id}': type change on an archived item is not allowed." + )); + } + + let old_type = item.item_type().map(|t| t.as_str()).unwrap_or("(inferred)"); + let new_type_str = req.new_type.as_str(); + + if !crate::crdt_state::set_item_type(story_id, Some(req.new_type)) { + return Err(format!( + "Failed to update item type for '{story_id}': CRDT write was rejected." + )); + } + + Ok(format!( + "Converted '{story_id}' from type '{old_type}' to '{new_type_str}'." + )) +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + use crate::http::test_helpers::test_ctx; + use crate::io::story_metadata::ItemType; + use serde_json::json; + + fn make_spike(spike_id: &str) { + crate::crdt_state::init_for_test(); + crate::db::ensure_content_store(); + crate::db::write_item_with_content( + spike_id, + "backlog", + "---\nname: Test Spike\n---\n", + crate::db::ItemMeta::named("Test Spike"), + ); + } + + #[test] + fn converts_spike_to_story_and_preserves_epic() { + crate::crdt_state::init_for_test(); + let spike_id = "9111_spike_convert_regression"; + make_spike(spike_id); + + // Attach an epic. + crate::crdt_state::set_item_type(spike_id, Some(ItemType::Spike)); + crate::crdt_state::set_epic(spike_id, crate::crdt_state::EpicId::from_crdt_str("9000")); + + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + + // (i) Convert spike → story. + let result = + tool_convert_item_type(&json!({"story_id": spike_id, "new_type": "story"}), &ctx); + assert!(result.is_ok(), "convert should succeed: {result:?}"); + assert!( + result.unwrap().contains("story"), + "response should mention new type" + ); + + // (i) Verify type is now Story in CRDT. + let item = crate::crdt_state::read_item(spike_id).expect("item must exist"); + assert_eq!( + item.item_type(), + Some(ItemType::Story), + "item_type should be Story after conversion" + ); + + // (ii) Verify the conversion is visible in dump_crdt. + let dump = crate::crdt_state::dump_crdt_state(Some(spike_id)); + let found = dump + .items + .iter() + .any(|i| i.item_type.as_deref() == Some("story") && !i.is_deleted); + assert!( + found, + "dump_crdt should show item_type='story' after conversion" + ); + + // (iii) Epic association is preserved. + assert_eq!( + item.epic(), + crate::crdt_state::EpicId::from_crdt_str("9000"), + "epic should be unchanged after type conversion" + ); + } + + #[test] + fn rejects_missing_story_id() { + crate::crdt_state::init_for_test(); + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + let err = tool_convert_item_type(&json!({"new_type": "story"}), &ctx).unwrap_err(); + assert!( + err.contains("story_id"), + "error should mention story_id: {err}" + ); + } + + #[test] + fn rejects_invalid_new_type() { + crate::crdt_state::init_for_test(); + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + let err = tool_convert_item_type( + &json!({"story_id": "9112_spike_foo", "new_type": "banana"}), + &ctx, + ) + .unwrap_err(); + assert!( + err.contains("new_type") || err.contains("InvalidValue"), + "error should mention new_type: {err}" + ); + } + + #[test] + fn rejects_nonexistent_item() { + crate::crdt_state::init_for_test(); + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + let err = tool_convert_item_type( + &json!({"story_id": "9999_spike_not_real", "new_type": "story"}), + &ctx, + ) + .unwrap_err(); + assert!( + err.contains("not found"), + "error should say not found: {err}" + ); + } + + #[test] + fn rejects_archived_item() { + crate::crdt_state::init_for_test(); + let spike_id = "9113_spike_archived_convert"; + crate::db::ensure_content_store(); + crate::db::write_item_with_content( + spike_id, + "archived", + "---\nname: Archived Spike\n---\n", + crate::db::ItemMeta::named("Archived Spike"), + ); + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + let err = tool_convert_item_type(&json!({"story_id": spike_id, "new_type": "story"}), &ctx) + .unwrap_err(); + assert!( + err.contains("archived"), + "error should mention archived: {err}" + ); + } +} diff --git a/server/src/http/mcp/story_tools/story/mod.rs b/server/src/http/mcp/story_tools/story/mod.rs index 94156758..4a5ba255 100644 --- a/server/src/http/mcp/story_tools/story/mod.rs +++ b/server/src/http/mcp/story_tools/story/mod.rs @@ -1,11 +1,13 @@ //! Story creation, listing, update, and lifecycle MCP tools. +mod convert; mod create; mod delete; mod freeze; mod query; mod update; +pub(crate) use convert::tool_convert_item_type; 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}; diff --git a/server/src/http/mcp/tools_list/mod.rs b/server/src/http/mcp/tools_list/mod.rs index 4423fed6..24fd2504 100644 --- a/server/src/http/mcp/tools_list/mod.rs +++ b/server/src/http/mcp/tools_list/mod.rs @@ -114,7 +114,8 @@ mod tests { assert!(names.contains(&"schedule_timer")); assert!(names.contains(&"list_timers")); assert!(names.contains(&"cancel_timer")); - assert_eq!(tools.len(), 82); + assert!(names.contains(&"convert_item_type")); + assert_eq!(tools.len(), 83); } #[test] diff --git a/server/src/http/mcp/tools_list/story_tools.rs b/server/src/http/mcp/tools_list/story_tools.rs index 88217ed5..d097b838 100644 --- a/server/src/http/mcp/tools_list/story_tools.rs +++ b/server/src/http/mcp/tools_list/story_tools.rs @@ -671,6 +671,25 @@ pub(super) fn story_tools() -> Vec { "required": ["story_id"] } }), + json!({ + "name": "convert_item_type", + "description": "Convert a work item's type in place (e.g. spike → story). The story_id, ACs, epic association, and all other registers are preserved; only the item_type register changes. Rejected for archived items.", + "inputSchema": { + "type": "object", + "properties": { + "story_id": { + "type": "string", + "description": "Work item identifier (filename stem, e.g. '42_spike_my_spike')" + }, + "new_type": { + "type": "string", + "enum": ["story", "bug", "spike", "refactor", "epic"], + "description": "Target item type" + } + }, + "required": ["story_id", "new_type"] + } + }), json!({ "name": "freeze_story", "description": "Freeze a work item at its current pipeline stage, suppressing pipeline advancement and auto-assign until unfrozen.", diff --git a/server/src/validation/mod.rs b/server/src/validation/mod.rs index 546f34ce..10c56e56 100644 --- a/server/src/validation/mod.rs +++ b/server/src/validation/mod.rs @@ -22,7 +22,8 @@ pub use newtypes::{ AcceptanceCriterion, DependsOnId, Description, StoryId, StoryName, TargetStage, }; pub use requests::{ - AddCriterionRequest, CreateBugRequest, CreateEpicRequest, CreateRefactorRequest, - CreateSpikeRequest, CreateStoryRequest, EditCriterionRequest, FreezeStoryRequest, - MoveStoryRequest, MoveStoryToMergeRequest, UnblockStoryRequest, UpdateStoryRequest, + AddCriterionRequest, ConvertItemTypeRequest, CreateBugRequest, CreateEpicRequest, + CreateRefactorRequest, CreateSpikeRequest, CreateStoryRequest, EditCriterionRequest, + FreezeStoryRequest, MoveStoryRequest, MoveStoryToMergeRequest, UnblockStoryRequest, + UpdateStoryRequest, }; diff --git a/server/src/validation/requests.rs b/server/src/validation/requests.rs index 92b4ee8f..d2e11a5b 100644 --- a/server/src/validation/requests.rs +++ b/server/src/validation/requests.rs @@ -1212,6 +1212,81 @@ impl FreezeStoryRequest { }) } } + +// --------------------------------------------------------------------------- +// ConvertItemTypeRequest +// --------------------------------------------------------------------------- + +/// Fully validated inputs for the `convert_item_type` MCP tool and `convert` chat command. +#[derive(Debug)] +pub struct ConvertItemTypeRequest { + /// Validated story identifier. + pub story_id: StoryId, + /// The target item type. + pub new_type: crate::io::story_metadata::ItemType, +} + +impl ConvertItemTypeRequest { + /// Parse and validate a `convert_item_type` JSON argument map. + /// + /// Required fields: `story_id` (work-item filename stem), `new_type` + /// (one of `"story"`, `"bug"`, `"spike"`, `"refactor"`, `"epic"`). + pub fn from_json(args: &serde_json::Value) -> Result { + let mut errors: Vec = Vec::new(); + + let story_id = match args.get("story_id").and_then(|v| v.as_str()) { + None => { + errors.push(ValidationError::FieldMissing { + field: "story_id".into(), + }); + None + } + Some(raw) => match StoryId::parse(raw) { + Ok(id) => Some(id), + Err(mut errs) => { + errors.append(&mut errs); + None + } + }, + }; + + let new_type = match args.get("new_type").and_then(|v| v.as_str()) { + None => { + errors.push(ValidationError::FieldMissing { + field: "new_type".into(), + }); + None + } + Some(raw) => match crate::io::story_metadata::ItemType::from_str(raw) { + Some(t) => Some(t), + None => { + errors.push(ValidationError::InvalidValue { + field: "new_type".into(), + actual: raw.to_string(), + allowed: vec![ + "story".into(), + "bug".into(), + "spike".into(), + "refactor".into(), + "epic".into(), + ], + }); + None + } + }, + }; + + if !errors.is_empty() { + return Err(format_errors_as_json(&errors)); + } + + Ok(ConvertItemTypeRequest { + story_id: story_id.unwrap(), + new_type: new_type.unwrap(), + }) + } +} + #[cfg(test)] mod tests { use super::*;