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

huskies: merge 879

Browse Source
This commit is contained in:
dave
2026-04-30 00:21:31 +00:00
parent 8fc581ad6b
commit a796bd933f
4 changed files with 98 additions and 129 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/chat/commands/depends.rs
+78 -56
View File
@@ -7,9 +7,7 @@
//! Passing no dependency numbers clears the field entirely.
use super::CommandContext;
use crate::io::story_metadata::{
parse_front_matter, write_depends_on, write_depends_on_in_content,
};
use crate::io::story_metadata::parse_front_matter;
/// Handle the `depends` command.
///
@@ -53,7 +51,7 @@ pub(super) fn handle_depends(ctx: &CommandContext) -> Option<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(ctx.effective_root(), num_str) {
Some(found) => found,
None => {
@@ -69,49 +67,20 @@ pub(super) fn handle_depends(ctx: &CommandContext) -> Option<String> {
.and_then(|m| m.name)
.unwrap_or_else(|| story_id.clone());
// 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);
// Sync depends_on to the typed CRDT register.
crate::crdt_state::set_depends_on(&story_id, &deps);
if deps.is_empty() {
Some(format!(
"Cleared all dependencies for **{story_name}** ({story_id})."
))
} else {
let nums: Vec<String> = deps.iter().map(|n| n.to_string()).collect();
Some(format!(
"Set depends_on: [{}] for **{story_name}** ({story_id}).",
nums.join(", ")
))
}
// Write depends_on to the typed CRDT register — single source of truth.
// No YAML mutation: the CRDT register is the canonical location for deps.
crate::crdt_state::set_depends_on(&story_id, &deps);
if deps.is_empty() {
Some(format!(
"Cleared all dependencies for **{story_name}** ({story_id})."
))
} else {
match write_depends_on(&path, &deps) {
Ok(()) if deps.is_empty() => {
crate::crdt_state::set_depends_on(&story_id, &[]);
Some(format!(
"Cleared all dependencies for **{story_name}** ({story_id})."
))
}
Ok(()) => {
crate::crdt_state::set_depends_on(&story_id, &deps);
let nums: Vec<String> = 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}")),
}
let nums: Vec<String> = deps.iter().map(|n| n.to_string()).collect();
Some(format!(
"Set depends_on: [{}] for **{story_name}** ({story_id}).",
nums.join(", ")
))
}
}
@@ -207,7 +176,7 @@ mod tests {
}
#[test]
fn depends_sets_deps_and_writes_to_content_store() {
fn depends_sets_deps_in_crdt_not_yaml() {
let tmp = tempfile::TempDir::new().unwrap();
write_story_file(
tmp.path(),
@@ -220,33 +189,86 @@ mod tests {
output.contains("477") && output.contains("478"),
"response should mention dep numbers: {output}"
);
let contents = crate::db::read_content("9910_story_foo")
.expect("content store should have updated story");
// CRDT register must hold the deps.
let view = crate::crdt_state::read_item("9910_story_foo").expect("CRDT should have story");
assert_eq!(
view.depends_on,
Some(vec![477, 478]),
"CRDT register should hold [477, 478]: {view:?}"
);
// Content store YAML must NOT be mutated with depends_on.
let contents =
crate::db::read_content("9910_story_foo").expect("content store should have story");
assert!(
contents.contains("depends_on: [477, 478]"),
"content store should have depends_on set: {contents}"
!contents.contains("depends_on"),
"content store YAML must not contain depends_on after chat command: {contents}"
);
}
#[test]
fn depends_clears_deps_when_no_deps_given() {
fn depends_clears_deps_in_crdt_not_yaml() {
let tmp = tempfile::TempDir::new().unwrap();
write_story_file(
tmp.path(),
"2_current",
"9911_story_bar.md",
"---\nname: Bar\ndepends_on: [477]\n---\n",
"---\nname: Bar\n---\n",
);
// Pre-seed CRDT with deps so we can verify clearing.
crate::crdt_state::set_depends_on("9911_story_bar", &[477]);
let output = depends_cmd_with_root(tmp.path(), "9911").unwrap();
assert!(
output.contains("Cleared"),
"should confirm clearing deps: {output}"
);
let contents = crate::db::read_content("9911_story_bar")
.expect("content store should have updated story");
// CRDT register must be empty after clear.
let view = crate::crdt_state::read_item("9911_story_bar").expect("CRDT should have story");
assert_eq!(
view.depends_on, None,
"CRDT register should be empty after clearing: {view:?}"
);
// Content store YAML must not be mutated.
let contents =
crate::db::read_content("9911_story_bar").expect("content store should have story");
assert!(
!contents.contains("depends_on"),
"content store should have depends_on cleared: {contents}"
"content store YAML must not contain depends_on after clear: {contents}"
);
}
/// Regression (AC3, chat path): chat `depends` must write deps to CRDT only —
/// no `depends_on` in the YAML content store.
/// The MCP counterpart is `tool_update_story_depends_on_routes_to_crdt_not_yaml`
/// in `http/mcp/story_tools/story/update.rs`. Both assert `Some(vec![500, 501])`
/// proving identical CRDT state across transports.
#[test]
fn chat_depends_sets_crdt_no_yaml_regression() {
let tmp = tempfile::TempDir::new().unwrap();
write_story_file(
tmp.path(),
"1_backlog",
"8790_story_chat_dep.md",
"---\nname: Chat Dep\n---\n",
);
let out = depends_cmd_with_root(tmp.path(), "8790 500 501").unwrap();
assert!(
out.contains("500"),
"chat response should mention dep: {out}"
);
let view =
crate::crdt_state::read_item("8790_story_chat_dep").expect("CRDT must have chat story");
assert_eq!(
view.depends_on,
Some(vec![500, 501]),
"CRDT must hold [500, 501]: {view:?}"
);
let content = crate::db::read_content("8790_story_chat_dep").unwrap();
assert!(
!content.contains("depends_on"),
"chat must not write depends_on to YAML: {content}"
);
}
server/src/http/mcp/story_tools/story/update.rs
+18 -17
View File
@@ -41,8 +41,10 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result<String
crate::crdt_state::set_qa_mode(story_id, mode);
}
// Sync `depends_on` to the typed CRDT register when present in the update.
if let Some(deps_val) = front_matter.get("depends_on") {
// Route `depends_on` to the typed CRDT register and remove it from the
// front-matter map so it is NOT written back to the YAML content store.
// This matches the `qa` field pattern: CRDT is the single source of truth.
if let Some(deps_val) = front_matter.remove("depends_on") {
if let Some(arr) = deps_val.as_array() {
let dep_nums: Vec<u32> = arr
.iter()
@@ -148,7 +150,7 @@ mod tests {
fs::create_dir_all(&current).unwrap();
fs::write(current.join(format!("{story_id}.md")), content).unwrap();
crate::db::ensure_content_store();
crate::db::write_content(story_id, content);
crate::db::write_item_with_content(story_id, "2_current", content);
}
#[test]
@@ -206,7 +208,7 @@ mod tests {
}
#[test]
fn tool_update_story_front_matter_json_array_written_as_yaml_sequence() {
fn tool_update_story_depends_on_routes_to_crdt_not_yaml() {
let tmp = tempfile::tempdir().unwrap();
setup_story_for_update(
tmp.path(),
@@ -221,20 +223,19 @@ mod tests {
);
assert!(result.is_ok(), "Expected ok: {result:?}");
let content = crate::db::read_content("504_arr_test").unwrap();
// YAML inline sequences use spaces after commas
assert!(
content.contains("depends_on: [490, 491]"),
"array should be unquoted YAML: {content}"
);
assert!(
!content.contains("depends_on: \""),
"array must not be quoted: {content}"
// CRDT register must hold the deps.
let view = crate::crdt_state::read_item("504_arr_test").expect("CRDT must have the story");
assert_eq!(
view.depends_on,
Some(vec![490, 491]),
"CRDT register should hold [490, 491]: {view:?}"
);
// The YAML must be parseable as a vec
let meta = crate::io::story_metadata::parse_front_matter(&content)
.expect("front matter should parse");
assert_eq!(meta.depends_on, Some(vec![490, 491]));
// Content store YAML must NOT contain depends_on — CRDT is sole source of truth.
let content = crate::db::read_content("504_arr_test").unwrap();
assert!(
!content.contains("depends_on"),
"depends_on must not be written to YAML content store: {content}"
);
}
}
server/src/io/story_metadata/fields.rs
-53
View File
@@ -94,25 +94,6 @@ pub fn write_review_hold(path: &Path) -> Result<(), String> {
Ok(())
}
/// Write or update `depends_on:` field in the YAML front matter of a story file.
///
/// Serialises `deps` as an inline YAML sequence, e.g. `[477, 478]`.
/// If `deps` is empty the field is removed.
/// If no front matter is present, this is a no-op (returns Ok).
pub fn write_depends_on(path: &Path, deps: &[u32]) -> Result<(), String> {
let contents =
fs::read_to_string(path).map_err(|e| format!("Failed to read story file: {e}"))?;
let updated = if deps.is_empty() {
remove_front_matter_field(&contents, "depends_on")
} else {
let nums: Vec<String> = deps.iter().map(|n| n.to_string()).collect();
let yaml_value = format!("[{}]", nums.join(", "));
set_front_matter_field(&contents, "depends_on", &yaml_value)
};
fs::write(path, &updated).map_err(|e| format!("Failed to write story file: {e}"))?;
Ok(())
}
/// Remove a key from the YAML front matter of a markdown string (pure function).
///
/// Returns the updated content. If no front matter or key is not found,
@@ -152,20 +133,6 @@ pub fn write_mergemaster_attempted_in_content(contents: &str) -> String {
set_front_matter_field(contents, "mergemaster_attempted", "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<String> = deps.iter().map(|n| n.to_string()).collect();
let yaml_value = format!("[{}]", nums.join(", "));
set_front_matter_field(contents, "depends_on", &yaml_value)
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -242,24 +209,4 @@ mod tests {
assert!(contents.contains("review_hold: true"));
assert!(contents.contains("name: My Spike"));
}
#[test]
fn write_depends_on_sets_field() {
let tmp = tempfile::tempdir().unwrap();
let path = tmp.path().join("story.md");
std::fs::write(&path, "---\nname: Test\n---\n# Story\n").unwrap();
write_depends_on(&path, &[477, 478]).unwrap();
let contents = std::fs::read_to_string(&path).unwrap();
assert!(contents.contains("depends_on: [477, 478]"), "{contents}");
}
#[test]
fn write_depends_on_removes_field_when_empty() {
let tmp = tempfile::tempdir().unwrap();
let path = tmp.path().join("story.md");
std::fs::write(&path, "---\nname: Test\ndepends_on: [477]\n---\n# Story\n").unwrap();
write_depends_on(&path, &[]).unwrap();
let contents = std::fs::read_to_string(&path).unwrap();
assert!(!contents.contains("depends_on"), "{contents}");
}
}
server/src/io/story_metadata/mod.rs
+2 -3
View File
@@ -14,9 +14,8 @@ mod types;
pub use deps::{check_archived_deps, check_archived_deps_from_list, check_unmet_deps};
pub use fields::{
clear_front_matter_field, clear_front_matter_field_in_content, set_front_matter_field,
write_depends_on, write_depends_on_in_content, write_merge_failure_in_content,
write_mergemaster_attempted_in_content, write_rejection_notes_to_content, write_review_hold,
write_review_hold_in_content,
write_merge_failure_in_content, write_mergemaster_attempted_in_content,
write_rejection_notes_to_content, write_review_hold, write_review_hold_in_content,
};
pub use parser::{
is_story_frozen_in_store, parse_front_matter, parse_unchecked_todos, resolve_qa_mode,