From 6e704a33b7caf5e2ddde4de0653e79580540fb8a Mon Sep 17 00:00:00 2001 From: Timmy Date: Tue, 12 May 2026 19:14:54 +0100 Subject: [PATCH] =?UTF-8?q?wip(929):=20stage=205=20=E2=80=94=20drop=20FS-b?= =?UTF-8?q?ased=20dep=20checks=20and=20qa-mode=20parser=20from=20io/story?= =?UTF-8?q?=5Fmetadata?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate the last three callers of the FS-scanning dependency helpers to the CRDT-direct equivalents and delete the dead helpers: - agents/pool/auto_assign/story_checks.rs: has_unmet_dependencies and check_archived_dependencies now wrap check_unmet_deps_crdt / check_archived_deps_crdt directly. Tests rewritten to seed the CRDT. - http/mcp/story_tools/story/update.rs: bug-503 archived-dep warning now reads from CRDT instead of scanning 6_archived. - agents/pool/pipeline/advance/helpers.rs: resolve_qa_mode_from_store is CRDT-only (the FS fallback for content-store-empty stories is gone). - io/story_metadata/parser.rs: resolve_qa_mode_from_content removed. - io/story_metadata/deps.rs: check_unmet_deps and dep_is_done deleted, along with the unused check_unmet_deps_from_list helper. - io/story_metadata/mod.rs: re-exports trimmed accordingly. check_archived_deps_from_list survives because story-creation still calls it before the CRDT entry exists (used from story_tools/story/create.rs). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../agents/pool/auto_assign/story_checks.rs | 194 ++++++++++-------- .../agents/pool/pipeline/advance/helpers.rs | 25 +-- server/src/http/mcp/story_tools/bug.rs | 4 +- server/src/http/mcp/story_tools/criteria.rs | 4 +- server/src/http/mcp/story_tools/refactor.rs | 4 +- server/src/http/mcp/story_tools/spike.rs | 4 +- .../src/http/mcp/story_tools/story/update.rs | 9 +- server/src/io/story_metadata/deps.rs | 106 ---------- server/src/io/story_metadata/mod.rs | 6 +- server/src/io/story_metadata/parser.rs | 10 - 10 files changed, 128 insertions(+), 238 deletions(-) diff --git a/server/src/agents/pool/auto_assign/story_checks.rs b/server/src/agents/pool/auto_assign/story_checks.rs index 3f3ca24a..b6ef53ee 100644 --- a/server/src/agents/pool/auto_assign/story_checks.rs +++ b/server/src/agents/pool/auto_assign/story_checks.rs @@ -98,49 +98,24 @@ pub(super) fn has_mergemaster_attempted( } /// Return `true` if the story has any `depends_on` entries that are not yet in -/// `5_done` or `6_archived`. -/// -/// Reads dependency state from the CRDT document first. Falls back to the -/// filesystem when the CRDT layer is not initialised. -pub(super) fn has_unmet_dependencies(project_root: &Path, stage_dir: &str, story_id: &str) -> bool { - // Prefer CRDT-based check. - let crdt_deps = crate::crdt_state::check_unmet_deps_crdt(story_id); - if !crdt_deps.is_empty() { - return true; - } - // If the CRDT had the item and returned empty deps, it means all are met. - if crate::pipeline_state::read_typed(story_id) - .ok() - .flatten() - .is_some() - { - return false; - } - // Fallback: filesystem check (CRDT not initialised or item not yet in CRDT). - !crate::io::story_metadata::check_unmet_deps(project_root, stage_dir, story_id).is_empty() +/// `5_done` or `6_archived`. Reads dependency state from the CRDT (story 929). +pub(super) fn has_unmet_dependencies( + _project_root: &Path, + _stage_dir: &str, + story_id: &str, +) -> bool { + !crate::crdt_state::check_unmet_deps_crdt(story_id).is_empty() } /// Return the list of dependency story numbers that are in `6_archived` (satisfied -/// via archive rather than via a clean `5_done` completion). -/// -/// Used to emit a warning when backlog promotion fires because one or more deps were -/// archived. Returns an empty `Vec` when no deps are archived. Reads from CRDT -/// first; falls back to filesystem when CRDT is not initialised. +/// via archive rather than via a clean `5_done` completion). Reads from the CRDT +/// (story 929). pub(super) fn check_archived_dependencies( - project_root: &Path, - stage_dir: &str, + _project_root: &Path, + _stage_dir: &str, story_id: &str, ) -> Vec { - // Prefer CRDT-based check when the item is known to CRDT. - if crate::pipeline_state::read_typed(story_id) - .ok() - .flatten() - .is_some() - { - return crate::crdt_state::check_archived_deps_crdt(story_id); - } - // Fallback: filesystem. - crate::io::story_metadata::check_archived_deps(project_root, stage_dir, story_id) + crate::crdt_state::check_archived_deps_crdt(story_id) } /// Return `true` if the story is in the `Frozen` pipeline stage. @@ -265,14 +240,20 @@ mod tests { #[test] fn has_unmet_dependencies_returns_true_when_dep_not_done() { + crate::crdt_state::init_for_test(); let tmp = tempfile::tempdir().unwrap(); - let current = tmp.path().join(".huskies/work/2_current"); - std::fs::create_dir_all(¤t).unwrap(); - std::fs::write( - current.join("10_story_blocked.md"), - "---\nname: Blocked\ndepends_on: [999]\n---\n", - ) - .unwrap(); + crate::crdt_state::write_item( + "10_story_blocked", + "2_current", + Some("Blocked"), + None, + None, + None, + Some("[999]"), + None, + None, + None, + ); assert!(has_unmet_dependencies( tmp.path(), "2_current", @@ -282,17 +263,32 @@ mod tests { #[test] fn has_unmet_dependencies_returns_false_when_dep_done() { + crate::crdt_state::init_for_test(); let tmp = tempfile::tempdir().unwrap(); - let current = tmp.path().join(".huskies/work/2_current"); - let done = tmp.path().join(".huskies/work/5_done"); - std::fs::create_dir_all(¤t).unwrap(); - std::fs::create_dir_all(&done).unwrap(); - std::fs::write(done.join("999_story_dep.md"), "---\nname: Dep\n---\n").unwrap(); - std::fs::write( - current.join("10_story_ok.md"), - "---\nname: Ok\ndepends_on: [999]\n---\n", - ) - .unwrap(); + crate::crdt_state::write_item( + "999_story_dep", + "5_done", + Some("Dep"), + None, + None, + None, + None, + None, + None, + None, + ); + crate::crdt_state::write_item( + "10_story_ok", + "2_current", + Some("Ok"), + None, + None, + None, + Some("[999]"), + None, + None, + None, + ); assert!(!has_unmet_dependencies( tmp.path(), "2_current", @@ -302,10 +298,20 @@ mod tests { #[test] fn has_unmet_dependencies_returns_false_when_no_deps() { + crate::crdt_state::init_for_test(); let tmp = tempfile::tempdir().unwrap(); - let current = tmp.path().join(".huskies/work/2_current"); - std::fs::create_dir_all(¤t).unwrap(); - std::fs::write(current.join("5_story_free.md"), "---\nname: Free\n---\n").unwrap(); + crate::crdt_state::write_item( + "5_story_free", + "2_current", + Some("Free"), + None, + None, + None, + None, + None, + None, + None, + ); assert!(!has_unmet_dependencies( tmp.path(), "2_current", @@ -318,21 +324,32 @@ mod tests { /// check_archived_dependencies returns dep IDs that are in 6_archived. #[test] fn check_archived_dependencies_returns_archived_ids() { + crate::crdt_state::init_for_test(); let tmp = tempfile::tempdir().unwrap(); - let backlog = tmp.path().join(".huskies/work/1_backlog"); - let archived = tmp.path().join(".huskies/work/6_archived"); - std::fs::create_dir_all(&backlog).unwrap(); - std::fs::create_dir_all(&archived).unwrap(); - std::fs::write( - archived.join("500_spike_crdt.md"), - "---\nname: CRDT Spike\n---\n", - ) - .unwrap(); - std::fs::write( - backlog.join("503_story_dependent.md"), - "---\nname: Dependent\ndepends_on: [500]\n---\n", - ) - .unwrap(); + crate::crdt_state::write_item( + "500_spike_crdt", + "6_archived", + Some("CRDT Spike"), + None, + None, + None, + None, + None, + None, + None, + ); + crate::crdt_state::write_item( + "503_story_dependent", + "1_backlog", + Some("Dependent"), + None, + None, + None, + Some("[500]"), + None, + None, + None, + ); let archived_deps = check_archived_dependencies(tmp.path(), "1_backlog", "503_story_dependent"); assert_eq!(archived_deps, vec![500]); @@ -341,17 +358,32 @@ mod tests { /// check_archived_dependencies returns empty when dep is in 5_done (not archived). #[test] fn check_archived_dependencies_empty_when_dep_in_done() { + crate::crdt_state::init_for_test(); let tmp = tempfile::tempdir().unwrap(); - let backlog = tmp.path().join(".huskies/work/1_backlog"); - let done = tmp.path().join(".huskies/work/5_done"); - std::fs::create_dir_all(&backlog).unwrap(); - std::fs::create_dir_all(&done).unwrap(); - std::fs::write(done.join("490_story_done.md"), "---\nname: Done\n---\n").unwrap(); - std::fs::write( - backlog.join("503_story_waiting.md"), - "---\nname: Waiting\ndepends_on: [490]\n---\n", - ) - .unwrap(); + crate::crdt_state::write_item( + "490_story_done", + "5_done", + Some("Done"), + None, + None, + None, + None, + None, + None, + None, + ); + crate::crdt_state::write_item( + "503_story_waiting", + "1_backlog", + Some("Waiting"), + None, + None, + None, + Some("[490]"), + None, + None, + None, + ); let archived_deps = check_archived_dependencies(tmp.path(), "1_backlog", "503_story_waiting"); assert!(archived_deps.is_empty()); diff --git a/server/src/agents/pool/pipeline/advance/helpers.rs b/server/src/agents/pool/pipeline/advance/helpers.rs index 05f0a4c9..39a24d90 100644 --- a/server/src/agents/pool/pipeline/advance/helpers.rs +++ b/server/src/agents/pool/pipeline/advance/helpers.rs @@ -52,29 +52,18 @@ pub(crate) fn spawn_pipeline_advance( /// Resolve QA mode for a story. /// -/// Checks the typed `qa_mode` CRDT register first. If the register holds a -/// recognised value, returns it immediately without touching the content store. -/// Otherwise reads the story content and falls back to YAML front-matter -/// parsing via [`crate::io::story_metadata::resolve_qa_mode_from_content`]. +/// Story 929: reads the typed `qa_mode` register; the legacy YAML fallback +/// is gone (the CRDT register is the only source). pub(super) fn resolve_qa_mode_from_store( _project_root: &Path, story_id: &str, default: crate::io::story_metadata::QaMode, ) -> crate::io::story_metadata::QaMode { - // CRDT register is the authoritative source; check it before the content store. - if let Some(view) = crate::crdt_state::read_item(story_id) - && let Some(s) = view.qa_mode() - && let Some(mode) = crate::io::story_metadata::QaMode::from_str(s) - { - return mode; - } - // Fall back to YAML front matter for backward compatibility. - if let Some(contents) = crate::db::read_content(story_id) { - return crate::io::story_metadata::resolve_qa_mode_from_content( - story_id, &contents, default, - ); - } - default + crate::crdt_state::read_item(story_id) + .and_then(|view| view.qa_mode().map(str::to_string)) + .as_deref() + .and_then(crate::io::story_metadata::QaMode::from_str) + .unwrap_or(default) } /// Write review_hold to the content store. diff --git a/server/src/http/mcp/story_tools/bug.rs b/server/src/http/mcp/story_tools/bug.rs index 601b61aa..31e68c93 100644 --- a/server/src/http/mcp/story_tools/bug.rs +++ b/server/src/http/mcp/story_tools/bug.rs @@ -13,9 +13,7 @@ use crate::http::workflow::{ list_refactor_files, load_pipeline_state, load_upcoming_stories, remove_criterion_from_file, update_story_in_file, validate_story_dirs, }; -use crate::io::story_metadata::{ - check_archived_deps, check_archived_deps_from_list, parse_unchecked_todos, -}; +use crate::io::story_metadata::{check_archived_deps_from_list, parse_unchecked_todos}; use crate::service::story::parse_test_cases; use crate::slog_warn; #[allow(unused_imports)] diff --git a/server/src/http/mcp/story_tools/criteria.rs b/server/src/http/mcp/story_tools/criteria.rs index b4199728..c88f8010 100644 --- a/server/src/http/mcp/story_tools/criteria.rs +++ b/server/src/http/mcp/story_tools/criteria.rs @@ -13,9 +13,7 @@ use crate::http::workflow::{ list_refactor_files, load_pipeline_state, load_upcoming_stories, remove_criterion_from_file, update_story_in_file, validate_story_dirs, }; -use crate::io::story_metadata::{ - check_archived_deps, check_archived_deps_from_list, parse_unchecked_todos, -}; +use crate::io::story_metadata::{check_archived_deps_from_list, parse_unchecked_todos}; use crate::service::story::parse_test_cases; use crate::slog_warn; #[allow(unused_imports)] diff --git a/server/src/http/mcp/story_tools/refactor.rs b/server/src/http/mcp/story_tools/refactor.rs index 84686348..55dd8ec8 100644 --- a/server/src/http/mcp/story_tools/refactor.rs +++ b/server/src/http/mcp/story_tools/refactor.rs @@ -13,9 +13,7 @@ use crate::http::workflow::{ list_refactor_files, load_pipeline_state, load_upcoming_stories, remove_criterion_from_file, update_story_in_file, validate_story_dirs, }; -use crate::io::story_metadata::{ - check_archived_deps, check_archived_deps_from_list, parse_unchecked_todos, -}; +use crate::io::story_metadata::{check_archived_deps_from_list, parse_unchecked_todos}; use crate::service::story::parse_test_cases; use crate::slog_warn; #[allow(unused_imports)] diff --git a/server/src/http/mcp/story_tools/spike.rs b/server/src/http/mcp/story_tools/spike.rs index 0e835351..a77539c4 100644 --- a/server/src/http/mcp/story_tools/spike.rs +++ b/server/src/http/mcp/story_tools/spike.rs @@ -13,9 +13,7 @@ use crate::http::workflow::{ list_refactor_files, load_pipeline_state, load_upcoming_stories, remove_criterion_from_file, update_story_in_file, validate_story_dirs, }; -use crate::io::story_metadata::{ - check_archived_deps, check_archived_deps_from_list, parse_unchecked_todos, -}; +use crate::io::story_metadata::{check_archived_deps_from_list, parse_unchecked_todos}; use crate::service::story::parse_test_cases; use crate::slog_warn; #[allow(unused_imports)] diff --git a/server/src/http/mcp/story_tools/story/update.rs b/server/src/http/mcp/story_tools/story/update.rs index 0a57b8e5..1cda668f 100644 --- a/server/src/http/mcp/story_tools/story/update.rs +++ b/server/src/http/mcp/story_tools/story/update.rs @@ -2,7 +2,6 @@ use crate::http::context::AppContext; use crate::http::workflow::update_story_in_file; -use crate::io::story_metadata::check_archived_deps; use crate::slog_warn; use serde_json::Value; use std::collections::HashMap; @@ -89,12 +88,8 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result bool { - let prefix = format!("{dep_number}_"); - let exact = dep_number.to_string(); - for stage in &["5_done", "6_archived"] { - let dir = project_root.join(".huskies").join("work").join(stage); - if let Ok(entries) = 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()) - && (stem == exact || stem.starts_with(&prefix)) - { - return true; - } - } - } - } - false -} - /// Return `true` if a story with the given numeric ID exists specifically in `6_archived`. fn dep_is_archived(project_root: &Path, dep_number: u32) -> bool { let prefix = format!("{dep_number}_"); @@ -56,19 +33,6 @@ fn dep_is_archived(project_root: &Path, dep_number: u32) -> bool { false } -/// Given an explicit list of dep numbers, return those that have NOT reached -/// `5_done` or `6_archived`. -/// -/// Used by callers that have the dep list in memory (e.g. story update at -/// promotion time) and want a filesystem fact rather than an in-memory CRDT -/// state which may be stale during transitions. -pub fn check_unmet_deps_from_list(project_root: &Path, deps: &[u32]) -> Vec { - deps.iter() - .copied() - .filter(|&dep| !dep_is_done(project_root, dep)) - .collect() -} - /// Given an explicit list of dep numbers, return those already in `6_archived`. /// /// Used at story-creation time when the dep list is known in memory (before @@ -81,80 +45,10 @@ pub fn check_archived_deps_from_list(project_root: &Path, deps: &[u32]) -> Vec/`. -/// -/// Reads the story's `depends_on` list from its YAML front matter and returns -/// the numeric deps still pending (not yet in `5_done` or `6_archived`). This -/// is the legacy API used by the auto-assigner when the CRDT layer is not yet -/// initialised; CRDT-aware callers should prefer `check_unmet_deps_crdt`. -pub fn check_unmet_deps(project_root: &Path, stage_dir: &str, story_id: &str) -> Vec { - let path = project_root - .join(".huskies") - .join("work") - .join(stage_dir) - .join(format!("{story_id}.md")); - let contents = match fs::read_to_string(&path) { - Ok(c) => c, - Err(_) => return Vec::new(), - }; - let deps = match crate::db::yaml_legacy::parse_front_matter(&contents) - .ok() - .and_then(|m| m.depends_on) - { - Some(d) => d, - None => return Vec::new(), - }; - check_unmet_deps_from_list(project_root, &deps) -} - -/// Filesystem-backed archived-dep check for a story file in `/`. -/// -/// Reads the story's `depends_on` list from its YAML front matter and returns -/// the numeric deps that satisfied via `6_archived` rather than `5_done`. -pub fn check_archived_deps(project_root: &Path, stage_dir: &str, story_id: &str) -> Vec { - let path = project_root - .join(".huskies") - .join("work") - .join(stage_dir) - .join(format!("{story_id}.md")); - let contents = match fs::read_to_string(&path) { - Ok(c) => c, - Err(_) => return Vec::new(), - }; - let deps = match crate::db::yaml_legacy::parse_front_matter(&contents) - .ok() - .and_then(|m| m.depends_on) - { - Some(d) => d, - None => return Vec::new(), - }; - check_archived_deps_from_list(project_root, &deps) -} - #[cfg(test)] mod tests { use super::*; - #[test] - fn dep_is_done_finds_story_in_archived() { - let tmp = tempfile::tempdir().unwrap(); - let archived = tmp.path().join(".huskies/work/6_archived"); - std::fs::create_dir_all(&archived).unwrap(); - std::fs::write(archived.join("100_story_old.md"), "# old\n").unwrap(); - assert!(dep_is_done(tmp.path(), 100)); - assert!(!dep_is_done(tmp.path(), 101)); - } - - #[test] - fn check_unmet_deps_from_list_returns_unmet_numbers() { - let tmp = tempfile::tempdir().unwrap(); - let done = tmp.path().join(".huskies/work/5_done"); - std::fs::create_dir_all(&done).unwrap(); - std::fs::write(done.join("477_story_dep.md"), "# dep\n").unwrap(); - let unmet = check_unmet_deps_from_list(tmp.path(), &[477, 478]); - assert_eq!(unmet, vec![478]); - } - #[test] fn check_archived_deps_from_list_returns_archived_ids() { let tmp = tempfile::tempdir().unwrap(); diff --git a/server/src/io/story_metadata/mod.rs b/server/src/io/story_metadata/mod.rs index 8243f31e..d9c0a7b0 100644 --- a/server/src/io/story_metadata/mod.rs +++ b/server/src/io/story_metadata/mod.rs @@ -10,8 +10,6 @@ mod deps; mod parser; mod types; -pub use deps::{check_archived_deps, check_archived_deps_from_list, check_unmet_deps}; -pub use parser::{ - is_story_frozen_in_store, parse_unchecked_todos, resolve_qa_mode, resolve_qa_mode_from_content, -}; +pub use deps::check_archived_deps_from_list; +pub use parser::{is_story_frozen_in_store, parse_unchecked_todos, resolve_qa_mode}; pub use types::QaMode; diff --git a/server/src/io/story_metadata/parser.rs b/server/src/io/story_metadata/parser.rs index 891d7eb5..204fdca9 100644 --- a/server/src/io/story_metadata/parser.rs +++ b/server/src/io/story_metadata/parser.rs @@ -29,16 +29,6 @@ pub fn resolve_qa_mode(story_id: &str, default: QaMode) -> QaMode { .unwrap_or(default) } -/// Resolve the effective QA mode by parsing legacy YAML front matter from a -/// markdown body. Used during one-time fallbacks when the CRDT register isn't -/// set; new code should always read `qa_mode` from the CRDT. -pub fn resolve_qa_mode_from_content(_story_id: &str, content: &str, default: QaMode) -> QaMode { - crate::db::yaml_legacy::parse_front_matter(content) - .ok() - .and_then(|m| m.qa) - .unwrap_or(default) -} - /// Return `true` if the story is in the `Frozen` pipeline stage. /// /// Checks the typed CRDT stage via `read_typed`. Used by the pipeline advance