From d1904209461fe3b41b3d316e164c22aa41f7a420 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?m=C3=BCde?= Date: Mon, 6 Jul 2026 21:53:48 +0200 Subject: [PATCH] refactor(hive-c0re): shared additive-migration helper in db MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit db::apply_migrations runs ALTER lists and ignores duplicate-column errors (turn_stats' pattern); approvals, operator_questions, broker reminders, and scheduled_prompts drop their hand-rolled pragma_table_info guards. broker's acked_at migration stays bespoke — its backfill must only run when the column was just created --- hive-ag3nt/src/{mcp.rs => mcp/mod.rs} | 0 hive-c0re/src/approvals.rs | 85 ++++++------------------ hive-c0re/src/broker.rs | 38 ++++------- hive-c0re/src/db.rs | 20 ++++++ hive-c0re/src/{forge.rs => forge/mod.rs} | 0 hive-c0re/src/operator_questions.rs | 49 ++++---------- hive-c0re/src/scheduled_prompts.rs | 11 ++- 7 files changed, 71 insertions(+), 132 deletions(-) rename hive-ag3nt/src/{mcp.rs => mcp/mod.rs} (100%) rename hive-c0re/src/{forge.rs => forge/mod.rs} (100%) diff --git a/hive-ag3nt/src/mcp.rs b/hive-ag3nt/src/mcp/mod.rs similarity index 100% rename from hive-ag3nt/src/mcp.rs rename to hive-ag3nt/src/mcp/mod.rs diff --git a/hive-c0re/src/approvals.rs b/hive-c0re/src/approvals.rs index 504d9302..2f1d4989 100644 --- a/hive-c0re/src/approvals.rs +++ b/hive-c0re/src/approvals.rs @@ -24,66 +24,26 @@ CREATE INDEX IF NOT EXISTS idx_approvals_pending ON approvals (id) WHERE status = 'pending'; "; -/// Add the `description` column to pre-description databases. Manager-supplied -/// note shown on the dashboard approval card at submission time (distinct from -/// `note` which is set on denial/failure). -fn ensure_description_column(conn: &Connection) -> Result<()> { - let has: bool = conn - .prepare("SELECT 1 FROM pragma_table_info('approvals') WHERE name = 'description'")? - .exists([])?; - if !has { - conn.execute_batch("ALTER TABLE approvals ADD COLUMN description TEXT;") - .context("add approvals.description column")?; - } - Ok(()) -} - -/// Add the `kind` column to pre-Phase-8 databases. ALTER TABLE ADD COLUMN is -/// idempotent here only via a column-existence check (sqlite doesn't support -/// IF NOT EXISTS on ADD COLUMN). Defaults legacy rows to `apply_commit`, -/// which matches their actual semantics. -fn ensure_kind_column(conn: &Connection) -> Result<()> { - let has_kind: bool = conn - .prepare("SELECT 1 FROM pragma_table_info('approvals') WHERE name = 'kind'")? - .exists([])?; - if !has_kind { - conn.execute_batch( - "ALTER TABLE approvals ADD COLUMN kind TEXT NOT NULL DEFAULT 'apply_commit';", - ) - .context("add approvals.kind column")?; - } - Ok(()) -} - -/// Same shape as `ensure_kind_column` but for `fetched_sha` — the -/// canonical sha hive-c0re vouched for at `request_apply_commit` time. -/// Distinct from `commit_ref` (manager-supplied, may not even resolve -/// in proposed by the time we approve). -fn ensure_fetched_sha_column(conn: &Connection) -> Result<()> { - let has: bool = conn - .prepare("SELECT 1 FROM pragma_table_info('approvals') WHERE name = 'fetched_sha'")? - .exists([])?; - if !has { - conn.execute_batch("ALTER TABLE approvals ADD COLUMN fetched_sha TEXT;") - .context("add approvals.fetched_sha column")?; - } - Ok(()) -} - -/// Same shape as `ensure_fetched_sha_column` but for `submitter` — the -/// agent that submitted the approval (the authenticated socket caller). -/// Approval-scoped helper events route to this agent. Legacy rows have -/// NULL; callers fall back to the root agent for those. -fn ensure_submitter_column(conn: &Connection) -> Result<()> { - let has: bool = conn - .prepare("SELECT 1 FROM pragma_table_info('approvals') WHERE name = 'submitter'")? - .exists([])?; - if !has { - conn.execute_batch("ALTER TABLE approvals ADD COLUMN submitter TEXT;") - .context("add approvals.submitter column")?; - } - Ok(()) -} +/// Additive column migrations for pre-existing databases, applied via +/// `db::apply_migrations` (try-and-ignore-duplicate-column). +const MIGRATIONS: &[&str] = &[ + // `kind` (pre-Phase-8 dbs): legacy rows default to `apply_commit`, + // which matches their actual semantics. + "ALTER TABLE approvals ADD COLUMN kind TEXT NOT NULL DEFAULT 'apply_commit'", + // `description`: manager-supplied note shown on the dashboard + // approval card at submission time (distinct from `note`, set on + // denial/failure). + "ALTER TABLE approvals ADD COLUMN description TEXT", + // `fetched_sha`: the canonical sha hive-c0re vouched for at + // `request_apply_commit` time. Distinct from `commit_ref` + // (manager-supplied, may not even resolve by approve time). + "ALTER TABLE approvals ADD COLUMN fetched_sha TEXT", + // `submitter`: the agent that submitted the approval (the + // authenticated socket caller); approval-scoped helper events + // route to it. Legacy rows are NULL → callers fall back to the + // root agent. + "ALTER TABLE approvals ADD COLUMN submitter TEXT", +]; pub struct Approvals { conn: Mutex, @@ -94,10 +54,7 @@ impl Approvals { let conn = crate::db::open(path, "approvals")?; conn.execute_batch(SCHEMA) .context("apply approvals schema")?; - ensure_kind_column(&conn).context("migrate approvals.kind")?; - ensure_fetched_sha_column(&conn).context("migrate approvals.fetched_sha")?; - ensure_description_column(&conn).context("migrate approvals.description")?; - ensure_submitter_column(&conn).context("migrate approvals.submitter")?; + crate::db::apply_migrations(&conn, "approvals", MIGRATIONS)?; Ok(Self { conn: Mutex::new(conn), }) diff --git a/hive-c0re/src/broker.rs b/hive-c0re/src/broker.rs index ba0e9fc6..4cc62bbb 100644 --- a/hive-c0re/src/broker.rs +++ b/hive-c0re/src/broker.rs @@ -1121,33 +1121,19 @@ fn ensure_message_columns(conn: &Connection) -> Result<()> { Ok(()) } -/// Idempotent reminder-table migrations. `ALTER TABLE ADD COLUMN` -/// has no `IF NOT EXISTS` form in sqlite, so we probe -/// `pragma_table_info` per column. New deploys (table created by -/// SCHEMA in this commit cycle) skip the ALTER; pre-existing -/// broker.sqlite files get the columns added on next boot. +/// Idempotent reminder-table migrations — plain additive columns, via +/// `db::apply_migrations`. (The messages-table migration above stays +/// bespoke: its backfill must run only when `acked_at` was just +/// created, which try-and-ignore can't express.) fn ensure_reminder_columns(conn: &Connection) -> Result<()> { - for (name, sql) in [ - ( - "attempt_count", - "ALTER TABLE reminders ADD COLUMN attempt_count INTEGER NOT NULL DEFAULT 0;", - ), - ( - "last_error", - "ALTER TABLE reminders ADD COLUMN last_error TEXT;", - ), - ] { - let has: bool = conn - .prepare(&format!( - "SELECT 1 FROM pragma_table_info('reminders') WHERE name = '{name}'" - ))? - .exists([])?; - if !has { - conn.execute_batch(sql) - .with_context(|| format!("add reminders.{name} column"))?; - } - } - Ok(()) + crate::db::apply_migrations( + conn, + "broker reminders", + &[ + "ALTER TABLE reminders ADD COLUMN attempt_count INTEGER NOT NULL DEFAULT 0", + "ALTER TABLE reminders ADD COLUMN last_error TEXT", + ], + ) } fn now_unix() -> i64 { diff --git a/hive-c0re/src/db.rs b/hive-c0re/src/db.rs index 3e86f629..f483d3d2 100644 --- a/hive-c0re/src/db.rs +++ b/hive-c0re/src/db.rs @@ -22,6 +22,26 @@ use rusqlite::Connection; /// means something is genuinely wedged, not ordinary contention. const BUSY_TIMEOUT: Duration = Duration::from_secs(5); +/// Apply additive, idempotent migrations: run each statement and +/// ignore `duplicate column name` errors (sqlite has no +/// `ADD COLUMN IF NOT EXISTS`; try-and-ignore is the portable path — +/// same pattern as hive-ag3nt's `turn_stats`). Any other error +/// propagates. Fits plain `ALTER TABLE ADD COLUMN` (new columns must +/// carry a default or tolerate NULL) and `IF NOT EXISTS` index DDL; +/// migrations with creation-conditional backfills (broker's +/// `acked_at`) stay bespoke in their store. +pub fn apply_migrations(conn: &Connection, subsystem: &str, statements: &[&str]) -> Result<()> { + for stmt in statements { + if let Err(e) = conn.execute_batch(stmt) { + if e.to_string().contains("duplicate column name") { + continue; + } + return Err(e).with_context(|| format!("{subsystem} migration failed: {stmt}")); + } + } + Ok(()) +} + /// Open a connection to the sqlite file at `path`, creating the parent /// directory if needed. `subsystem` labels error contexts (`"broker"`, /// `"approvals"`, …). diff --git a/hive-c0re/src/forge.rs b/hive-c0re/src/forge/mod.rs similarity index 100% rename from hive-c0re/src/forge.rs rename to hive-c0re/src/forge/mod.rs diff --git a/hive-c0re/src/operator_questions.rs b/hive-c0re/src/operator_questions.rs index 1986bd8a..945d7ae9 100644 --- a/hive-c0re/src/operator_questions.rs +++ b/hive-c0re/src/operator_questions.rs @@ -33,41 +33,18 @@ CREATE INDEX IF NOT EXISTS idx_operator_questions_pending ON operator_questions (id) WHERE answered_at IS NULL; "; -/// Add late-added columns to pre-existing databases. `ALTER TABLE -/// ADD COLUMN` has no `IF NOT EXISTS` form in sqlite, so we check -/// `pragma_table_info` first per column. -fn ensure_columns(conn: &Connection) -> Result<()> { - for (name, sql) in [ - ( - "multi", - "ALTER TABLE operator_questions ADD COLUMN multi INTEGER NOT NULL DEFAULT 0;", - ), - ( - "deadline_at", - "ALTER TABLE operator_questions ADD COLUMN deadline_at INTEGER;", - ), - // `target` = recipient of the question. NULL = operator - // (back-compat default for rows written before agent-to-agent - // questions existed); a non-null agent name = peer-to-peer - // question. Dashboard's `pending()` filters on `target IS NULL` - // so peer questions never leak into the operator's queue. - ( - "target", - "ALTER TABLE operator_questions ADD COLUMN target TEXT;", - ), - ] { - let has: bool = conn - .prepare(&format!( - "SELECT 1 FROM pragma_table_info('operator_questions') WHERE name = '{name}'" - ))? - .exists([])?; - if !has { - conn.execute_batch(sql) - .with_context(|| format!("add operator_questions.{name} column"))?; - } - } - Ok(()) -} +/// Additive column migrations for pre-existing databases, applied via +/// `db::apply_migrations` (try-and-ignore-duplicate-column). +const MIGRATIONS: &[&str] = &[ + "ALTER TABLE operator_questions ADD COLUMN multi INTEGER NOT NULL DEFAULT 0", + "ALTER TABLE operator_questions ADD COLUMN deadline_at INTEGER", + // `target` = recipient of the question. NULL = operator + // (back-compat default for rows written before agent-to-agent + // questions existed); a non-null agent name = peer-to-peer + // question. Dashboard's `pending()` filters on `target IS NULL` + // so peer questions never leak into the operator's queue. + "ALTER TABLE operator_questions ADD COLUMN target TEXT", +]; #[derive(Debug, Clone, Serialize)] pub struct OpQuestion { @@ -100,7 +77,7 @@ impl OperatorQuestions { let conn = crate::db::open(path, "operator_questions")?; conn.execute_batch(SCHEMA) .context("apply operator_questions schema")?; - ensure_columns(&conn).context("migrate operator_questions columns")?; + crate::db::apply_migrations(&conn, "operator_questions", MIGRATIONS)?; Ok(Self { conn: Mutex::new(conn), }) diff --git a/hive-c0re/src/scheduled_prompts.rs b/hive-c0re/src/scheduled_prompts.rs index bec95999..24607782 100644 --- a/hive-c0re/src/scheduled_prompts.rs +++ b/hive-c0re/src/scheduled_prompts.rs @@ -186,12 +186,11 @@ impl ScheduledPrompts { conn.execute_batch(SCHEMA) .context("apply scheduled_prompts schema")?; // Migration: add paused_at_unix to existing databases. - // Silently ignores "duplicate column name" errors so this is - // idempotent across daemon restarts on already-migrated DBs. - let _ = conn.execute( - "ALTER TABLE scheduled_prompts ADD COLUMN paused_at_unix INTEGER", - [], - ); + crate::db::apply_migrations( + &conn, + "scheduled_prompts", + &["ALTER TABLE scheduled_prompts ADD COLUMN paused_at_unix INTEGER"], + )?; // Migration: recreate the due-rows index to also exclude paused // rows. `CREATE INDEX IF NOT EXISTS` won't update an existing // index's WHERE clause, so we drop + recreate on every open.