huskies: merge 1095 bug Shadow drift: set_name writes CRDT name register without updating pipeline_items.name
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
|
||||
+45
-1
@@ -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<String>,) =
|
||||
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
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user