From adf936be07574b0f148ba9b24f2ea93944ff5037 Mon Sep 17 00:00:00 2001 From: dave Date: Mon, 27 Apr 2026 02:13:31 +0000 Subject: [PATCH] refactor: split http/workflow/story_ops.rs (1256) into create + criterion + update The 1256-line story_ops.rs is split: - create.rs: create_story_file + tests (~232 lines) - criterion.rs: check/add/remove/edit_criterion_in_file + tests (~525 lines) - update.rs: update_story_in_file + yaml helpers + tests (~640 lines) - mod.rs: re-exports (~12 lines) Workflow helpers (read_story_content, write_story_content, slugify_name, etc.) bumped from pub(super) to pub(crate) since they're now consumed across nested sub-modules and from http/mcp/story_tools/. Tests stay co-located. All 2636 tests pass; clippy clean. --- server/src/http/workflow/mod.rs | 14 +- server/src/http/workflow/story_ops.rs | 1256 ----------------- server/src/http/workflow/story_ops/create.rs | 232 +++ .../src/http/workflow/story_ops/criterion.rs | 522 +++++++ server/src/http/workflow/story_ops/mod.rs | 12 + server/src/http/workflow/story_ops/update.rs | 626 ++++++++ 6 files changed, 1399 insertions(+), 1263 deletions(-) delete mode 100644 server/src/http/workflow/story_ops.rs create mode 100644 server/src/http/workflow/story_ops/create.rs create mode 100644 server/src/http/workflow/story_ops/criterion.rs create mode 100644 server/src/http/workflow/story_ops/mod.rs create mode 100644 server/src/http/workflow/story_ops/update.rs diff --git a/server/src/http/workflow/mod.rs b/server/src/http/workflow/mod.rs index 2191edea..a60e888a 100644 --- a/server/src/http/workflow/mod.rs +++ b/server/src/http/workflow/mod.rs @@ -300,13 +300,13 @@ pub fn validate_story_dirs(_root: &std::path::Path) -> Result Result { +pub(crate) fn read_story_content(_project_root: &Path, story_id: &str) -> Result { crate::db::read_content(story_id) .ok_or_else(|| format!("Story '{story_id}' not found in any pipeline stage.")) } /// Write story content to the DB content store and CRDT. -pub(super) fn write_story_content( +pub(crate) fn write_story_content( _project_root: &Path, story_id: &str, stage: &str, @@ -316,7 +316,7 @@ pub(super) fn write_story_content( } /// Determine what stage a story is in (from CRDT). -pub(super) fn story_stage(story_id: &str) -> Option { +pub(crate) fn story_stage(story_id: &str) -> Option { crate::pipeline_state::read_typed(story_id) .ok() .flatten() @@ -328,7 +328,7 @@ pub(super) fn story_stage(story_id: &str) -> Option { /// Finds the first occurrence of `## {section_name}` and replaces everything /// until the next `##` heading (or end of file) with the provided text. /// Returns an error if the section is not found. -pub(super) fn replace_section_content( +pub(crate) fn replace_section_content( content: &str, section_name: &str, new_text: &str, @@ -381,7 +381,7 @@ pub(super) fn replace_section_content( /// The new section is placed immediately before the first occurrence of /// `## {before_section}`. If `before_section` is `None` or not found in the /// document, the section is appended at the end. -pub(super) fn create_section_content( +pub(crate) fn create_section_content( content: &str, section_name: &str, new_text: &str, @@ -468,7 +468,7 @@ pub(super) fn replace_or_append_section(contents: &str, header: &str, new_sectio } } -pub(super) fn slugify_name(name: &str) -> String { +pub(crate) fn slugify_name(name: &str) -> String { let slug: String = name .chars() .map(|c| { @@ -501,7 +501,7 @@ pub(super) fn slugify_name(name: &str) -> String { } /// Get the next available item number from the database/CRDT. -pub(super) fn next_item_number(_root: &std::path::Path) -> Result { +pub(crate) fn next_item_number(_root: &std::path::Path) -> Result { Ok(crate::db::next_item_number()) } diff --git a/server/src/http/workflow/story_ops.rs b/server/src/http/workflow/story_ops.rs deleted file mode 100644 index de277df3..00000000 --- a/server/src/http/workflow/story_ops.rs +++ /dev/null @@ -1,1256 +0,0 @@ -//! Story operations — creates, updates, and manages acceptance criteria in story files. -use crate::io::story_metadata::set_front_matter_field; -use serde_json::Value; -use std::collections::HashMap; -use std::path::Path; - -use super::{ - create_section_content, next_item_number, read_story_content, replace_section_content, - slugify_name, story_stage, write_story_content, -}; - -/// Shared create-story logic used by both the OpenApi and MCP handlers. -/// -/// Writes the new story to the database content store and CRDT. -/// The `commit` parameter is retained for API compatibility but ignored. -pub fn create_story_file( - root: &std::path::Path, - name: &str, - user_story: Option<&str>, - description: Option<&str>, - acceptance_criteria: Option<&[String]>, - depends_on: Option<&[u32]>, - _commit: bool, -) -> Result { - let story_number = next_item_number(root)?; - let slug = slugify_name(name); - - if slug.is_empty() { - return Err("Name must contain at least one alphanumeric character.".to_string()); - } - - let story_id = format!("{story_number}_story_{slug}"); - - let mut content = String::new(); - content.push_str("---\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")); - - 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'); - - 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); - - Ok(story_id) -} - -/// Check off the Nth unchecked acceptance criterion in a story. -/// -/// `criterion_index` is 0-based among unchecked (`- [ ]`) items. -pub fn check_criterion_in_file( - project_root: &Path, - story_id: &str, - criterion_index: usize, -) -> Result<(), String> { - let contents = read_story_content(project_root, story_id)?; - - let mut unchecked_count: usize = 0; - let mut found = false; - let new_lines: Vec = contents - .lines() - .map(|line| { - let trimmed = line.trim(); - if let Some(rest) = trimmed.strip_prefix("- [ ] ") { - if unchecked_count == criterion_index { - unchecked_count += 1; - found = true; - let indent_len = line.len() - trimmed.len(); - let indent = &line[..indent_len]; - return format!("{indent}- [x] {rest}"); - } - unchecked_count += 1; - } - line.to_string() - }) - .collect(); - - if !found { - return Err(format!( - "Criterion index {criterion_index} out of range. Story '{story_id}' has \ - {unchecked_count} unchecked criteria (indices 0..{}).", - unchecked_count.saturating_sub(1) - )); - } - - let mut new_str = new_lines.join("\n"); - if contents.ends_with('\n') { - new_str.push('\n'); - } - - // Write back to content store. - let stage = story_stage(story_id).unwrap_or_else(|| "2_current".to_string()); - write_story_content(project_root, story_id, &stage, &new_str); - - Ok(()) -} - -/// Remove an acceptance criterion from a story by its 0-based index (counting all criteria, -/// both checked and unchecked). Returns an error if the index is out of range. -pub fn remove_criterion_from_file( - project_root: &Path, - story_id: &str, - criterion_index: usize, -) -> Result<(), String> { - let contents = read_story_content(project_root, story_id)?; - - let mut count: usize = 0; - let mut found = false; - let new_lines: Vec = contents - .lines() - .filter(|line| { - let trimmed = line.trim(); - if trimmed.starts_with("- [ ] ") || trimmed.starts_with("- [x] ") { - if count == criterion_index { - count += 1; - found = true; - return false; - } - count += 1; - } - true - }) - .map(|s| s.to_string()) - .collect(); - - if !found { - return Err(format!( - "Criterion index {criterion_index} out of range. Story '{story_id}' has \ - {count} criteria (indices 0..{}).", - count.saturating_sub(1) - )); - } - - let mut new_str = new_lines.join("\n"); - if contents.ends_with('\n') { - new_str.push('\n'); - } - - let stage = story_stage(story_id).unwrap_or_else(|| "2_current".to_string()); - write_story_content(project_root, story_id, &stage, &new_str); - - Ok(()) -} - -/// Edit the text of an existing acceptance criterion without changing its checked state. -/// -/// Finds the criterion at `criterion_index` (0-based, counting all criteria regardless -/// of checked state) and replaces its text with `new_text`. -pub fn edit_criterion_in_file( - project_root: &Path, - story_id: &str, - criterion_index: usize, - new_text: &str, -) -> Result<(), String> { - let contents = read_story_content(project_root, story_id)?; - - let mut count: usize = 0; - let mut found = false; - let new_lines: Vec = contents - .lines() - .map(|line| { - let trimmed = line.trim(); - let prefix = if trimmed.starts_with("- [ ] ") { - Some("- [ ] ") - } else if trimmed.starts_with("- [x] ") { - Some("- [x] ") - } else { - None - }; - if let Some(p) = prefix { - if count == criterion_index { - count += 1; - found = true; - let indent_len = line.len() - trimmed.len(); - let indent = &line[..indent_len]; - return format!("{indent}{p}{new_text}"); - } - count += 1; - } - line.to_string() - }) - .collect(); - - if !found { - return Err(format!( - "Criterion index {criterion_index} out of range. Story '{story_id}' has \ - {count} criteria (indices 0..{}).", - count.saturating_sub(1) - )); - } - - let mut new_str = new_lines.join("\n"); - if contents.ends_with('\n') { - new_str.push('\n'); - } - - let stage = story_stage(story_id).unwrap_or_else(|| "2_current".to_string()); - write_story_content(project_root, story_id, &stage, &new_str); - - Ok(()) -} - -/// Add a new acceptance criterion to a story. -/// -/// Appends `- [ ] {criterion}` after the last existing criterion line in the -/// "## Acceptance Criteria" section. -pub fn add_criterion_to_file( - project_root: &Path, - story_id: &str, - criterion: &str, -) -> Result<(), String> { - let contents = read_story_content(project_root, story_id)?; - - let lines: Vec<&str> = contents.lines().collect(); - let mut in_ac_section = false; - let mut ac_section_start: Option = None; - let mut last_criterion_line: Option = None; - - for (i, line) in lines.iter().enumerate() { - let trimmed = line.trim(); - if trimmed == "## Acceptance Criteria" { - in_ac_section = true; - ac_section_start = Some(i); - continue; - } - if in_ac_section { - if trimmed.starts_with("## ") { - break; - } - if trimmed.starts_with("- [ ] ") || trimmed.starts_with("- [x] ") { - last_criterion_line = Some(i); - } - } - } - - let mut new_lines: Vec = lines.iter().map(|s| s.to_string()).collect(); - - let insert_after = if let Some(idx) = last_criterion_line.or(ac_section_start) { - idx - } else { - // No ## Acceptance Criteria section — create one on demand. - if new_lines.last().map(|l| !l.is_empty()).unwrap_or(false) { - new_lines.push(String::new()); - } - new_lines.push("## Acceptance Criteria".to_string()); - new_lines.push(String::new()); - new_lines.len() - 1 - }; - - new_lines.insert(insert_after + 1, format!("- [ ] {criterion}")); - - let mut new_str = new_lines.join("\n"); - if contents.ends_with('\n') { - new_str.push('\n'); - } - - // Write back to content store. - let stage = story_stage(story_id).unwrap_or_else(|| "2_current".to_string()); - write_story_content(project_root, story_id, &stage, &new_str); - - Ok(()) -} - -/// Encode a JSON value as a YAML scalar string. -/// -/// Native JSON types map to native YAML types: -/// - Bool → unquoted `true`/`false` -/// - Integer → unquoted integer -/// - Float → unquoted float -/// - Array → unquoted inline sequence (e.g. `[490, 491]`) -/// - String → quoted unless it looks like a bool, integer, or inline sequence -fn json_value_to_yaml_scalar(value: &Value) -> String { - match value { - Value::Bool(b) => b.to_string(), - Value::Number(n) => n.to_string(), - Value::Array(arr) => { - let items: Vec = arr.iter().map(|v| v.to_string()).collect(); - format!("[{}]", items.join(", ")) - } - Value::String(s) => yaml_encode_str(s), - // Null and Object are not meaningful as YAML scalars; store as quoted strings. - other => format!( - "\"{}\"", - other - .to_string() - .replace('"', "\\\"") - .replace('\n', " ") - .replace('\r', "") - ), - } -} - -/// Encode a plain string as a YAML scalar. -/// -/// Booleans (`true`/`false`), integers, and inline sequences (`[...]`) are -/// written unquoted. Everything else is quoted to avoid ambiguity. -fn yaml_encode_str(s: &str) -> String { - match s { - "true" | "false" => s.to_string(), - s if s.parse::().is_ok() => s.to_string(), - s if s.parse::().is_ok() => s.to_string(), - // YAML inline sequences like [490] or [490, 491] — write unquoted so - // serde_yaml can deserialise them as Vec. - s if s.starts_with('[') && s.ends_with(']') => s.to_string(), - s => format!( - "\"{}\"", - s.replace('"', "\\\"").replace('\n', " ").replace('\r', "") - ), - } -} - -/// Update the user story text and/or description in a story. -/// -/// At least one of `user_story` or `description` must be provided. -/// Replaces the content of the corresponding `##` section in place. -pub fn update_story_in_file( - project_root: &Path, - story_id: &str, - user_story: Option<&str>, - description: Option<&str>, - front_matter: Option<&HashMap>, -) -> Result<(), String> { - let has_front_matter_updates = front_matter.map(|m| !m.is_empty()).unwrap_or(false); - if user_story.is_none() && description.is_none() && !has_front_matter_updates { - return Err( - "At least one of 'user_story', 'description', or 'front_matter' must be provided." - .to_string(), - ); - } - - let mut contents = read_story_content(project_root, story_id)?; - - if let Some(fields) = front_matter { - if fields.contains_key("acceptance_criteria") { - return Err( - "'acceptance_criteria' is a reserved field managed via the story body \ - (use add_criterion / remove_criterion / edit_criterion instead)." - .to_string(), - ); - } - for (key, value) in fields { - let yaml_value = json_value_to_yaml_scalar(value); - contents = set_front_matter_field(&contents, key, &yaml_value); - } - } - - if let Some(us) = user_story { - contents = match replace_section_content(&contents, "User Story", us) { - Ok(updated) => updated, - Err(_) => { - create_section_content(&contents, "User Story", us, Some("Acceptance Criteria")) - } - }; - } - if let Some(desc) = description { - contents = match replace_section_content(&contents, "Description", desc) { - Ok(updated) => updated, - Err(_) => { - create_section_content(&contents, "Description", desc, Some("Acceptance Criteria")) - } - }; - } - - // Write back to content store. - let stage = story_stage(story_id).unwrap_or_else(|| "2_current".to_string()); - write_story_content(project_root, story_id, &stage, &contents); - - Ok(()) -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::io::story_metadata::parse_front_matter; - use std::fs; - - fn setup_git_repo(root: &std::path::Path) { - std::process::Command::new("git") - .args(["init"]) - .current_dir(root) - .output() - .unwrap(); - std::process::Command::new("git") - .args(["config", "user.email", "test@test.com"]) - .current_dir(root) - .output() - .unwrap(); - std::process::Command::new("git") - .args(["config", "user.name", "Test"]) - .current_dir(root) - .output() - .unwrap(); - std::process::Command::new("git") - .args(["commit", "--allow-empty", "-m", "init"]) - .current_dir(root) - .output() - .unwrap(); - } - - fn story_with_criteria(n: usize) -> String { - let mut s = "---\nname: Test Story\n---\n\n## Acceptance Criteria\n\n".to_string(); - for i in 0..n { - s.push_str(&format!("- [ ] Criterion {i}\n")); - } - s - } - - /// Helper to set up a story in the filesystem and content store for tests - /// that use check/add criterion. - fn setup_story_in_fs(root: &std::path::Path, story_id: &str, content: &str) { - let current = root.join(".huskies/work/2_current"); - fs::create_dir_all(¤t).unwrap(); - fs::write(current.join(format!("{story_id}.md")), content).unwrap(); - // Also write to the global content store so read_story_content picks up this - // content even when a previous test has left a stale entry for the same ID. - crate::db::ensure_content_store(); - crate::db::write_content(story_id, content); - } - - // --- create_story integration tests --- - - #[test] - fn create_story_writes_correct_content() { - crate::db::ensure_content_store(); - let tmp = tempfile::tempdir().unwrap(); - let backlog = tmp.path().join(".huskies/work/1_backlog"); - fs::create_dir_all(&backlog).unwrap(); - fs::write(backlog.join("36_story_existing.md"), "").unwrap(); - // Also write to content store so next_item_number sees it. - crate::db::write_item_with_content( - "36_story_existing", - "1_backlog", - "---\nname: Existing\n---\n", - ); - - let number = super::super::next_item_number(tmp.path()).unwrap(); - // The number must be >= 37 (at least higher than the existing "36_story_existing.md"), - // but the global content store may have higher-numbered items from parallel tests. - assert!(number >= 37, "expected number >= 37, got: {number}"); - - let slug = super::super::slugify_name("My New Feature"); - assert_eq!(slug, "my_new_feature"); - - let filename = format!("{number}_{slug}.md"); - let filepath = backlog.join(&filename); - - let mut content = String::new(); - content.push_str("---\n"); - content.push_str("name: \"My New Feature\"\n"); - content.push_str("---\n\n"); - content.push_str(&format!("# Story {number}: My New Feature\n\n")); - content.push_str("## User Story\n\n"); - content.push_str("As a dev, I want this feature\n\n"); - content.push_str("## Acceptance Criteria\n\n"); - content.push_str("- [ ] It works\n"); - content.push_str("- [ ] It is tested\n\n"); - content.push_str("## Out of Scope\n\n"); - content.push_str("- TBD\n"); - - fs::write(&filepath, &content).unwrap(); - - let written = fs::read_to_string(&filepath).unwrap(); - assert!(written.starts_with("---\nname: \"My New Feature\"\n---")); - assert!(written.contains(&format!("# Story {number}: My New Feature"))); - assert!(written.contains("- [ ] It works")); - assert!(written.contains("- [ ] It is tested")); - assert!(written.contains("## Out of Scope")); - } - - #[test] - fn create_story_with_colon_in_name_produces_valid_yaml() { - 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); - assert!(result.is_ok(), "create_story_file failed: {result:?}"); - - let story_id = result.unwrap(); - // Read from content store or filesystem. - let content = crate::db::read_content(&story_id) - .or_else(|| { - let backlog = tmp.path().join(".huskies/work/1_backlog"); - fs::read_to_string(backlog.join(format!("{story_id}.md"))).ok() - }) - .expect("story content should exist"); - - let meta = parse_front_matter(&content).expect("front matter should be valid YAML"); - assert_eq!(meta.name.as_deref(), Some(name)); - } - - // ── check_criterion_in_file tests ───────────────────────────────────────── - - #[test] - fn check_criterion_marks_first_unchecked() { - let tmp = tempfile::tempdir().unwrap(); - setup_git_repo(tmp.path()); - setup_story_in_fs(tmp.path(), "1_test", &story_with_criteria(3)); - - check_criterion_in_file(tmp.path(), "1_test", 0).unwrap(); - - // Read the updated content. - let contents = read_story_content(tmp.path(), "1_test").unwrap(); - assert!( - contents.contains("- [x] Criterion 0"), - "first should be checked" - ); - assert!( - contents.contains("- [ ] Criterion 1"), - "second should stay unchecked" - ); - assert!( - contents.contains("- [ ] Criterion 2"), - "third should stay unchecked" - ); - } - - #[test] - fn check_criterion_marks_second_unchecked() { - let tmp = tempfile::tempdir().unwrap(); - setup_git_repo(tmp.path()); - setup_story_in_fs(tmp.path(), "2_test", &story_with_criteria(3)); - - check_criterion_in_file(tmp.path(), "2_test", 1).unwrap(); - - let contents = read_story_content(tmp.path(), "2_test").unwrap(); - assert!( - contents.contains("- [ ] Criterion 0"), - "first should stay unchecked" - ); - assert!( - contents.contains("- [x] Criterion 1"), - "second should be checked" - ); - assert!( - contents.contains("- [ ] Criterion 2"), - "third should stay unchecked" - ); - } - - #[test] - fn check_criterion_out_of_range_returns_error() { - let tmp = tempfile::tempdir().unwrap(); - setup_git_repo(tmp.path()); - setup_story_in_fs(tmp.path(), "3_test", &story_with_criteria(2)); - - let result = check_criterion_in_file(tmp.path(), "3_test", 5); - assert!(result.is_err(), "should fail for out-of-range index"); - assert!(result.unwrap_err().contains("out of range")); - } - - // ── add_criterion_to_file tests ─────────────────────────────────────────── - - fn story_with_ac_section(criteria: &[&str]) -> String { - let mut s = - "---\nname: Test\n---\n\n## User Story\n\nAs a user...\n\n## Acceptance Criteria\n\n" - .to_string(); - for c in criteria { - s.push_str(&format!("- [ ] {c}\n")); - } - s.push_str("\n## Out of Scope\n\n- N/A\n"); - s - } - - #[test] - fn add_criterion_appends_after_last_criterion() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs( - tmp.path(), - "10_test", - &story_with_ac_section(&["First", "Second"]), - ); - - add_criterion_to_file(tmp.path(), "10_test", "Third").unwrap(); - - let contents = read_story_content(tmp.path(), "10_test").unwrap(); - assert!(contents.contains("- [ ] First\n")); - assert!(contents.contains("- [ ] Second\n")); - assert!(contents.contains("- [ ] Third\n")); - let pos_second = contents.find("- [ ] Second").unwrap(); - let pos_third = contents.find("- [ ] Third").unwrap(); - assert!(pos_third > pos_second, "Third should appear after Second"); - } - - #[test] - fn add_criterion_to_empty_section() { - let tmp = tempfile::tempdir().unwrap(); - let content = - "---\nname: Test\n---\n\n## Acceptance Criteria\n\n## Out of Scope\n\n- N/A\n"; - setup_story_in_fs(tmp.path(), "11_test", content); - - add_criterion_to_file(tmp.path(), "11_test", "New AC").unwrap(); - - let contents = read_story_content(tmp.path(), "11_test").unwrap(); - assert!( - contents.contains("- [ ] New AC\n"), - "criterion should be present" - ); - } - - #[test] - fn add_criterion_creates_section_when_missing() { - // Bug 625: adding a criterion to a story that has no ## Acceptance Criteria - // section (e.g. a spike converted to a story) must auto-create the section. - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs( - tmp.path(), - "12_test", - "---\nname: Test\n---\n\nNo AC section here.\n", - ); - - let result = add_criterion_to_file(tmp.path(), "12_test", "X"); - assert!( - result.is_ok(), - "should succeed by creating the section: {result:?}" - ); - - let contents = read_story_content(tmp.path(), "12_test").unwrap(); - assert!( - contents.contains("## Acceptance Criteria"), - "section should be created" - ); - assert!(contents.contains("- [ ] X"), "criterion should be present"); - } - - /// Bug 625: spike-to-story conversion + AC addition end-to-end. - /// - /// Simulates: create spike content (no AC section), convert to story via - /// update_story front_matter type field, then add_criterion three times, - /// and assert all three appear in the AC section. - #[test] - fn spike_converted_to_story_accepts_add_criterion() { - let tmp = tempfile::tempdir().unwrap(); - - // Spike content: no ## Acceptance Criteria section. - let spike_content = "---\nname: \"My Spike\"\n---\n\n\ - ## Question\n\n- What is the best approach?\n\n\ - ## Hypothesis\n\n- TBD\n\n\ - ## Findings\n\n- TBD\n\n\ - ## Recommendation\n\n- TBD\n"; - setup_story_in_fs(tmp.path(), "100_spike_my_spike", spike_content); - - // Convert spike to story by updating the type field. - let mut fields = HashMap::new(); - fields.insert("type".to_string(), Value::String("story".to_string())); - update_story_in_file(tmp.path(), "100_spike_my_spike", None, None, Some(&fields)) - .expect("converting spike type to story should succeed"); - - // Add three acceptance criteria. - add_criterion_to_file(tmp.path(), "100_spike_my_spike", "First criterion") - .expect("add first criterion"); - add_criterion_to_file(tmp.path(), "100_spike_my_spike", "Second criterion") - .expect("add second criterion"); - add_criterion_to_file(tmp.path(), "100_spike_my_spike", "Third criterion") - .expect("add third criterion"); - - let contents = read_story_content(tmp.path(), "100_spike_my_spike").unwrap(); - assert!( - contents.contains("## Acceptance Criteria"), - "AC section should be present" - ); - assert!( - contents.contains("- [ ] First criterion"), - "first AC missing" - ); - assert!( - contents.contains("- [ ] Second criterion"), - "second AC missing" - ); - assert!( - contents.contains("- [ ] Third criterion"), - "third AC missing" - ); - } - - #[test] - fn update_story_acceptance_criteria_in_front_matter_returns_error() { - // Bug 625: passing acceptance_criteria via front_matter must return an - // explicit error rather than silently dropping the value. - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs( - tmp.path(), - "101_test", - "---\nname: T\n---\n\n## Acceptance Criteria\n\n- [ ] Existing\n", - ); - - let mut fields = HashMap::new(); - fields.insert( - "acceptance_criteria".to_string(), - serde_json::json!(["crit 1", "crit 2"]), - ); - let result = update_story_in_file(tmp.path(), "101_test", None, None, Some(&fields)); - assert!(result.is_err(), "should fail with reserved-field error"); - let err = result.unwrap_err(); - assert!( - err.contains("acceptance_criteria"), - "error should name the reserved field: {err}" - ); - } - - // ── remove_criterion_from_file tests ────────────────────────────────────── - - #[test] - fn remove_criterion_removes_by_index() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs( - tmp.path(), - "20_remove_test", - &story_with_ac_section(&["First", "Second", "Third"]), - ); - - remove_criterion_from_file(tmp.path(), "20_remove_test", 1).unwrap(); - - let contents = read_story_content(tmp.path(), "20_remove_test").unwrap(); - assert!(contents.contains("- [ ] First"), "First should remain"); - assert!(!contents.contains("Second"), "Second should be removed"); - assert!(contents.contains("- [ ] Third"), "Third should remain"); - } - - #[test] - fn remove_criterion_shifts_indices() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs( - tmp.path(), - "21_remove_test", - &story_with_ac_section(&["A", "B", "C"]), - ); - - remove_criterion_from_file(tmp.path(), "21_remove_test", 0).unwrap(); - - let contents = read_story_content(tmp.path(), "21_remove_test").unwrap(); - assert!(!contents.contains("- [ ] A"), "A should be removed"); - assert!(contents.contains("- [ ] B"), "B should remain"); - assert!(contents.contains("- [ ] C"), "C should remain"); - // B is now at index 0, C at index 1 — verify by removing B next - remove_criterion_from_file(tmp.path(), "21_remove_test", 0).unwrap(); - let contents2 = read_story_content(tmp.path(), "21_remove_test").unwrap(); - assert!(!contents2.contains("- [ ] B"), "B should now be removed"); - assert!(contents2.contains("- [ ] C"), "C should still remain"); - } - - #[test] - fn remove_criterion_out_of_range_returns_error() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs( - tmp.path(), - "22_remove_test", - &story_with_ac_section(&["Only"]), - ); - - let result = remove_criterion_from_file(tmp.path(), "22_remove_test", 5); - assert!(result.is_err(), "should fail for out-of-range index"); - assert!(result.unwrap_err().contains("out of range")); - } - - // ── update_story_in_file tests ───────────────────────────────────────────── - - #[test] - fn update_story_replaces_user_story_section() { - let tmp = tempfile::tempdir().unwrap(); - let content = "---\nname: T\n---\n\n## User Story\n\nOld text\n\n## Acceptance Criteria\n\n- [ ] AC\n"; - setup_story_in_fs(tmp.path(), "20_test", content); - - update_story_in_file( - tmp.path(), - "20_test", - Some("New user story text"), - None, - None, - ) - .unwrap(); - - let result = read_story_content(tmp.path(), "20_test").unwrap(); - assert!( - result.contains("New user story text"), - "new text should be present" - ); - assert!(!result.contains("Old text"), "old text should be replaced"); - assert!( - result.contains("## Acceptance Criteria"), - "other sections preserved" - ); - } - - #[test] - fn update_story_replaces_description_section() { - let tmp = tempfile::tempdir().unwrap(); - let content = "---\nname: T\n---\n\n## Description\n\nOld description\n\n## Acceptance Criteria\n\n- [ ] AC\n"; - setup_story_in_fs(tmp.path(), "21_test", content); - - update_story_in_file(tmp.path(), "21_test", None, Some("New description"), None).unwrap(); - - let result = read_story_content(tmp.path(), "21_test").unwrap(); - assert!( - result.contains("New description"), - "new description present" - ); - assert!( - !result.contains("Old description"), - "old description replaced" - ); - } - - #[test] - fn update_story_no_args_returns_error() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs(tmp.path(), "22_test", "---\nname: T\n---\n"); - - let result = update_story_in_file(tmp.path(), "22_test", None, None, None); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("At least one")); - } - - #[test] - fn update_story_creates_user_story_section_if_missing() { - let tmp = tempfile::tempdir().unwrap(); - // Story with no ## User Story section but has ## Acceptance Criteria. - let content = "---\nname: T\n---\n\n## Acceptance Criteria\n\n- [ ] AC\n"; - setup_story_in_fs(tmp.path(), "23_test", content); - - let result = - update_story_in_file(tmp.path(), "23_test", Some("New user story"), None, None); - assert!( - result.is_ok(), - "should succeed when section is missing: {result:?}" - ); - - let updated = read_story_content(tmp.path(), "23_test").unwrap(); - assert!( - updated.contains("## User Story"), - "section should be created" - ); - assert!(updated.contains("New user story"), "text should be present"); - // Section should appear before Acceptance Criteria. - let pos_us = updated.find("## User Story").unwrap(); - let pos_ac = updated.find("## Acceptance Criteria").unwrap(); - assert!( - pos_us < pos_ac, - "User Story should be before Acceptance Criteria" - ); - } - - #[test] - fn update_story_creates_description_section_if_missing() { - let tmp = tempfile::tempdir().unwrap(); - // Story with no ## Description section but has ## Acceptance Criteria. - let content = "---\nname: T\n---\n\n## User Story\n\nAs a user...\n\n## Acceptance Criteria\n\n- [ ] AC\n"; - setup_story_in_fs(tmp.path(), "32_test", content); - - let result = update_story_in_file( - tmp.path(), - "32_test", - None, - Some("New description text"), - None, - ); - assert!( - result.is_ok(), - "should succeed when section is missing: {result:?}" - ); - - let updated = read_story_content(tmp.path(), "32_test").unwrap(); - assert!( - updated.contains("## Description"), - "section should be created" - ); - assert!( - updated.contains("New description text"), - "text should be present" - ); - // Section should appear before Acceptance Criteria. - let pos_desc = updated.find("## Description").unwrap(); - let pos_ac = updated.find("## Acceptance Criteria").unwrap(); - assert!( - pos_desc < pos_ac, - "Description should be before Acceptance Criteria" - ); - } - - #[test] - fn update_story_creates_description_section_no_ac_section() { - let tmp = tempfile::tempdir().unwrap(); - // Story with no ## Description and no ## Acceptance Criteria. - let content = "---\nname: T\n---\n\nSome content here.\n"; - setup_story_in_fs(tmp.path(), "33_test", content); - - let result = update_story_in_file( - tmp.path(), - "33_test", - None, - Some("Appended description"), - None, - ); - assert!( - result.is_ok(), - "should succeed even with no Acceptance Criteria: {result:?}" - ); - - let updated = read_story_content(tmp.path(), "33_test").unwrap(); - assert!( - updated.contains("## Description"), - "section should be created" - ); - assert!( - updated.contains("Appended description"), - "text should be present" - ); - } - - #[test] - fn update_story_sets_agent_front_matter_field() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs( - tmp.path(), - "24_test", - "---\nname: T\n---\n\n## User Story\n\nSome story\n", - ); - - let mut fields = HashMap::new(); - fields.insert("agent".to_string(), Value::String("dev".to_string())); - update_story_in_file(tmp.path(), "24_test", None, None, Some(&fields)).unwrap(); - - let result = read_story_content(tmp.path(), "24_test").unwrap(); - assert!( - result.contains("agent: \"dev\""), - "agent field should be set" - ); - assert!(result.contains("name: T"), "name field preserved"); - } - - #[test] - fn update_story_sets_arbitrary_front_matter_fields() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs( - tmp.path(), - "25_test", - "---\nname: T\n---\n\n## User Story\n\nSome story\n", - ); - - let mut fields = HashMap::new(); - fields.insert("qa".to_string(), Value::String("human".to_string())); - fields.insert("priority".to_string(), Value::String("high".to_string())); - update_story_in_file(tmp.path(), "25_test", None, None, Some(&fields)).unwrap(); - - let result = read_story_content(tmp.path(), "25_test").unwrap(); - assert!(result.contains("qa: \"human\""), "qa field should be set"); - assert!( - result.contains("priority: \"high\""), - "priority field should be set" - ); - assert!(result.contains("name: T"), "name field preserved"); - } - - #[test] - fn update_story_front_matter_only_no_section_required() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs( - tmp.path(), - "26_test", - "---\nname: T\n---\n\nNo sections here.\n", - ); - - let mut fields = HashMap::new(); - fields.insert("agent".to_string(), Value::String("dev".to_string())); - let result = update_story_in_file(tmp.path(), "26_test", None, None, Some(&fields)); - assert!( - result.is_ok(), - "front-matter-only update should not require body sections" - ); - - let contents = read_story_content(tmp.path(), "26_test").unwrap(); - assert!(contents.contains("agent: \"dev\"")); - } - - #[test] - fn update_story_bool_front_matter_written_unquoted() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs(tmp.path(), "27_test", "---\nname: T\n---\n\nNo sections.\n"); - - // String "false" still works (backwards compatibility). - let mut fields = HashMap::new(); - fields.insert("blocked".to_string(), Value::String("false".to_string())); - update_story_in_file(tmp.path(), "27_test", None, None, Some(&fields)).unwrap(); - - let result = read_story_content(tmp.path(), "27_test").unwrap(); - assert!( - result.contains("blocked: false"), - "bool should be unquoted: {result}" - ); - assert!( - !result.contains("blocked: \"false\""), - "bool must not be quoted: {result}" - ); - } - - #[test] - fn update_story_integer_front_matter_written_unquoted() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs(tmp.path(), "28_test", "---\nname: T\n---\n\nNo sections.\n"); - - // String "0" still works (backwards compatibility). - let mut fields = HashMap::new(); - fields.insert("retry_count".to_string(), Value::String("0".to_string())); - update_story_in_file(tmp.path(), "28_test", None, None, Some(&fields)).unwrap(); - - let result = read_story_content(tmp.path(), "28_test").unwrap(); - assert!( - result.contains("retry_count: 0"), - "integer should be unquoted: {result}" - ); - assert!( - !result.contains("retry_count: \"0\""), - "integer must not be quoted: {result}" - ); - } - - #[test] - fn update_story_bool_front_matter_parseable_after_write() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs( - tmp.path(), - "29_test", - "---\nname: My Story\n---\n\nNo sections.\n", - ); - - let mut fields = HashMap::new(); - fields.insert("blocked".to_string(), Value::String("false".to_string())); - update_story_in_file(tmp.path(), "29_test", None, None, Some(&fields)).unwrap(); - - let contents = read_story_content(tmp.path(), "29_test").unwrap(); - let meta = parse_front_matter(&contents).expect("front matter should parse"); - assert_eq!( - meta.name.as_deref(), - Some("My Story"), - "name preserved after writing bool field" - ); - } - - // ── Bug 493 regression tests ────────────────────────────────────────────── - - #[test] - fn update_story_depends_on_stored_as_yaml_array_not_quoted_string() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs(tmp.path(), "30_test", "---\nname: T\n---\n\nNo sections.\n"); - - // String "[490]" still works (backwards compatibility). - let mut fields = HashMap::new(); - fields.insert("depends_on".to_string(), Value::String("[490]".to_string())); - update_story_in_file(tmp.path(), "30_test", None, None, Some(&fields)).unwrap(); - - let result = read_story_content(tmp.path(), "30_test").unwrap(); - assert!( - result.contains("depends_on: [490]"), - "should be unquoted array: {result}" - ); - assert!( - !result.contains("depends_on: \"[490]\""), - "must not be quoted: {result}" - ); - - let meta = parse_front_matter(&result).expect("front matter should parse"); - assert_eq!(meta.depends_on, Some(vec![490])); - } - - #[test] - fn create_story_with_depends_on_writes_front_matter_array() { - let tmp = tempfile::tempdir().unwrap(); - let story_id = create_story_file( - tmp.path(), - "Dependent Story", - None, - None, - None, - Some(&[489]), - false, - ) - .unwrap(); - - let contents = crate::db::read_content(&story_id) - .or_else(|| { - let backlog = tmp.path().join(".huskies/work/1_backlog"); - fs::read_to_string(backlog.join(format!("{story_id}.md"))).ok() - }) - .expect("story content should exist"); - - assert!( - contents.contains("depends_on: [489]"), - "missing front matter: {contents}" - ); - assert!( - !contents.contains("- [ ] depends_on"), - "must not appear as checkbox: {contents}" - ); - - let meta = parse_front_matter(&contents).expect("front matter should parse"); - assert_eq!(meta.depends_on, Some(vec![489])); - } - - // ── Story 504: native JSON types in front_matter ─────────────────────────── - - #[test] - fn update_story_native_bool_written_unquoted() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs(tmp.path(), "31_test", "---\nname: T\n---\n\nNo sections.\n"); - - let mut fields = HashMap::new(); - fields.insert("blocked".to_string(), Value::Bool(false)); - update_story_in_file(tmp.path(), "31_test", None, None, Some(&fields)).unwrap(); - - let result = read_story_content(tmp.path(), "31_test").unwrap(); - assert!( - result.contains("blocked: false"), - "native bool false should be unquoted: {result}" - ); - assert!( - !result.contains("blocked: \"false\""), - "must not be quoted: {result}" - ); - } - - #[test] - fn update_story_native_bool_true_written_unquoted() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs(tmp.path(), "32_test", "---\nname: T\n---\n\nNo sections.\n"); - - let mut fields = HashMap::new(); - fields.insert("blocked".to_string(), Value::Bool(true)); - update_story_in_file(tmp.path(), "32_test", None, None, Some(&fields)).unwrap(); - - let result = read_story_content(tmp.path(), "32_test").unwrap(); - assert!( - result.contains("blocked: true"), - "native bool true should be unquoted: {result}" - ); - assert!( - !result.contains("blocked: \"true\""), - "must not be quoted: {result}" - ); - } - - #[test] - fn update_story_native_integer_written_unquoted() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs( - tmp.path(), - "33b_test", - "---\nname: T\n---\n\nNo sections.\n", - ); - - let mut fields = HashMap::new(); - fields.insert("retry_count".to_string(), serde_json::json!(3)); - update_story_in_file(tmp.path(), "33b_test", None, None, Some(&fields)).unwrap(); - - let result = read_story_content(tmp.path(), "33b_test").unwrap(); - assert!( - result.contains("retry_count: 3"), - "native integer should be unquoted: {result}" - ); - assert!( - !result.contains("retry_count: \"3\""), - "must not be quoted: {result}" - ); - } - - #[test] - fn update_story_native_array_written_as_yaml_sequence() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs(tmp.path(), "34_test", "---\nname: T\n---\n\nNo sections.\n"); - - let mut fields = HashMap::new(); - fields.insert("depends_on".to_string(), serde_json::json!([490, 491])); - update_story_in_file(tmp.path(), "34_test", None, None, Some(&fields)).unwrap(); - - let result = read_story_content(tmp.path(), "34_test").unwrap(); - assert!( - result.contains("depends_on: [490, 491]"), - "native array should be YAML sequence: {result}" - ); - assert!( - !result.contains("depends_on: \"["), - "must not be quoted: {result}" - ); - - let meta = parse_front_matter(&result).expect("front matter should parse"); - assert_eq!(meta.depends_on, Some(vec![490, 491])); - } - - #[test] - fn update_story_native_bool_parseable_after_write() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs( - tmp.path(), - "35_test", - "---\nname: My Story\n---\n\nNo sections.\n", - ); - - let mut fields = HashMap::new(); - fields.insert("blocked".to_string(), Value::Bool(false)); - update_story_in_file(tmp.path(), "35_test", None, None, Some(&fields)).unwrap(); - - let contents = read_story_content(tmp.path(), "35_test").unwrap(); - let meta = parse_front_matter(&contents).expect("front matter should parse"); - assert_eq!( - meta.name.as_deref(), - Some("My Story"), - "name preserved after writing native bool" - ); - } - - #[test] - fn update_story_depends_on_multi_element_array() { - let tmp = tempfile::tempdir().unwrap(); - setup_story_in_fs(tmp.path(), "31_test", "---\nname: T\n---\n\nNo sections.\n"); - - // String "[490, 491]" still works (backwards compatibility). - let mut fields = HashMap::new(); - fields.insert( - "depends_on".to_string(), - Value::String("[490, 491]".to_string()), - ); - update_story_in_file(tmp.path(), "31_test", None, None, Some(&fields)).unwrap(); - - let result = read_story_content(tmp.path(), "31_test").unwrap(); - let meta = parse_front_matter(&result).expect("front matter should parse"); - assert_eq!(meta.depends_on, Some(vec![490, 491])); - } -} diff --git a/server/src/http/workflow/story_ops/create.rs b/server/src/http/workflow/story_ops/create.rs new file mode 100644 index 00000000..e5b99d20 --- /dev/null +++ b/server/src/http/workflow/story_ops/create.rs @@ -0,0 +1,232 @@ +//! 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}; + +pub fn create_story_file( + root: &std::path::Path, + name: &str, + user_story: Option<&str>, + description: Option<&str>, + acceptance_criteria: Option<&[String]>, + depends_on: Option<&[u32]>, + _commit: bool, +) -> Result { + let story_number = next_item_number(root)?; + let slug = slugify_name(name); + + if slug.is_empty() { + return Err("Name must contain at least one alphanumeric character.".to_string()); + } + + let story_id = format!("{story_number}_story_{slug}"); + + let mut content = String::new(); + content.push_str("---\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")); + + 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'); + + 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); + + Ok(story_id) +} + +/// Check off the Nth unchecked acceptance criterion in a story. +/// +/// `criterion_index` is 0-based among unchecked (`- [ ]`) items. +#[cfg(test)] +mod tests { + use super::*; + use crate::io::story_metadata::parse_front_matter; + use std::fs; + + #[allow(dead_code)] + fn setup_git_repo(root: &std::path::Path) { + std::process::Command::new("git") + .args(["init"]) + .current_dir(root) + .output() + .unwrap(); + std::process::Command::new("git") + .args(["config", "user.email", "test@test.com"]) + .current_dir(root) + .output() + .unwrap(); + std::process::Command::new("git") + .args(["config", "user.name", "Test"]) + .current_dir(root) + .output() + .unwrap(); + std::process::Command::new("git") + .args(["commit", "--allow-empty", "-m", "init"]) + .current_dir(root) + .output() + .unwrap(); + } + + #[allow(dead_code)] + fn story_with_criteria(n: usize) -> String { + let mut s = "---\nname: Test Story\n---\n\n## Acceptance Criteria\n\n".to_string(); + for i in 0..n { + s.push_str(&format!("- [ ] Criterion {i}\n")); + } + s + } + + /// Helper to set up a story in the filesystem and content store for tests + /// that use check/add criterion. + #[allow(dead_code)] + fn setup_story_in_fs(root: &std::path::Path, story_id: &str, content: &str) { + let current = root.join(".huskies/work/2_current"); + fs::create_dir_all(¤t).unwrap(); + fs::write(current.join(format!("{story_id}.md")), content).unwrap(); + // Also write to the global content store so read_story_content picks up this + // content even when a previous test has left a stale entry for the same ID. + crate::db::ensure_content_store(); + crate::db::write_content(story_id, content); + } + + // --- create_story integration tests --- + + #[test] + fn create_story_writes_correct_content() { + crate::db::ensure_content_store(); + let tmp = tempfile::tempdir().unwrap(); + let backlog = tmp.path().join(".huskies/work/1_backlog"); + fs::create_dir_all(&backlog).unwrap(); + fs::write(backlog.join("36_story_existing.md"), "").unwrap(); + // Also write to content store so next_item_number sees it. + crate::db::write_item_with_content( + "36_story_existing", + "1_backlog", + "---\nname: Existing\n---\n", + ); + + let number = super::super::super::next_item_number(tmp.path()).unwrap(); + // The number must be >= 37 (at least higher than the existing "36_story_existing.md"), + // but the global content store may have higher-numbered items from parallel tests. + assert!(number >= 37, "expected number >= 37, got: {number}"); + + let slug = super::super::super::slugify_name("My New Feature"); + assert_eq!(slug, "my_new_feature"); + + let filename = format!("{number}_{slug}.md"); + let filepath = backlog.join(&filename); + + let mut content = String::new(); + content.push_str("---\n"); + content.push_str("name: \"My New Feature\"\n"); + content.push_str("---\n\n"); + content.push_str(&format!("# Story {number}: My New Feature\n\n")); + content.push_str("## User Story\n\n"); + content.push_str("As a dev, I want this feature\n\n"); + content.push_str("## Acceptance Criteria\n\n"); + content.push_str("- [ ] It works\n"); + content.push_str("- [ ] It is tested\n\n"); + content.push_str("## Out of Scope\n\n"); + content.push_str("- TBD\n"); + + fs::write(&filepath, &content).unwrap(); + + let written = fs::read_to_string(&filepath).unwrap(); + assert!(written.starts_with("---\nname: \"My New Feature\"\n---")); + assert!(written.contains(&format!("# Story {number}: My New Feature"))); + assert!(written.contains("- [ ] It works")); + assert!(written.contains("- [ ] It is tested")); + assert!(written.contains("## Out of Scope")); + } + + #[test] + fn create_story_with_colon_in_name_produces_valid_yaml() { + 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); + assert!(result.is_ok(), "create_story_file failed: {result:?}"); + + let story_id = result.unwrap(); + // Read from content store or filesystem. + let content = crate::db::read_content(&story_id) + .or_else(|| { + let backlog = tmp.path().join(".huskies/work/1_backlog"); + fs::read_to_string(backlog.join(format!("{story_id}.md"))).ok() + }) + .expect("story content should exist"); + + let meta = parse_front_matter(&content).expect("front matter should be valid YAML"); + assert_eq!(meta.name.as_deref(), Some(name)); + } + + // ── check_criterion_in_file tests ───────────────────────────────────────── + + #[test] + fn create_story_with_depends_on_writes_front_matter_array() { + let tmp = tempfile::tempdir().unwrap(); + let story_id = create_story_file( + tmp.path(), + "Dependent Story", + None, + None, + None, + Some(&[489]), + false, + ) + .unwrap(); + + let contents = crate::db::read_content(&story_id) + .or_else(|| { + let backlog = tmp.path().join(".huskies/work/1_backlog"); + fs::read_to_string(backlog.join(format!("{story_id}.md"))).ok() + }) + .expect("story content should exist"); + + assert!( + contents.contains("depends_on: [489]"), + "missing front matter: {contents}" + ); + assert!( + !contents.contains("- [ ] depends_on"), + "must not appear as checkbox: {contents}" + ); + + let meta = parse_front_matter(&contents).expect("front matter should parse"); + assert_eq!(meta.depends_on, Some(vec![489])); + } + + // ── Story 504: native JSON types in front_matter ─────────────────────────── +} diff --git a/server/src/http/workflow/story_ops/criterion.rs b/server/src/http/workflow/story_ops/criterion.rs new file mode 100644 index 00000000..8e2adf12 --- /dev/null +++ b/server/src/http/workflow/story_ops/criterion.rs @@ -0,0 +1,522 @@ +//! Acceptance-criterion file edits: check / add / remove / edit by index. + +use std::path::Path; + +#[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}; + + +pub fn check_criterion_in_file( + project_root: &Path, + story_id: &str, + criterion_index: usize, +) -> Result<(), String> { + let contents = read_story_content(project_root, story_id)?; + + let mut unchecked_count: usize = 0; + let mut found = false; + let new_lines: Vec = contents + .lines() + .map(|line| { + let trimmed = line.trim(); + if let Some(rest) = trimmed.strip_prefix("- [ ] ") { + if unchecked_count == criterion_index { + unchecked_count += 1; + found = true; + let indent_len = line.len() - trimmed.len(); + let indent = &line[..indent_len]; + return format!("{indent}- [x] {rest}"); + } + unchecked_count += 1; + } + line.to_string() + }) + .collect(); + + if !found { + return Err(format!( + "Criterion index {criterion_index} out of range. Story '{story_id}' has \ + {unchecked_count} unchecked criteria (indices 0..{}).", + unchecked_count.saturating_sub(1) + )); + } + + let mut new_str = new_lines.join("\n"); + if contents.ends_with('\n') { + new_str.push('\n'); + } + + // Write back to content store. + let stage = story_stage(story_id).unwrap_or_else(|| "2_current".to_string()); + write_story_content(project_root, story_id, &stage, &new_str); + + Ok(()) +} + +/// Remove an acceptance criterion from a story by its 0-based index (counting all criteria, +/// both checked and unchecked). Returns an error if the index is out of range. +pub fn remove_criterion_from_file( + project_root: &Path, + story_id: &str, + criterion_index: usize, +) -> Result<(), String> { + let contents = read_story_content(project_root, story_id)?; + + let mut count: usize = 0; + let mut found = false; + let new_lines: Vec = contents + .lines() + .filter(|line| { + let trimmed = line.trim(); + if trimmed.starts_with("- [ ] ") || trimmed.starts_with("- [x] ") { + if count == criterion_index { + count += 1; + found = true; + return false; + } + count += 1; + } + true + }) + .map(|s| s.to_string()) + .collect(); + + if !found { + return Err(format!( + "Criterion index {criterion_index} out of range. Story '{story_id}' has \ + {count} criteria (indices 0..{}).", + count.saturating_sub(1) + )); + } + + let mut new_str = new_lines.join("\n"); + if contents.ends_with('\n') { + new_str.push('\n'); + } + + let stage = story_stage(story_id).unwrap_or_else(|| "2_current".to_string()); + write_story_content(project_root, story_id, &stage, &new_str); + + Ok(()) +} + +/// Edit the text of an existing acceptance criterion without changing its checked state. +/// +/// Finds the criterion at `criterion_index` (0-based, counting all criteria regardless +/// of checked state) and replaces its text with `new_text`. +pub fn edit_criterion_in_file( + project_root: &Path, + story_id: &str, + criterion_index: usize, + new_text: &str, +) -> Result<(), String> { + let contents = read_story_content(project_root, story_id)?; + + let mut count: usize = 0; + let mut found = false; + let new_lines: Vec = contents + .lines() + .map(|line| { + let trimmed = line.trim(); + let prefix = if trimmed.starts_with("- [ ] ") { + Some("- [ ] ") + } else if trimmed.starts_with("- [x] ") { + Some("- [x] ") + } else { + None + }; + if let Some(p) = prefix { + if count == criterion_index { + count += 1; + found = true; + let indent_len = line.len() - trimmed.len(); + let indent = &line[..indent_len]; + return format!("{indent}{p}{new_text}"); + } + count += 1; + } + line.to_string() + }) + .collect(); + + if !found { + return Err(format!( + "Criterion index {criterion_index} out of range. Story '{story_id}' has \ + {count} criteria (indices 0..{}).", + count.saturating_sub(1) + )); + } + + let mut new_str = new_lines.join("\n"); + if contents.ends_with('\n') { + new_str.push('\n'); + } + + let stage = story_stage(story_id).unwrap_or_else(|| "2_current".to_string()); + write_story_content(project_root, story_id, &stage, &new_str); + + Ok(()) +} + +/// Add a new acceptance criterion to a story. +/// +/// Appends `- [ ] {criterion}` after the last existing criterion line in the +/// "## Acceptance Criteria" section. +pub fn add_criterion_to_file( + project_root: &Path, + story_id: &str, + criterion: &str, +) -> Result<(), String> { + let contents = read_story_content(project_root, story_id)?; + + let lines: Vec<&str> = contents.lines().collect(); + let mut in_ac_section = false; + let mut ac_section_start: Option = None; + let mut last_criterion_line: Option = None; + + for (i, line) in lines.iter().enumerate() { + let trimmed = line.trim(); + if trimmed == "## Acceptance Criteria" { + in_ac_section = true; + ac_section_start = Some(i); + continue; + } + if in_ac_section { + if trimmed.starts_with("## ") { + break; + } + if trimmed.starts_with("- [ ] ") || trimmed.starts_with("- [x] ") { + last_criterion_line = Some(i); + } + } + } + + let mut new_lines: Vec = lines.iter().map(|s| s.to_string()).collect(); + + let insert_after = if let Some(idx) = last_criterion_line.or(ac_section_start) { + idx + } else { + // No ## Acceptance Criteria section — create one on demand. + if new_lines.last().map(|l| !l.is_empty()).unwrap_or(false) { + new_lines.push(String::new()); + } + new_lines.push("## Acceptance Criteria".to_string()); + new_lines.push(String::new()); + new_lines.len() - 1 + }; + + new_lines.insert(insert_after + 1, format!("- [ ] {criterion}")); + + let mut new_str = new_lines.join("\n"); + if contents.ends_with('\n') { + new_str.push('\n'); + } + + // Write back to content store. + let stage = story_stage(story_id).unwrap_or_else(|| "2_current".to_string()); + write_story_content(project_root, story_id, &stage, &new_str); + + Ok(()) +} + +/// Encode a JSON value as a YAML scalar string. +/// +/// Native JSON types map to native YAML types: +/// - Bool → unquoted `true`/`false` +/// - Integer → unquoted integer +/// - Float → unquoted float +/// - Array → unquoted inline sequence (e.g. `[490, 491]`) +/// - String → quoted unless it looks like a bool, integer, or inline sequence +#[cfg(test)] +mod tests { + use super::*; + use serde_json::Value; + use std::collections::HashMap; + use super::super::update_story_in_file; + use std::fs; + + #[allow(dead_code)] + fn setup_git_repo(root: &std::path::Path) { + std::process::Command::new("git") + .args(["init"]) + .current_dir(root) + .output() + .unwrap(); + std::process::Command::new("git") + .args(["config", "user.email", "test@test.com"]) + .current_dir(root) + .output() + .unwrap(); + std::process::Command::new("git") + .args(["config", "user.name", "Test"]) + .current_dir(root) + .output() + .unwrap(); + std::process::Command::new("git") + .args(["commit", "--allow-empty", "-m", "init"]) + .current_dir(root) + .output() + .unwrap(); + } + + #[allow(dead_code)] + fn story_with_criteria(n: usize) -> String { + let mut s = "---\nname: Test Story\n---\n\n## Acceptance Criteria\n\n".to_string(); + for i in 0..n { + s.push_str(&format!("- [ ] Criterion {i}\n")); + } + s + } + + /// Helper to set up a story in the filesystem and content store for tests + /// that use check/add criterion. + #[allow(dead_code)] + fn setup_story_in_fs(root: &std::path::Path, story_id: &str, content: &str) { + let current = root.join(".huskies/work/2_current"); + fs::create_dir_all(¤t).unwrap(); + fs::write(current.join(format!("{story_id}.md")), content).unwrap(); + // Also write to the global content store so read_story_content picks up this + // content even when a previous test has left a stale entry for the same ID. + crate::db::ensure_content_store(); + crate::db::write_content(story_id, content); + } + + // --- create_story integration tests --- + + #[test] + fn check_criterion_marks_first_unchecked() { + let tmp = tempfile::tempdir().unwrap(); + setup_git_repo(tmp.path()); + setup_story_in_fs(tmp.path(), "1_test", &story_with_criteria(3)); + + check_criterion_in_file(tmp.path(), "1_test", 0).unwrap(); + + // Read the updated content. + let contents = read_story_content(tmp.path(), "1_test").unwrap(); + assert!( + contents.contains("- [x] Criterion 0"), + "first should be checked" + ); + assert!( + contents.contains("- [ ] Criterion 1"), + "second should stay unchecked" + ); + assert!( + contents.contains("- [ ] Criterion 2"), + "third should stay unchecked" + ); + } + + #[test] + fn check_criterion_marks_second_unchecked() { + let tmp = tempfile::tempdir().unwrap(); + setup_git_repo(tmp.path()); + setup_story_in_fs(tmp.path(), "2_test", &story_with_criteria(3)); + + check_criterion_in_file(tmp.path(), "2_test", 1).unwrap(); + + let contents = read_story_content(tmp.path(), "2_test").unwrap(); + assert!( + contents.contains("- [ ] Criterion 0"), + "first should stay unchecked" + ); + assert!( + contents.contains("- [x] Criterion 1"), + "second should be checked" + ); + assert!( + contents.contains("- [ ] Criterion 2"), + "third should stay unchecked" + ); + } + + #[test] + fn check_criterion_out_of_range_returns_error() { + let tmp = tempfile::tempdir().unwrap(); + setup_git_repo(tmp.path()); + setup_story_in_fs(tmp.path(), "3_test", &story_with_criteria(2)); + + let result = check_criterion_in_file(tmp.path(), "3_test", 5); + assert!(result.is_err(), "should fail for out-of-range index"); + assert!(result.unwrap_err().contains("out of range")); + } + + // ── add_criterion_to_file tests ─────────────────────────────────────────── + + #[allow(dead_code)] + fn story_with_ac_section(criteria: &[&str]) -> String { + let mut s = + "---\nname: Test\n---\n\n## User Story\n\nAs a user...\n\n## Acceptance Criteria\n\n" + .to_string(); + for c in criteria { + s.push_str(&format!("- [ ] {c}\n")); + } + s.push_str("\n## Out of Scope\n\n- N/A\n"); + s + } + + #[test] + fn add_criterion_appends_after_last_criterion() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs( + tmp.path(), + "10_test", + &story_with_ac_section(&["First", "Second"]), + ); + + add_criterion_to_file(tmp.path(), "10_test", "Third").unwrap(); + + let contents = read_story_content(tmp.path(), "10_test").unwrap(); + assert!(contents.contains("- [ ] First\n")); + assert!(contents.contains("- [ ] Second\n")); + assert!(contents.contains("- [ ] Third\n")); + let pos_second = contents.find("- [ ] Second").unwrap(); + let pos_third = contents.find("- [ ] Third").unwrap(); + assert!(pos_third > pos_second, "Third should appear after Second"); + } + + #[test] + fn add_criterion_to_empty_section() { + let tmp = tempfile::tempdir().unwrap(); + let content = + "---\nname: Test\n---\n\n## Acceptance Criteria\n\n## Out of Scope\n\n- N/A\n"; + setup_story_in_fs(tmp.path(), "11_test", content); + + add_criterion_to_file(tmp.path(), "11_test", "New AC").unwrap(); + + let contents = read_story_content(tmp.path(), "11_test").unwrap(); + assert!( + contents.contains("- [ ] New AC\n"), + "criterion should be present" + ); + } + + #[test] + fn add_criterion_creates_section_when_missing() { + // Bug 625: adding a criterion to a story that has no ## Acceptance Criteria + // section (e.g. a spike converted to a story) must auto-create the section. + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs( + tmp.path(), + "12_test", + "---\nname: Test\n---\n\nNo AC section here.\n", + ); + + let result = add_criterion_to_file(tmp.path(), "12_test", "X"); + assert!( + result.is_ok(), + "should succeed by creating the section: {result:?}" + ); + + let contents = read_story_content(tmp.path(), "12_test").unwrap(); + assert!( + contents.contains("## Acceptance Criteria"), + "section should be created" + ); + assert!(contents.contains("- [ ] X"), "criterion should be present"); + } + + /// Bug 625: spike-to-story conversion + AC addition end-to-end. + /// + /// Simulates: create spike content (no AC section), convert to story via + /// update_story front_matter type field, then add_criterion three times, + #[test] + fn spike_converted_to_story_accepts_add_criterion() { + let tmp = tempfile::tempdir().unwrap(); + + // Spike content: no ## Acceptance Criteria section. + let spike_content = "---\nname: \"My Spike\"\n---\n\n\ + ## Question\n\n- What is the best approach?\n\n\ + ## Hypothesis\n\n- TBD\n\n\ + ## Findings\n\n- TBD\n\n\ + ## Recommendation\n\n- TBD\n"; + setup_story_in_fs(tmp.path(), "100_spike_my_spike", spike_content); + + // Convert spike to story by updating the type field. + let mut fields = HashMap::new(); + fields.insert("type".to_string(), Value::String("story".to_string())); + update_story_in_file(tmp.path(), "100_spike_my_spike", None, None, Some(&fields)) + .expect("converting spike type to story should succeed"); + + // Add three acceptance criteria. + add_criterion_to_file(tmp.path(), "100_spike_my_spike", "First criterion") + .expect("add first criterion"); + add_criterion_to_file(tmp.path(), "100_spike_my_spike", "Second criterion") + .expect("add second criterion"); + add_criterion_to_file(tmp.path(), "100_spike_my_spike", "Third criterion") + .expect("add third criterion"); + + let contents = read_story_content(tmp.path(), "100_spike_my_spike").unwrap(); + assert!( + contents.contains("## Acceptance Criteria"), + "AC section should be present" + ); + assert!( + contents.contains("- [ ] First criterion"), + "first AC missing" + ); + assert!( + contents.contains("- [ ] Second criterion"), + "second AC missing" + ); + assert!( + contents.contains("- [ ] Third criterion"), + "third AC missing" + ); + } + + #[test] + fn remove_criterion_removes_by_index() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs( + tmp.path(), + "20_remove_test", + &story_with_ac_section(&["First", "Second", "Third"]), + ); + + remove_criterion_from_file(tmp.path(), "20_remove_test", 1).unwrap(); + + let contents = read_story_content(tmp.path(), "20_remove_test").unwrap(); + assert!(contents.contains("- [ ] First"), "First should remain"); + assert!(!contents.contains("Second"), "Second should be removed"); + assert!(contents.contains("- [ ] Third"), "Third should remain"); + } + + #[test] + fn remove_criterion_shifts_indices() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs( + tmp.path(), + "21_remove_test", + &story_with_ac_section(&["A", "B", "C"]), + ); + + remove_criterion_from_file(tmp.path(), "21_remove_test", 0).unwrap(); + + let contents = read_story_content(tmp.path(), "21_remove_test").unwrap(); + assert!(!contents.contains("- [ ] A"), "A should be removed"); + assert!(contents.contains("- [ ] B"), "B should remain"); + assert!(contents.contains("- [ ] C"), "C should remain"); + // B is now at index 0, C at index 1 — verify by removing B next + remove_criterion_from_file(tmp.path(), "21_remove_test", 0).unwrap(); + let contents2 = read_story_content(tmp.path(), "21_remove_test").unwrap(); + assert!(!contents2.contains("- [ ] B"), "B should now be removed"); + assert!(contents2.contains("- [ ] C"), "C should still remain"); + } + + #[test] + fn remove_criterion_out_of_range_returns_error() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs( + tmp.path(), + "22_remove_test", + &story_with_ac_section(&["Only"]), + ); + + let result = remove_criterion_from_file(tmp.path(), "22_remove_test", 5); + assert!(result.is_err(), "should fail for out-of-range index"); + assert!(result.unwrap_err().contains("out of range")); + } + + // ── update_story_in_file tests ───────────────────────────────────────────── +} diff --git a/server/src/http/workflow/story_ops/mod.rs b/server/src/http/workflow/story_ops/mod.rs new file mode 100644 index 00000000..5863c7b0 --- /dev/null +++ b/server/src/http/workflow/story_ops/mod.rs @@ -0,0 +1,12 @@ +//! Story operations — creates, updates, and manages acceptance criteria in story files. + +mod create; +mod criterion; +mod update; + +pub use create::create_story_file; +pub use criterion::{ + add_criterion_to_file, check_criterion_in_file, edit_criterion_in_file, + remove_criterion_from_file, +}; +pub use update::update_story_in_file; diff --git a/server/src/http/workflow/story_ops/update.rs b/server/src/http/workflow/story_ops/update.rs new file mode 100644 index 00000000..889cb779 --- /dev/null +++ b/server/src/http/workflow/story_ops/update.rs @@ -0,0 +1,626 @@ +//! update_story_in_file: replaces sections / writes front matter on existing stories. + +use serde_json::Value; +use std::collections::HashMap; +use std::path::Path; + +#[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 crate::io::story_metadata::set_front_matter_field; + + +fn json_value_to_yaml_scalar(value: &Value) -> String { + match value { + Value::Bool(b) => b.to_string(), + Value::Number(n) => n.to_string(), + Value::Array(arr) => { + let items: Vec = arr.iter().map(|v| v.to_string()).collect(); + format!("[{}]", items.join(", ")) + } + Value::String(s) => yaml_encode_str(s), + // Null and Object are not meaningful as YAML scalars; store as quoted strings. + other => format!( + "\"{}\"", + other + .to_string() + .replace('"', "\\\"") + .replace('\n', " ") + .replace('\r', "") + ), + } +} + +/// Encode a plain string as a YAML scalar. +/// +/// Booleans (`true`/`false`), integers, and inline sequences (`[...]`) are +/// written unquoted. Everything else is quoted to avoid ambiguity. +fn yaml_encode_str(s: &str) -> String { + match s { + "true" | "false" => s.to_string(), + s if s.parse::().is_ok() => s.to_string(), + s if s.parse::().is_ok() => s.to_string(), + // YAML inline sequences like [490] or [490, 491] — write unquoted so + // serde_yaml can deserialise them as Vec. + s if s.starts_with('[') && s.ends_with(']') => s.to_string(), + s => format!( + "\"{}\"", + s.replace('"', "\\\"").replace('\n', " ").replace('\r', "") + ), + } +} + +/// Update the user story text and/or description in a story. +/// +/// At least one of `user_story` or `description` must be provided. +/// Replaces the content of the corresponding `##` section in place. +pub fn update_story_in_file( + project_root: &Path, + story_id: &str, + user_story: Option<&str>, + description: Option<&str>, + front_matter: Option<&HashMap>, +) -> Result<(), String> { + let has_front_matter_updates = front_matter.map(|m| !m.is_empty()).unwrap_or(false); + if user_story.is_none() && description.is_none() && !has_front_matter_updates { + return Err( + "At least one of 'user_story', 'description', or 'front_matter' must be provided." + .to_string(), + ); + } + + let mut contents = read_story_content(project_root, story_id)?; + + if let Some(fields) = front_matter { + if fields.contains_key("acceptance_criteria") { + return Err( + "'acceptance_criteria' is a reserved field managed via the story body \ + (use add_criterion / remove_criterion / edit_criterion instead)." + .to_string(), + ); + } + for (key, value) in fields { + let yaml_value = json_value_to_yaml_scalar(value); + contents = set_front_matter_field(&contents, key, &yaml_value); + } + } + + if let Some(us) = user_story { + contents = match replace_section_content(&contents, "User Story", us) { + Ok(updated) => updated, + Err(_) => { + create_section_content(&contents, "User Story", us, Some("Acceptance Criteria")) + } + }; + } + if let Some(desc) = description { + contents = match replace_section_content(&contents, "Description", desc) { + Ok(updated) => updated, + Err(_) => { + create_section_content(&contents, "Description", desc, Some("Acceptance Criteria")) + } + }; + } + + // Write back to content store. + let stage = story_stage(story_id).unwrap_or_else(|| "2_current".to_string()); + write_story_content(project_root, story_id, &stage, &contents); + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::io::story_metadata::parse_front_matter; + use std::fs; + + #[allow(dead_code)] + fn setup_git_repo(root: &std::path::Path) { + std::process::Command::new("git") + .args(["init"]) + .current_dir(root) + .output() + .unwrap(); + std::process::Command::new("git") + .args(["config", "user.email", "test@test.com"]) + .current_dir(root) + .output() + .unwrap(); + std::process::Command::new("git") + .args(["config", "user.name", "Test"]) + .current_dir(root) + .output() + .unwrap(); + std::process::Command::new("git") + .args(["commit", "--allow-empty", "-m", "init"]) + .current_dir(root) + .output() + .unwrap(); + } + + #[allow(dead_code)] + fn story_with_criteria(n: usize) -> String { + let mut s = "---\nname: Test Story\n---\n\n## Acceptance Criteria\n\n".to_string(); + for i in 0..n { + s.push_str(&format!("- [ ] Criterion {i}\n")); + } + s + } + + /// Helper to set up a story in the filesystem and content store for tests + /// that use check/add criterion. + #[allow(dead_code)] + fn setup_story_in_fs(root: &std::path::Path, story_id: &str, content: &str) { + let current = root.join(".huskies/work/2_current"); + fs::create_dir_all(¤t).unwrap(); + fs::write(current.join(format!("{story_id}.md")), content).unwrap(); + // Also write to the global content store so read_story_content picks up this + // content even when a previous test has left a stale entry for the same ID. + crate::db::ensure_content_store(); + crate::db::write_content(story_id, content); + } + + // --- create_story integration tests --- + + #[test] + fn update_story_acceptance_criteria_in_front_matter_returns_error() { + // Bug 625: passing acceptance_criteria via front_matter must return an + // explicit error rather than silently dropping the value. + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs( + tmp.path(), + "101_test", + "---\nname: T\n---\n\n## Acceptance Criteria\n\n- [ ] Existing\n", + ); + + let mut fields = HashMap::new(); + fields.insert( + "acceptance_criteria".to_string(), + serde_json::json!(["crit 1", "crit 2"]), + ); + let result = update_story_in_file(tmp.path(), "101_test", None, None, Some(&fields)); + assert!(result.is_err(), "should fail with reserved-field error"); + let err = result.unwrap_err(); + assert!( + err.contains("acceptance_criteria"), + "error should name the reserved field: {err}" + ); + } + + // ── remove_criterion_from_file tests ────────────────────────────────────── + + #[test] + fn update_story_replaces_user_story_section() { + let tmp = tempfile::tempdir().unwrap(); + let content = "---\nname: T\n---\n\n## User Story\n\nOld text\n\n## Acceptance Criteria\n\n- [ ] AC\n"; + setup_story_in_fs(tmp.path(), "20_test", content); + + update_story_in_file( + tmp.path(), + "20_test", + Some("New user story text"), + None, + None, + ) + .unwrap(); + + let result = read_story_content(tmp.path(), "20_test").unwrap(); + assert!( + result.contains("New user story text"), + "new text should be present" + ); + assert!(!result.contains("Old text"), "old text should be replaced"); + assert!( + result.contains("## Acceptance Criteria"), + "other sections preserved" + ); + } + + #[test] + fn update_story_replaces_description_section() { + let tmp = tempfile::tempdir().unwrap(); + let content = "---\nname: T\n---\n\n## Description\n\nOld description\n\n## Acceptance Criteria\n\n- [ ] AC\n"; + setup_story_in_fs(tmp.path(), "21_test", content); + + update_story_in_file(tmp.path(), "21_test", None, Some("New description"), None).unwrap(); + + let result = read_story_content(tmp.path(), "21_test").unwrap(); + assert!( + result.contains("New description"), + "new description present" + ); + assert!( + !result.contains("Old description"), + "old description replaced" + ); + } + + #[test] + fn update_story_no_args_returns_error() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "22_test", "---\nname: T\n---\n"); + + let result = update_story_in_file(tmp.path(), "22_test", None, None, None); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("At least one")); + } + + #[test] + fn update_story_creates_user_story_section_if_missing() { + let tmp = tempfile::tempdir().unwrap(); + // Story with no ## User Story section but has ## Acceptance Criteria. + let content = "---\nname: T\n---\n\n## Acceptance Criteria\n\n- [ ] AC\n"; + setup_story_in_fs(tmp.path(), "23_test", content); + + let result = + update_story_in_file(tmp.path(), "23_test", Some("New user story"), None, None); + assert!( + result.is_ok(), + "should succeed when section is missing: {result:?}" + ); + + let updated = read_story_content(tmp.path(), "23_test").unwrap(); + assert!( + updated.contains("## User Story"), + "section should be created" + ); + assert!(updated.contains("New user story"), "text should be present"); + // Section should appear before Acceptance Criteria. + let pos_us = updated.find("## User Story").unwrap(); + let pos_ac = updated.find("## Acceptance Criteria").unwrap(); + assert!( + pos_us < pos_ac, + "User Story should be before Acceptance Criteria" + ); + } + + #[test] + fn update_story_creates_description_section_if_missing() { + let tmp = tempfile::tempdir().unwrap(); + // Story with no ## Description section but has ## Acceptance Criteria. + let content = "---\nname: T\n---\n\n## User Story\n\nAs a user...\n\n## Acceptance Criteria\n\n- [ ] AC\n"; + setup_story_in_fs(tmp.path(), "32_test", content); + + let result = update_story_in_file( + tmp.path(), + "32_test", + None, + Some("New description text"), + None, + ); + assert!( + result.is_ok(), + "should succeed when section is missing: {result:?}" + ); + + let updated = read_story_content(tmp.path(), "32_test").unwrap(); + assert!( + updated.contains("## Description"), + "section should be created" + ); + assert!( + updated.contains("New description text"), + "text should be present" + ); + // Section should appear before Acceptance Criteria. + let pos_desc = updated.find("## Description").unwrap(); + let pos_ac = updated.find("## Acceptance Criteria").unwrap(); + assert!( + pos_desc < pos_ac, + "Description should be before Acceptance Criteria" + ); + } + + #[test] + fn update_story_creates_description_section_no_ac_section() { + let tmp = tempfile::tempdir().unwrap(); + // Story with no ## Description and no ## Acceptance Criteria. + let content = "---\nname: T\n---\n\nSome content here.\n"; + setup_story_in_fs(tmp.path(), "33_test", content); + + let result = update_story_in_file( + tmp.path(), + "33_test", + None, + Some("Appended description"), + None, + ); + assert!( + result.is_ok(), + "should succeed even with no Acceptance Criteria: {result:?}" + ); + + let updated = read_story_content(tmp.path(), "33_test").unwrap(); + assert!( + updated.contains("## Description"), + "section should be created" + ); + assert!( + updated.contains("Appended description"), + "text should be present" + ); + } + + #[test] + fn update_story_sets_agent_front_matter_field() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs( + tmp.path(), + "24_test", + "---\nname: T\n---\n\n## User Story\n\nSome story\n", + ); + + let mut fields = HashMap::new(); + fields.insert("agent".to_string(), Value::String("dev".to_string())); + update_story_in_file(tmp.path(), "24_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "24_test").unwrap(); + assert!( + result.contains("agent: \"dev\""), + "agent field should be set" + ); + assert!(result.contains("name: T"), "name field preserved"); + } + + #[test] + fn update_story_sets_arbitrary_front_matter_fields() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs( + tmp.path(), + "25_test", + "---\nname: T\n---\n\n## User Story\n\nSome story\n", + ); + + let mut fields = HashMap::new(); + fields.insert("qa".to_string(), Value::String("human".to_string())); + fields.insert("priority".to_string(), Value::String("high".to_string())); + update_story_in_file(tmp.path(), "25_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "25_test").unwrap(); + assert!(result.contains("qa: \"human\""), "qa field should be set"); + assert!( + result.contains("priority: \"high\""), + "priority field should be set" + ); + assert!(result.contains("name: T"), "name field preserved"); + } + + #[test] + fn update_story_front_matter_only_no_section_required() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs( + tmp.path(), + "26_test", + "---\nname: T\n---\n\nNo sections here.\n", + ); + + let mut fields = HashMap::new(); + fields.insert("agent".to_string(), Value::String("dev".to_string())); + let result = update_story_in_file(tmp.path(), "26_test", None, None, Some(&fields)); + assert!( + result.is_ok(), + "front-matter-only update should not require body sections" + ); + + let contents = read_story_content(tmp.path(), "26_test").unwrap(); + assert!(contents.contains("agent: \"dev\"")); + } + + #[test] + fn update_story_bool_front_matter_written_unquoted() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "27_test", "---\nname: T\n---\n\nNo sections.\n"); + + // String "false" still works (backwards compatibility). + let mut fields = HashMap::new(); + fields.insert("blocked".to_string(), Value::String("false".to_string())); + update_story_in_file(tmp.path(), "27_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "27_test").unwrap(); + assert!( + result.contains("blocked: false"), + "bool should be unquoted: {result}" + ); + assert!( + !result.contains("blocked: \"false\""), + "bool must not be quoted: {result}" + ); + } + + #[test] + fn update_story_integer_front_matter_written_unquoted() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "28_test", "---\nname: T\n---\n\nNo sections.\n"); + + // String "0" still works (backwards compatibility). + let mut fields = HashMap::new(); + fields.insert("retry_count".to_string(), Value::String("0".to_string())); + update_story_in_file(tmp.path(), "28_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "28_test").unwrap(); + assert!( + result.contains("retry_count: 0"), + "integer should be unquoted: {result}" + ); + assert!( + !result.contains("retry_count: \"0\""), + "integer must not be quoted: {result}" + ); + } + + #[test] + fn update_story_bool_front_matter_parseable_after_write() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs( + tmp.path(), + "29_test", + "---\nname: My Story\n---\n\nNo sections.\n", + ); + + let mut fields = HashMap::new(); + fields.insert("blocked".to_string(), Value::String("false".to_string())); + update_story_in_file(tmp.path(), "29_test", None, None, Some(&fields)).unwrap(); + + let contents = read_story_content(tmp.path(), "29_test").unwrap(); + let meta = parse_front_matter(&contents).expect("front matter should parse"); + assert_eq!( + meta.name.as_deref(), + Some("My Story"), + "name preserved after writing bool field" + ); + } + + // ── Bug 493 regression tests ────────────────────────────────────────────── + + #[test] + fn update_story_depends_on_stored_as_yaml_array_not_quoted_string() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "30_test", "---\nname: T\n---\n\nNo sections.\n"); + + // String "[490]" still works (backwards compatibility). + let mut fields = HashMap::new(); + fields.insert("depends_on".to_string(), Value::String("[490]".to_string())); + update_story_in_file(tmp.path(), "30_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "30_test").unwrap(); + assert!( + result.contains("depends_on: [490]"), + "should be unquoted array: {result}" + ); + assert!( + !result.contains("depends_on: \"[490]\""), + "must not be quoted: {result}" + ); + + let meta = parse_front_matter(&result).expect("front matter should parse"); + assert_eq!(meta.depends_on, Some(vec![490])); + } + + #[test] + fn update_story_native_bool_written_unquoted() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "31_test", "---\nname: T\n---\n\nNo sections.\n"); + + let mut fields = HashMap::new(); + fields.insert("blocked".to_string(), Value::Bool(false)); + update_story_in_file(tmp.path(), "31_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "31_test").unwrap(); + assert!( + result.contains("blocked: false"), + "native bool false should be unquoted: {result}" + ); + assert!( + !result.contains("blocked: \"false\""), + "must not be quoted: {result}" + ); + } + + #[test] + fn update_story_native_bool_true_written_unquoted() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "32_test", "---\nname: T\n---\n\nNo sections.\n"); + + let mut fields = HashMap::new(); + fields.insert("blocked".to_string(), Value::Bool(true)); + update_story_in_file(tmp.path(), "32_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "32_test").unwrap(); + assert!( + result.contains("blocked: true"), + "native bool true should be unquoted: {result}" + ); + assert!( + !result.contains("blocked: \"true\""), + "must not be quoted: {result}" + ); + } + + #[test] + fn update_story_native_integer_written_unquoted() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs( + tmp.path(), + "33b_test", + "---\nname: T\n---\n\nNo sections.\n", + ); + + let mut fields = HashMap::new(); + fields.insert("retry_count".to_string(), serde_json::json!(3)); + update_story_in_file(tmp.path(), "33b_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "33b_test").unwrap(); + assert!( + result.contains("retry_count: 3"), + "native integer should be unquoted: {result}" + ); + assert!( + !result.contains("retry_count: \"3\""), + "must not be quoted: {result}" + ); + } + + #[test] + fn update_story_native_array_written_as_yaml_sequence() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "34_test", "---\nname: T\n---\n\nNo sections.\n"); + + let mut fields = HashMap::new(); + fields.insert("depends_on".to_string(), serde_json::json!([490, 491])); + update_story_in_file(tmp.path(), "34_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "34_test").unwrap(); + assert!( + result.contains("depends_on: [490, 491]"), + "native array should be YAML sequence: {result}" + ); + assert!( + !result.contains("depends_on: \"["), + "must not be quoted: {result}" + ); + + let meta = parse_front_matter(&result).expect("front matter should parse"); + assert_eq!(meta.depends_on, Some(vec![490, 491])); + } + + #[test] + fn update_story_native_bool_parseable_after_write() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs( + tmp.path(), + "35_test", + "---\nname: My Story\n---\n\nNo sections.\n", + ); + + let mut fields = HashMap::new(); + fields.insert("blocked".to_string(), Value::Bool(false)); + update_story_in_file(tmp.path(), "35_test", None, None, Some(&fields)).unwrap(); + + let contents = read_story_content(tmp.path(), "35_test").unwrap(); + let meta = parse_front_matter(&contents).expect("front matter should parse"); + assert_eq!( + meta.name.as_deref(), + Some("My Story"), + "name preserved after writing native bool" + ); + } + + #[test] + fn update_story_depends_on_multi_element_array() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "31_test", "---\nname: T\n---\n\nNo sections.\n"); + + // String "[490, 491]" still works (backwards compatibility). + let mut fields = HashMap::new(); + fields.insert( + "depends_on".to_string(), + Value::String("[490, 491]".to_string()), + ); + update_story_in_file(tmp.path(), "31_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "31_test").unwrap(); + let meta = parse_front_matter(&result).expect("front matter should parse"); + assert_eq!(meta.depends_on, Some(vec![490, 491])); + } +}