diff --git a/server/src/http/mcp/story_tools/bug.rs b/server/src/http/mcp/story_tools/bug.rs index 00fa4e74..8e313ebc 100644 --- a/server/src/http/mcp/story_tools/bug.rs +++ b/server/src/http/mcp/story_tools/bug.rs @@ -71,7 +71,7 @@ pub(crate) fn tool_create_bug(args: &Value, ctx: &AppContext) -> Result Result Result, + acceptance_criteria: &[String], depends_on: Option<&[u32]>, ) -> Result { - let bug_number = next_item_number(root)?; - let slug = slugify_name(name); + let name_owned = name.to_string(); + let description_owned = description.to_string(); + let steps_owned = steps_to_reproduce.to_string(); + let actual_owned = actual_result.to_string(); + let expected_owned = expected_result.to_string(); + let depends_on_owned: Option> = depends_on.map(<[u32]>::to_vec); + let acs_owned: Vec = acceptance_criteria.to_vec(); - if slug.is_empty() { - return Err("Name must contain at least one alphanumeric character.".to_string()); - } - - let bug_id = format!("{bug_number}"); - - let mut content = String::new(); - content.push_str("---\n"); - content.push_str("type: bug\n"); - content.push_str(&format!("name: \"{}\"\n", name.replace('"', "\\\""))); - if let Some(deps) = depends_on.filter(|d| !d.is_empty()) { - let nums: Vec = deps.iter().map(|n| n.to_string()).collect(); - content.push_str(&format!("depends_on: [{}]\n", nums.join(", "))); - } - content.push_str("---\n\n"); - content.push_str(&format!("# Bug {bug_number}: {name}\n\n")); - content.push_str("## Description\n\n"); - content.push_str(description); - content.push_str("\n\n"); - content.push_str("## How to Reproduce\n\n"); - content.push_str(steps_to_reproduce); - content.push_str("\n\n"); - content.push_str("## Actual Result\n\n"); - content.push_str(actual_result); - content.push_str("\n\n"); - content.push_str("## Expected Result\n\n"); - content.push_str(expected_result); - content.push_str("\n\n"); - content.push_str("## Acceptance Criteria\n\n"); - if let Some(criteria) = acceptance_criteria { - for criterion in criteria { - content.push_str(&format!("- [ ] {criterion}\n")); - } - } else { - content.push_str("- [ ] Bug is fixed and verified\n"); - } - - // Write to database content store and CRDT. - write_story_content(root, &bug_id, "1_backlog", &content, Some(name)); - - // Sync depends_on to the typed CRDT register. - crate::crdt_state::set_depends_on(&bug_id, depends_on.unwrap_or(&[])); - - // Story 933: typed CRDT register for item_type. - crate::crdt_state::set_item_type(&bug_id, Some("bug")); - - Ok(bug_id) + create_item_in_backlog( + root, + "bug", + name, + acceptance_criteria, + depends_on, + move |bug_number| { + let mut content = String::new(); + content.push_str("---\n"); + content.push_str("type: bug\n"); + content.push_str(&format!("name: \"{}\"\n", name_owned.replace('"', "\\\""))); + if let Some(ref deps) = depends_on_owned.filter(|d| !d.is_empty()) { + let nums: Vec = deps.iter().map(|n| n.to_string()).collect(); + content.push_str(&format!("depends_on: [{}]\n", nums.join(", "))); + } + content.push_str("---\n\n"); + content.push_str(&format!("# Bug {bug_number}: {name_owned}\n\n")); + content.push_str("## Description\n\n"); + content.push_str(&description_owned); + content.push_str("\n\n"); + content.push_str("## How to Reproduce\n\n"); + content.push_str(&steps_owned); + content.push_str("\n\n"); + content.push_str("## Actual Result\n\n"); + content.push_str(&actual_owned); + content.push_str("\n\n"); + content.push_str("## Expected Result\n\n"); + content.push_str(&expected_owned); + content.push_str("\n\n"); + content.push_str("## Acceptance Criteria\n\n"); + for criterion in &acs_owned { + content.push_str(&format!("- [ ] {criterion}\n")); + } + content + }, + ) } /// Returns true if the item stem is a bug item. diff --git a/server/src/http/workflow/bug_ops/refactor.rs b/server/src/http/workflow/bug_ops/refactor.rs index d5b5f5d8..0269ea8c 100644 --- a/server/src/http/workflow/bug_ops/refactor.rs +++ b/server/src/http/workflow/bug_ops/refactor.rs @@ -2,69 +2,62 @@ use std::path::Path; -use super::super::{next_item_number, slugify_name, write_story_content}; +use super::super::create_item_in_backlog; /// Create a refactor work item and store it in the database. /// +/// Routes through `create_item_in_backlog`, the single internal creation path. +/// `acceptance_criteria` must be non-empty; an empty slice returns an error. /// Returns the refactor_id (e.g. `"5"`). pub fn create_refactor_file( root: &Path, name: &str, description: Option<&str>, - acceptance_criteria: Option<&[String]>, + acceptance_criteria: &[String], depends_on: Option<&[u32]>, ) -> Result { - let refactor_number = next_item_number(root)?; - let slug = slugify_name(name); + let name_owned = name.to_string(); + let description_owned = description.map(str::to_string); + let depends_on_owned: Option> = depends_on.map(<[u32]>::to_vec); + let acs_owned: Vec = acceptance_criteria.to_vec(); - if slug.is_empty() { - return Err("Name must contain at least one alphanumeric character.".to_string()); - } - - let refactor_id = format!("{refactor_number}"); - - let mut content = String::new(); - content.push_str("---\n"); - content.push_str("type: refactor\n"); - content.push_str(&format!("name: \"{}\"\n", name.replace('"', "\\\""))); - if let Some(deps) = depends_on.filter(|d| !d.is_empty()) { - let nums: Vec = deps.iter().map(|n| n.to_string()).collect(); - content.push_str(&format!("depends_on: [{}]\n", nums.join(", "))); - } - content.push_str("---\n\n"); - content.push_str(&format!("# Refactor {refactor_number}: {name}\n\n")); - content.push_str("## Current State\n\n"); - content.push_str("- TBD\n\n"); - content.push_str("## Desired State\n\n"); - if let Some(desc) = description { - content.push_str(desc); - content.push('\n'); - } else { - content.push_str("- TBD\n"); - } - content.push('\n'); - content.push_str("## Acceptance Criteria\n\n"); - if let Some(criteria) = acceptance_criteria { - for criterion in criteria { - content.push_str(&format!("- [ ] {criterion}\n")); - } - } else { - content.push_str("- [ ] Refactoring complete and all tests pass\n"); - } - content.push('\n'); - content.push_str("## Out of Scope\n\n"); - content.push_str("- TBD\n"); - - // Write to database content store and CRDT. - write_story_content(root, &refactor_id, "1_backlog", &content, Some(name)); - - // Sync depends_on to the typed CRDT register. - crate::crdt_state::set_depends_on(&refactor_id, depends_on.unwrap_or(&[])); - - // Story 933: typed CRDT register for item_type. - crate::crdt_state::set_item_type(&refactor_id, Some("refactor")); - - Ok(refactor_id) + create_item_in_backlog( + root, + "refactor", + name, + acceptance_criteria, + depends_on, + move |refactor_number| { + let mut content = String::new(); + content.push_str("---\n"); + content.push_str("type: refactor\n"); + content.push_str(&format!("name: \"{}\"\n", name_owned.replace('"', "\\\""))); + if let Some(ref deps) = depends_on_owned.filter(|d| !d.is_empty()) { + let nums: Vec = deps.iter().map(|n| n.to_string()).collect(); + content.push_str(&format!("depends_on: [{}]\n", nums.join(", "))); + } + content.push_str("---\n\n"); + content.push_str(&format!("# Refactor {refactor_number}: {name_owned}\n\n")); + content.push_str("## Current State\n\n"); + content.push_str("- TBD\n\n"); + content.push_str("## Desired State\n\n"); + if let Some(ref desc) = description_owned { + content.push_str(desc); + content.push('\n'); + } else { + content.push_str("- TBD\n"); + } + content.push('\n'); + content.push_str("## Acceptance Criteria\n\n"); + for criterion in &acs_owned { + content.push_str(&format!("- [ ] {criterion}\n")); + } + content.push('\n'); + content.push_str("## Out of Scope\n\n"); + content.push_str("- TBD\n"); + content + }, + ) } /// Returns true if the item stem is a refactor item. diff --git a/server/src/http/workflow/bug_ops/spike.rs b/server/src/http/workflow/bug_ops/spike.rs index 85cbe396..e7db17db 100644 --- a/server/src/http/workflow/bug_ops/spike.rs +++ b/server/src/http/workflow/bug_ops/spike.rs @@ -2,10 +2,12 @@ use std::path::Path; -use super::super::{next_item_number, slugify_name, write_story_content}; +use super::super::create_item_in_backlog; /// Create a spike file and store it in the database. /// +/// Routes through `create_item_in_backlog`, the single internal creation path. +/// `acceptance_criteria` must be non-empty; an empty slice returns an error. /// Returns the spike_id (e.g. `"4"`). pub fn create_spike_file( root: &Path, @@ -14,60 +16,51 @@ pub fn create_spike_file( acceptance_criteria: &[String], depends_on: Option<&[u32]>, ) -> Result { - let spike_number = next_item_number(root)?; - let slug = slugify_name(name); + let name_owned = name.to_string(); + let description_owned = description.map(str::to_string); + let depends_on_owned: Option> = depends_on.map(<[u32]>::to_vec); + let acs_owned: Vec = acceptance_criteria.to_vec(); - if slug.is_empty() { - return Err("Name must contain at least one alphanumeric character.".to_string()); - } - - let spike_id = format!("{spike_number}"); - - let mut content = String::new(); - content.push_str("---\n"); - content.push_str("type: spike\n"); - content.push_str(&format!("name: \"{}\"\n", name.replace('"', "\\\""))); - if let Some(deps) = depends_on.filter(|d| !d.is_empty()) { - let nums: Vec = deps.iter().map(|n| n.to_string()).collect(); - content.push_str(&format!("depends_on: [{}]\n", nums.join(", "))); - } - content.push_str("---\n\n"); - content.push_str(&format!("# Spike {spike_number}: {name}\n\n")); - content.push_str("## Question\n\n"); - if let Some(desc) = description { - content.push_str(desc); - content.push('\n'); - } else { - content.push_str("- TBD\n"); - } - content.push('\n'); - content.push_str("## Hypothesis\n\n"); - content.push_str("- TBD\n\n"); - content.push_str("## Timebox\n\n"); - content.push_str("- TBD\n\n"); - content.push_str("## Investigation Plan\n\n"); - content.push_str("- TBD\n\n"); - content.push_str("## Findings\n\n"); - content.push_str("- TBD\n\n"); - content.push_str("## Recommendation\n\n"); - content.push_str("- TBD\n\n"); - content.push_str("## Acceptance Criteria\n\n"); - if acceptance_criteria.is_empty() { - content.push_str("- [ ] TBD\n"); - } else { - for criterion in acceptance_criteria { - content.push_str(&format!("- [ ] {criterion}\n")); - } - } - - // Write to database content store and CRDT. - write_story_content(root, &spike_id, "1_backlog", &content, Some(name)); - - // Sync depends_on to the typed CRDT register. - crate::crdt_state::set_depends_on(&spike_id, depends_on.unwrap_or(&[])); - - // Story 933: typed CRDT register for item_type. - crate::crdt_state::set_item_type(&spike_id, Some("spike")); - - Ok(spike_id) + create_item_in_backlog( + root, + "spike", + name, + acceptance_criteria, + depends_on, + move |spike_number| { + let mut content = String::new(); + content.push_str("---\n"); + content.push_str("type: spike\n"); + content.push_str(&format!("name: \"{}\"\n", name_owned.replace('"', "\\\""))); + if let Some(ref deps) = depends_on_owned.filter(|d| !d.is_empty()) { + let nums: Vec = deps.iter().map(|n| n.to_string()).collect(); + content.push_str(&format!("depends_on: [{}]\n", nums.join(", "))); + } + content.push_str("---\n\n"); + content.push_str(&format!("# Spike {spike_number}: {name_owned}\n\n")); + content.push_str("## Question\n\n"); + if let Some(ref desc) = description_owned { + content.push_str(desc); + content.push('\n'); + } else { + content.push_str("- TBD\n"); + } + content.push('\n'); + content.push_str("## Hypothesis\n\n"); + content.push_str("- TBD\n\n"); + content.push_str("## Timebox\n\n"); + content.push_str("- TBD\n\n"); + content.push_str("## Investigation Plan\n\n"); + content.push_str("- TBD\n\n"); + content.push_str("## Findings\n\n"); + content.push_str("- TBD\n\n"); + content.push_str("## Recommendation\n\n"); + content.push_str("- TBD\n\n"); + content.push_str("## Acceptance Criteria\n\n"); + for criterion in &acs_owned { + content.push_str(&format!("- [ ] {criterion}\n")); + } + content + }, + ) } diff --git a/server/src/http/workflow/bug_ops/tests.rs b/server/src/http/workflow/bug_ops/tests.rs index c7f1066d..c96d8bef 100644 --- a/server/src/http/workflow/bug_ops/tests.rs +++ b/server/src/http/workflow/bug_ops/tests.rs @@ -176,7 +176,7 @@ fn create_bug_file_writes_correct_content() { "1. Go to /login\n2. Click submit", "Page crashes with 500 error", "Login succeeds", - Some(&["Login form submits without error".to_string()]), + &["Login form submits without error".to_string()], None, ) .unwrap(); @@ -226,7 +226,7 @@ fn create_bug_file_rejects_empty_name() { "steps", "actual", "expected", - None, + &[], None, ); assert!(result.is_err()); @@ -234,29 +234,23 @@ fn create_bug_file_rejects_empty_name() { } #[test] -fn create_bug_file_uses_default_acceptance_criterion() { +fn create_bug_file_rejects_empty_acceptance_criteria() { let tmp = tempfile::tempdir().unwrap(); - setup_git_repo(tmp.path()); - - let bug_id = create_bug_file( + let err = create_bug_file( tmp.path(), "Some Bug", "desc", "steps", "actual", "expected", - None, + &[], None, ) - .unwrap(); - - let contents = crate::db::read_content(&bug_id).expect("bug content should exist"); - + .unwrap_err(); assert!( - contents.starts_with("---\ntype: bug\nname: \"Some Bug\"\n---"), - "bug file must have YAML front matter with type field" + err.contains("acceptance criterion"), + "error should mention acceptance criterion, got: {err}" ); - assert!(contents.contains("- [ ] Bug is fixed and verified")); } // ── create_spike_file tests ──────────────────────────────────────────────── @@ -269,7 +263,7 @@ fn create_spike_file_writes_correct_content() { tmp.path(), "Filesystem Watcher Architecture", None, - &[], + &["Architecture documented".to_string()], None, ) .unwrap(); @@ -302,8 +296,14 @@ fn create_spike_file_uses_description_when_provided() { let tmp = tempfile::tempdir().unwrap(); let description = "What is the best approach for watching filesystem events?"; - let spike_id = - create_spike_file(tmp.path(), "FS Watcher Spike", Some(description), &[], None).unwrap(); + let spike_id = create_spike_file( + tmp.path(), + "FS Watcher Spike", + Some(description), + &["Findings documented".to_string()], + None, + ) + .unwrap(); let contents = crate::db::read_content(&spike_id) .or_else(|| { @@ -319,7 +319,14 @@ fn create_spike_file_uses_description_when_provided() { #[test] fn create_spike_file_uses_placeholder_when_no_description() { let tmp = tempfile::tempdir().unwrap(); - let spike_id = create_spike_file(tmp.path(), "My Spike", None, &[], None).unwrap(); + let spike_id = create_spike_file( + tmp.path(), + "My Spike", + None, + &["Findings documented".to_string()], + None, + ) + .unwrap(); let contents = crate::db::read_content(&spike_id) .or_else(|| { @@ -335,16 +342,22 @@ fn create_spike_file_uses_placeholder_when_no_description() { #[test] fn create_spike_file_rejects_empty_name() { let tmp = tempfile::tempdir().unwrap(); - let result = create_spike_file(tmp.path(), "!!!", None, &[], None); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("alphanumeric")); + // Name "!!!" has no alphanumeric chars — fails before AC check. + let err = create_spike_file(tmp.path(), "!!!", None, &["AC".to_string()], None).unwrap_err(); + assert!(err.contains("alphanumeric"), "got: {err}"); } #[test] fn create_spike_file_with_special_chars_in_name_produces_valid_yaml() { let tmp = tempfile::tempdir().unwrap(); let name = "Spike: compare \"fast\" vs slow encoders"; - let result = create_spike_file(tmp.path(), name, None, &[], None); + let result = create_spike_file( + tmp.path(), + name, + None, + &["Findings documented".to_string()], + None, + ); assert!(result.is_ok(), "create_spike_file failed: {result:?}"); let spike_id = result.unwrap(); @@ -364,7 +377,14 @@ fn create_spike_file_increments_from_existing_items() { crate::db::ItemMeta::named("Existing"), ); - let spike_id = create_spike_file(tmp.path(), "My Spike", None, &[], None).unwrap(); + let spike_id = create_spike_file( + tmp.path(), + "My Spike", + None, + &["Findings documented".to_string()], + None, + ) + .unwrap(); assert!( spike_id.chars().all(|c| c.is_ascii_digit()), "spike ID must be numeric-only, got: {spike_id}" @@ -391,7 +411,7 @@ fn create_bug_file_with_depends_on_persists_to_crdt() { "steps", "actual", "expected", - None, + &["Bug fixed".to_string()], Some(&[42, 43]), ) .unwrap(); @@ -412,7 +432,7 @@ fn create_bug_file_without_depends_on_omits_field() { "steps", "actual", "expected", - None, + &["Bug fixed".to_string()], None, ) .unwrap(); @@ -438,8 +458,14 @@ fn create_refactor_file_with_depends_on_persists_to_crdt() { let tmp = tempfile::tempdir().unwrap(); setup_git_repo(tmp.path()); - let refactor_id = - create_refactor_file(tmp.path(), "Dep Refactor", None, None, Some(&[99])).unwrap(); + let refactor_id = create_refactor_file( + tmp.path(), + "Dep Refactor", + None, + &["Refactoring complete".to_string()], + Some(&[99]), + ) + .unwrap(); let view = crate::crdt_state::read_item(&refactor_id).expect("CRDT entry should exist"); assert_eq!(view.depends_on(), &[99]); @@ -450,8 +476,14 @@ fn create_refactor_file_without_depends_on_omits_field() { let tmp = tempfile::tempdir().unwrap(); setup_git_repo(tmp.path()); - let refactor_id = - create_refactor_file(tmp.path(), "No Dep Refactor", None, None, None).unwrap(); + let refactor_id = create_refactor_file( + tmp.path(), + "No Dep Refactor", + None, + &["Refactoring complete".to_string()], + None, + ) + .unwrap(); let contents = crate::db::read_content(&refactor_id) .or_else(|| { diff --git a/server/src/http/workflow/mod.rs b/server/src/http/workflow/mod.rs index c9c8796a..8604b976 100644 --- a/server/src/http/workflow/mod.rs +++ b/server/src/http/workflow/mod.rs @@ -19,6 +19,7 @@ pub use story_ops::{ pub use test_results::{read_test_results_from_story_file, write_test_results_to_story_file}; pub(crate) use utils::{ - create_section_content, next_item_number, read_story_content, replace_or_append_section, - replace_section_content, slugify_name, story_stage, write_story_content, + create_item_in_backlog, create_section_content, next_item_number, read_story_content, + replace_or_append_section, replace_section_content, slugify_name, story_stage, + write_story_content, }; diff --git a/server/src/http/workflow/story_ops/create.rs b/server/src/http/workflow/story_ops/create.rs index 68421e38..720fb8de 100644 --- a/server/src/http/workflow/story_ops/create.rs +++ b/server/src/http/workflow/story_ops/create.rs @@ -1,80 +1,71 @@ //! create_story_file: write new story to CRDT/content store. -#[allow(unused_imports)] -use super::super::{ - create_section_content, next_item_number, read_story_content, replace_section_content, - slugify_name, story_stage, write_story_content, -}; +use super::super::create_item_in_backlog; /// Write a new story file to the CRDT content store and return the generated story ID. +/// +/// Routes through `create_item_in_backlog`, the single internal creation path. +/// Validates non-empty title and ≥ 1 acceptance criterion before writing anything. pub fn create_story_file( root: &std::path::Path, name: &str, user_story: Option<&str>, description: Option<&str>, - acceptance_criteria: Option<&[String]>, + acceptance_criteria: &[String], depends_on: Option<&[u32]>, _commit: bool, ) -> Result { - let story_number = next_item_number(root)?; - let slug = slugify_name(name); + let name_owned = name.to_string(); + let user_story_owned = user_story.map(str::to_string); + let description_owned = description.map(str::to_string); + let depends_on_owned: Option> = depends_on.map(<[u32]>::to_vec); + let acs_owned: Vec = acceptance_criteria.to_vec(); - if slug.is_empty() { - return Err("Name must contain at least one alphanumeric character.".to_string()); - } + create_item_in_backlog( + root, + "story", + name, + acceptance_criteria, + depends_on, + move |story_number| { + let mut content = String::new(); + content.push_str("---\n"); + content.push_str("type: story\n"); + content.push_str(&format!("name: \"{}\"\n", name_owned.replace('"', "\\\""))); + if let Some(ref deps) = depends_on_owned.filter(|d| !d.is_empty()) { + let nums: Vec = deps.iter().map(|n| n.to_string()).collect(); + content.push_str(&format!("depends_on: [{}]\n", nums.join(", "))); + } + content.push_str("---\n\n"); + content.push_str(&format!("# Story {story_number}: {name_owned}\n\n")); - let story_id = format!("{story_number}"); + content.push_str("## User Story\n\n"); + if let Some(ref us) = user_story_owned { + content.push_str(us); + content.push('\n'); + } else { + content.push_str("As a ..., I want ..., so that ...\n"); + } + content.push('\n'); - let mut content = String::new(); - content.push_str("---\n"); - content.push_str("type: story\n"); - content.push_str(&format!("name: \"{}\"\n", name.replace('"', "\\\""))); - if let Some(deps) = depends_on.filter(|d| !d.is_empty()) { - let nums: Vec = deps.iter().map(|n| n.to_string()).collect(); - content.push_str(&format!("depends_on: [{}]\n", nums.join(", "))); - } - content.push_str("---\n\n"); - content.push_str(&format!("# Story {story_number}: {name}\n\n")); + if let Some(ref desc) = description_owned { + content.push_str("## Description\n\n"); + content.push_str(desc); + content.push('\n'); + content.push('\n'); + } - content.push_str("## User Story\n\n"); - if let Some(us) = user_story { - content.push_str(us); - content.push('\n'); - } else { - content.push_str("As a ..., I want ..., so that ...\n"); - } - content.push('\n'); + content.push_str("## Acceptance Criteria\n\n"); + for criterion in &acs_owned { + content.push_str(&format!("- [ ] {criterion}\n")); + } + content.push('\n'); - if let Some(desc) = description { - content.push_str("## Description\n\n"); - content.push_str(desc); - content.push('\n'); - content.push('\n'); - } - - content.push_str("## Acceptance Criteria\n\n"); - if let Some(criteria) = acceptance_criteria { - for criterion in criteria { - content.push_str(&format!("- [ ] {criterion}\n")); - } - } else { - content.push_str("- [ ] TODO\n"); - } - content.push('\n'); - - content.push_str("## Out of Scope\n\n"); - content.push_str("- TBD\n"); - - // Write to database content store and CRDT. - write_story_content(root, &story_id, "1_backlog", &content, Some(name)); - - // Sync depends_on to the typed CRDT register. - crate::crdt_state::set_depends_on(&story_id, depends_on.unwrap_or(&[])); - - // Story 933: typed CRDT register for item_type. - crate::crdt_state::set_item_type(&story_id, Some("story")); - - Ok(story_id) + content.push_str("## Out of Scope\n\n"); + content.push_str("- TBD\n"); + content + }, + ) } /// Check off the Nth unchecked acceptance criterion in a story. @@ -187,7 +178,8 @@ mod tests { crate::crdt_state::init_for_test(); let tmp = tempfile::tempdir().unwrap(); let name = "Server-owned agent completion: remove report_completion dependency"; - let result = create_story_file(tmp.path(), name, None, None, None, None, false); + let acs = vec!["Completion handled server-side".to_string()]; + let result = create_story_file(tmp.path(), name, None, None, &acs, None, false); assert!(result.is_ok(), "create_story_file failed: {result:?}"); let story_id = result.unwrap(); @@ -202,12 +194,13 @@ mod tests { fn create_story_with_depends_on_persists_to_crdt() { crate::crdt_state::init_for_test(); let tmp = tempfile::tempdir().unwrap(); + let acs = vec!["Dependent criterion".to_string()]; let story_id = create_story_file( tmp.path(), "Dependent Story", None, None, - None, + &acs, Some(&[489]), false, ) @@ -224,7 +217,8 @@ mod tests { fn create_story_file_returns_numeric_only_id() { crate::db::ensure_content_store(); let tmp = tempfile::tempdir().unwrap(); - let result = create_story_file(tmp.path(), "My Feature", None, None, None, None, false); + let acs = vec!["Feature works".to_string()]; + let result = create_story_file(tmp.path(), "My Feature", None, None, &acs, None, false); assert!( result.is_ok(), "create_story_file should succeed: {result:?}" @@ -241,8 +235,9 @@ mod tests { crate::crdt_state::init_for_test(); crate::db::ensure_content_store(); let tmp = tempfile::tempdir().unwrap(); + let acs = vec!["Type validated".to_string()]; let story_id = - create_story_file(tmp.path(), "Type Test Story", None, None, None, None, false) + create_story_file(tmp.path(), "Type Test Story", None, None, &acs, None, false) .unwrap(); let view = crate::crdt_state::read_item(&story_id).expect("CRDT entry must exist"); assert_eq!( @@ -252,5 +247,38 @@ mod tests { ); } + #[test] + fn create_story_file_rejects_empty_title() { + let tmp = tempfile::tempdir().unwrap(); + let acs = vec!["Some criterion".to_string()]; + let err = create_story_file(tmp.path(), "", None, None, &acs, None, false).unwrap_err(); + assert!( + err.contains("empty") || err.contains("whitespace"), + "error should mention empty/whitespace, got: {err}" + ); + } + + #[test] + fn create_story_file_rejects_whitespace_only_title() { + let tmp = tempfile::tempdir().unwrap(); + let acs = vec!["Some criterion".to_string()]; + let err = create_story_file(tmp.path(), " ", None, None, &acs, None, false).unwrap_err(); + assert!( + err.contains("empty") || err.contains("whitespace"), + "error should mention empty/whitespace, got: {err}" + ); + } + + #[test] + fn create_story_file_rejects_empty_acceptance_criteria() { + let tmp = tempfile::tempdir().unwrap(); + let result = create_story_file(tmp.path(), "Valid Title", None, None, &[], None, false); + assert!(result.is_err(), "empty ACs should be rejected"); + assert!( + result.unwrap_err().contains("acceptance criterion"), + "error should mention acceptance criterion" + ); + } + // ── Story 504: native JSON types in front_matter ─────────────────────────── } diff --git a/server/src/http/workflow/utils.rs b/server/src/http/workflow/utils.rs index ef5b9636..e87cb07b 100644 --- a/server/src/http/workflow/utils.rs +++ b/server/src/http/workflow/utils.rs @@ -222,6 +222,55 @@ pub(crate) fn next_item_number(_root: &std::path::Path) -> Result { Ok(crate::db::next_item_number()) } +/// Single internal entry point for creating a new pipeline work item in the backlog. +/// +/// This is the canonical creation path. All `create_*_file` functions for pipeline +/// item types (story, bug, spike, refactor) route through here. On validation failure +/// this function returns `Err` and writes nothing. +/// +/// Validates: +/// - `name` is not empty or whitespace-only +/// - `name` contains at least one alphanumeric character +/// - `acceptance_criteria` has at least one entry +/// - `item_type` is a known pipeline item type +/// +/// `build_content` receives the assigned item number and returns the full markdown +/// content to persist (including front matter and all type-specific sections). +pub(crate) fn create_item_in_backlog( + root: &Path, + item_type: &str, + name: &str, + acceptance_criteria: &[String], + depends_on: Option<&[u32]>, + build_content: impl FnOnce(u32) -> String, +) -> Result { + if name.trim().is_empty() { + return Err("Title must not be empty or whitespace-only.".to_string()); + } + if slugify_name(name).is_empty() { + return Err("Title must contain at least one alphanumeric character.".to_string()); + } + if acceptance_criteria.is_empty() { + return Err("At least one acceptance criterion is required.".to_string()); + } + const VALID_TYPES: &[&str] = &["story", "bug", "spike", "refactor"]; + if !VALID_TYPES.contains(&item_type) { + return Err(format!( + "Invalid item type '{item_type}': must be one of story, bug, spike, refactor." + )); + } + + let item_number = next_item_number(root)?; + let item_id = format!("{item_number}"); + let content = build_content(item_number); + + write_story_content(root, &item_id, "1_backlog", &content, Some(name)); + crate::crdt_state::set_depends_on(&item_id, depends_on.unwrap_or(&[])); + crate::crdt_state::set_item_type(&item_id, Some(item_type)); + + Ok(item_id) +} + #[cfg(test)] mod tests { use super::*;