From c47faf0af96151baa748953bf330b98fc8063943 Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 10 Jul 2026 16:25:02 +0200 Subject: [PATCH] fix: idempotent migrations (ADD COLUMN IF NOT EXISTS), stale doc, safety note --- hive-c0re/src/stores/approvals.rs | 9 ++-- hive-c0re/src/stores/broker.rs | 36 ++++++++------- hive-c0re/src/stores/db.rs | 51 ++++++++++++++++++++-- hive-c0re/src/stores/operator_questions.rs | 7 +-- hive-c0re/src/stores/scheduled_prompts.rs | 3 +- 5 files changed, 80 insertions(+), 26 deletions(-) diff --git a/hive-c0re/src/stores/approvals.rs b/hive-c0re/src/stores/approvals.rs index 37e809ca..a8f39633 100644 --- a/hive-c0re/src/stores/approvals.rs +++ b/hive-c0re/src/stores/approvals.rs @@ -30,14 +30,15 @@ CREATE INDEX IF NOT EXISTS idx_approvals_pending /// all known migrations. New columns go here as v5, v6, … const MIGRATIONS: &[&str] = &[ // v1: `kind` (pre-Phase-8 dbs): legacy rows default to `apply_commit`. - "ALTER TABLE approvals ADD COLUMN kind TEXT NOT NULL DEFAULT 'apply_commit'", + "ALTER TABLE approvals ADD COLUMN IF NOT EXISTS \ + kind TEXT NOT NULL DEFAULT 'apply_commit'", // v2: `description`: manager-supplied note on the dashboard card. - "ALTER TABLE approvals ADD COLUMN description TEXT", + "ALTER TABLE approvals ADD COLUMN IF NOT EXISTS description TEXT", // v3: `fetched_sha`: canonical sha hive-c0re resolved at submit time. - "ALTER TABLE approvals ADD COLUMN fetched_sha TEXT", + "ALTER TABLE approvals ADD COLUMN IF NOT EXISTS fetched_sha TEXT", // v4: `submitter`: authenticated agent that submitted the approval. // Legacy rows are NULL → callers fall back to the root agent. - "ALTER TABLE approvals ADD COLUMN submitter TEXT", + "ALTER TABLE approvals ADD COLUMN IF NOT EXISTS submitter TEXT", ]; pub struct Approvals { diff --git a/hive-c0re/src/stores/broker.rs b/hive-c0re/src/stores/broker.rs index b779674b..c614008e 100644 --- a/hive-c0re/src/stores/broker.rs +++ b/hive-c0re/src/stores/broker.rs @@ -162,23 +162,34 @@ pub struct Broker { /// `messages` — the last column added before versioning. const BROKER_MIGRATIONS: &[&str] = &[ // v1: acked_at on messages, with backfill so existing delivered rows - // are not phantom-requeued on the next open. The multi-statement - // batch runs atomically (execute_batch wraps in a transaction). - "ALTER TABLE messages ADD COLUMN acked_at INTEGER;\ - UPDATE messages SET acked_at = delivered_at WHERE delivered_at IS NOT NULL;", + // are not phantom-requeued on the next open. + // + // IF NOT EXISTS: safe to run on a partially-migrated legacy DB where + // acked_at already exists but priority (the legacy marker) does not. + // The BEGIN/COMMIT block makes ALTER + UPDATE atomic — if the UPDATE + // fails the version counter is not bumped and the whole step retries. + "BEGIN;\ + ALTER TABLE messages ADD COLUMN IF NOT EXISTS acked_at INTEGER;\ + UPDATE messages SET acked_at = delivered_at \ + WHERE delivered_at IS NOT NULL AND acked_at IS NULL;\ + COMMIT;", // v2: in_reply_to for thread-parent tracking. NULL = root of a thread. - "ALTER TABLE messages ADD COLUMN in_reply_to INTEGER", + "ALTER TABLE messages ADD COLUMN IF NOT EXISTS in_reply_to INTEGER", // v3: priority for operator-message fast-path. Rebuild the delivery // index to include priority as a secondary sort key. - "ALTER TABLE messages ADD COLUMN priority INTEGER NOT NULL DEFAULT 0;\ + "BEGIN;\ + ALTER TABLE messages ADD COLUMN IF NOT EXISTS \ + priority INTEGER NOT NULL DEFAULT 0;\ DROP INDEX IF EXISTS idx_messages_undelivered;\ - CREATE INDEX idx_messages_undelivered \ - ON messages (recipient, priority DESC, id) WHERE delivered_at IS NULL", + CREATE INDEX IF NOT EXISTS idx_messages_undelivered \ + ON messages (recipient, priority DESC, id) WHERE delivered_at IS NULL;\ + COMMIT;", // v4: attempt_count on reminders for the MAX_REMINDER_ATTEMPTS cap. - "ALTER TABLE reminders ADD COLUMN attempt_count INTEGER NOT NULL DEFAULT 0", + "ALTER TABLE reminders ADD COLUMN IF NOT EXISTS \ + attempt_count INTEGER NOT NULL DEFAULT 0", // v5: last_error on reminders — last delivery failure surfaced on the // dashboard so a stuck reminder is visible without digging in logs. - "ALTER TABLE reminders ADD COLUMN last_error TEXT", + "ALTER TABLE reminders ADD COLUMN IF NOT EXISTS last_error TEXT", ]; impl Broker { @@ -1134,11 +1145,6 @@ impl Broker { } } -/// Idempotent messages-table migrations. Adds `acked_at` and -/// back-fills it for every already-delivered row, so the -/// pre-migration sessions count as "fully handled" and won't be -/// resurfaced by the first `requeue_inflight` after upgrade. - #[cfg(test)] mod tests { use super::*; diff --git a/hive-c0re/src/stores/db.rs b/hive-c0re/src/stores/db.rs index 886fa96e..c3810a0c 100644 --- a/hive-c0re/src/stores/db.rs +++ b/hive-c0re/src/stores/db.rs @@ -80,6 +80,11 @@ pub fn apply_versioned_migrations( // old try-and-ignore path — skip them. If it doesn't exist, start // at 0 (fresh install or partial state). let (legacy_table, legacy_col) = legacy_column; + // SAFETY: `legacy_table` is always a static table-name literal + // supplied by each store's `open` (e.g. `"messages"`). Dynamic + // or untrusted table names must not be passed here — if that + // ever changes, switch to `pragma_table_info(?1)` with two + // bound parameters. let is_legacy: bool = conn .prepare(&format!( "SELECT 1 FROM pragma_table_info('{legacy_table}') WHERE name = ?1" @@ -143,9 +148,9 @@ mod tests { const TEST_SCHEMA: &str = "CREATE TABLE IF NOT EXISTS things (id INTEGER PRIMARY KEY)"; const TEST_MIGRATIONS: &[&str] = &[ - "ALTER TABLE things ADD COLUMN alpha TEXT", - "ALTER TABLE things ADD COLUMN beta INTEGER NOT NULL DEFAULT 0", - "ALTER TABLE things ADD COLUMN gamma TEXT", + "ALTER TABLE things ADD COLUMN IF NOT EXISTS alpha TEXT", + "ALTER TABLE things ADD COLUMN IF NOT EXISTS beta INTEGER NOT NULL DEFAULT 0", + "ALTER TABLE things ADD COLUMN IF NOT EXISTS gamma TEXT", ]; #[test] @@ -289,4 +294,44 @@ mod tests { assert_eq!(va, 1); assert_eq!(vb, 1); } + + /// A partially-migrated legacy database: some columns from the old + /// try-and-ignore path exist, but the legacy marker column (used to + /// fast-forward fresh schema_versions entries to `len`) is absent. + /// With idempotent `ADD COLUMN IF NOT EXISTS` migrations this must + /// succeed — the already-present columns are skipped and only the + /// missing ones are added. + #[test] + fn partial_legacy_completes_without_error() { + let conn = open_tmp(); + // Simulate a DB that has `alpha` (v1) but not `beta` (v2) or + // `gamma` (v3). No schema_versions row exists yet, and `gamma` + // (the legacy marker) is absent → the function cannot fast-forward + // and must start from v0 with idempotent migrations. + conn.execute_batch( + "CREATE TABLE IF NOT EXISTS things (id INTEGER PRIMARY KEY, alpha TEXT)", + ) + .unwrap(); + apply_versioned_migrations(&conn, "things", ("things", "gamma"), TEST_MIGRATIONS).unwrap(); + // All three columns must now exist. + for col in ["alpha", "beta", "gamma"] { + let exists: bool = conn + .prepare(&format!( + "SELECT 1 FROM pragma_table_info('things') WHERE name = '{col}'" + )) + .unwrap() + .exists([]) + .unwrap(); + assert!(exists, "column {col} missing after partial-legacy migration"); + } + // Version must be at the latest. + let v: i64 = conn + .query_row( + "SELECT version FROM schema_versions WHERE store = 'things'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(v, 3, "must reach latest version after completing partial legacy"); + } } diff --git a/hive-c0re/src/stores/operator_questions.rs b/hive-c0re/src/stores/operator_questions.rs index 87ac6214..4f352020 100644 --- a/hive-c0re/src/stores/operator_questions.rs +++ b/hive-c0re/src/stores/operator_questions.rs @@ -39,13 +39,14 @@ CREATE INDEX IF NOT EXISTS idx_operator_questions_pending /// past all known migrations. const MIGRATIONS: &[&str] = &[ // v1: `multi` — checkbox-style multi-option questions. - "ALTER TABLE operator_questions ADD COLUMN multi INTEGER NOT NULL DEFAULT 0", + "ALTER TABLE operator_questions ADD COLUMN IF NOT EXISTS \ + multi INTEGER NOT NULL DEFAULT 0", // v2: `deadline_at` — optional TTL after which the watchdog auto-resolves. - "ALTER TABLE operator_questions ADD COLUMN deadline_at INTEGER", + "ALTER TABLE operator_questions ADD COLUMN IF NOT EXISTS deadline_at INTEGER", // v3: `target` — recipient of the question. NULL = operator (back-compat // default); non-null = peer-to-peer question. Dashboard's `pending()` // filters on `target IS NULL` so peer questions never leak to the operator. - "ALTER TABLE operator_questions ADD COLUMN target TEXT", + "ALTER TABLE operator_questions ADD COLUMN IF NOT EXISTS target TEXT", ]; #[derive(Debug, Clone, Serialize)] diff --git a/hive-c0re/src/stores/scheduled_prompts.rs b/hive-c0re/src/stores/scheduled_prompts.rs index 7d745aa9..4ffa132e 100644 --- a/hive-c0re/src/stores/scheduled_prompts.rs +++ b/hive-c0re/src/stores/scheduled_prompts.rs @@ -195,7 +195,8 @@ impl ScheduledPrompts { ("scheduled_prompts", "paused_at_unix"), &[ // v1: add paused_at_unix for per-schedule pause support. - "ALTER TABLE scheduled_prompts ADD COLUMN paused_at_unix INTEGER", + "ALTER TABLE scheduled_prompts \ + ADD COLUMN IF NOT EXISTS paused_at_unix INTEGER", ], )?; // Migration: recreate the due-rows index to also exclude paused