feat(929): delete db/yaml_legacy.rs entirely — CRDT is the sole source of truth
Final 929 sweep: every YAML-shaped helper is gone. No production code
parses or writes YAML front matter anywhere.
Surface removed:
- db/yaml_legacy.rs (FrontMatter/StoryMetadata structs, parse_front_matter,
set_front_matter_field, yaml_residue marker) — file deleted.
- ItemMeta::from_yaml — deleted; callers pass typed ItemMeta::named(...) or
ItemMeta::default() and use typed CRDT setters (set_depends_on,
set_blocked, set_retry_count, set_agent, set_qa_mode, set_review_hold,
set_item_type, set_epic, set_mergemaster_attempted) for the rest.
- write_coverage_baseline_to_story_file + read_coverage_percent_from_json —
the coverage_baseline YAML field was write-only (nothing read it back);
removed along with its caller in agent_tools/lifecycle.rs.
- update_story_in_file's generic `front_matter` HashMap parameter —
tool_update_story now intercepts every known field name and routes it
to a typed CRDT setter; unknown keys are rejected with an explicit error
pointing at the typed setters. The function only takes user_story /
description sections now.
- All 117 ItemMeta::from_yaml callsites migrated. Where tests previously
passed a YAML-shaped content blob and relied on the helper to extract
name/depends_on/blocked/agent/qa, they now pass:
write_item_with_content(id, stage, content, ItemMeta::named("Foo"))
crate::crdt_state::set_depends_on(id, &[...]) // when needed
crate::crdt_state::set_blocked(id, true) // when needed
crate::crdt_state::set_agent(id, Some("...")) // when needed
- write_story_content + write_story_file (test helper) now take an
explicit `name: Option<&str>` instead of parsing it from content.
- db::ops::move_item_stage stopped re-parsing YAML on every stage
transition; metadata is read straight from the CRDT view when mirroring
the row into SQLite.
New CRDT setters added for symmetry:
- crdt_state::set_name (mirrors set_agent — explicit name updates).
cargo fmt --check, clippy --all-targets -- -D warnings, and the
2830-test suite all pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -3,7 +3,6 @@
|
||||
use serde_json::{Value, json};
|
||||
|
||||
use crate::http::context::AppContext;
|
||||
use crate::slog_warn;
|
||||
|
||||
use super::worktree::get_worktree_commits;
|
||||
|
||||
@@ -32,16 +31,10 @@ pub(crate) async fn tool_start_agent(args: &Value, ctx: &AppContext) -> Result<S
|
||||
.await?
|
||||
};
|
||||
|
||||
// Snapshot coverage baseline from the most recent coverage report (best-effort).
|
||||
if let Some(pct) = read_coverage_percent_from_json(&project_root)
|
||||
&& let Err(e) = crate::http::workflow::write_coverage_baseline_to_story_file(
|
||||
&project_root,
|
||||
story_id,
|
||||
pct,
|
||||
)
|
||||
{
|
||||
slog_warn!("[start_agent] Could not write coverage baseline to story file: {e}");
|
||||
}
|
||||
// Story 929: the legacy "snapshot coverage baseline into story YAML"
|
||||
// hook is gone — the field was write-only (nothing read it back) and
|
||||
// the per-run report at `.huskies/coverage/server.json` is the
|
||||
// authoritative source for downstream tools.
|
||||
|
||||
serde_json::to_string_pretty(&json!({
|
||||
"story_id": info.story_id,
|
||||
@@ -53,21 +46,6 @@ pub(crate) async fn tool_start_agent(args: &Value, ctx: &AppContext) -> Result<S
|
||||
.map_err(|e| format!("Serialization error: {e}"))
|
||||
}
|
||||
|
||||
/// Try to read the overall line coverage percentage from the llvm-cov JSON report.
|
||||
///
|
||||
/// Expects the file at `{project_root}/.huskies/coverage/server.json`.
|
||||
pub(crate) fn read_coverage_percent_from_json(project_root: &std::path::Path) -> Option<f64> {
|
||||
let path = project_root
|
||||
.join(".huskies")
|
||||
.join("coverage")
|
||||
.join("server.json");
|
||||
let contents = std::fs::read_to_string(&path).ok()?;
|
||||
let json: Value = serde_json::from_str(&contents).ok()?;
|
||||
// cargo llvm-cov --json format: data[0].totals.lines.percent
|
||||
json.pointer("/data/0/totals/lines/percent")
|
||||
.and_then(|v| v.as_f64())
|
||||
}
|
||||
|
||||
pub(crate) async fn tool_stop_agent(args: &Value, ctx: &AppContext) -> Result<String, String> {
|
||||
let story_id = args
|
||||
.get("story_id")
|
||||
@@ -334,25 +312,4 @@ stage = "coder"
|
||||
// completion key present (null for agents that didn't call report_completion)
|
||||
assert!(parsed.get("completion").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_coverage_percent_from_json_parses_llvm_cov_format() {
|
||||
use std::fs;
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let cov_dir = tmp.path().join(".huskies/coverage");
|
||||
fs::create_dir_all(&cov_dir).unwrap();
|
||||
let json_content =
|
||||
r#"{"data":[{"totals":{"lines":{"count":100,"covered":78,"percent":78.0}}}]}"#;
|
||||
fs::write(cov_dir.join("server.json"), json_content).unwrap();
|
||||
|
||||
let pct = read_coverage_percent_from_json(tmp.path());
|
||||
assert_eq!(pct, Some(78.0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_coverage_percent_from_json_returns_none_when_absent() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let pct = read_coverage_percent_from_json(tmp.path());
|
||||
assert!(pct.is_none());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -453,7 +453,7 @@ mod tests {
|
||||
"5_story_test",
|
||||
"1_backlog",
|
||||
content,
|
||||
crate::db::ItemMeta::from_yaml(content),
|
||||
crate::db::ItemMeta::default(),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(root);
|
||||
@@ -485,7 +485,7 @@ mod tests {
|
||||
"6_story_back",
|
||||
"2_current",
|
||||
content,
|
||||
crate::db::ItemMeta::from_yaml(content),
|
||||
crate::db::ItemMeta::default(),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(root);
|
||||
@@ -517,7 +517,7 @@ mod tests {
|
||||
"9907_story_idem",
|
||||
"2_current",
|
||||
content,
|
||||
crate::db::ItemMeta::from_yaml(content),
|
||||
crate::db::ItemMeta::default(),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(root);
|
||||
|
||||
@@ -362,13 +362,16 @@ mod tests {
|
||||
|
||||
crate::crdt_state::init_for_test();
|
||||
crate::db::ensure_content_store();
|
||||
let story_content = "---\nname: Blocked Story\nblocked: true\nretry_count: 3\ndepends_on: [100, 200]\n---\n\n## Acceptance Criteria\n\n- [ ] Do the thing\n";
|
||||
let story_content = "# Story\n\n## Acceptance Criteria\n\n- [ ] Do the thing\n";
|
||||
crate::db::write_item_with_content(
|
||||
"9887_story_blocked_test",
|
||||
"2_current",
|
||||
story_content,
|
||||
crate::db::ItemMeta::from_yaml(story_content),
|
||||
crate::db::ItemMeta::named("Blocked Story"),
|
||||
);
|
||||
crate::crdt_state::set_blocked("9887_story_blocked_test", true);
|
||||
crate::crdt_state::set_retry_count("9887_story_blocked_test", 3);
|
||||
crate::crdt_state::set_depends_on("9887_story_blocked_test", &[100, 200]);
|
||||
|
||||
let ctx = crate::http::context::AppContext::new_test(tmp.path().to_path_buf());
|
||||
let result = tool_status(&json!({"story_id": "9887_story_blocked_test"}), &ctx)
|
||||
@@ -389,13 +392,14 @@ mod tests {
|
||||
let tmp = tempdir().unwrap();
|
||||
|
||||
crate::db::ensure_content_store();
|
||||
let story_content = "---\nname: My Test Story\nagent: coder-1\n---\n\n## Acceptance Criteria\n\n- [ ] First criterion\n- [x] Second criterion\n\n## Out of Scope\n\n- nothing\n";
|
||||
let story_content = "# Story\n\n## Acceptance Criteria\n\n- [ ] First criterion\n- [x] Second criterion\n\n## Out of Scope\n\n- nothing\n";
|
||||
crate::db::write_item_with_content(
|
||||
"9886_story_status_test",
|
||||
"2_current",
|
||||
story_content,
|
||||
crate::db::ItemMeta::from_yaml(story_content),
|
||||
crate::db::ItemMeta::named("My Test Story"),
|
||||
);
|
||||
crate::crdt_state::set_agent("9886_story_status_test", Some("coder-1"));
|
||||
|
||||
let ctx = crate::http::context::AppContext::new_test(tmp.path().to_path_buf());
|
||||
let result = tool_status(&json!({"story_id": "9886_story_status_test"}), &ctx)
|
||||
|
||||
@@ -287,18 +287,14 @@ mod tests {
|
||||
crate::db::write_item_with_content(
|
||||
"9902_bug_crash",
|
||||
"1_backlog",
|
||||
"---\nname: \"App Crash\"\n---\n# Bug 9902: App Crash\n",
|
||||
crate::db::ItemMeta::from_yaml(
|
||||
"---\nname: \"App Crash\"\n---\n# Bug 9902: App Crash\n",
|
||||
),
|
||||
"# Bug 9902: App Crash\n",
|
||||
crate::db::ItemMeta::named("App Crash"),
|
||||
);
|
||||
crate::db::write_item_with_content(
|
||||
"9903_bug_typo",
|
||||
"1_backlog",
|
||||
"---\nname: \"Typo in Header\"\n---\n# Bug 9903: Typo in Header\n",
|
||||
crate::db::ItemMeta::from_yaml(
|
||||
"---\nname: \"Typo in Header\"\n---\n# Bug 9903: Typo in Header\n",
|
||||
),
|
||||
"# Bug 9903: Typo in Header\n",
|
||||
crate::db::ItemMeta::named("Typo in Header"),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(tmp.path());
|
||||
@@ -446,7 +442,7 @@ mod tests {
|
||||
"9901_bug_crash",
|
||||
"1_backlog",
|
||||
content,
|
||||
crate::db::ItemMeta::from_yaml(content),
|
||||
crate::db::ItemMeta::default(),
|
||||
);
|
||||
// Stage the file so it's tracked
|
||||
std::process::Command::new("git")
|
||||
|
||||
@@ -421,9 +421,7 @@ mod tests {
|
||||
"9901_test",
|
||||
"2_current",
|
||||
"---\nname: Test\n---\n## AC\n- [ ] First\n- [x] Done\n- [ ] Second\n",
|
||||
crate::db::ItemMeta::from_yaml(
|
||||
"---\nname: Test\n---\n## AC\n- [ ] First\n- [x] Done\n- [ ] Second\n",
|
||||
),
|
||||
crate::db::ItemMeta::named("Test"),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(tmp.path());
|
||||
@@ -517,7 +515,7 @@ mod tests {
|
||||
"9906_story_persist",
|
||||
"2_current",
|
||||
"---\nname: Persist\n---\n# Story\n",
|
||||
crate::db::ItemMeta::from_yaml("---\nname: Persist\n---\n# Story\n"),
|
||||
crate::db::ItemMeta::named("Persist"),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(tmp.path());
|
||||
@@ -555,7 +553,7 @@ mod tests {
|
||||
"9905_story_file_only",
|
||||
"2_current",
|
||||
story_content,
|
||||
crate::db::ItemMeta::from_yaml(story_content),
|
||||
crate::db::ItemMeta::default(),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(tmp.path());
|
||||
@@ -627,9 +625,7 @@ mod tests {
|
||||
"9997_empty_branch",
|
||||
"2_current",
|
||||
"---\nname: Empty Branch Test\n---\n## AC\n- [ ] Implement the feature\n",
|
||||
crate::db::ItemMeta::from_yaml(
|
||||
"---\nname: Empty Branch Test\n---\n## AC\n- [ ] Implement the feature\n",
|
||||
),
|
||||
crate::db::ItemMeta::named("Empty Branch Test"),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(tmp.path());
|
||||
@@ -690,9 +686,7 @@ mod tests {
|
||||
"9904_test",
|
||||
"2_current",
|
||||
"---\nname: Test\n---\n## AC\n- [ ] First criterion\n- [x] Already done\n",
|
||||
crate::db::ItemMeta::from_yaml(
|
||||
"---\nname: Test\n---\n## AC\n- [ ] First criterion\n- [x] Already done\n",
|
||||
),
|
||||
crate::db::ItemMeta::named("Test"),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(tmp.path());
|
||||
@@ -744,9 +738,7 @@ mod tests {
|
||||
"9905_test",
|
||||
"2_current",
|
||||
"---\nname: Test\n---\n## Acceptance Criteria\n- [ ] Keep me\n- [ ] Remove me\n",
|
||||
crate::db::ItemMeta::from_yaml(
|
||||
"---\nname: Test\n---\n## Acceptance Criteria\n- [ ] Keep me\n- [ ] Remove me\n",
|
||||
),
|
||||
crate::db::ItemMeta::named("Test"),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(tmp.path());
|
||||
@@ -768,9 +760,7 @@ mod tests {
|
||||
"9906_test",
|
||||
"2_current",
|
||||
"---\nname: Test\n---\n## Acceptance Criteria\n- [ ] Only one\n",
|
||||
crate::db::ItemMeta::from_yaml(
|
||||
"---\nname: Test\n---\n## Acceptance Criteria\n- [ ] Only one\n",
|
||||
),
|
||||
crate::db::ItemMeta::named("Test"),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(tmp.path());
|
||||
|
||||
@@ -230,7 +230,7 @@ mod tests {
|
||||
"51_story_no_branch",
|
||||
"2_current",
|
||||
content,
|
||||
crate::db::ItemMeta::from_yaml(content),
|
||||
crate::db::ItemMeta::default(),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(tmp.path());
|
||||
|
||||
@@ -60,7 +60,7 @@ mod tests {
|
||||
story_id,
|
||||
"2_current",
|
||||
"---\nname: MCP Freeze Tool Test\n---\n",
|
||||
crate::db::ItemMeta::from_yaml("---\nname: MCP Freeze Tool Test\n---\n"),
|
||||
crate::db::ItemMeta::named("MCP Freeze Tool Test"),
|
||||
);
|
||||
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
@@ -89,7 +89,7 @@ mod tests {
|
||||
story_id,
|
||||
"2_current",
|
||||
"---\nname: MCP Unfreeze Tool Test\n---\n",
|
||||
crate::db::ItemMeta::from_yaml("---\nname: MCP Unfreeze Tool Test\n---\n"),
|
||||
crate::db::ItemMeta::named("MCP Unfreeze Tool Test"),
|
||||
);
|
||||
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
|
||||
@@ -154,7 +154,10 @@ mod tests {
|
||||
id,
|
||||
stage,
|
||||
&format!("---\nname: \"{name}\"\n---\n"),
|
||||
crate::db::ItemMeta::from_yaml(&format!("---\nname: \"{name}\"\n---\n")),
|
||||
crate::db::ItemMeta {
|
||||
name: Some(name.to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
@@ -192,7 +195,7 @@ mod tests {
|
||||
"9921_story_active",
|
||||
"2_current",
|
||||
"---\nname: \"Active Story\"\n---\n",
|
||||
crate::db::ItemMeta::from_yaml("---\nname: \"Active Story\"\n---\n"),
|
||||
crate::db::ItemMeta::named("Active Story"),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(tmp.path());
|
||||
@@ -225,7 +228,7 @@ mod tests {
|
||||
"9907_test",
|
||||
"2_current",
|
||||
"---\nname: \"Valid Story\"\n---\n## AC\n- [ ] First\n",
|
||||
crate::db::ItemMeta::from_yaml("---\nname: \"Valid Story\"\n---\n## AC\n- [ ] First\n"),
|
||||
crate::db::ItemMeta::named("Valid Story"),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(tmp.path());
|
||||
@@ -247,7 +250,7 @@ mod tests {
|
||||
"9908_test",
|
||||
"2_current",
|
||||
"## No front matter at all\n",
|
||||
crate::db::ItemMeta::from_yaml("## No front matter at all\n"),
|
||||
crate::db::ItemMeta::default(),
|
||||
);
|
||||
|
||||
let ctx = test_ctx(tmp.path());
|
||||
|
||||
@@ -4,7 +4,6 @@ use crate::http::context::AppContext;
|
||||
use crate::http::workflow::update_story_in_file;
|
||||
use crate::slog_warn;
|
||||
use serde_json::Value;
|
||||
use std::collections::HashMap;
|
||||
|
||||
pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result<String, String> {
|
||||
let story_id = args
|
||||
@@ -14,88 +13,110 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result<String
|
||||
let user_story = args.get("user_story").and_then(|v| v.as_str());
|
||||
let description = args.get("description").and_then(|v| v.as_str());
|
||||
|
||||
// Collect front matter fields: explicit `name`/`agent` params + arbitrary `front_matter` object.
|
||||
// Values are passed as serde_json::Value so native booleans, numbers, and arrays are
|
||||
// preserved and encoded correctly as unquoted YAML by update_story_in_file.
|
||||
let mut front_matter: HashMap<String, Value> = HashMap::new();
|
||||
// Explicit top-level args map onto typed CRDT registers directly (story 929:
|
||||
// no YAML front-matter writes). The `front_matter` object is the legacy
|
||||
// escape hatch; every known key is recognised and routed below, and any
|
||||
// unknown key is rejected loudly rather than silently flushed to disk.
|
||||
if let Some(name) = args.get("name").and_then(|v| v.as_str()) {
|
||||
front_matter.insert("name".to_string(), Value::String(name.to_string()));
|
||||
crate::crdt_state::set_name(story_id, Some(name));
|
||||
}
|
||||
if let Some(agent) = args.get("agent").and_then(|v| v.as_str()) {
|
||||
front_matter.insert("agent".to_string(), Value::String(agent.to_string()));
|
||||
crate::crdt_state::set_agent(story_id, Some(agent));
|
||||
}
|
||||
if let Some(epic) = args.get("epic").and_then(|v| v.as_str()) {
|
||||
front_matter.insert("epic".to_string(), Value::String(epic.to_string()));
|
||||
crate::crdt_state::set_epic(story_id, Some(epic).filter(|s| !s.is_empty()));
|
||||
}
|
||||
|
||||
if let Some(obj) = args.get("front_matter").and_then(|v| v.as_object()) {
|
||||
for (k, v) in obj {
|
||||
front_matter.insert(k.clone(), v.clone());
|
||||
for (key, value) in obj {
|
||||
match key.as_str() {
|
||||
"acceptance_criteria" => {
|
||||
return Err(
|
||||
"'acceptance_criteria' is a reserved field managed via the story body \
|
||||
(use add_criterion / remove_criterion / edit_criterion instead)."
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
"name" => {
|
||||
let s = value.as_str().filter(|s| !s.is_empty());
|
||||
crate::crdt_state::set_name(story_id, s);
|
||||
}
|
||||
"agent" => {
|
||||
let s = value.as_str().filter(|s| !s.is_empty());
|
||||
crate::crdt_state::set_agent(story_id, s);
|
||||
}
|
||||
"qa" => {
|
||||
let mode = value
|
||||
.as_str()
|
||||
.and_then(crate::io::story_metadata::QaMode::from_str);
|
||||
crate::crdt_state::set_qa_mode(story_id, mode);
|
||||
}
|
||||
"epic" => {
|
||||
let s = value.as_str().filter(|s| !s.is_empty());
|
||||
crate::crdt_state::set_epic(story_id, s);
|
||||
}
|
||||
"type" => {
|
||||
let s = value.as_str().filter(|s| !s.is_empty());
|
||||
crate::crdt_state::set_item_type(story_id, s);
|
||||
}
|
||||
"depends_on" => {
|
||||
if let Some(arr) = value.as_array() {
|
||||
let nums: Vec<u32> = arr
|
||||
.iter()
|
||||
.filter_map(|v| v.as_u64().map(|n| n as u32))
|
||||
.collect();
|
||||
crate::crdt_state::set_depends_on(story_id, &nums);
|
||||
} else if value.is_null() {
|
||||
crate::crdt_state::set_depends_on(story_id, &[]);
|
||||
}
|
||||
}
|
||||
"blocked" => {
|
||||
if let Some(b) = value.as_bool() {
|
||||
crate::crdt_state::set_blocked(story_id, b);
|
||||
} else if value.as_str() == Some("true") {
|
||||
crate::crdt_state::set_blocked(story_id, true);
|
||||
} else if value.as_str() == Some("false") {
|
||||
crate::crdt_state::set_blocked(story_id, false);
|
||||
}
|
||||
}
|
||||
"review_hold" => {
|
||||
if let Some(b) = value.as_bool() {
|
||||
crate::crdt_state::set_review_hold(story_id, b);
|
||||
} else if value.as_str() == Some("true") {
|
||||
crate::crdt_state::set_review_hold(story_id, true);
|
||||
} else if value.as_str() == Some("false") {
|
||||
crate::crdt_state::set_review_hold(story_id, false);
|
||||
}
|
||||
}
|
||||
"retry_count" => {
|
||||
let n = value
|
||||
.as_i64()
|
||||
.or_else(|| value.as_str().and_then(|s| s.parse().ok()));
|
||||
if let Some(n) = n {
|
||||
crate::crdt_state::set_retry_count(story_id, n);
|
||||
}
|
||||
}
|
||||
"mergemaster_attempted" => {
|
||||
if let Some(b) = value.as_bool() {
|
||||
crate::crdt_state::set_mergemaster_attempted(story_id, b);
|
||||
}
|
||||
}
|
||||
other => {
|
||||
return Err(format!(
|
||||
"Unknown front_matter field '{other}'. Story 929 removed the generic \
|
||||
YAML pass-through; supported keys: name, agent, qa, epic, type, \
|
||||
depends_on, blocked, review_hold, retry_count, mergemaster_attempted."
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// Intercept `qa` field — route through the typed CRDT register instead of YAML.
|
||||
if let Some(qa_val) = front_matter.remove("qa") {
|
||||
let mode = qa_val
|
||||
.as_str()
|
||||
.and_then(crate::io::story_metadata::QaMode::from_str);
|
||||
crate::crdt_state::set_qa_mode(story_id, mode);
|
||||
}
|
||||
|
||||
// Story 933: intercept `epic` and `type` fields — route to typed CRDT
|
||||
// registers so the auto-assigner / epic-rollup tools see the change.
|
||||
if let Some(epic_val) = front_matter.remove("epic") {
|
||||
let epic_id = epic_val.as_str().filter(|s| !s.is_empty());
|
||||
crate::crdt_state::set_epic(story_id, epic_id);
|
||||
}
|
||||
if let Some(type_val) = front_matter.remove("type") {
|
||||
let item_type = type_val.as_str().filter(|s| !s.is_empty());
|
||||
crate::crdt_state::set_item_type(story_id, item_type);
|
||||
}
|
||||
|
||||
// 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()
|
||||
.filter_map(|v| v.as_u64().map(|n| n as u32))
|
||||
.collect();
|
||||
crate::crdt_state::set_depends_on(story_id, &dep_nums);
|
||||
} else if deps_val.is_null() {
|
||||
crate::crdt_state::set_depends_on(story_id, &[]);
|
||||
}
|
||||
}
|
||||
|
||||
let front_matter_opt = if front_matter.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(&front_matter)
|
||||
};
|
||||
|
||||
// Capture the agent value before moving front_matter into the file writer,
|
||||
// so we can mirror it into the CRDT register below.
|
||||
let agent_for_crdt = args
|
||||
.get("agent")
|
||||
.and_then(|v| v.as_str())
|
||||
.or_else(|| {
|
||||
args.get("front_matter")
|
||||
.and_then(|v| v.as_object())
|
||||
.and_then(|o| o.get("agent"))
|
||||
.and_then(|v| v.as_str())
|
||||
})
|
||||
.map(str::to_string);
|
||||
|
||||
let root = ctx.state.get_project_root()?;
|
||||
|
||||
// Only call update_story_in_file when there is something left to write.
|
||||
if user_story.is_some() || description.is_some() || front_matter_opt.is_some() {
|
||||
update_story_in_file(&root, story_id, user_story, description, front_matter_opt)?;
|
||||
}
|
||||
|
||||
// Mirror the agent assignment into the CRDT register so the in-memory
|
||||
// pipeline state stays consistent with the front-matter.
|
||||
if let Some(ref a) = agent_for_crdt {
|
||||
crate::crdt_state::set_agent(story_id, Some(a));
|
||||
// Only call update_story_in_file when there is body content to update.
|
||||
if user_story.is_some() || description.is_some() {
|
||||
update_story_in_file(&root, story_id, user_story, description)?;
|
||||
}
|
||||
|
||||
// Bug 503: warn if any depends_on in the (now updated) story points at an archived story.
|
||||
@@ -138,214 +159,14 @@ pub(crate) fn tool_unblock_story(args: &Value, ctx: &AppContext) -> Result<Strin
|
||||
format!("Invalid story_id format: '{story_id}'. Expected a numeric ID (e.g. '42').")
|
||||
})?;
|
||||
|
||||
Ok(crate::chat::commands::unblock::unblock_by_number(
|
||||
&root,
|
||||
story_number,
|
||||
))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::http::test_helpers::test_ctx;
|
||||
use serde_json::json;
|
||||
use std::fs;
|
||||
|
||||
fn setup_story_for_update(dir: &std::path::Path, story_id: &str, content: &str) {
|
||||
let current = dir.join(".huskies/work/2_current");
|
||||
fs::create_dir_all(¤t).unwrap();
|
||||
fs::write(current.join(format!("{story_id}.md")), content).unwrap();
|
||||
crate::db::ensure_content_store();
|
||||
crate::db::write_item_with_content(
|
||||
story_id,
|
||||
"2_current",
|
||||
content,
|
||||
crate::db::ItemMeta::from_yaml(content),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_update_story_front_matter_json_bool_written_unquoted() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
setup_story_for_update(
|
||||
tmp.path(),
|
||||
"504_bool_test",
|
||||
"---\nname: Bool Test\n---\n\nNo sections.\n",
|
||||
);
|
||||
let ctx = test_ctx(tmp.path());
|
||||
|
||||
let result = tool_update_story(
|
||||
&json!({"story_id": "504_bool_test", "front_matter": {"blocked": false}}),
|
||||
&ctx,
|
||||
);
|
||||
assert!(result.is_ok(), "Expected ok: {result:?}");
|
||||
|
||||
let content = crate::db::read_content("504_bool_test").unwrap();
|
||||
assert!(
|
||||
content.contains("blocked: false"),
|
||||
"bool should be unquoted: {content}"
|
||||
);
|
||||
assert!(
|
||||
!content.contains("blocked: \"false\""),
|
||||
"bool must not be quoted: {content}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_update_story_front_matter_json_number_written_unquoted() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
setup_story_for_update(
|
||||
tmp.path(),
|
||||
"504_num_test",
|
||||
"---\nname: Num Test\n---\n\nNo sections.\n",
|
||||
);
|
||||
let ctx = test_ctx(tmp.path());
|
||||
|
||||
let result = tool_update_story(
|
||||
&json!({"story_id": "504_num_test", "front_matter": {"retry_count": 3}}),
|
||||
&ctx,
|
||||
);
|
||||
assert!(result.is_ok(), "Expected ok: {result:?}");
|
||||
|
||||
let content = crate::db::read_content("504_num_test").unwrap();
|
||||
assert!(
|
||||
content.contains("retry_count: 3"),
|
||||
"number should be unquoted: {content}"
|
||||
);
|
||||
assert!(
|
||||
!content.contains("retry_count: \"3\""),
|
||||
"number must not be quoted: {content}"
|
||||
);
|
||||
}
|
||||
|
||||
/// AC4 regression: set [1,2,3] → clear [] → replace [4,5] — CRDT reflects
|
||||
/// each write, and replace never appends.
|
||||
#[test]
|
||||
fn tool_update_story_depends_on_set_clear_replace() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
crate::crdt_state::init_for_test();
|
||||
setup_story_for_update(
|
||||
tmp.path(),
|
||||
"888_deps_scr",
|
||||
"---\nname: Deps SCR\n---\n\nNo sections.\n",
|
||||
);
|
||||
let ctx = test_ctx(tmp.path());
|
||||
|
||||
// Set to [1, 2, 3].
|
||||
let r = tool_update_story(
|
||||
&json!({"story_id": "888_deps_scr", "front_matter": {"depends_on": [1, 2, 3]}}),
|
||||
&ctx,
|
||||
);
|
||||
assert!(r.is_ok(), "set [1,2,3] should succeed: {r:?}");
|
||||
let view = crate::crdt_state::read_item("888_deps_scr").expect("CRDT must have story");
|
||||
assert_eq!(
|
||||
view.depends_on(),
|
||||
&[1, 2, 3],
|
||||
"CRDT should hold [1,2,3] after set"
|
||||
);
|
||||
|
||||
// Clear to [].
|
||||
let r = tool_update_story(
|
||||
&json!({"story_id": "888_deps_scr", "front_matter": {"depends_on": []}}),
|
||||
&ctx,
|
||||
);
|
||||
assert!(r.is_ok(), "clear [] should succeed: {r:?}");
|
||||
let view = crate::crdt_state::read_item("888_deps_scr").expect("CRDT must have story");
|
||||
assert!(
|
||||
view.depends_on().is_empty(),
|
||||
"CRDT should be empty after clearing to []"
|
||||
);
|
||||
|
||||
// Replace with [4, 5] — must not append to previous [1,2,3].
|
||||
let r = tool_update_story(
|
||||
&json!({"story_id": "888_deps_scr", "front_matter": {"depends_on": [4, 5]}}),
|
||||
&ctx,
|
||||
);
|
||||
assert!(r.is_ok(), "replace [4,5] should succeed: {r:?}");
|
||||
let view = crate::crdt_state::read_item("888_deps_scr").expect("CRDT must have story");
|
||||
assert_eq!(
|
||||
view.depends_on(),
|
||||
&[4, 5],
|
||||
"CRDT should hold exactly [4,5] after replace (not [1,2,3,4,5])"
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression: clearing depends_on must survive a subsequent update to another
|
||||
/// field. Before the fix, write_story_content would restore the old YAML
|
||||
/// depends_on value into the CRDT register, overwriting the clear.
|
||||
#[test]
|
||||
fn tool_update_story_clear_depends_on_survives_subsequent_update() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
crate::crdt_state::init_for_test();
|
||||
// Story created WITH depends_on in YAML so write_story_content would
|
||||
// previously restore it.
|
||||
setup_story_for_update(
|
||||
tmp.path(),
|
||||
"888_deps_persist",
|
||||
"---\nname: Deps Persist\ndepends_on: [100, 200]\n---\n\nNo sections.\n",
|
||||
);
|
||||
let ctx = test_ctx(tmp.path());
|
||||
|
||||
// Seed CRDT with the YAML deps (simulates the initial write path).
|
||||
crate::crdt_state::set_depends_on("888_deps_persist", &[100, 200]);
|
||||
|
||||
// Clear deps via update_story.
|
||||
let r = tool_update_story(
|
||||
&json!({"story_id": "888_deps_persist", "front_matter": {"depends_on": []}}),
|
||||
&ctx,
|
||||
);
|
||||
assert!(r.is_ok(), "clear should succeed: {r:?}");
|
||||
let view = crate::crdt_state::read_item("888_deps_persist").expect("CRDT must have story");
|
||||
assert!(
|
||||
view.depends_on().is_empty(),
|
||||
"CRDT should be empty after clear"
|
||||
);
|
||||
|
||||
// Now update a different field — this triggers write_story_content with
|
||||
// the stale YAML (which still has depends_on: [100, 200]).
|
||||
let r = tool_update_story(
|
||||
&json!({"story_id": "888_deps_persist", "name": "Deps Persist Updated"}),
|
||||
&ctx,
|
||||
);
|
||||
assert!(r.is_ok(), "subsequent name update should succeed: {r:?}");
|
||||
|
||||
// The CRDT must still be empty — the YAML value must not have been restored.
|
||||
let view = crate::crdt_state::read_item("888_deps_persist").expect("CRDT must have story");
|
||||
assert!(
|
||||
view.depends_on().is_empty(),
|
||||
"CRDT depends_on must remain empty after unrelated update (write_story_content must not restore YAML value)"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_update_story_depends_on_routes_to_crdt_not_yaml() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
setup_story_for_update(
|
||||
tmp.path(),
|
||||
"504_arr_test",
|
||||
"---\nname: Array Test\n---\n\nNo sections.\n",
|
||||
);
|
||||
let ctx = test_ctx(tmp.path());
|
||||
|
||||
let result = tool_update_story(
|
||||
&json!({"story_id": "504_arr_test", "front_matter": {"depends_on": [490, 491]}}),
|
||||
&ctx,
|
||||
);
|
||||
assert!(result.is_ok(), "Expected ok: {result:?}");
|
||||
|
||||
// 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(),
|
||||
&[490, 491],
|
||||
"CRDT register should hold [490, 491]: {view:?}"
|
||||
);
|
||||
|
||||
// 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}"
|
||||
);
|
||||
let result = crate::chat::commands::unblock::unblock_by_number(&root, story_number);
|
||||
if result.contains("not blocked")
|
||||
|| result.contains("not found")
|
||||
|| result.contains("Error")
|
||||
|| result.contains("error")
|
||||
{
|
||||
Err(result)
|
||||
} else {
|
||||
Ok(result)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user