huskies: merge 1026

This commit is contained in:
dave
2026-05-14 12:53:14 +00:00
parent a80d0a497a
commit 72d79deec9
13 changed files with 1443 additions and 127 deletions
+28 -23
View File
@@ -6,38 +6,27 @@
use crate::http::context::AppContext;
use crate::http::workflow::create_epic_file;
use crate::validation::CreateEpicRequest;
use serde_json::{Value, json};
/// Create a new epic and store it in the CRDT items list.
pub(crate) fn tool_create_epic(args: &Value, ctx: &AppContext) -> Result<String, String> {
let name = args
.get("name")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: name")?;
let goal = args
.get("goal")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: goal")?;
let motivation = args.get("motivation").and_then(|v| v.as_str());
let key_files = args.get("key_files").and_then(|v| v.as_str());
let success_criteria: Option<Vec<String>> = args
.get("success_criteria")
.and_then(|v| v.as_array())
.map(|arr| {
arr.iter()
.filter_map(|v| v.as_str().map(str::to_string))
.collect()
});
let req = CreateEpicRequest::from_json(args)?;
let root = ctx.state.get_project_root()?;
let success_criteria = req.success_criteria_strings();
let epic_id = create_epic_file(
&root,
name,
goal,
motivation,
key_files,
success_criteria.as_deref(),
req.name.as_ref(),
req.goal.as_str(),
req.motivation.as_ref().map(|d| d.as_ref()),
req.key_files.as_deref(),
if success_criteria.is_empty() {
None
} else {
Some(success_criteria.as_slice())
},
)?;
Ok(format!("Created epic: {epic_id}"))
@@ -204,6 +193,22 @@ mod tests {
assert!(result.unwrap_err().contains("goal"));
}
#[test]
fn tool_create_epic_rejects_grammar_token_in_name() {
let tmp = tempfile::tempdir().unwrap();
let ctx = test_ctx(tmp.path());
let result = tool_create_epic(
&json!({"name": "Epic </description> bad", "goal": "some goal"}),
&ctx,
);
assert!(result.is_err());
assert!(
result.unwrap_err().contains("AntiGrammarToken"),
"expected AntiGrammarToken error"
);
}
#[test]
fn tool_list_epics_includes_created_epic() {
let tmp = tempfile::tempdir().unwrap();
+82 -47
View File
@@ -3,55 +3,36 @@
use crate::http::context::AppContext;
use crate::http::workflow::create_story_file;
use crate::slog_warn;
use crate::validation::CreateStoryRequest;
use serde_json::Value;
/// Create a new story in the backlog.
///
/// Deserialises the JSON arguments into a `CreateStoryRequest`, runs the full
/// validation pipeline (field-level newtypes + cross-field garde rules), then
/// delegates to `create_story_file` / `create_item_in_backlog`. All ad-hoc
/// string checks have been removed; `CreateStoryRequest` is now the sole gate.
pub(crate) fn tool_create_story(args: &Value, ctx: &AppContext) -> Result<String, String> {
let name = args
.get("name")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: name")?;
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 acceptance_criteria: Vec<String> = args
.get("acceptance_criteria")
.and_then(|v| serde_json::from_value(v.clone()).ok())
.ok_or("Missing required argument: acceptance_criteria")?;
if acceptance_criteria.is_empty() {
return Err("acceptance_criteria must contain at least one entry".to_string());
}
const JUNK_AC: &[&str] = &["", "todo", "tbd", "fixme", "xxx", "???"];
let all_junk = acceptance_criteria
.iter()
.all(|ac| JUNK_AC.contains(&ac.trim().to_lowercase().as_str()));
if all_junk {
return Err(
"acceptance_criteria must contain at least one real entry (not just TODO/TBD/FIXME/XXX/???)."
.to_string(),
);
}
let depends_on: Option<Vec<u32>> = args
.get("depends_on")
.and_then(|v| serde_json::from_value(v.clone()).ok());
// Spike 61: write the file only — the filesystem watcher detects the new
// .md file in work/1_backlog/ and auto-commits with a deterministic message.
let commit = false;
let req = CreateStoryRequest::from_json(args)?;
let root = ctx.state.get_project_root()?;
let depends_on_ids = req.depends_on_ids();
let story_id = create_story_file(
&root,
name,
user_story,
description,
&acceptance_criteria,
depends_on.as_deref(),
commit,
req.name.as_ref(),
req.user_story.as_ref().map(|d| d.as_ref()),
req.description.as_ref().map(|d| d.as_ref()),
&req.acceptance_criteria
.iter()
.map(|ac| ac.as_ref().to_string())
.collect::<Vec<_>>(),
depends_on_ids.as_deref(),
false,
)?;
// Bug 503: warn at creation time if any depends_on points at an already-archived story.
// Archived = satisfied semantics: the dep will resolve immediately on the next promotion
// tick, which is surprising if the archived story was abandoned rather than cleanly done.
// Story 929: dep archived-status now comes from the CRDT, not a FS scan of 6_archived/.
let archived_deps: Vec<u32> = depends_on
let archived_deps: Vec<u32> = depends_on_ids
.as_deref()
.map(|deps| {
deps.iter()
@@ -199,7 +180,11 @@ mod tests {
let ctx = test_ctx(tmp.path());
let result = tool_create_story(&json!({}), &ctx);
assert!(result.is_err());
assert!(result.unwrap_err().contains("Missing required argument"));
let err = result.unwrap_err();
assert!(
err.contains("FieldMissing") || err.contains("name"),
"expected FieldMissing/name in: {err}"
);
}
#[test]
@@ -211,7 +196,6 @@ mod tests {
&ctx,
);
assert!(result.is_err());
assert!(result.unwrap_err().contains("alphanumeric"));
}
#[test]
@@ -224,8 +208,8 @@ mod tests {
)
.unwrap_err();
assert!(
err.contains("empty") || err.contains("whitespace"),
"error should mention empty/whitespace, got: {err}"
err.contains("EmptyAfterTrim") || err.contains("empty") || err.contains("whitespace"),
"error should mention EmptyAfterTrim/empty/whitespace, got: {err}"
);
}
@@ -277,10 +261,6 @@ mod tests {
&ctx,
);
assert!(result.is_err());
assert!(
result.unwrap_err().contains("real entry"),
"error should mention real entry"
);
}
#[test]
@@ -326,4 +306,59 @@ mod tests {
"Description text missing from story: {content}"
);
}
#[test]
fn tool_create_story_rejects_grammar_token_in_name() {
let tmp = tempfile::tempdir().unwrap();
let ctx = test_ctx(tmp.path());
let err = tool_create_story(
&json!({
"name": "Bad </description> Story",
"acceptance_criteria": ["AC1"]
}),
&ctx,
)
.unwrap_err();
assert!(
err.contains("AntiGrammarToken") || err.contains("grammar"),
"expected grammar-token error, got: {err}"
);
}
#[test]
fn tool_create_story_rejects_grammar_token_in_ac() {
let tmp = tempfile::tempdir().unwrap();
let ctx = test_ctx(tmp.path());
let err = tool_create_story(
&json!({
"name": "Valid Story",
"acceptance_criteria": ["<thinking>bad output</thinking>"]
}),
&ctx,
)
.unwrap_err();
assert!(
err.contains("AntiGrammarToken") || err.contains("grammar"),
"expected grammar-token error, got: {err}"
);
}
#[test]
fn tool_create_story_html_sanitised_in_name() {
let tmp = tempfile::tempdir().unwrap();
let ctx = test_ctx(tmp.path());
// HTML in name is sanitised (not rejected)
let result = tool_create_story(
&json!({
"name": "Story with <b>bold</b> name",
"acceptance_criteria": ["AC1"]
}),
&ctx,
);
// Should succeed (HTML is sanitised, not rejected)
assert!(
result.is_ok(),
"HTML in name should be sanitised: {result:?}"
);
}
}
+50 -52
View File
@@ -6,11 +6,16 @@
use std::path::Path;
use super::super::{next_item_number, slugify_name, write_story_content};
use super::super::create_item_in_backlog;
/// Create an epic file and store it in the database.
/// Create an epic file, storing it in the database via `create_item_in_backlog`.
///
/// Returns the epic_id (e.g. `"880"`).
/// Routes through `create_item_in_backlog` so that the tombstone-skip allocator
/// defence and post-write CRDT verification (added by bug 1001's fix) apply to
/// epics too. Previously this function had its own ad-hoc allocate-and-write
/// path that bypassed those safety checks.
///
/// Returns the epic_id (numeric only, e.g. `"880"`).
pub fn create_epic_file(
root: &Path,
name: &str,
@@ -19,61 +24,54 @@ pub fn create_epic_file(
key_files: Option<&str>,
success_criteria: Option<&[String]>,
) -> Result<String, String> {
let epic_number = next_item_number(root)?;
let slug = slugify_name(name);
let name_owned = name.to_string();
let goal_owned = goal.to_string();
let motivation_owned = motivation.map(str::to_string);
let key_files_owned = key_files.map(str::to_string);
let success_criteria_owned: Vec<String> =
success_criteria.map(|sc| sc.to_vec()).unwrap_or_default();
if slug.is_empty() {
return Err("Name must contain at least one alphanumeric character.".to_string());
}
// Epics don't have acceptance criteria; pass an empty slice.
// create_item_in_backlog skips the AC check for type "epic".
create_item_in_backlog(root, "epic", name, &[], None, move |epic_number| {
let mut content = String::new();
content.push_str("---\n");
content.push_str("type: epic\n");
content.push_str(&format!("name: \"{}\"\n", name_owned.replace('"', "\\\"")));
content.push_str("---\n\n");
content.push_str(&format!("# Epic {epic_number}: {name_owned}\n\n"));
let epic_id = format!("{epic_number}");
content.push_str("## Goal\n\n");
content.push_str(&goal_owned);
content.push_str("\n\n");
let mut content = String::new();
content.push_str("---\n");
content.push_str("type: epic\n");
content.push_str(&format!("name: \"{}\"\n", name.replace('"', "\\\"")));
content.push_str("---\n\n");
content.push_str(&format!("# Epic {epic_number}: {name}\n\n"));
content.push_str("## Goal\n\n");
content.push_str(goal);
content.push_str("\n\n");
content.push_str("## Motivation\n\n");
if let Some(m) = motivation {
content.push_str(m);
content.push('\n');
} else {
content.push_str("- TBD\n");
}
content.push('\n');
content.push_str("## Key Files\n\n");
if let Some(kf) = key_files {
content.push_str(kf);
content.push('\n');
} else {
content.push_str("- TBD\n");
}
content.push('\n');
content.push_str("## Success Criteria\n\n");
match success_criteria {
Some(criteria) if !criteria.is_empty() => {
for c in criteria {
content.push_str(&format!("- {c}\n"));
}
}
_ => {
content.push_str("## Motivation\n\n");
if let Some(ref m) = motivation_owned {
content.push_str(m);
content.push('\n');
} else {
content.push_str("- TBD\n");
}
}
content.push('\n');
// Epics are stored in backlog (no pipeline advancement).
write_story_content(root, &epic_id, "1_backlog", &content, Some(name));
content.push_str("## Key Files\n\n");
if let Some(ref kf) = key_files_owned {
content.push_str(kf);
content.push('\n');
} else {
content.push_str("- TBD\n");
}
content.push('\n');
// Story 933: typed CRDT register for item_type.
crate::crdt_state::set_item_type(&epic_id, Some(crate::io::story_metadata::ItemType::Epic));
content.push_str("## Success Criteria\n\n");
if !success_criteria_owned.is_empty() {
for c in &success_criteria_owned {
content.push_str(&format!("- {c}\n"));
}
} else {
content.push_str("- TBD\n");
}
Ok(epic_id)
content
})
}
+7 -5
View File
@@ -250,15 +250,17 @@ pub(crate) fn create_item_in_backlog(
if slugify_name(name).is_empty() {
return Err("Title must contain at least one alphanumeric character.".to_string());
}
if acceptance_criteria.is_empty() {
return Err("At least one acceptance criterion is required.".to_string());
}
const VALID_TYPES: &[&str] = &["story", "bug", "spike", "refactor"];
const VALID_TYPES: &[&str] = &["story", "bug", "spike", "refactor", "epic"];
if !VALID_TYPES.contains(&item_type) {
return Err(format!(
"Invalid item type '{item_type}': must be one of story, bug, spike, refactor."
"Invalid item type '{item_type}': must be one of story, bug, spike, refactor, epic."
));
}
// Epics use success_criteria (optional); the acceptance_criteria check is
// only meaningful for pipeline work items.
if item_type != "epic" && acceptance_criteria.is_empty() {
return Err("At least one acceptance criterion is required.".to_string());
}
let item_number = next_item_number(root)?;
let item_id = format!("{item_number}");