From 71e303ed622e372a46c5f4c524a78f55cd26a674 Mon Sep 17 00:00:00 2001 From: damocles Date: Wed, 21 Oct 2026 21:42:29 +0200 Subject: [PATCH] db migrate: retry the legacy reminders copy while the legacy file is still there --- hive-agent/src/db_migrate.rs | 127 +++++++++++++++++++++++++++++------ 1 file changed, 107 insertions(+), 20 deletions(-) diff --git a/hive-agent/src/db_migrate.rs b/hive-agent/src/db_migrate.rs index 368edf20..9c8941d3 100644 --- a/hive-agent/src/db_migrate.rs +++ b/hive-agent/src/db_migrate.rs @@ -4,7 +4,8 @@ //! another sqlite ... add it to one of the existing ones"). Idempotent + best //! effort: `run` is called once in `main.rs`, before anything opens the //! consolidated db — a fresh harness boot with no legacy files (or one that -//! already migrated) is a silent no-op. +//! already migrated) is a silent no-op, and a failed step leaves its legacy +//! file in place to be retried on the next boot. //! //! Two legacy shapes fold in: //! - `hyperhive-todos.sqlite` already has exactly the `todos` table the @@ -23,26 +24,24 @@ use anyhow::{Context, Result}; use rusqlite::{Connection, params}; /// Fold the legacy `legacy_todos`/`legacy_reminders` files (if present) into -/// the single consolidated `state_db` path. No-op if `state_db` already -/// exists — legacy files are only ever consulted on the first boot after -/// the db-consolidation upgrade. +/// the single consolidated `state_db` path. The legacy todos file is only +/// renamed in while `state_db` does not exist yet; the legacy reminders file +/// is copied in whenever it is still on disk, so a copy that failed on one +/// boot is retried on the next. /// /// # Errors /// /// Propagates filesystem rename / sqlite attach-copy failures. Callers /// treat this as best-effort (log and continue) — `Todos`/`Reminders::open` /// against `state_db` still work standalone either way (todos would simply -/// start empty rather than inheriting the legacy rows). Retry-next-boot -/// only holds for failures before `state_db` exists (the todos rename, or -/// the reminders copy itself): once the copy has landed, `state_db` exists, -/// so a `remove_file(legacy_reminders)` failure afterwards leaves an -/// orphaned (already-copied, now-inert) legacy file behind rather than -/// retrying — the rows are safe, but the leftover file won't self-clean. +/// start empty rather than inheriting the legacy rows). A failed todos +/// rename or reminders copy leaves its legacy file in place and is retried +/// on the next boot, and so is a failed `remove_file(legacy_reminders)`. +/// A retry skips rows already present with the same content, but a legacy +/// row that was copied and then cancelled or pruned from the consolidated +/// db comes back, which is the cost of retrying at all. pub fn run(state_db: &Path, legacy_todos: &Path, legacy_reminders: &Path) -> Result<()> { - if state_db.exists() { - return Ok(()); - } - if legacy_todos.exists() { + if !state_db.exists() && legacy_todos.exists() { std::fs::rename(legacy_todos, state_db).with_context(|| { format!( "rename legacy todos db {} -> {}", @@ -80,11 +79,31 @@ fn migrate_reminders_rows(state_db: &Path, legacy_reminders: &Path) -> Result<() ) .context("attach legacy reminders db")?; let copied = conn - .execute_batch( - "INSERT INTO reminders (id, message, file_path, due_at, created_at, sent_at) \ - SELECT id, message, file_path, due_at, created_at, sent_at FROM legacy.reminders;", - ) - .context("copy legacy reminders rows"); + .query_row("SELECT COUNT(*) FROM main.reminders", [], |row| { + row.get::<_, i64>(0) + }) + .context("count consolidated reminders") + .and_then(|existing| { + // Ids are kept while the table is empty (the first-boot case an + // id seen just before the upgrade relies on). A retry after the + // harness already stored reminders of its own cannot reuse them: + // a colliding id would silently drop the legacy row. Those rows + // are appended under fresh ids instead, skipping any that are + // already there with the same content. + let sql = if existing == 0 { + "INSERT INTO reminders (id, message, file_path, due_at, created_at, sent_at) \ + SELECT id, message, file_path, due_at, created_at, sent_at FROM legacy.reminders;" + } else { + "INSERT INTO reminders (message, file_path, due_at, created_at, sent_at) \ + SELECT l.message, l.file_path, l.due_at, l.created_at, l.sent_at \ + FROM legacy.reminders l WHERE NOT EXISTS ( \ + SELECT 1 FROM main.reminders r WHERE r.message = l.message \ + AND r.due_at = l.due_at AND r.created_at = l.created_at \ + AND r.file_path IS l.file_path);" + }; + conn.execute_batch(sql) + .context("copy legacy reminders rows") + }); // DETACH regardless of the copy's outcome so the connection doesn't // leak the attachment on an early return via `?` inside this fn. let detach = conn.execute("DETACH DATABASE legacy", []); @@ -109,7 +128,7 @@ mod tests { } #[test] - fn state_db_already_present_short_circuits() { + fn an_existing_state_db_leaves_a_legacy_todos_file_alone() { let dir = tempfile::tempdir().unwrap(); let state = dir.path().join("hyperhive-state.sqlite"); let legacy_todos = dir.path().join("hyperhive-todos.sqlite"); @@ -175,6 +194,74 @@ mod tests { assert_eq!(pending[0].due_at.timestamp(), 1000); } + /// A copy that fails after `state_db` was created must be retried on the + /// next boot, not abandoned because the consolidated db now exists. + #[test] + fn a_failed_reminders_copy_is_retried_once_the_state_db_exists() { + let dir = tempfile::tempdir().unwrap(); + let legacy_reminders = dir.path().join("hyperhive-reminders.sqlite"); + let state = dir.path().join("hyperhive-state.sqlite"); + let legacy_todos = dir.path().join("hyperhive-todos.sqlite"); + std::fs::write(&legacy_reminders, b"not a sqlite file").unwrap(); + + run(&state, &legacy_todos, &legacy_reminders).unwrap_err(); + assert!(state.exists(), "the failed copy still created the db"); + assert!(legacy_reminders.exists(), "legacy file kept for a retry"); + + std::fs::remove_file(&legacy_reminders).unwrap(); + Reminders::open(&legacy_reminders) + .unwrap() + .store("ping", None, 500) + .unwrap(); + run(&state, &legacy_todos, &legacy_reminders).unwrap(); + + assert!(!legacy_reminders.exists(), "removed after the retry"); + assert_eq!( + Reminders::open(&state) + .unwrap() + .list_pending() + .unwrap() + .len(), + 1 + ); + } + + /// The harness boots after a failed copy and stores reminders of its own, + /// so their ids overlap the legacy ones. The retry must keep both sets. + #[test] + fn a_retry_keeps_legacy_reminders_whose_ids_collide_with_new_ones() { + let dir = tempfile::tempdir().unwrap(); + let legacy_reminders = dir.path().join("hyperhive-reminders.sqlite"); + let state = dir.path().join("hyperhive-state.sqlite"); + let legacy_todos = dir.path().join("hyperhive-todos.sqlite"); + std::fs::write(&legacy_reminders, b"not a sqlite file").unwrap(); + run(&state, &legacy_todos, &legacy_reminders).unwrap_err(); + Reminders::open(&state) + .unwrap() + .store("new while degraded", None, 900) + .unwrap(); + + std::fs::remove_file(&legacy_reminders).unwrap(); + { + let legacy = Reminders::open(&legacy_reminders).unwrap(); + legacy.store("legacy one", None, 100).unwrap(); + legacy.store("legacy two", None, 200).unwrap(); + } + run(&state, &legacy_todos, &legacy_reminders).unwrap(); + + let messages: Vec = Reminders::open(&state) + .unwrap() + .list_pending() + .unwrap() + .into_iter() + .map(|r| r.message) + .collect(); + assert_eq!(messages.len(), 3, "got {messages:?}"); + for expected in ["new while degraded", "legacy one", "legacy two"] { + assert!(messages.iter().any(|m| m == expected), "missing {expected}"); + } + } + #[test] fn folds_both_legacy_files_into_one_consolidated_db() { let dir = tempfile::tempdir().unwrap();