huskies: merge 512_story_migrate_chat_commands_from_filesystem_lookup_to_crdt_db

2026-04-09 23:00:01 +00:00
parent c324452b38
commit 0de9200d48
9 changed files with 318 additions and 452 deletions
@@ -0,0 +1,223 @@
+//! 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):
+//!
+//! 1. **CRDT** — authoritative in-memory state, works even when the
+//!    filesystem shadow does not exist.
+//! 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:
+/// - `story_id` — full stem, e.g. `"503_story_some_feature"`
+/// - `stage_dir` — pipeline directory name, e.g. `"4_merge"`
+/// - `path` — canonical path under `.huskies/work/`; may not exist on disk
+///   for CRDT-only stories
+/// - `content` — markdown body from the content store when available;
+///   for the filesystem fallback this is the file contents read from disk;
+///   `None` only when the story was found in CRDT but has no content entry
+///   (callers may fall back to `fs::read_to_string(&path)` in that case)
+///
+/// Returns `None` when no matching work item is found by any mechanism.
+pub(crate) fn find_story_by_number(
+    project_root: &Path,
+    number: &str,
+) -> Option<(String, String, PathBuf, Option<String>)> {
+    // ── 1. CRDT (authoritative) ──────────────────────────────────────────
+    // `read_all_items` returns None only when the CRDT layer is not yet
+    // initialised (e.g. in unit tests or very early startup).
+    if let Some(items) = crate::crdt_state::read_all_items() {
+        for item in items {
+            if item.story_id.split('_').next().unwrap_or("") == number {
+                let path = project_root
+                    .join(".huskies")
+                    .join("work")
+                    .join(&item.stage)
+                    .join(format!("{}.md", item.story_id));
+                let content = crate::db::read_content(&item.story_id);
+                return Some((item.story_id, item.stage, path, content));
+            }
+        }
+    }
+
+    // ── 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).
+    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() {
+            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,
+                        ));
+                    }
+                }
+            }
+        }
+    }
+
+    None
+}
+
+// ---------------------------------------------------------------------------
+// Tests
+// ---------------------------------------------------------------------------
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::chat::test_helpers::write_story_file;
+
+    #[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() {
+        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",
+        );
+        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");
+        assert!(
+            path.ends_with("1_backlog/42_story_some_feature.md"),
+            "unexpected path: {path:?}"
+        );
+        assert!(
+            content.as_deref().unwrap_or("").contains("Some Feature"),
+            "content should include story text"
+        );
+    }
+
+    #[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() {
+        let tmp = tempfile::TempDir::new().unwrap();
+        write_story_file(
+            tmp.path(),
+            "2_current",
+            "7_bug_crash_on_login.md",
+            "---\nname: Crash on login\n---\n",
+        );
+        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".
+        write_story_file(
+            tmp.path(),
+            "1_backlog",
+            "1_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");
+    }
+
+    #[test]
+    fn returned_path_points_into_project_root() {
+        let tmp = tempfile::TempDir::new().unwrap();
+        write_story_file(
+            tmp.path(),
+            "4_merge",
+            "503_story_migration.md",
+            "---\nname: Migration\n---\n",
+        );
+        let (_, _, path, _) =
+            find_story_by_number(tmp.path(), "503").expect("should find story 503");
+        assert!(
+            path.starts_with(tmp.path()),
+            "path should be under the project root"
+        );
+        assert!(path.exists(), "filesystem-fallback path should exist on disk");
+    }
+}