crashlabs/huskies
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 26 Wiki Activity

huskies: merge 730_story_use_numeric_only_story_ids_across_mcp_worktrees_git_branches_and_log_paths

Browse Source
This commit is contained in:
dave
2026-04-27 20:17:03 +00:00
parent 63d5a500de
commit 1388658ae8
10 changed files with 223 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/agent_log.rs
+18
View File
@@ -349,6 +349,24 @@ mod tests {
assert!(expected_path.exists(), "Log file should exist");
}
#[test]
fn numeric_id_log_dir_uses_number_only() {
let tmp = tempdir().unwrap();
let root = tmp.path();
let _writer = AgentLogWriter::new(root, "664", "coder-1", "sess-xyz").unwrap();
let expected_path = root
.join(".huskies")
.join("logs")
.join("664")
.join("coder-1-sess-xyz.log");
assert!(
expected_path.exists(),
"Log file for numeric ID should be at .huskies/logs/664/..."
);
}
#[test]
fn test_log_writer_writes_jsonl_with_timestamps() {
let tmp = tempdir().unwrap();
server/src/agents/lifecycle.rs
+52 -5
View File
@@ -10,16 +10,34 @@ use crate::slog;
type ContentTransform = Option<Box<dyn Fn(&str) -> String>>;
/// Determine the item type ("story", "bug", "spike", or "refactor") from the item ID.
///
/// For slug-format IDs (e.g. `"4_bug_login_crash"`), the type is embedded in the ID.
/// For numeric-only IDs (e.g. `"4"`), the type is read from the `type:` field in
/// the content-store front matter. Falls back to `"story"` if not found.
pub(crate) fn item_type_from_id(item_id: &str) -> &'static str {
// New format: {digits}_{type}_{slug}
let after_num = item_id.trim_start_matches(|c: char| c.is_ascii_digit());
if after_num.starts_with("_bug_") {
"bug"
return "bug";
} else if after_num.starts_with("_spike_") {
"spike"
} else {
"story"
return "spike";
} else if after_num.starts_with("_refactor_") {
return "refactor";
}
// Numeric-only ID: check content store front matter for explicit type.
if after_num.is_empty()
&& let Some(content) = crate::db::read_content(item_id)
&& let Ok(meta) = crate::io::story_metadata::parse_front_matter(&content)
&& let Some(t) = meta.item_type.as_deref()
{
return match t {
"bug" => "bug",
"spike" => "spike",
"refactor" => "refactor",
_ => "story",
};
}
"story"
}
/// Move a work item to a new pipeline stage via the database.
@@ -332,6 +350,35 @@ mod tests {
assert_eq!(item_type_from_id("1_spike_research"), "spike");
assert_eq!(item_type_from_id("50_story_my_story"), "story");
assert_eq!(item_type_from_id("1_story_simple"), "story");
assert_eq!(item_type_from_id("1_refactor_cleanup"), "refactor");
}
#[test]
fn item_type_from_id_falls_back_to_content_store_for_numeric_ids() {
crate::db::ensure_content_store();
// Write a bug item with numeric-only ID into the content store.
let bug_content = "---\ntype: bug\nname: \"Test Bug\"\n---\n\n# Bug 9999: Test Bug\n";
crate::db::write_content("9999", bug_content);
let spike_content =
"---\ntype: spike\nname: \"Test Spike\"\n---\n\n# Spike 9998: Test Spike\n";
crate::db::write_content("9998", spike_content);
let refactor_content =
"---\ntype: refactor\nname: \"Test Refactor\"\n---\n\n# Refactor 9997: Test Refactor\n";
crate::db::write_content("9997", refactor_content);
let story_content =
"---\ntype: story\nname: \"Test Story\"\n---\n\n# Story 9996: Test Story\n";
crate::db::write_content("9996", story_content);
assert_eq!(item_type_from_id("9999"), "bug");
assert_eq!(item_type_from_id("9998"), "spike");
assert_eq!(item_type_from_id("9997"), "refactor");
assert_eq!(item_type_from_id("9996"), "story");
// No content store entry → defaults to "story".
assert_eq!(item_type_from_id("99999"), "story");
}
// ── feature_branch_has_unmerged_changes tests ────────────────────────────
server/src/http/mcp/story_tools/bug.rs
+2 -2
View File
@@ -257,8 +257,8 @@ mod tests {
.unwrap();
assert!(
result.contains("_bug_login_crash"),
"result should contain bug ID: {result}"
result.starts_with("Created bug: "),
"result should be a 'Created bug: <id>' message: {result}"
);
// Extract the actual bug ID from the result message (format: "Created bug: <id>").
let bug_id = result.trim_start_matches("Created bug: ").trim();
server/src/http/mcp/story_tools/spike.rs
+6 -6
View File
@@ -111,14 +111,14 @@ mod tests {
.unwrap();
assert!(
result.contains("_spike_compare_encoders"),
"result should contain spike ID: {result}"
result.starts_with("Created spike: "),
"result should be a 'Created spike: <id>' message: {result}"
);
// Extract the actual spike ID from the result message (format: "Created spike: <id>").
let spike_id = result.trim_start_matches("Created spike: ").trim();
// Spike content should exist in the CRDT content store.
let contents = crate::db::read_content(spike_id).expect("expected spike content in CRDT");
assert!(contents.starts_with("---\nname: \"Compare Encoders\"\n---"));
assert!(contents.starts_with("---\ntype: spike\nname: \"Compare Encoders\"\n---"));
assert!(contents.contains("Which encoder is fastest?"));
}
@@ -133,15 +133,15 @@ mod tests {
)
.unwrap();
assert!(
result.contains("_spike_my_spike"),
"result should contain spike ID: {result}"
result.starts_with("Created spike: "),
"result should be a 'Created spike: <id>' message: {result}"
);
// Extract the actual spike ID from the result message (format: "Created spike: <id>").
let spike_id = result.trim_start_matches("Created spike: ").trim();
// Spike content should exist in the CRDT content store.
let contents = crate::db::read_content(spike_id).expect("expected spike content in CRDT");
assert!(contents.starts_with("---\nname: \"My Spike\"\n---"));
assert!(contents.starts_with("---\ntype: spike\nname: \"My Spike\"\n---"));
assert!(contents.contains("## Question\n\n- TBD\n"));
}
server/src/http/mcp/story_tools/story.rs
+4 -2
View File
@@ -300,12 +300,14 @@ pub(crate) fn tool_unblock_story(args: &Value, ctx: &AppContext) -> Result<Strin
let root = ctx.state.get_project_root()?;
// Extract the numeric prefix (e.g. "42" from "42_story_foo")
// Extract the numeric prefix (e.g. "42" from "42" or from legacy "42_story_foo").
let story_number = story_id
.split('_')
.next()
.filter(|s| !s.is_empty() && s.chars().all(|c| c.is_ascii_digit()))
.ok_or_else(|| format!("Invalid story_id format: '{story_id}'. Expected a numeric prefix (e.g. '42_story_foo')."))?;
.ok_or_else(|| {
format!("Invalid story_id format: '{story_id}'. Expected a numeric ID (e.g. '42').")
})?;
Ok(crate::chat::commands::unblock::unblock_by_number(
&root,
server/src/http/workflow/bug_ops.rs
+56 -41
View File
@@ -7,7 +7,7 @@ use super::{next_item_number, slugify_name, write_story_content};
/// Create a bug file and store it in the database.
///
/// Also writes to the filesystem for backwards compatibility during migration.
/// Returns the bug_id (e.g. `"4_bug_login_crash"`).
/// Returns the bug_id (e.g. `"4"`).
#[allow(clippy::too_many_arguments)]
pub fn create_bug_file(
root: &Path,
@@ -26,10 +26,11 @@ pub fn create_bug_file(
return Err("Name must contain at least one alphanumeric character.".to_string());
}
let bug_id = format!("{bug_number}_bug_{slug}");
let bug_id = format!("{bug_number}");
let mut content = String::new();
content.push_str("---\n");
content.push_str("type: bug\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();
@@ -66,7 +67,7 @@ pub fn create_bug_file(
/// Create a spike file and store it in the database.
///
/// Returns the spike_id (e.g. `"4_spike_filesystem_watcher_architecture"`).
/// Returns the spike_id (e.g. `"4"`).
pub fn create_spike_file(
root: &Path,
name: &str,
@@ -80,10 +81,11 @@ pub fn create_spike_file(
return Err("Name must contain at least one alphanumeric character.".to_string());
}
let spike_id = format!("{spike_number}_spike_{slug}");
let spike_id = format!("{spike_number}");
let mut content = String::new();
content.push_str("---\n");
content.push_str("type: spike\n");
content.push_str(&format!("name: \"{}\"\n", name.replace('"', "\\\"")));
content.push_str("---\n\n");
content.push_str(&format!("# Spike {spike_number}: {name}\n\n"));
@@ -122,7 +124,7 @@ pub fn create_spike_file(
/// Create a refactor work item and store it in the database.
///
/// Returns the refactor_id (e.g. `"5_refactor_split_agents_rs"`).
/// Returns the refactor_id (e.g. `"5"`).
pub fn create_refactor_file(
root: &Path,
name: &str,
@@ -137,10 +139,11 @@ pub fn create_refactor_file(
return Err("Name must contain at least one alphanumeric character.".to_string());
}
let refactor_id = format!("{refactor_number}_refactor_{slug}");
let refactor_id = format!("{refactor_number}");
let mut content = String::new();
content.push_str("---\n");
content.push_str("type: refactor\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();
@@ -176,10 +179,24 @@ pub fn create_refactor_file(
Ok(refactor_id)
}
/// Returns true if the item stem (filename without extension) is a bug item.
/// Returns true if the item stem is a bug item.
///
/// Checks the slug-based ID format first (e.g. `"4_bug_login_crash"`), then
/// falls back to reading `type: bug` from the content store for numeric-only IDs.
fn is_bug_item(stem: &str) -> bool {
let after_num = stem.trim_start_matches(|c: char| c.is_ascii_digit());
after_num.starts_with("_bug_")
if after_num.starts_with("_bug_") {
return true;
}
// Numeric-only ID: check content store front matter.
if after_num.is_empty() {
return crate::db::read_content(stem)
.and_then(|c| parse_front_matter(&c).ok())
.and_then(|m| m.item_type)
.map(|t| t == "bug")
.unwrap_or(false);
}
false
}
/// Extract bug name from content (heading or front matter).
@@ -229,9 +246,23 @@ pub fn list_bug_files(_root: &Path) -> Result<Vec<(String, String)>, String> {
}
/// Returns true if the item stem is a refactor item.
///
/// Checks the slug-based ID format first (e.g. `"5_refactor_split_agents_rs"`), then
/// falls back to reading `type: refactor` from the content store for numeric-only IDs.
fn is_refactor_item(stem: &str) -> bool {
let after_num = stem.trim_start_matches(|c: char| c.is_ascii_digit());
after_num.starts_with("_refactor_")
if after_num.starts_with("_refactor_") {
return true;
}
// Numeric-only ID: check content store front matter.
if after_num.is_empty() {
return crate::db::read_content(stem)
.and_then(|c| parse_front_matter(&c).ok())
.and_then(|m| m.item_type)
.map(|t| t == "refactor")
.unwrap_or(false);
}
false
}
/// List all open refactors from CRDT + content store.
@@ -431,8 +462,8 @@ mod tests {
.unwrap();
assert!(
bug_id.ends_with("_bug_login_crash"),
"expected ID to end with _bug_login_crash, got: {bug_id}"
bug_id.chars().all(|c| c.is_ascii_digit()),
"bug ID must be numeric-only, got: {bug_id}"
);
// Check content exists (either in DB or filesystem).
@@ -446,8 +477,8 @@ mod tests {
.expect("bug content should exist");
assert!(
contents.starts_with("---\nname: \"Login Crash\"\n---"),
"bug file must start with YAML front matter"
contents.starts_with("---\ntype: bug\nname: \"Login Crash\"\n---"),
"bug file must start with YAML front matter including type field"
);
assert!(
contents.contains("Login Crash"),
@@ -499,16 +530,11 @@ mod tests {
)
.unwrap();
let contents = crate::db::read_content(&bug_id)
.or_else(|| {
let filepath = tmp.path().join(".huskies/work/1_backlog/1_bug_some_bug.md");
fs::read_to_string(filepath).ok()
})
.expect("bug content should exist");
let contents = crate::db::read_content(&bug_id).expect("bug content should exist");
assert!(
contents.starts_with("---\nname: \"Some Bug\"\n---"),
"bug file must have YAML front matter"
contents.starts_with("---\ntype: bug\nname: \"Some Bug\"\n---"),
"bug file must have YAML front matter with type field"
);
assert!(contents.contains("- [ ] Bug is fixed and verified"));
}
@@ -523,22 +549,16 @@ mod tests {
create_spike_file(tmp.path(), "Filesystem Watcher Architecture", None, &[]).unwrap();
assert!(
spike_id.ends_with("_spike_filesystem_watcher_architecture"),
"expected ID to end with _spike_filesystem_watcher_architecture, got: {spike_id}"
spike_id.chars().all(|c| c.is_ascii_digit()),
"spike ID must be numeric-only, got: {spike_id}"
);
let contents = crate::db::read_content(&spike_id)
.or_else(|| {
let filepath = tmp
.path()
.join(format!(".huskies/work/1_backlog/{spike_id}.md"));
fs::read_to_string(filepath).ok()
})
.expect("spike content should exist");
let contents = crate::db::read_content(&spike_id).expect("spike content should exist");
assert!(
contents.starts_with("---\nname: \"Filesystem Watcher Architecture\"\n---"),
"spike file must start with YAML front matter"
contents
.starts_with("---\ntype: spike\nname: \"Filesystem Watcher Architecture\"\n---"),
"spike file must start with YAML front matter including type field"
);
assert!(
contents.contains("Filesystem Watcher Architecture"),
@@ -627,15 +647,10 @@ mod tests {
let spike_id = create_spike_file(tmp.path(), "My Spike", None, &[]).unwrap();
assert!(
spike_id.ends_with("_spike_my_spike"),
"expected ID to end with _spike_my_spike, got: {spike_id}"
spike_id.chars().all(|c| c.is_ascii_digit()),
"spike ID must be numeric-only, got: {spike_id}"
);
let num: u32 = spike_id
.chars()
.take_while(|c| c.is_ascii_digit())
.collect::<String>()
.parse()
.unwrap();
let num: u32 = spike_id.parse().unwrap();
assert!(
num >= 7051,
"expected spike number >= 7051, got: {spike_id}"
server/src/http/workflow/story_ops/create.rs
+38 -2
View File
@@ -22,10 +22,11 @@ pub fn create_story_file(
return Err("Name must contain at least one alphanumeric character.".to_string());
}
let story_id = format!("{story_number}_story_{slug}");
let story_id = format!("{story_number}");
let mut content = String::new();
content.push_str("---\n");
content.push_str("type: story\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();
@@ -167,7 +168,7 @@ mod tests {
fs::write(&filepath, &content).unwrap();
let written = fs::read_to_string(&filepath).unwrap();
assert!(written.starts_with("---\nname: \"My New Feature\"\n---"));
assert!(written.starts_with("---\n"));
assert!(written.contains(&format!("# Story {number}: My New Feature")));
assert!(written.contains("- [ ] It works"));
assert!(written.contains("- [ ] It is tested"));
@@ -230,5 +231,40 @@ mod tests {
assert_eq!(meta.depends_on, Some(vec![489]));
}
// ── Story 730: numeric-only story IDs ─────────────────────────────────────
#[test]
fn create_story_file_returns_numeric_only_id() {
crate::db::ensure_content_store();
let tmp = tempfile::tempdir().unwrap();
let result = create_story_file(tmp.path(), "My Feature", None, None, None, None, false);
assert!(
result.is_ok(),
"create_story_file should succeed: {result:?}"
);
let story_id = result.unwrap();
assert!(
story_id.chars().all(|c| c.is_ascii_digit()),
"story ID must be numeric-only, got: '{story_id}'"
);
}
#[test]
fn create_story_file_writes_type_field_in_front_matter() {
crate::db::ensure_content_store();
let tmp = tempfile::tempdir().unwrap();
let story_id =
create_story_file(tmp.path(), "Type Test Story", None, None, None, None, false)
.unwrap();
let content = crate::db::read_content(&story_id).expect("content must exist");
let meta = crate::io::story_metadata::parse_front_matter(&content)
.expect("front matter should be valid");
assert_eq!(
meta.item_type.as_deref(),
Some("story"),
"front matter must contain type: story"
);
}
// ── Story 504: native JSON types in front_matter ───────────────────────────
}
server/src/io/story_metadata.rs
+9
View File
@@ -64,6 +64,11 @@ pub struct StoryMetadata {
/// Used by the bug-645 salvage path to require real test evidence, not just
/// compilation success.
pub run_tests_passed: Option<bool>,
/// Item type: "story", "bug", "spike", or "refactor".
///
/// Present on items created with numeric-only IDs (no slug suffix).
/// Used by the pipeline to determine routing (e.g. spikes skip QA).
pub item_type: Option<String>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -102,6 +107,9 @@ struct FrontMatter {
/// Used by the bug-645 salvage path to distinguish a genuine test-passing
/// session from one that merely compiled.
run_tests_passed: Option<bool>,
/// Item type: "story", "bug", "spike", or "refactor".
#[serde(rename = "type")]
item_type: Option<String>,
}
pub fn parse_front_matter(contents: &str) -> Result<StoryMetadata, StoryMetaError> {
@@ -144,6 +152,7 @@ fn build_metadata(front: FrontMatter) -> StoryMetadata {
depends_on: front.depends_on,
frozen: front.frozen,
run_tests_passed: front.run_tests_passed,
item_type: front.item_type,
}
}
server/src/service/common/item_id.rs
+22 -6
View File
@@ -1,12 +1,14 @@
//! Pure helpers for pipeline item ID parsing.
//!
//! Pipeline item IDs share the format `{number}_{type}_{slug}`, e.g.
//! `"42_story_foo"`, `"7_bug_bar"`, `"100_refactor_baz"`. The functions here
//! extract or validate the leading numeric segment without performing any I/O.
//! Pipeline item IDs are numeric strings, e.g. `"42"`, `"730"`. Legacy items
//! may use the old `{number}_{type}_{slug}` format (e.g. `"42_story_foo"`).
//! The functions here extract or validate the leading numeric segment without
//! performing any I/O.
/// Extract the numeric prefix from a pipeline item ID.
///
/// Returns the leading digit sequence from IDs like `"42_story_foo"` → `"42"`.
/// Works for both numeric-only IDs (`"42"` → `"42"`) and legacy slug-format
/// IDs (`"42_story_foo"` → `"42"`).
/// Returns `None` if the ID has no leading digit sequence.
pub fn extract_item_number(item_id: &str) -> Option<&str> {
item_id
@@ -16,9 +18,9 @@ pub fn extract_item_number(item_id: &str) -> Option<&str> {
}
#[allow(dead_code)]
/// Return `true` if `item_id` has a valid `{digits}_` prefix format.
/// Return `true` if `item_id` starts with a numeric prefix.
///
/// Valid: `"42_story_foo"`, `"1_bug_bar"`.
/// Valid: `"42"`, `"42_story_foo"`, `"1_bug_bar"`.
/// Invalid: `"story_without_number"`, `""`, `"abc_story"`.
pub fn has_valid_id_prefix(item_id: &str) -> bool {
extract_item_number(item_id).is_some()
@@ -42,6 +44,20 @@ mod tests {
assert_eq!(extract_item_number("1_spike_research"), Some("1"));
}
#[test]
fn extract_item_number_works_for_numeric_only_ids() {
// Numeric-only IDs (the new canonical format).
assert_eq!(extract_item_number("42"), Some("42"));
assert_eq!(extract_item_number("730"), Some("730"));
assert_eq!(extract_item_number("1"), Some("1"));
}
#[test]
fn has_valid_id_prefix_returns_true_for_numeric_only() {
assert!(has_valid_id_prefix("42"));
assert!(has_valid_id_prefix("730"));
}
#[test]
fn extract_item_number_returns_none_for_no_numeric_prefix() {
assert_eq!(extract_item_number("story_without_number"), None);
server/src/worktree.rs
+16
View File
@@ -476,6 +476,22 @@ mod tests {
assert_eq!(branch_name("1_test"), "feature/story-1_test");
}
#[test]
fn numeric_id_worktree_path_uses_number_only() {
let project_root = Path::new("/home/user/my-project");
let path = worktree_path(project_root, "664");
assert_eq!(
path,
Path::new("/home/user/my-project/.huskies/worktrees/664")
);
}
#[test]
fn numeric_id_branch_name_uses_number_only() {
assert_eq!(branch_name("664"), "feature/story-664");
assert_eq!(branch_name("730"), "feature/story-730");
}
#[test]
fn detect_base_branch_returns_branch_in_git_repo() {
let tmp = TempDir::new().unwrap();