From 62d1535e76d01e594135562d245907184a551d6d Mon Sep 17 00:00:00 2001 From: dave Date: Fri, 15 May 2026 12:03:15 +0000 Subject: [PATCH] huskies: merge 1095 bug Shadow drift: set_name writes CRDT name register without updating pipeline_items.name --- server/src/crdt_state/write/item.rs | 3 ++ server/src/db/mod.rs | 46 ++++++++++++++++++++++++++++- server/src/db/ops.rs | 34 +++++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) diff --git a/server/src/crdt_state/write/item.rs b/server/src/crdt_state/write/item.rs index 6a227ece..07d1edc2 100644 --- a/server/src/crdt_state/write/item.rs +++ b/server/src/crdt_state/write/item.rs @@ -155,6 +155,9 @@ pub fn set_name(story_id: &str, name: Option<&str>) -> bool { apply_and_persist(&mut state, |s| { s.crdt.doc.items[idx].name.set(value.clone()) }); + // Drop the lock before the shadow write so `read_item` can acquire it. + drop(state); + crate::db::sync_item_name(story_id); true } diff --git a/server/src/db/mod.rs b/server/src/db/mod.rs index 670d77ed..f204ced9 100644 --- a/server/src/db/mod.rs +++ b/server/src/db/mod.rs @@ -28,7 +28,10 @@ pub mod recover; pub mod shadow_write; 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 ops::{ + ItemMeta, delete_item, move_item_stage, next_item_number, sync_item_name, + write_item_with_content, +}; pub use shadow_write::{check_schema_drift, get_shared_pool, init}; #[cfg(test)] @@ -589,6 +592,47 @@ mod tests { ); } + /// Regression for story 1095: `set_name` must propagate the new name to the + /// SQLite shadow table via `sync_item_name`. Before the fix, the CRDT + /// register was updated but `pipeline_items.name` stayed stale. + #[tokio::test] + async fn set_name_updates_shadow_name_column() { + crate::crdt_state::init_for_test(); + ensure_content_store(); + + let tmp = tempfile::tempdir().unwrap(); + let db_path = tmp.path().join("pipeline.db"); + shadow_write::init(&db_path).await.expect("db init"); + + let story_id = "9095_story_set_name_shadow"; + write_item_with_content( + story_id, + "1_backlog", + "---\nname: Original Name\n---\n", + ItemMeta::named("Original Name"), + ); + + // Rename via the CRDT setter — now also triggers sync_item_name. + crate::crdt_state::set_name(story_id, Some("Updated Name")); + + // Wait for the background write task to flush. + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + + let pool = shadow_write::get_shared_pool().expect("pool must be initialised"); + let row: (Option,) = + sqlx::query_as("SELECT name FROM pipeline_items WHERE id = ?1") + .bind(story_id) + .fetch_one(pool) + .await + .unwrap(); + + assert_eq!( + row.0.as_deref(), + Some("Updated Name"), + "set_name must propagate the new name to the shadow table" + ); + } + /// Story 1087, AC2: the split-stage migration projects every supported /// wire-form `stage` string into the canonical `(pipeline, status)` pair. /// The fixture covers each Stage variant (and the legacy numeric-prefix diff --git a/server/src/db/ops.rs b/server/src/db/ops.rs index 0ba9817f..25394378 100644 --- a/server/src/db/ops.rs +++ b/server/src/db/ops.rs @@ -198,6 +198,40 @@ pub fn delete_item(story_id: &str) { } } +/// Sync the shadow table's `name` column after a CRDT name-register write. +/// +/// Reads the current item from the CRDT (which already holds the new name after +/// `apply_and_persist`) and sends a `PipelineWriteMsg` so the SQLite mirror +/// stays in sync. All other columns (stage, agent, retry_count, depends_on) +/// are preserved from the live CRDT view; `content` is left as `None` so the +/// UPSERT's `COALESCE` keeps the existing value. +/// +/// No-ops if the DB is not initialised or the item is not in the CRDT. +pub fn sync_item_name(story_id: &str) { + let Some(db) = PIPELINE_DB.get() else { return }; + let Some(view) = crate::crdt_state::read_item(story_id) else { + return; + }; + let depends_on = { + let d = view.depends_on(); + if d.is_empty() { + None + } else { + serde_json::to_string(d).ok() + } + }; + let msg = PipelineWriteMsg { + story_id: story_id.to_string(), + stage: view.stage().dir_name().to_string(), + name: Some(view.name().to_string()), + agent: view.agent().map(|a| a.to_string()), + retry_count: Some(view.retry_count() as i64), + depends_on, + content: None, + }; + let _ = db.tx.send(msg); +} + /// Get the next available item number by scanning the CRDT state, the /// in-memory content store, AND the tombstone set for the highest existing /// number.