From 0bf715d9bb4b4754b159121e9fb536c4078eb84d Mon Sep 17 00:00:00 2001 From: dave Date: Wed, 15 Apr 2026 13:33:45 +0000 Subject: [PATCH] huskies: merge 574_bug_depends_bot_command_broken_after_removing_filesystem_story_files --- server/src/chat/commands/depends.rs | 81 +++++++++++++++++++---------- server/src/io/story_metadata.rs | 14 +++++ 2 files changed, 67 insertions(+), 28 deletions(-) diff --git a/server/src/chat/commands/depends.rs b/server/src/chat/commands/depends.rs index d59bb9db..2e46658b 100644 --- a/server/src/chat/commands/depends.rs +++ b/server/src/chat/commands/depends.rs @@ -7,7 +7,9 @@ //! Passing no dependency numbers clears the field entirely. use super::CommandContext; -use crate::io::story_metadata::{parse_front_matter, write_depends_on}; +use crate::io::story_metadata::{ + parse_front_matter, write_depends_on, write_depends_on_in_content, +}; /// Handle the `depends` command. /// @@ -51,7 +53,7 @@ pub(super) fn handle_depends(ctx: &CommandContext) -> Option { } // 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(ctx.project_root, num_str) { Some(found) => found, None => { @@ -62,23 +64,48 @@ pub(super) fn handle_depends(ctx: &CommandContext) -> Option { }; let story_name = content - .or_else(|| std::fs::read_to_string(&path).ok()) - .and_then(|c| parse_front_matter(&c).ok()) + .as_deref() + .and_then(|c| parse_front_matter(c).ok()) .and_then(|m| m.name) .unwrap_or_else(|| story_id.clone()); - match write_depends_on(&path, &deps) { - Ok(()) if deps.is_empty() => Some(format!( - "Cleared all dependencies for **{story_name}** ({story_id})." - )), - Ok(()) => { + // Prefer the CRDT content store; fall back to filesystem only when the + // story has not been loaded into the DB (e.g. very early startup or tests + // that haven't called write_item_with_content). + if let Some(existing) = crate::db::read_content(&story_id) { + let updated = write_depends_on_in_content(&existing, &deps); + crate::db::write_content(&story_id, &updated); + let stage = crate::pipeline_state::read_typed(&story_id) + .ok() + .flatten() + .map(|i| i.stage.dir_name().to_string()) + .unwrap_or_else(|| stage_dir.clone()); + crate::db::write_item_with_content(&story_id, &stage, &updated); + if deps.is_empty() { + Some(format!( + "Cleared all dependencies for **{story_name}** ({story_id})." + )) + } else { let nums: Vec = deps.iter().map(|n| n.to_string()).collect(); Some(format!( "Set depends_on: [{}] for **{story_name}** ({story_id}).", nums.join(", ") )) } - Err(e) => Some(format!("Failed to update dependencies for {story_id}: {e}")), + } else { + match write_depends_on(&path, &deps) { + Ok(()) if deps.is_empty() => Some(format!( + "Cleared all dependencies for **{story_name}** ({story_id})." + )), + Ok(()) => { + let nums: Vec = deps.iter().map(|n| n.to_string()).collect(); + Some(format!( + "Set depends_on: [{}] for **{story_name}** ({story_id}).", + nums.join(", ") + )) + } + Err(e) => Some(format!("Failed to update dependencies for {story_id}: {e}")), + } } } @@ -170,10 +197,10 @@ mod tests { write_story_file( tmp.path(), "1_backlog", - "42_story_foo.md", + "9912_story_foo.md", "---\nname: Foo\n---\n", ); - let output = depends_cmd_with_root(tmp.path(), "42 abc").unwrap(); + let output = depends_cmd_with_root(tmp.path(), "9912 abc").unwrap(); assert!( output.contains("Invalid dependency number"), "non-numeric dep should error: {output}" @@ -181,25 +208,24 @@ mod tests { } #[test] - fn depends_sets_deps_and_writes_to_file() { + fn depends_sets_deps_and_writes_to_content_store() { let tmp = tempfile::TempDir::new().unwrap(); write_story_file( tmp.path(), "1_backlog", - "42_story_foo.md", + "9910_story_foo.md", "---\nname: Foo\n---\n", ); - let output = depends_cmd_with_root(tmp.path(), "42 477 478").unwrap(); + let output = depends_cmd_with_root(tmp.path(), "9910 477 478").unwrap(); assert!( output.contains("477") && output.contains("478"), "response should mention dep numbers: {output}" ); - let contents = - std::fs::read_to_string(tmp.path().join(".huskies/work/1_backlog/42_story_foo.md")) - .unwrap(); + let contents = crate::db::read_content("9910_story_foo") + .expect("content store should have updated story"); assert!( contents.contains("depends_on: [477, 478]"), - "file should have depends_on set: {contents}" + "content store should have depends_on set: {contents}" ); } @@ -209,20 +235,19 @@ mod tests { write_story_file( tmp.path(), "2_current", - "10_story_bar.md", + "9911_story_bar.md", "---\nname: Bar\ndepends_on: [477]\n---\n", ); - let output = depends_cmd_with_root(tmp.path(), "10").unwrap(); + let output = depends_cmd_with_root(tmp.path(), "9911").unwrap(); assert!( output.contains("Cleared"), "should confirm clearing deps: {output}" ); - let contents = - std::fs::read_to_string(tmp.path().join(".huskies/work/2_current/10_story_bar.md")) - .unwrap(); + let contents = crate::db::read_content("9911_story_bar") + .expect("content store should have updated story"); assert!( !contents.contains("depends_on"), - "file should have depends_on cleared: {contents}" + "content store should have depends_on cleared: {contents}" ); } @@ -232,12 +257,12 @@ mod tests { write_story_file( tmp.path(), "3_qa", - "55_story_inqa.md", + "9913_story_inqa.md", "---\nname: In QA\n---\n", ); - let output = depends_cmd_with_root(tmp.path(), "55 100").unwrap(); + let output = depends_cmd_with_root(tmp.path(), "9913 100").unwrap(); assert!( - output.contains("In QA") || output.contains("55_story_inqa"), + output.contains("In QA") || output.contains("9913_story_inqa"), "should find story in qa stage: {output}" ); assert!(output.contains("100"), "should mention dep 100: {output}"); diff --git a/server/src/io/story_metadata.rs b/server/src/io/story_metadata.rs index f2466288..434bf9b1 100644 --- a/server/src/io/story_metadata.rs +++ b/server/src/io/story_metadata.rs @@ -459,6 +459,20 @@ pub fn write_review_hold_in_content(contents: &str) -> String { set_front_matter_field(contents, "review_hold", "true") } +/// Write or update `depends_on` in story content (pure function). +/// +/// Serialises `deps` as an inline YAML sequence, e.g. `[477, 478]`. +/// If `deps` is empty the field is removed. +pub fn write_depends_on_in_content(contents: &str, deps: &[u32]) -> String { + if deps.is_empty() { + remove_front_matter_field(contents, "depends_on") + } else { + let nums: Vec = deps.iter().map(|n| n.to_string()).collect(); + let yaml_value = format!("[{}]", nums.join(", ")); + set_front_matter_field(contents, "depends_on", &yaml_value) + } +} + /// Resolve the effective QA mode for a story file. /// /// Reads the `qa` front matter field. If absent, falls back to `default`.