huskies: merge 961

server/src/db/content_store.rs

@@ -7,6 +7,44 @@
 use std::collections::HashMap;
 use std::sync::{Mutex, OnceLock};
 
+/// Typed key for the in-memory content store.
+///
+/// Each variant maps to a distinct raw key namespace so that content written
+/// under one variant is never visible under another — no raw `format!()`
+/// key construction is needed at call sites outside `db/`.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum ContentKey<'a> {
+    /// Main markdown body of a work item (story, bug, spike, refactor, epic).
+    Story(&'a str),
+    /// Gate failure output from the last failed agent run.
+    GateOutput(&'a str),
+    /// Consecutive abort-respawn counter.
+    AbortRespawnCount(&'a str),
+    /// Mergemaster re-spawn counter.
+    MergeMasterSpawnCount(&'a str),
+    /// Evidence that `run_tests` passed during an agent session.
+    RunTestsOk(&'a str),
+    /// Flag indicating a commit-recovery respawn is in progress.
+    CommitRecoveryPending(&'a str),
+}
+
+impl<'a> ContentKey<'a> {
+    /// Lower this typed key to the underlying storage string used by the
+    /// CRDT content store (`{story_id}` for the base story, `{story_id}:<kind>`
+    /// for per-purpose sub-keys). Internal — callers should use the typed
+    /// `read_content` / `write_content` wrappers instead of touching strings.
+    pub(super) fn as_raw_key(&self) -> String {
+        match self {
+            ContentKey::Story(id) => id.to_string(),
+            ContentKey::GateOutput(id) => format!("{id}:gate_output"),
+            ContentKey::AbortRespawnCount(id) => format!("{id}:abort_respawn_count"),
+            ContentKey::MergeMasterSpawnCount(id) => format!("{id}:mergemaster_spawn_count"),
+            ContentKey::RunTestsOk(id) => format!("{id}:run_tests_ok"),
+            ContentKey::CommitRecoveryPending(id) => format!("{id}:commit_recovery_pending"),
+        }
+    }
+}
+
 static CONTENT_STORE: OnceLock<Mutex<HashMap<String, String>>> = OnceLock::new();
 
 #[cfg(test)]
@@ -41,30 +79,28 @@ pub(super) fn get_content_store() -> Option<&'static Mutex<HashMap<String, Strin
     }
 }
 
-/// Read the full markdown content of a story from the in-memory store.
-pub fn read_content(story_id: &str) -> Option<String> {
+/// Read content from the in-memory store by typed key.
+pub fn read_content(key: ContentKey<'_>) -> Option<String> {
     let store = get_content_store()?;
     let map = store.lock().ok()?;
-    map.get(story_id).cloned()
+    map.get(&key.as_raw_key()).cloned()
 }
 
-/// Write (or overwrite) the full markdown content of a story.
-///
-/// Updates the in-memory store immediately.
-pub fn write_content(story_id: &str, content: &str) {
+/// Write (or overwrite) content in the in-memory store by typed key.
+pub fn write_content(key: ContentKey<'_>, content: &str) {
     if let Some(store) = get_content_store()
         && let Ok(mut map) = store.lock()
     {
-        map.insert(story_id.to_string(), content.to_string());
+        map.insert(key.as_raw_key(), content.to_string());
     }
 }
 
-/// Remove a story's content from the in-memory store.
-pub fn delete_content(story_id: &str) {
+/// Remove an entry from the in-memory store by typed key.
+pub fn delete_content(key: ContentKey<'_>) {
     if let Some(store) = get_content_store()
         && let Ok(mut map) = store.lock()
     {
-        map.remove(story_id);
+        map.remove(&key.as_raw_key());
     }
 }
 
@@ -103,3 +139,52 @@ pub fn all_content_ids() -> Vec<String> {
 pub(super) fn init_content_store(map: HashMap<String, String>) {
     let _ = CONTENT_STORE.set(Mutex::new(map));
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// AC 2 regression: writing under `ContentKey::Story` is not visible under
+    /// `ContentKey::GateOutput` (and vice versa).  The typed key namespace, not
+    /// runtime substring matching, enforces the separation.
+    #[test]
+    fn wrong_key_variant_is_isolated() {
+        ensure_content_store();
+        let id = "9961_regression_key_isolation";
+
+        write_content(ContentKey::Story(id), "story body");
+
+        // A different variant for the same base id must not surface the story body.
+        assert!(
+            read_content(ContentKey::GateOutput(id)).is_none(),
+            "GateOutput key must not read Story content"
+        );
+        assert!(
+            read_content(ContentKey::RunTestsOk(id)).is_none(),
+            "RunTestsOk key must not read Story content"
+        );
+
+        // The Story variant itself must still return the content.
+        assert_eq!(
+            read_content(ContentKey::Story(id)).as_deref(),
+            Some("story body")
+        );
+
+        // Write under a second variant; reading under Story must still return
+        // the original body, not the gate output.
+        write_content(ContentKey::GateOutput(id), "gate failure text");
+        assert_eq!(
+            read_content(ContentKey::Story(id)).as_deref(),
+            Some("story body"),
+            "Story key must not be polluted by GateOutput write"
+        );
+        assert_eq!(
+            read_content(ContentKey::GateOutput(id)).as_deref(),
+            Some("gate failure text")
+        );
+
+        // Cleanup.
+        delete_content(ContentKey::Story(id));
+        delete_content(ContentKey::GateOutput(id));
+    }
+}

server/src/db/mod.rs

@@ -20,7 +20,7 @@ pub mod ops;
 /// Background shadow-write task — persists pipeline items to SQLite asynchronously.
 pub mod shadow_write;
 
-pub use content_store::{all_content_ids, delete_content, read_content, write_content};
+pub use content_store::{ContentKey, all_content_ids, delete_content, read_content, write_content};
 pub use ops::{ItemMeta, delete_item, move_item_stage, next_item_number, write_item_with_content};
 pub use shadow_write::init;
 
@@ -217,17 +217,23 @@ mod tests {
         let markdown = "---\nname: Content Test\n---\n# Story\n";
 
         // Write.
-        write_content(story_id, markdown);
-        assert_eq!(read_content(story_id).as_deref(), Some(markdown));
+        write_content(ContentKey::Story(story_id), markdown);
+        assert_eq!(
+            read_content(ContentKey::Story(story_id)).as_deref(),
+            Some(markdown)
+        );
 
         // Overwrite.
         let updated = "---\nname: Updated\n---\n# Updated Story\n";
-        write_content(story_id, updated);
-        assert_eq!(read_content(story_id).as_deref(), Some(updated));
+        write_content(ContentKey::Story(story_id), updated);
+        assert_eq!(
+            read_content(ContentKey::Story(story_id)).as_deref(),
+            Some(updated)
+        );
 
         // Delete.
-        delete_content(story_id);
-        assert!(read_content(story_id).is_none());
+        delete_content(ContentKey::Story(story_id));
+        assert!(read_content(ContentKey::Story(story_id)).is_none());
     }
 
     #[test]
@@ -327,7 +333,10 @@ mod tests {
         assert_eq!(view.depends_on(), &[100, 200]);
 
         // Content is stored verbatim (no parsing, no rewrite).
-        assert_eq!(read_content(story_id).as_deref(), Some(content));
+        assert_eq!(
+            read_content(ContentKey::Story(story_id)).as_deref(),
+            Some(content)
+        );
     }
 
     /// Story 864: passing `ItemMeta::default()` against a content blob that
@@ -371,7 +380,7 @@ mod tests {
             },
         );
 
-        let read_back = read_content(story_id).expect("content present");
+        let read_back = read_content(ContentKey::Story(story_id)).expect("content present");
         assert_eq!(read_back, body, "plain content must be readable as-is");
         assert!(
             !read_back.trim_start().starts_with("---"),
@@ -402,7 +411,7 @@ mod tests {
             None,
         );
         write_content(
-            story_id,
+            ContentKey::Story(story_id),
             "---\nname: Retry reset test\nretry_count: 3\n---\n",
         );
         let typed = crate::pipeline_state::read_typed(story_id)

server/src/db/ops.rs

@@ -4,7 +4,7 @@
 //! content store, the CRDT (source of truth for metadata), and the SQLite
 //! shadow table (via the background channel).
 use super::content_store::{
-    all_content_ids, delete_content, ensure_content_store, read_content, write_content,
+    ContentKey, all_content_ids, delete_content, ensure_content_store, read_content, write_content,
 };
 use super::shadow_write::{PIPELINE_DB, PipelineWriteMsg};
 
@@ -74,7 +74,7 @@ pub fn write_item_with_content(story_id: &str, stage: &str, content: &str, meta:
 
     // Update in-memory content store.
     ensure_content_store();
-    write_content(story_id, content);
+    write_content(ContentKey::Story(story_id), content);
 
     // Primary: CRDT ops.
     let stage = normalise_stage_str(stage);
@@ -123,12 +123,12 @@ pub fn move_item_stage(
     new_stage: &str,
     content_transform: Option<&dyn Fn(&str) -> String>,
 ) {
-    let current_content = read_content(story_id);
+    let current_content = read_content(ContentKey::Story(story_id));
 
     let content = match (&current_content, content_transform) {
         (Some(c), Some(transform)) => {
             let new_content = transform(c);
-            write_content(story_id, &new_content);
+            write_content(ContentKey::Story(story_id), &new_content);
             Some(new_content)
         }
         (Some(c), None) => Some(c.clone()),
@@ -192,7 +192,7 @@ pub fn move_item_stage(
 
 /// Delete a story from the shadow table (fire-and-forget).
 pub fn delete_item(story_id: &str) {
-    delete_content(story_id);
+    delete_content(ContentKey::Story(story_id));
 
     if let Some(db) = PIPELINE_DB.get() {
         // Reuse the channel with a special "deleted" stage marker.