diff --git a/server/src/http/workflow/story_ops.rs b/server/src/http/workflow/story_ops.rs index c9552acb..de277df3 100644 --- a/server/src/http/workflow/story_ops.rs +++ b/server/src/http/workflow/story_ops.rs @@ -264,11 +264,20 @@ pub fn add_criterion_to_file( } } - let insert_after = last_criterion_line - .or(ac_section_start) - .ok_or_else(|| format!("Story '{story_id}' has no '## Acceptance Criteria' section."))?; - 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"); @@ -353,6 +362,13 @@ pub fn update_story_in_file( 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); @@ -612,7 +628,9 @@ mod tests { } #[test] - fn add_criterion_missing_section_returns_error() { + 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(), @@ -621,8 +639,92 @@ mod tests { ); let result = add_criterion_to_file(tmp.path(), "12_test", "X"); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("Acceptance Criteria")); + 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 ──────────────────────────────────────