story-kit: merge 177_bug_no_mcp_tool_to_edit_story_acceptance_criteria

This commit is contained in:
Dave
2026-02-25 11:34:37 +00:00
parent e419c198a6
commit aa423cae22
2 changed files with 342 additions and 3 deletions

View File

@@ -5,8 +5,9 @@ use crate::slog_warn;
use crate::http::context::AppContext; use crate::http::context::AppContext;
use crate::http::settings::get_editor_command_from_store; use crate::http::settings::get_editor_command_from_store;
use crate::http::workflow::{ use crate::http::workflow::{
check_criterion_in_file, create_bug_file, create_spike_file, create_story_file, list_bug_files, add_criterion_to_file, check_criterion_in_file, create_bug_file, create_spike_file,
load_upcoming_stories, validate_story_dirs, create_story_file, list_bug_files, load_upcoming_stories, update_story_in_file,
validate_story_dirs,
}; };
use crate::worktree; use crate::worktree;
use crate::io::story_metadata::{parse_front_matter, parse_unchecked_todos}; use crate::io::story_metadata::{parse_front_matter, parse_unchecked_todos};
@@ -616,6 +617,46 @@ fn handle_tools_list(id: Option<Value>) -> JsonRpcResponse {
"required": ["story_id", "criterion_index"] "required": ["story_id", "criterion_index"]
} }
}, },
{
"name": "add_criterion",
"description": "Add an acceptance criterion to an existing story file. Appends '- [ ] {criterion}' after the last existing criterion in the '## Acceptance Criteria' section. Auto-commits via the filesystem watcher.",
"inputSchema": {
"type": "object",
"properties": {
"story_id": {
"type": "string",
"description": "Story identifier (filename stem, e.g. '28_my_story')"
},
"criterion": {
"type": "string",
"description": "The acceptance criterion text to add (without the '- [ ] ' prefix)"
}
},
"required": ["story_id", "criterion"]
}
},
{
"name": "update_story",
"description": "Update the user story text and/or description of an existing story file. Replaces the content of the '## User Story' and/or '## Description' section in place. Auto-commits via the filesystem watcher.",
"inputSchema": {
"type": "object",
"properties": {
"story_id": {
"type": "string",
"description": "Story identifier (filename stem, e.g. '28_my_story')"
},
"user_story": {
"type": "string",
"description": "New user story text to replace the '## User Story' section content"
},
"description": {
"type": "string",
"description": "New description text to replace the '## Description' section content"
}
},
"required": ["story_id"]
}
},
{ {
"name": "create_spike", "name": "create_spike",
"description": "Create a spike file in .story_kit/work/1_upcoming/ with a deterministic filename and YAML front matter. Returns the spike_id.", "description": "Create a spike file in .story_kit/work/1_upcoming/ with a deterministic filename and YAML front matter. Returns the spike_id.",
@@ -828,6 +869,8 @@ async fn handle_tools_call(
"accept_story" => tool_accept_story(&args, ctx), "accept_story" => tool_accept_story(&args, ctx),
// Story mutation tools (auto-commit to master) // Story mutation tools (auto-commit to master)
"check_criterion" => tool_check_criterion(&args, ctx), "check_criterion" => tool_check_criterion(&args, ctx),
"add_criterion" => tool_add_criterion(&args, ctx),
"update_story" => tool_update_story(&args, ctx),
// Spike lifecycle tools // Spike lifecycle tools
"create_spike" => tool_create_spike(&args, ctx), "create_spike" => tool_create_spike(&args, ctx),
// Bug lifecycle tools // Bug lifecycle tools
@@ -1378,6 +1421,38 @@ fn tool_check_criterion(args: &Value, ctx: &AppContext) -> Result<String, String
)) ))
} }
fn tool_add_criterion(args: &Value, ctx: &AppContext) -> Result<String, String> {
let story_id = args
.get("story_id")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: story_id")?;
let criterion = args
.get("criterion")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: criterion")?;
let root = ctx.state.get_project_root()?;
add_criterion_to_file(&root, story_id, criterion)?;
Ok(format!(
"Added criterion to story '{story_id}': - [ ] {criterion}"
))
}
fn tool_update_story(args: &Value, ctx: &AppContext) -> Result<String, String> {
let story_id = args
.get("story_id")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: story_id")?;
let user_story = args.get("user_story").and_then(|v| v.as_str());
let description = args.get("description").and_then(|v| v.as_str());
let root = ctx.state.get_project_root()?;
update_story_in_file(&root, story_id, user_story, description)?;
Ok(format!("Updated story '{story_id}'."))
}
// ── Spike lifecycle tool implementations ───────────────────────── // ── Spike lifecycle tool implementations ─────────────────────────
fn tool_create_spike(args: &Value, ctx: &AppContext) -> Result<String, String> { fn tool_create_spike(args: &Value, ctx: &AppContext) -> Result<String, String> {
@@ -1795,6 +1870,8 @@ mod tests {
assert!(!names.contains(&"report_completion")); assert!(!names.contains(&"report_completion"));
assert!(names.contains(&"accept_story")); assert!(names.contains(&"accept_story"));
assert!(names.contains(&"check_criterion")); assert!(names.contains(&"check_criterion"));
assert!(names.contains(&"add_criterion"));
assert!(names.contains(&"update_story"));
assert!(names.contains(&"create_spike")); assert!(names.contains(&"create_spike"));
assert!(names.contains(&"create_bug")); assert!(names.contains(&"create_bug"));
assert!(names.contains(&"list_bugs")); assert!(names.contains(&"list_bugs"));
@@ -1804,7 +1881,7 @@ mod tests {
assert!(names.contains(&"request_qa")); assert!(names.contains(&"request_qa"));
assert!(names.contains(&"get_server_logs")); assert!(names.contains(&"get_server_logs"));
assert!(names.contains(&"prompt_permission")); assert!(names.contains(&"prompt_permission"));
assert_eq!(tools.len(), 28); assert_eq!(tools.len(), 30);
} }
#[test] #[test]

View File

@@ -469,6 +469,146 @@ pub fn check_criterion_in_file(
Ok(()) Ok(())
} }
/// Add a new acceptance criterion to a story file.
///
/// Appends `- [ ] {criterion}` after the last existing criterion line in the
/// "## Acceptance Criteria" section, or directly after the section heading if
/// the section is empty. The filesystem watcher auto-commits the change.
pub fn add_criterion_to_file(
project_root: &Path,
story_id: &str,
criterion: &str,
) -> Result<(), String> {
let filepath = find_story_file(project_root, story_id)?;
let contents = fs::read_to_string(&filepath)
.map_err(|e| format!("Failed to read story file: {e}"))?;
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 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<String> = lines.iter().map(|s| s.to_string()).collect();
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');
}
fs::write(&filepath, &new_str)
.map_err(|e| format!("Failed to write story file: {e}"))?;
// Watcher handles the git commit asynchronously.
Ok(())
}
/// Replace the content of a named `## Section` in a story file.
///
/// 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.
fn replace_section_content(content: &str, section_name: &str, new_text: &str) -> Result<String, String> {
let lines: Vec<&str> = content.lines().collect();
let heading = format!("## {section_name}");
let mut section_start: Option<usize> = None;
let mut section_end: Option<usize> = None;
for (i, line) in lines.iter().enumerate() {
let trimmed = line.trim();
if trimmed == heading {
section_start = Some(i);
continue;
}
if section_start.is_some() && trimmed.starts_with("## ") {
section_end = Some(i);
break;
}
}
let section_start =
section_start.ok_or_else(|| format!("Section '{heading}' not found in story file."))?;
let mut new_lines: Vec<String> = Vec::new();
// Keep everything up to and including the section heading.
for line in lines.iter().take(section_start + 1) {
new_lines.push(line.to_string());
}
// Blank line, new content, blank line.
new_lines.push(String::new());
new_lines.push(new_text.to_string());
new_lines.push(String::new());
// Resume from the next section heading (or EOF).
let resume_from = section_end.unwrap_or(lines.len());
for line in lines.iter().skip(resume_from) {
new_lines.push(line.to_string());
}
let mut new_str = new_lines.join("\n");
if content.ends_with('\n') {
new_str.push('\n');
}
Ok(new_str)
}
/// Update the user story text and/or description in a story file.
///
/// At least one of `user_story` or `description` must be provided.
/// Replaces the content of the corresponding `##` section in place.
/// The filesystem watcher auto-commits the change.
pub fn update_story_in_file(
project_root: &Path,
story_id: &str,
user_story: Option<&str>,
description: Option<&str>,
) -> Result<(), String> {
if user_story.is_none() && description.is_none() {
return Err(
"At least one of 'user_story' or 'description' must be provided.".to_string(),
);
}
let filepath = find_story_file(project_root, story_id)?;
let mut contents = fs::read_to_string(&filepath)
.map_err(|e| format!("Failed to read story file: {e}"))?;
if let Some(us) = user_story {
contents = replace_section_content(&contents, "User Story", us)?;
}
if let Some(desc) = description {
contents = replace_section_content(&contents, "Description", desc)?;
}
fs::write(&filepath, &contents)
.map_err(|e| format!("Failed to write story file: {e}"))?;
// Watcher handles the git commit asynchronously.
Ok(())
}
fn slugify_name(name: &str) -> String { fn slugify_name(name: &str) -> String {
let slug: String = name let slug: String = name
.chars() .chars()
@@ -1265,6 +1405,128 @@ mod tests {
assert!(result.unwrap_err().contains("not found")); assert!(result.unwrap_err().contains("not found"));
} }
// ── 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();
let current = tmp.path().join(".story_kit/work/2_current");
fs::create_dir_all(&current).unwrap();
let filepath = current.join("10_test.md");
fs::write(&filepath, story_with_ac_section(&["First", "Second"])).unwrap();
add_criterion_to_file(tmp.path(), "10_test", "Third").unwrap();
let contents = fs::read_to_string(&filepath).unwrap();
assert!(contents.contains("- [ ] First\n"));
assert!(contents.contains("- [ ] Second\n"));
assert!(contents.contains("- [ ] Third\n"));
// Third should come after Second
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 current = tmp.path().join(".story_kit/work/2_current");
fs::create_dir_all(&current).unwrap();
let filepath = current.join("11_test.md");
let content = "---\nname: Test\n---\n\n## Acceptance Criteria\n\n## Out of Scope\n\n- N/A\n";
fs::write(&filepath, content).unwrap();
add_criterion_to_file(tmp.path(), "11_test", "New AC").unwrap();
let contents = fs::read_to_string(&filepath).unwrap();
assert!(contents.contains("- [ ] New AC\n"), "criterion should be present");
}
#[test]
fn add_criterion_missing_section_returns_error() {
let tmp = tempfile::tempdir().unwrap();
let current = tmp.path().join(".story_kit/work/2_current");
fs::create_dir_all(&current).unwrap();
let filepath = current.join("12_test.md");
fs::write(&filepath, "---\nname: Test\n---\n\nNo AC section here.\n").unwrap();
let result = add_criterion_to_file(tmp.path(), "12_test", "X");
assert!(result.is_err());
assert!(result.unwrap_err().contains("Acceptance Criteria"));
}
// ── update_story_in_file tests ─────────────────────────────────────────────
#[test]
fn update_story_replaces_user_story_section() {
let tmp = tempfile::tempdir().unwrap();
let current = tmp.path().join(".story_kit/work/2_current");
fs::create_dir_all(&current).unwrap();
let filepath = current.join("20_test.md");
let content = "---\nname: T\n---\n\n## User Story\n\nOld text\n\n## Acceptance Criteria\n\n- [ ] AC\n";
fs::write(&filepath, content).unwrap();
update_story_in_file(tmp.path(), "20_test", Some("New user story text"), None).unwrap();
let result = fs::read_to_string(&filepath).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 current = tmp.path().join(".story_kit/work/2_current");
fs::create_dir_all(&current).unwrap();
let filepath = current.join("21_test.md");
let content = "---\nname: T\n---\n\n## Description\n\nOld description\n\n## Acceptance Criteria\n\n- [ ] AC\n";
fs::write(&filepath, content).unwrap();
update_story_in_file(tmp.path(), "21_test", None, Some("New description")).unwrap();
let result = fs::read_to_string(&filepath).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();
let current = tmp.path().join(".story_kit/work/2_current");
fs::create_dir_all(&current).unwrap();
fs::write(current.join("22_test.md"), "---\nname: T\n---\n").unwrap();
let result = update_story_in_file(tmp.path(), "22_test", None, None);
assert!(result.is_err());
assert!(result.unwrap_err().contains("At least one"));
}
#[test]
fn update_story_missing_section_returns_error() {
let tmp = tempfile::tempdir().unwrap();
let current = tmp.path().join(".story_kit/work/2_current");
fs::create_dir_all(&current).unwrap();
fs::write(
current.join("23_test.md"),
"---\nname: T\n---\n\nNo sections here.\n",
)
.unwrap();
let result = update_story_in_file(tmp.path(), "23_test", Some("new text"), None);
assert!(result.is_err());
assert!(result.unwrap_err().contains("User Story"));
}
// ── Bug file helper tests ────────────────────────────────────────────────── // ── Bug file helper tests ──────────────────────────────────────────────────
#[test] #[test]