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.
This commit is contained in:
dave
2026-04-27 02:13:31 +00:00
parent 34a399b838
commit adf936be07
6 changed files with 1399 additions and 1263 deletions
+7 -7
View File
@@ -300,13 +300,13 @@ pub fn validate_story_dirs(_root: &std::path::Path) -> Result<Vec<StoryValidatio
/// Read story content from the database content store.
///
/// Returns the story content or an error if not found.
pub(super) fn read_story_content(_project_root: &Path, story_id: &str) -> Result<String, String> {
pub(crate) fn read_story_content(_project_root: &Path, story_id: &str) -> Result<String, String> {
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<String> {
pub(crate) fn story_stage(story_id: &str) -> Option<String> {
crate::pipeline_state::read_typed(story_id)
.ok()
.flatten()
@@ -328,7 +328,7 @@ pub(super) fn story_stage(story_id: &str) -> Option<String> {
/// 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<u32, String> {
pub(crate) fn next_item_number(_root: &std::path::Path) -> Result<u32, String> {
Ok(crate::db::next_item_number())
}
File diff suppressed because it is too large Load Diff
@@ -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<String, String> {
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<String> = 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(&current).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 ───────────────────────────
}
@@ -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<String> = 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<String> = 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<String> = 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<usize> = None;
let mut last_criterion_line: Option<usize> = 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<String> = 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(&current).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 ─────────────────────────────────────────────
}
+12
View File
@@ -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;
@@ -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<String> = 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::<i64>().is_ok() => s.to_string(),
s if s.parse::<f64>().is_ok() => s.to_string(),
// YAML inline sequences like [490] or [490, 491] — write unquoted so
// serde_yaml can deserialise them as Vec<u32>.
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<String, Value>>,
) -> 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(&current).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]));
}
}