huskies: merge 1111 bug Test isolation: init_for_test() and ensure_content_store() are once-per-thread, not once-per-test, polluting CRDT state across tests

server/src/crdt_state/state/mod.rs

@@ -122,49 +122,57 @@ pub(super) fn get_crdt() -> Option<&'static Mutex<CrdtState>> {
 /// This avoids the async SQLite setup from `init()`.  Ops are sent to a
 /// channel whose receiver is leaked (so nothing is persisted, but the channel
 /// stays open and `apply_and_persist` succeeds silently).
-/// Safe to call multiple times — subsequent calls are no-ops (thread-local).
+/// Always resets all thread-local state so each call produces a clean slate —
+/// no cross-test pollution when two tests share the same thread.
 #[cfg(test)]
 pub fn init_for_test() {
-    // Initialise thread-local CRDT for test isolation.
-    // Only creates a new CRDT if one isn't set yet on this thread;
-    // subsequent calls are no-ops (matching the old OnceLock semantics
-    // while keeping each thread isolated).
+    let keypair = make_keypair();
+    let crdt = BaseCrdt::<PipelineDoc>::new(&keypair);
+    let (persist_tx, rx) = mpsc::unbounded_channel();
+    // Leak the receiver so the channel stays open: apply_and_persist
+    // can then send without error, preventing [crdt_persist] WARNs
+    // from racing with other tests that watch the global log buffer.
+    std::mem::forget(rx);
+    let fresh = CrdtState {
+        crdt,
+        keypair,
+        index: HashMap::new(),
+        node_index: HashMap::new(),
+        token_index: HashMap::new(),
+        merge_job_index: HashMap::new(),
+        active_agent_index: HashMap::new(),
+        test_job_index: HashMap::new(),
+        agent_throttle_index: HashMap::new(),
+        gateway_project_index: HashMap::new(),
+        persist_tx,
+        lamport_floor: 0,
+        tombstones: HashSet::new(),
+    };
     CRDT_STATE_TL.with(|lock| {
-        if lock.get().is_none() {
-            let keypair = make_keypair();
-            let crdt = BaseCrdt::<PipelineDoc>::new(&keypair);
-            let (persist_tx, rx) = mpsc::unbounded_channel();
-            // Leak the receiver so the channel stays open: apply_and_persist
-            // can then send without error, preventing [crdt_persist] WARNs
-            // from racing with other tests that watch the global log buffer.
-            std::mem::forget(rx);
-            let state = CrdtState {
-                crdt,
-                keypair,
-                index: HashMap::new(),
-                node_index: HashMap::new(),
-                token_index: HashMap::new(),
-                merge_job_index: HashMap::new(),
-                active_agent_index: HashMap::new(),
-                test_job_index: HashMap::new(),
-                agent_throttle_index: HashMap::new(),
-                gateway_project_index: HashMap::new(),
-                persist_tx,
-                lamport_floor: 0,
-                tombstones: HashSet::new(),
-            };
-            let _ = lock.set(Mutex::new(state));
+        if let Some(mutex) = lock.get() {
+            // Already set on this thread — replace contents so the second
+            // (and subsequent) test on the same thread starts clean.
+            *mutex.lock().unwrap() = fresh;
+        } else {
+            let _ = lock.set(Mutex::new(fresh));
         }
     });
     let _ = statics::CRDT_EVENT_TX.get_or_init(|| broadcast::channel::<CrdtEvent>(256).0);
     let _ = statics::SYNC_TX.get_or_init(|| broadcast::channel::<SignedOp>(1024).0);
-    // Per-thread op journal + vector clock — keeps parallel tests' writes
-    // from corrupting each other's view of ALL_OPS (notably, one thread's
-    // `apply_compaction` could otherwise prune another thread's ops).
+    // Per-thread op journal + vector clock — always cleared so a second test
+    // on the same thread cannot see ops written by the first.
     statics::ALL_OPS_TL.with(|lock| {
-        let _ = lock.set(Mutex::new(Vec::new()));
+        if let Some(mutex) = lock.get() {
+            mutex.lock().unwrap().clear();
+        } else {
+            let _ = lock.set(Mutex::new(Vec::new()));
+        }
     });
     statics::VECTOR_CLOCK_TL.with(|lock| {
-        let _ = lock.set(Mutex::new(VectorClock::new()));
+        if let Some(mutex) = lock.get() {
+            mutex.lock().unwrap().clear();
+        } else {
+            let _ = lock.set(Mutex::new(VectorClock::new()));
+        }
     });
 }

server/src/db/content_store.rs

@@ -165,7 +165,9 @@ pub fn delete_content(key: ContentKey<'_>) {
 
 /// Ensure the in-memory content store is initialised.
 ///
-/// Safe to call multiple times — the `OnceLock` is set at most once.
+/// In non-test builds: init-once via `OnceLock` (safe to call multiple times).
+/// In test builds: always resets `CONTENT_STORE_TL` to an empty `HashMap` so
+/// each test on the same thread starts with a clean store.
 pub fn ensure_content_store() {
     #[cfg(not(test))]
     {
@@ -175,7 +177,11 @@ pub fn ensure_content_store() {
     #[cfg(test)]
     {
         CONTENT_STORE_TL.with(|lock| {
-            if lock.get().is_none() {
+            if let Some(mutex) = lock.get() {
+                // Already initialised on this thread — reset to empty so the
+                // next test does not see content written by a previous test.
+                mutex.lock().unwrap().clear();
+            } else {
                 let _ = lock.set(Mutex::new(HashMap::new()));
             }
         });
@@ -203,6 +209,41 @@ pub(super) fn init_content_store(map: HashMap<String, String>) {
 mod tests {
     use super::*;
 
+    /// Regression: two sequential `ensure_content_store()` + write + read cycles
+    /// in the same test body must not see each other's content.  Before the fix,
+    /// `ensure_content_store()` was a no-op on the second call (OnceLock gating),
+    /// so the second cycle could read items written in the first cycle.
+    #[test]
+    fn sequential_ensure_content_store_resets_state() {
+        // ── Cycle 1 ──────────────────────────────────────────────────────────
+        ensure_content_store();
+        write_content(ContentKey::Story("1111_cycle1"), "cycle-one body");
+        assert_eq!(
+            read_content(ContentKey::Story("1111_cycle1")).as_deref(),
+            Some("cycle-one body"),
+            "cycle 1: item must be readable after write"
+        );
+
+        // ── Cycle 2: reset, write a different item ────────────────────────────
+        ensure_content_store();
+        // Cycle-1 item must no longer be visible.
+        assert!(
+            read_content(ContentKey::Story("1111_cycle1")).is_none(),
+            "cycle 2: store must be empty; cycle-1 content must not bleed through"
+        );
+        write_content(ContentKey::Story("1111_cycle2"), "cycle-two body");
+        assert_eq!(
+            read_content(ContentKey::Story("1111_cycle2")).as_deref(),
+            Some("cycle-two body"),
+            "cycle 2: own item must be readable"
+        );
+        // And cycle-1 key must still be absent.
+        assert!(
+            read_content(ContentKey::Story("1111_cycle1")).is_none(),
+            "cycle 2: cycle-1 content must remain absent after cycle-2 write"
+        );
+    }
+
     /// 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.

server/src/db/ops.rs

@@ -72,6 +72,12 @@ pub fn write_item_with_content(story_id: &str, stage: &str, content: &str, meta:
         .and_then(|d| serde_json::to_string(d).ok());
 
     // Update in-memory content store.
+    // In test builds, the caller (test setup) is responsible for calling
+    // ensure_content_store() once before writing — calling it here would
+    // reset the store on every write, losing items from prior writes in the
+    // same test.  In production, the lazy-init call is safe because nothing
+    // resets the store between writes.
+    #[cfg(not(test))]
     ensure_content_store();
     write_content(ContentKey::Story(story_id), content);
 

server/src/http/mcp/story_tools/bug.rs

@@ -86,6 +86,7 @@ mod tests {
     use crate::http::test_helpers::test_ctx;
 
     fn setup_git_repo_in(dir: &std::path::Path) {
+        crate::db::ensure_content_store();
         std::process::Command::new("git")
             .args(["init"])
             .current_dir(dir)

server/src/http/mcp/story_tools/refactor.rs

@@ -115,6 +115,7 @@ mod tests {
 
     #[test]
     fn tool_create_refactor_accepts_single_criterion() {
+        crate::db::ensure_content_store();
         let tmp = tempfile::tempdir().unwrap();
         let ctx = test_ctx(tmp.path());
         let result = tool_create_refactor(
@@ -146,6 +147,7 @@ mod tests {
 
     #[test]
     fn tool_create_refactor_accepts_mixed_junk_and_real_acceptance_criteria() {
+        crate::db::ensure_content_store();
         let tmp = tempfile::tempdir().unwrap();
         let ctx = test_ctx(tmp.path());
         let result = tool_create_refactor(

server/src/http/mcp/story_tools/spike.rs

@@ -118,6 +118,7 @@ mod tests {
 
     #[test]
     fn tool_create_spike_creates_file() {
+        crate::db::ensure_content_store();
         let tmp = tempfile::tempdir().unwrap();
         let ctx = test_ctx(tmp.path());
 
@@ -147,6 +148,7 @@ mod tests {
 
     #[test]
     fn tool_create_spike_creates_file_without_description() {
+        crate::db::ensure_content_store();
         let tmp = tempfile::tempdir().unwrap();
         let ctx = test_ctx(tmp.path());
 
@@ -202,6 +204,7 @@ mod tests {
 
     #[test]
     fn tool_create_spike_accepts_single_criterion() {
+        crate::db::ensure_content_store();
         let tmp = tempfile::tempdir().unwrap();
         let ctx = test_ctx(tmp.path());
         let result = tool_create_spike(
@@ -233,6 +236,7 @@ mod tests {
 
     #[test]
     fn tool_create_spike_accepts_mixed_junk_and_real_acceptance_criteria() {
+        crate::db::ensure_content_store();
         let tmp = tempfile::tempdir().unwrap();
         let ctx = test_ctx(tmp.path());
         let result = tool_create_spike(

server/src/http/mcp/story_tools/story/create.rs

@@ -256,6 +256,7 @@ mod tests {
 
     #[test]
     fn tool_create_story_accepts_single_criterion() {
+        crate::db::ensure_content_store();
         let tmp = tempfile::tempdir().unwrap();
         let ctx = test_ctx(tmp.path());
         let result = tool_create_story(
@@ -283,6 +284,7 @@ mod tests {
 
     #[test]
     fn tool_create_story_accepts_mixed_junk_and_real_acceptance_criteria() {
+        crate::db::ensure_content_store();
         let tmp = tempfile::tempdir().unwrap();
         let ctx = test_ctx(tmp.path());
         let result = tool_create_story(
@@ -299,6 +301,7 @@ mod tests {
 
     #[test]
     fn tool_create_story_description_is_written_to_file() {
+        crate::db::ensure_content_store();
         let tmp = tempfile::tempdir().unwrap();
         let ctx = test_ctx(tmp.path());
 
@@ -368,6 +371,7 @@ mod tests {
 
     #[test]
     fn tool_create_story_html_sanitised_in_name() {
+        crate::db::ensure_content_store();
         let tmp = tempfile::tempdir().unwrap();
         let ctx = test_ctx(tmp.path());
         // HTML in name is sanitised (not rejected)

server/src/http/mcp/story_tools/story/query.rs

@@ -124,6 +124,7 @@ mod tests {
 
     #[test]
     fn tool_create_story_and_list_upcoming() {
+        crate::db::ensure_content_store();
         let tmp = tempfile::tempdir().unwrap();
         // No git repo needed: spike 61 — create_story just writes the file;
         // the filesystem watcher handles the commit asynchronously.

server/src/http/workflow/bug_ops/tests.rs

@@ -6,6 +6,7 @@ use super::spike::create_spike_file;
 use std::fs;
 
 fn setup_git_repo(root: &std::path::Path) {
+    crate::db::ensure_content_store();
     std::process::Command::new("git")
         .args(["init"])
         .current_dir(root)
@@ -166,6 +167,7 @@ fn extract_bug_name_from_content_parses_heading() {
 
 #[test]
 fn create_bug_file_writes_correct_content() {
+    crate::db::ensure_content_store();
     let tmp = tempfile::tempdir().unwrap();
     setup_git_repo(tmp.path());
 
@@ -257,6 +259,7 @@ fn create_bug_file_rejects_empty_acceptance_criteria() {
 
 #[test]
 fn create_spike_file_writes_correct_content() {
+    crate::db::ensure_content_store();
     let tmp = tempfile::tempdir().unwrap();
 
     let spike_id = create_spike_file(
@@ -294,6 +297,7 @@ fn create_spike_file_writes_correct_content() {
 
 #[test]
 fn create_spike_file_uses_description_when_provided() {
+    crate::db::ensure_content_store();
     let tmp = tempfile::tempdir().unwrap();
     let description = "What is the best approach for watching filesystem events?";
 
@@ -319,6 +323,7 @@ fn create_spike_file_uses_description_when_provided() {
 
 #[test]
 fn create_spike_file_uses_placeholder_when_no_description() {
+    crate::db::ensure_content_store();
     let tmp = tempfile::tempdir().unwrap();
     let spike_id = create_spike_file(
         tmp.path(),
@@ -350,6 +355,7 @@ fn create_spike_file_rejects_empty_name() {
 
 #[test]
 fn create_spike_file_with_special_chars_in_name_produces_valid_yaml() {
+    crate::db::ensure_content_store();
     let tmp = tempfile::tempdir().unwrap();
     let name = "Spike: compare \"fast\" vs slow encoders";
     let result = create_spike_file(
@@ -423,6 +429,7 @@ fn create_bug_file_with_depends_on_persists_to_crdt() {
 
 #[test]
 fn create_bug_file_without_depends_on_omits_field() {
+    crate::db::ensure_content_store();
     let tmp = tempfile::tempdir().unwrap();
     setup_git_repo(tmp.path());
 
@@ -474,6 +481,7 @@ fn create_refactor_file_with_depends_on_persists_to_crdt() {
 
 #[test]
 fn create_refactor_file_without_depends_on_omits_field() {
+    crate::db::ensure_content_store();
     let tmp = tempfile::tempdir().unwrap();
     setup_git_repo(tmp.path());