huskies: merge 530_story_eliminate_filesystem_markdown_shadows_entirely_crdt_db_is_the_only_story_store

This commit is contained in:
dave
2026-04-10 14:56:13 +00:00
parent 1dd675796b
commit 11d19d8902
26 changed files with 966 additions and 1668 deletions
+4 -4
View File
@@ -207,12 +207,12 @@ mod tests {
write_story_file(
tmp.path(),
"2_current",
"10_story_test.md",
"---\nname: Test\n---\n",
"8810_story_case_test.md",
"---\nname: CaseTest\n---\n",
);
let output = move_cmd_with_root(tmp.path(), "10 BACKLOG").unwrap();
let output = move_cmd_with_root(tmp.path(), "8810 BACKLOG").unwrap();
assert!(
output.contains("Test") && output.contains("backlog"),
output.contains("CaseTest") && output.contains("backlog"),
"stage matching should be case-insensitive: {output}"
);
}
+28 -99
View File
@@ -1,34 +1,15 @@
//! Shared story-lookup helper for chat commands.
//!
//! All chat commands that need to find a work item by its numeric prefix
//! use [`find_story_by_number`]. The lookup priority matches the MCP
//! `move_story` tool (which already worked correctly post-491/492 migration):
//! use [`find_story_by_number`]. The lookup reads from:
//!
//! 1. **CRDT** — authoritative in-memory state, works even when the
//! filesystem shadow does not exist.
//! 1. **CRDT** — authoritative in-memory state.
//! 2. **Content store / pipeline_items** — in-memory mirror of the
//! `pipeline_items` table; catches items that are in the DB but whose
//! CRDT entry hasn't been synced yet.
//! 3. **Filesystem** — backward-compatible fallback for stories that have
//! not yet been imported into the DB (pre-migration window).
//!
//! **Why this module exists (Story 512):** before this change, `move` and
//! `show` used pure filesystem lookups, causing them to silently fail with
//! "No story found" for any story whose filesystem shadow didn't exist — even
//! when the story was fully present in CRDT and `pipeline_items`.
use std::path::{Path, PathBuf};
/// Pipeline stage directories searched by the filesystem fallback, in order.
pub(crate) const STAGES: &[&str] = &[
"1_backlog",
"2_current",
"3_qa",
"4_merge",
"5_done",
"6_archived",
];
/// Locate a work item by its numeric ID prefix.
///
/// Returns `(story_id, stage_dir, path, content)` where:
@@ -65,52 +46,22 @@ pub(crate) fn find_story_by_number(
// ── 2. Content store + CRDT stage lookup ────────────────────────────
// Handles the edge case where an item is in the content store but was
// somehow missing from the CRDT iteration above (e.g. concurrent write).
// somehow missing from the CRDT iteration above (e.g. concurrent write
// or CRDT not yet initialised, such as in unit tests).
for id in crate::db::all_content_ids() {
if id.split('_').next().unwrap_or("") == number
&& let Some(view) = crate::crdt_state::read_item(&id)
{
let stage_dir = view.stage;
let path = project_root
.join(".huskies")
.join("work")
.join(&stage_dir)
.join(format!("{id}.md"));
let content = crate::db::read_content(&id);
return Some((id, stage_dir, path, content));
}
}
// ── 3. Filesystem (backward-compat for pre-migration stories) ────────
for stage in STAGES {
let dir = project_root.join(".huskies").join("work").join(stage);
if !dir.exists() {
if id.split('_').next().unwrap_or("") != number {
continue;
}
if let Ok(entries) = std::fs::read_dir(&dir) {
for entry in entries.flatten() {
let path = entry.path();
if path.extension().and_then(|e| e.to_str()) != Some("md") {
continue;
}
if let Some(stem) = path.file_stem().and_then(|s| s.to_str()) {
let file_num = stem
.split('_')
.next()
.filter(|s| !s.is_empty() && s.chars().all(|c| c.is_ascii_digit()))
.unwrap_or("");
if file_num == number {
let content = std::fs::read_to_string(&path).ok();
return Some((
stem.to_string(),
stage.to_string(),
path,
content,
));
}
}
}
}
let stage_dir = crate::crdt_state::read_item(&id)
.map(|v| v.stage)
.unwrap_or_else(|| "1_backlog".to_string());
let path = project_root
.join(".huskies")
.join("work")
.join(&stage_dir)
.join(format!("{id}.md"));
let content = crate::db::read_content(&id);
return Some((id, stage_dir, path, content));
}
None
@@ -128,29 +79,24 @@ mod tests {
#[test]
fn not_found_returns_none() {
let tmp = tempfile::TempDir::new().unwrap();
// Create the pipeline directories so the search runs.
for stage in STAGES {
std::fs::create_dir_all(tmp.path().join(".huskies/work").join(stage)).unwrap();
}
let result = find_story_by_number(tmp.path(), "999");
assert!(result.is_none(), "should return None when story is not found");
}
#[test]
fn finds_story_in_backlog_via_filesystem() {
fn finds_story_in_content_store() {
let tmp = tempfile::TempDir::new().unwrap();
write_story_file(
tmp.path(),
"1_backlog",
"42_story_some_feature.md",
"---\nname: Some Feature\n---\n\n# Story 42\n",
"9970_story_some_feature.md",
"---\nname: Some Feature\n---\n\n# Story 9970\n",
);
let (story_id, stage_dir, path, content) =
find_story_by_number(tmp.path(), "42").expect("should find story 42");
assert_eq!(story_id, "42_story_some_feature");
assert_eq!(stage_dir, "1_backlog");
let (story_id, _stage_dir, path, content) =
find_story_by_number(tmp.path(), "9970").expect("should find story 9970");
assert_eq!(story_id, "9970_story_some_feature");
assert!(
path.ends_with("1_backlog/42_story_some_feature.md"),
path.ends_with("9970_story_some_feature.md"),
"unexpected path: {path:?}"
);
assert!(
@@ -160,22 +106,7 @@ mod tests {
}
#[test]
fn finds_story_in_any_stage_via_filesystem() {
let tmp = tempfile::TempDir::new().unwrap();
write_story_file(
tmp.path(),
"3_qa",
"55_story_inqa.md",
"---\nname: In QA\n---\n",
);
let (story_id, stage_dir, _, _) =
find_story_by_number(tmp.path(), "55").expect("should find story 55");
assert_eq!(story_id, "55_story_inqa");
assert_eq!(stage_dir, "3_qa");
}
#[test]
fn finds_bug_by_number_via_filesystem() {
fn finds_bug_by_number() {
let tmp = tempfile::TempDir::new().unwrap();
write_story_file(
tmp.path(),
@@ -183,24 +114,23 @@ mod tests {
"7_bug_crash_on_login.md",
"---\nname: Crash on login\n---\n",
);
let (story_id, stage_dir, _, _) =
let (story_id, _stage_dir, _, _) =
find_story_by_number(tmp.path(), "7").expect("should find bug 7");
assert_eq!(story_id, "7_bug_crash_on_login");
assert_eq!(stage_dir, "2_current");
}
#[test]
fn numeric_prefix_must_match_exactly() {
let tmp = tempfile::TempDir::new().unwrap();
// Story 1 exists; searching for "10" must not match "1_story_foo".
// Story 9971 exists; searching for "99710" must not match "9971_story_foo".
write_story_file(
tmp.path(),
"1_backlog",
"1_story_foo.md",
"9971_story_foo.md",
"---\nname: Foo\n---\n",
);
let result = find_story_by_number(tmp.path(), "10");
assert!(result.is_none(), "number 10 should not match story 1");
let result = find_story_by_number(tmp.path(), "99710");
assert!(result.is_none(), "number 99710 should not match story 9971");
}
#[test]
@@ -218,6 +148,5 @@ mod tests {
path.starts_with(tmp.path()),
"path should be under the project root"
);
assert!(path.exists(), "filesystem-fallback path should exist on disk");
}
}
+8 -12
View File
@@ -4,24 +4,20 @@
use std::path::Path;
/// Write a work-item file into the standard pipeline directory structure.
/// Write a work-item into the content store and CRDT for testing.
///
/// Creates `.huskies/work/{stage}/{filename}` under `root`, creating any
/// missing parent directories. Also writes to the global content store so
/// that code paths that prefer the content store over the filesystem (e.g.
/// `unblock_by_number`) see this test's content rather than a stale entry
/// left by a parallel test with the same numeric prefix.
/// Also creates the filesystem directory structure and file so that tests
/// which still verify filesystem state (e.g. assign tests that check the
/// physical file) continue to work.
///
/// Uses `write_item_with_content` to populate both the in-memory content
/// store and the CRDT, matching the production write path.
pub(crate) fn write_story_file(root: &Path, stage: &str, filename: &str, content: &str) {
let dir = root.join(".huskies/work").join(stage);
std::fs::create_dir_all(&dir).unwrap();
std::fs::write(dir.join(filename), content).unwrap();
// Seed the in-memory content store so lifecycle functions that read from
// the content store (instead of the filesystem) see this entry. Use
// write_content (not write_item_with_content) to avoid writing to the
// CRDT — tests must not initialise the global CRDT OnceLock because that
// would pollute every subsequent test in the same process.
let story_id = filename.trim_end_matches(".md");
crate::db::ensure_content_store();
crate::db::write_content(story_id, content);
crate::db::write_item_with_content(story_id, stage, content);
}
+4 -4
View File
@@ -972,13 +972,13 @@ mod tests {
#[tokio::test]
async fn handle_schedule_story_not_in_backlog_or_current() {
let dir = TempDir::new().unwrap();
// Set up directory structure with no story in backlog or current
std::fs::create_dir_all(dir.path().join(".huskies/work/1_backlog")).unwrap();
std::fs::create_dir_all(dir.path().join(".huskies/work/2_current")).unwrap();
// Ensure CRDT content store is initialised so the DB-first lookup works.
crate::db::ensure_content_store();
// No story written — "9950_story_timer_neg" should not be found.
let store = TimerStore::load(dir.path().join("timers.json"));
let result = handle_timer_command(
TimerCommand::Schedule {
story_number_or_id: "421_story_foo".to_string(),
story_number_or_id: "9950_story_timer_neg".to_string(),
hhmm: "14:30".to_string(),
},
&store,
+27 -34
View File
@@ -92,7 +92,7 @@ pub async fn handle_assign(
agents: &AgentPool,
) -> String {
// Find the story by numeric prefix: CRDT → content store → filesystem.
let (story_id, _stage_dir, path, content) =
let (story_id, _stage_dir, _path, content) =
match crate::chat::lookup::find_story_by_number(project_root, story_number) {
Some(found) => found,
None => {
@@ -102,21 +102,24 @@ pub async fn handle_assign(
}
};
let story_name = content
.or_else(|| std::fs::read_to_string(&path).ok())
.and_then(|contents| parse_front_matter(&contents).ok().and_then(|m| m.name))
let current_content = content.or_else(|| crate::db::read_content(&story_id));
let story_name = current_content
.as_ref()
.and_then(|c| parse_front_matter(c).ok().and_then(|m| m.name))
.unwrap_or_else(|| story_id.clone());
let agent_name = resolve_agent_name(model_str);
// Write `agent: <agent_name>` into the story's front matter.
let write_result = std::fs::read_to_string(&path)
.map_err(|e| format!("Failed to read story file: {e}"))
.and_then(|contents| {
// Write `agent: <agent_name>` into the story's front matter via content store.
let write_result = match current_content {
Some(contents) => {
let updated = set_front_matter_field(&contents, "agent", &agent_name);
std::fs::write(&path, &updated)
.map_err(|e| format!("Failed to write story file: {e}"))
});
crate::db::write_item_with_content(&story_id, &_stage_dir, &updated);
Ok(())
}
None => Err(format!("Story content not found for {story_id}")),
};
if let Err(e) = write_result {
return format!("Failed to assign model to **{story_name}**: {e}");
@@ -304,15 +307,11 @@ mod tests {
// -- handle_assign (no running coder) ------------------------------------
use crate::chat::lookup::STAGES;
use crate::chat::test_helpers::write_story_file;
#[tokio::test]
async fn handle_assign_returns_not_found_for_unknown_number() {
let tmp = tempfile::tempdir().unwrap();
for stage in STAGES {
std::fs::create_dir_all(tmp.path().join(".huskies/work").join(stage)).unwrap();
}
let agents = std::sync::Arc::new(AgentPool::new_test(3000));
let response = handle_assign("Timmy", "999", "opus", tmp.path(), &agents).await;
assert!(
@@ -327,12 +326,12 @@ mod tests {
write_story_file(
tmp.path(),
"1_backlog",
"42_story_test.md",
"---\nname: Test Feature\n---\n\n# Story 42\n",
"9972_story_test.md",
"---\nname: Test Feature\n---\n\n# Story 9972\n",
);
let agents = std::sync::Arc::new(AgentPool::new_test(3000));
let response = handle_assign("Timmy", "42", "opus", tmp.path(), &agents).await;
let response = handle_assign("Timmy", "9972", "opus", tmp.path(), &agents).await;
assert!(
response.contains("coder-opus"),
@@ -348,10 +347,8 @@ mod tests {
"response should indicate assignment for future start: {response}"
);
let contents = std::fs::read_to_string(
tmp.path().join(".huskies/work/1_backlog/42_story_test.md"),
)
.unwrap();
let contents = crate::db::read_content("9972_story_test")
.expect("content store should have updated content");
assert!(
contents.contains("agent: coder-opus"),
"front matter should contain agent field: {contents}"
@@ -364,12 +361,12 @@ mod tests {
write_story_file(
tmp.path(),
"1_backlog",
"7_story_small.md",
"9973_story_small.md",
"---\nname: Small Story\n---\n",
);
let agents = std::sync::Arc::new(AgentPool::new_test(3000));
let response = handle_assign("Timmy", "7", "coder-opus", tmp.path(), &agents).await;
let response = handle_assign("Timmy", "9973", "coder-opus", tmp.path(), &agents).await;
assert!(
response.contains("coder-opus"),
@@ -380,10 +377,8 @@ mod tests {
"must not double-prefix: {response}"
);
let contents = std::fs::read_to_string(
tmp.path().join(".huskies/work/1_backlog/7_story_small.md"),
)
.unwrap();
let contents = crate::db::read_content("9973_story_small")
.expect("content store should have updated content");
assert!(
contents.contains("agent: coder-opus"),
"must write coder-opus, not coder-coder-opus: {contents}"
@@ -396,17 +391,15 @@ mod tests {
write_story_file(
tmp.path(),
"1_backlog",
"5_story_existing.md",
"9974_story_existing.md",
"---\nname: Existing\nagent: coder-sonnet\n---\n",
);
let agents = std::sync::Arc::new(AgentPool::new_test(3000));
handle_assign("Timmy", "5", "opus", tmp.path(), &agents).await;
handle_assign("Timmy", "9974", "opus", tmp.path(), &agents).await;
let contents = std::fs::read_to_string(
tmp.path().join(".huskies/work/1_backlog/5_story_existing.md"),
)
.unwrap();
let contents = crate::db::read_content("9974_story_existing")
.expect("content store should have updated content");
assert!(
contents.contains("agent: coder-opus"),
"should overwrite old agent: {contents}"
+16 -54
View File
@@ -61,7 +61,7 @@ pub async fn handle_delete(
agents: &AgentPool,
) -> String {
// Find the story by numeric prefix: CRDT → content store → filesystem.
let (story_id, stage, path, content) =
let (story_id, stage, _path, content) =
match crate::chat::lookup::find_story_by_number(project_root, story_number) {
Some(found) => found,
None => {
@@ -72,7 +72,6 @@ pub async fn handle_delete(
};
let story_name = content
.or_else(|| std::fs::read_to_string(&path).ok())
.and_then(|contents| {
crate::io::story_metadata::parse_front_matter(&contents)
.ok()
@@ -103,23 +102,9 @@ pub async fn handle_delete(
// Remove the worktree if one exists (best-effort; ignore errors).
let _ = crate::worktree::prune_worktree_sync(project_root, &story_id);
// Delete the story file.
if let Err(e) = std::fs::remove_file(&path) {
return format!("Failed to delete story {story_number}: {e}");
}
// Commit the deletion to git.
let commit_msg = format!("huskies: delete {story_id}");
let work_rel = std::path::PathBuf::from(".huskies").join("work");
let _ = std::process::Command::new("git")
.args(["add", "-A"])
.arg(&work_rel)
.current_dir(project_root)
.output();
let _ = std::process::Command::new("git")
.args(["commit", "-m", &commit_msg])
.current_dir(project_root)
.output();
// Delete from the content store and CRDT.
crate::db::delete_content(&story_id);
crate::db::delete_item(&story_id);
// Build the response.
let stage_label = stage_display_name(&stage);
@@ -265,47 +250,24 @@ mod tests {
let tmp = tempfile::tempdir().unwrap();
let project_root = tmp.path();
// Init a bare git repo so the commit step doesn't fail fatally.
std::process::Command::new("git")
.args(["init"])
.current_dir(project_root)
.output()
.unwrap();
std::process::Command::new("git")
.args(["config", "user.email", "test@test.com"])
.current_dir(project_root)
.output()
.unwrap();
std::process::Command::new("git")
.args(["config", "user.name", "Test"])
.current_dir(project_root)
.output()
.unwrap();
let backlog_dir = project_root.join(".huskies").join("work").join("1_backlog");
std::fs::create_dir_all(&backlog_dir).unwrap();
let story_path = backlog_dir.join("42_story_some_feature.md");
std::fs::write(&story_path, "---\nname: Some Feature\n---\n\n# Story 42\n").unwrap();
// Initial commit so git doesn't complain about no commits.
std::process::Command::new("git")
.args(["add", "-A"])
.current_dir(project_root)
.output()
.unwrap();
std::process::Command::new("git")
.args(["commit", "-m", "init"])
.current_dir(project_root)
.output()
.unwrap();
// Seed the story in the content store + CRDT (no filesystem needed).
crate::db::ensure_content_store();
crate::db::write_item_with_content(
"9975_story_some_feature",
"1_backlog",
"---\nname: Some Feature\n---\n\n# Story 9975\n",
);
let agents = std::sync::Arc::new(crate::agents::AgentPool::new_test(3000));
let response = handle_delete("Timmy", "42", project_root, &agents).await;
let response = handle_delete("Timmy", "9975", project_root, &agents).await;
assert!(
response.contains("Some Feature") && response.contains("backlog"),
"unexpected response: {response}"
);
assert!(!story_path.exists(), "story file should have been deleted");
assert!(
crate::db::read_content("9975_story_some_feature").is_none(),
"content store should no longer contain the deleted story"
);
}
}
+10 -9
View File
@@ -80,7 +80,7 @@ pub async fn handle_start(
agents: &AgentPool,
) -> String {
// Find the story by numeric prefix: CRDT → content store → filesystem.
let (story_id, _stage_dir, path, content) =
let (story_id, _stage_dir, _path, content) =
match crate::chat::lookup::find_story_by_number(project_root, story_number) {
Some(found) => found,
None => {
@@ -91,7 +91,6 @@ pub async fn handle_start(
};
let story_name = content
.or_else(|| std::fs::read_to_string(&path).ok())
.and_then(|contents| {
crate::io::story_metadata::parse_front_matter(&contents)
.ok()
@@ -252,23 +251,25 @@ mod tests {
let tmp = tempfile::tempdir().unwrap();
let project_root = tmp.path();
let sk = project_root.join(".huskies");
let backlog = sk.join("work/1_backlog");
std::fs::create_dir_all(&backlog).unwrap();
std::fs::create_dir_all(&sk).unwrap();
std::fs::write(
sk.join("project.toml"),
"[[agent]]\nname = \"coder-1\"\nstage = \"coder\"\n",
)
.unwrap();
std::fs::write(
backlog.join("356_story_test.md"),
// Seed the story in the content store + CRDT (no filesystem needed).
crate::db::ensure_content_store();
crate::db::write_item_with_content(
"9976_story_test",
"1_backlog",
"---\nname: Test Story\n---\n",
)
.unwrap();
);
let agents = Arc::new(AgentPool::new_test(3000));
agents.inject_test_agent("other-story", "coder-1", AgentStatus::Running);
let response = handle_start("Timmy", "356", None, project_root, &agents).await;
let response = handle_start("Timmy", "9976", None, project_root, &agents).await;
assert!(
!response.contains("Failed"),