diff --git a/packages/gittensory-miner/lib/claim-ledger.js b/packages/gittensory-miner/lib/claim-ledger.js index 74493a0f39..5cfc96fcb4 100644 --- a/packages/gittensory-miner/lib/claim-ledger.js +++ b/packages/gittensory-miner/lib/claim-ledger.js @@ -1,4 +1,5 @@ import { normalizeLocalStoreDbPath, openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js"; +import { applySchemaMigrations } from "./schema-version.js"; // The miner's local soft-claim ledger (#2314): a 100% client-side record of "I'm working on issue #N in repo X", // so Phase 2's soft-claim adjudication (sibling issues) has somewhere to persist claims. Schema + CRUD only — no @@ -71,6 +72,8 @@ export function openClaimLedger(dbPath = resolveClaimLedgerDbPath()) { UNIQUE (repo_full_name, issue_number) ) `); + // Schema-version convention (#4832): stamp the baseline and run any post-baseline migrations (none yet). + applySchemaMigrations(db, []); // Idempotent claim in ONE atomic statement: insert a new active claim, or — only if the existing row is NOT // already active — re-activate it (a released/expired claim can be re-claimed). The `WHERE status <> 'active'` diff --git a/packages/gittensory-miner/lib/event-ledger.js b/packages/gittensory-miner/lib/event-ledger.js index 2794fcf527..09c0585f60 100644 --- a/packages/gittensory-miner/lib/event-ledger.js +++ b/packages/gittensory-miner/lib/event-ledger.js @@ -1,5 +1,6 @@ import { isDeepStrictEqual } from "node:util"; import { normalizeLocalStoreDbPath, openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js"; +import { applySchemaMigrations } from "./schema-version.js"; // The miner's local, append-only event ledger (#2290): an immutable audit trail of every significant miner-loop // event (discovered_issue, plan_built, plan_step_completed, pr_prepared, … — a small fixed vocabulary for this @@ -103,6 +104,8 @@ export function initEventLedger(dbPath = resolveEventLedgerDbPath()) { created_at TEXT NOT NULL ) `); + // Schema-version convention (#4832): stamp the baseline and run any post-baseline migrations (none yet). + applySchemaMigrations(db, []); const nextSeqStatement = db.prepare("SELECT COALESCE(MAX(seq), 0) + 1 AS nextSeq FROM miner_event_ledger"); const appendStatement = db.prepare(` diff --git a/packages/gittensory-miner/lib/governor-ledger.js b/packages/gittensory-miner/lib/governor-ledger.js index 4c52c49742..0c39c47080 100644 --- a/packages/gittensory-miner/lib/governor-ledger.js +++ b/packages/gittensory-miner/lib/governor-ledger.js @@ -3,6 +3,7 @@ import { homedir } from "node:os"; import { dirname, join } from "node:path"; import { DatabaseSync } from "node:sqlite"; import { normalizeGovernorLedgerEvent } from "@jsonbored/gittensory-engine"; +import { applySchemaMigrations } from "./schema-version.js"; // Append-only governor decision ledger (#2328): every allowed/denied/throttled/kill-switch outcome lands in a // local SQLite table for contributor audit. IMMUTABILITY INVARIANT: INSERT + SELECT only — never UPDATE/DELETE. @@ -87,6 +88,8 @@ export function initGovernorLedger(dbPath = resolveGovernorLedgerDbPath()) { ) `); db.exec("CREATE INDEX IF NOT EXISTS idx_governor_events_repo ON governor_events (repo_full_name, id)"); + // Schema-version convention (#4832): stamp the baseline and run any post-baseline migrations (none yet). + applySchemaMigrations(db, []); const appendStatement = db.prepare(` INSERT INTO governor_events (ts, event_type, repo_full_name, action_class, decision, reason, payload_json) diff --git a/packages/gittensory-miner/lib/laptop-init.js b/packages/gittensory-miner/lib/laptop-init.js index b9c47bc557..85c625a315 100644 --- a/packages/gittensory-miner/lib/laptop-init.js +++ b/packages/gittensory-miner/lib/laptop-init.js @@ -2,6 +2,7 @@ import { accessSync, chmodSync, constants, existsSync, mkdirSync } from "node:fs import { homedir } from "node:os"; import { delimiter, join } from "node:path"; import { DatabaseSync } from "node:sqlite"; +import { applySchemaMigrations } from "./schema-version.js"; const defaultDbFileName = "laptop-state.sqlite3"; @@ -36,6 +37,8 @@ export function initLaptopState(env = process.env) { value TEXT NOT NULL ) `); + // Schema-version convention (#4832): stamp the baseline and run any post-baseline migrations (none yet). + applySchemaMigrations(db, []); if (created) { db.prepare("INSERT INTO laptop_meta (key, value) VALUES ('initialized_at', ?)") .run(new Date().toISOString()); diff --git a/packages/gittensory-miner/lib/plan-store.js b/packages/gittensory-miner/lib/plan-store.js index fbd27591b3..251c3781b6 100644 --- a/packages/gittensory-miner/lib/plan-store.js +++ b/packages/gittensory-miner/lib/plan-store.js @@ -2,6 +2,7 @@ import { chmodSync, mkdirSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join } from "node:path"; import { DatabaseSync } from "node:sqlite"; +import { applySchemaMigrations } from "./schema-version.js"; // Local SQLite persistence for the stateless MCP plan DAG (#2318). `gittensory_build_plan`/`plan_status`/ // `record_step_result` are stateless — the caller holds the plan and passes it back each call — so a miner running @@ -164,6 +165,8 @@ export function openPlanStore(dbPath = resolvePlanStoreDbPath()) { updated_at TEXT NOT NULL ) `); + // Schema-version convention (#4832): stamp the baseline and run any post-baseline migrations (none yet). + applySchemaMigrations(db, []); const saveStatement = db.prepare(` INSERT INTO miner_plans (plan_id, plan_json, status, updated_at) diff --git a/packages/gittensory-miner/lib/portfolio-queue.js b/packages/gittensory-miner/lib/portfolio-queue.js index 923e9a3382..31f993c58e 100644 --- a/packages/gittensory-miner/lib/portfolio-queue.js +++ b/packages/gittensory-miner/lib/portfolio-queue.js @@ -1,4 +1,5 @@ import { normalizeLocalStoreDbPath, openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js"; +import { applySchemaMigrations } from "./schema-version.js"; // The miner's local portfolio/queue store (#2292): a 100% client-side, prioritized backlog of candidate work // items across every repo the miner has been pointed at ("what should I look at next, across everything I'm @@ -88,12 +89,18 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) // swept back to 'queued' by age (see portfolio-queue-expiry.js) instead of stranding the item forever — the same // recovery the claim-ledger and worktree-allocator stores already provide for their own tables (#4827). Additive // migration for stores created before this column: CREATE TABLE IF NOT EXISTS never adds a column to a pre-existing - // table, so add it idempotently. - const hasLeasedAtColumn = db - .prepare("PRAGMA table_info(miner_portfolio_queue)") - .all() - .some((column) => column.name === "leased_at"); - if (!hasLeasedAtColumn) db.exec("ALTER TABLE miner_portfolio_queue ADD COLUMN leased_at TEXT"); + // table, so add it idempotently. Expressed as the store's first schema migration (#4832): the baseline table is + // version 1; migration 1→2 adds `leased_at`. The migration stays defensive (checks table_info) so a version-0 + // file that already ran the pre-convention ad-hoc ALTER is not re-altered into a duplicate-column error. + applySchemaMigrations(db, [ + (migrationDb) => { + const hasLeasedAtColumn = migrationDb + .prepare("PRAGMA table_info(miner_portfolio_queue)") + .all() + .some((column) => column.name === "leased_at"); + if (!hasLeasedAtColumn) migrationDb.exec("ALTER TABLE miner_portfolio_queue ADD COLUMN leased_at TEXT"); + }, + ]); // `rowid` is a stable, unique key assigned once at first insert (re-enqueue updates in place, never re-inserts), // so it is a deterministic total-order tie-break: two items sharing a priority AND an `enqueued_at` timestamp diff --git a/packages/gittensory-miner/lib/run-state.js b/packages/gittensory-miner/lib/run-state.js index 047e8b1075..f7c890100c 100644 --- a/packages/gittensory-miner/lib/run-state.js +++ b/packages/gittensory-miner/lib/run-state.js @@ -1,4 +1,5 @@ import { normalizeLocalStoreDbPath, openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js"; +import { applySchemaMigrations } from "./schema-version.js"; export const RUN_STATES = Object.freeze(["idle", "discovering", "planning", "preparing"]); @@ -41,6 +42,8 @@ export function initRunStateStore(dbPath = resolveRunStateDbPath()) { updated_at TEXT NOT NULL ) `); + // Schema-version convention (#4832): stamp the baseline and run any post-baseline migrations (none yet). + applySchemaMigrations(db, []); const getStatement = db.prepare( "SELECT state FROM miner_run_state WHERE repo_full_name = ?", diff --git a/packages/gittensory-miner/lib/schema-version.d.ts b/packages/gittensory-miner/lib/schema-version.d.ts new file mode 100644 index 0000000000..5c52cd1c07 --- /dev/null +++ b/packages/gittensory-miner/lib/schema-version.d.ts @@ -0,0 +1,17 @@ +import type { DatabaseSync } from "node:sqlite"; + +/** A single post-baseline schema migration: mutate the store in place to advance it exactly one version. */ +export type SchemaMigration = (db: DatabaseSync) => void; + +/** The bootstrap schema every store creates inline is, by convention, schema version 1. */ +export const BASELINE_SCHEMA_VERSION: number; + +/** Read a store's current `PRAGMA user_version`, coercing any absent/invalid value to 0 (pre-versioning). */ +export function readSchemaVersion(db: DatabaseSync): number; + +/** + * Run pending post-baseline migrations in order and stamp the resulting `user_version`. `migrations[i]` upgrades + * from version i+1 to i+2, so the target version is `BASELINE_SCHEMA_VERSION + migrations.length`. Returns the + * resulting version. + */ +export function applySchemaMigrations(db: DatabaseSync, migrations?: SchemaMigration[]): number; diff --git a/packages/gittensory-miner/lib/schema-version.js b/packages/gittensory-miner/lib/schema-version.js new file mode 100644 index 0000000000..8a08eba6e6 --- /dev/null +++ b/packages/gittensory-miner/lib/schema-version.js @@ -0,0 +1,56 @@ +// Lightweight schema-versioning convention shared across the miner's local SQLite stores (#4832). +// +// Every store bootstraps its tables with `CREATE TABLE IF NOT EXISTS ...` but, until now, carried no +// `user_version`/migration mechanism at all — so an older on-disk file was silently reused with a stale shape. +// This module adds the missing convention without the weight of the main product's `migrations/` runner: each +// store's bootstrap schema is treated as version 1 (BASELINE_SCHEMA_VERSION), and a store's `migrations` array +// describes ONLY the changes AFTER that baseline (`migrations[i]` upgrades the schema from version i+1 to i+2). +// `applySchemaMigrations` reads the file's current `PRAGMA user_version`, runs exactly the pending migrations in +// order, and stamps the new version — so opening an older-schema file runs its outstanding migrations instead of +// silently continuing on an incompatible shape. A pre-versioning file (user_version 0) already carries the +// baseline tables (the idempotent `CREATE TABLE IF NOT EXISTS` ran), so it is treated as the baseline before any +// post-baseline migration is applied. Pure control flow over an injected `DatabaseSync` handle: no IO of its own +// beyond the PRAGMA read/write and the caller-supplied migration functions, and deterministic given the same +// handle + migration list. + +/** The bootstrap schema every store creates inline is, by convention, schema version 1. */ +export const BASELINE_SCHEMA_VERSION = 1; + +/** Read a store's current `PRAGMA user_version`, coercing any absent/invalid value to 0 (pre-versioning). */ +export function readSchemaVersion(db) { + const row = db.prepare("PRAGMA user_version").get(); + const raw = row ? Number(row.user_version) : 0; + return Number.isInteger(raw) && raw >= 0 ? raw : 0; +} + +/** + * Bring a store's on-disk schema up to date, then stamp its `user_version`. `migrations[i]` upgrades from + * version i+1 to i+2, so the target version is `BASELINE_SCHEMA_VERSION + migrations.length`. Every migration + * whose resulting version is above the file's current version runs, in order; a file already at (or past) the + * target runs none. Returns the resulting version. Never runs a migration twice (the stamped `user_version` + * gates re-runs on the next open) and never DOWNGRADES: a file written by newer code with more migrations is + * left at its higher version rather than stamped back down. + * + * @param {import("node:sqlite").DatabaseSync} db - an open store handle whose baseline tables already exist. + * @param {Array<(db: import("node:sqlite").DatabaseSync) => void>} [migrations] - post-baseline migrations. + * @returns {number} the schema version after applying any pending migrations. + */ +export function applySchemaMigrations(db, migrations = []) { + const target = BASELINE_SCHEMA_VERSION + migrations.length; + const current = readSchemaVersion(db); + // A pre-versioning file (0) already holds the baseline schema, so advance from the baseline, not from 0. + const effective = current < BASELINE_SCHEMA_VERSION ? BASELINE_SCHEMA_VERSION : current; + for (let version = effective; version < target; version += 1) { + migrations[version - BASELINE_SCHEMA_VERSION](db); + } + if (current < target) { + // Only ever stamp when UPGRADING. A file written by NEWER code carrying more migrations (current > target) + // must be left at its higher version: stamping it back down to `target` would corrupt its version and hide + // the migrations it already ran. `user_version` is an integer PRAGMA that cannot be parameterized; `target` + // is a computed integer, never caller text, so interpolating it is safe. + db.exec(`PRAGMA user_version = ${target}`); + } + // The resulting on-disk version: `target` after an upgrade, or the file's own higher version when it was + // written by newer code (never downgraded). + return Math.max(current, target); +} diff --git a/test/unit/miner-schema-version.test.ts b/test/unit/miner-schema-version.test.ts new file mode 100644 index 0000000000..17f8bf6671 --- /dev/null +++ b/test/unit/miner-schema-version.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from "vitest"; +import { DatabaseSync } from "node:sqlite"; +import { + applySchemaMigrations, + readSchemaVersion, + BASELINE_SCHEMA_VERSION, +} from "../../packages/gittensory-miner/lib/schema-version.js"; + +type Migration = (db: DatabaseSync) => void; + +/** A minimal store whose bootstrap table already exists (the `CREATE TABLE IF NOT EXISTS` convention). */ +function freshStore(): DatabaseSync { + const db = new DatabaseSync(":memory:"); + db.exec("CREATE TABLE IF NOT EXISTS t (id INTEGER PRIMARY KEY)"); + return db; +} + +describe("schema-version migration runner (#4832)", () => { + it("treats a pre-versioning file as version 0 and stamps the baseline when there are no migrations", () => { + const db = freshStore(); + expect(readSchemaVersion(db)).toBe(0); + expect(applySchemaMigrations(db, [])).toBe(BASELINE_SCHEMA_VERSION); + expect(readSchemaVersion(db)).toBe(1); + db.close(); + }); + + it("runs every pending migration in order on a pre-versioning file and stamps the target version", () => { + const db = freshStore(); + const calls: number[] = []; + const migrations: Migration[] = [ + (d) => { + d.exec("ALTER TABLE t ADD COLUMN a TEXT"); + calls.push(1); + }, + (d) => { + d.exec("ALTER TABLE t ADD COLUMN b TEXT"); + calls.push(2); + }, + ]; + expect(applySchemaMigrations(db, migrations)).toBe(3); // baseline 1 + 2 migrations + expect(calls).toEqual([1, 2]); + expect(readSchemaVersion(db)).toBe(3); + db.exec("INSERT INTO t (id, a, b) VALUES (1, 'x', 'y')"); // both added columns exist + db.close(); + }); + + it("is idempotent: re-applying the same migrations on an up-to-date file runs none and does not re-stamp", () => { + const db = freshStore(); + let runs = 0; + const migrations: Migration[] = [ + (d) => { + d.exec("ALTER TABLE t ADD COLUMN a TEXT"); + runs += 1; + }, + ]; + applySchemaMigrations(db, migrations); + expect(runs).toBe(1); + expect(applySchemaMigrations(db, migrations)).toBe(2); + expect(runs).toBe(1); // the already-applied migration did not run again + db.close(); + }); + + it("runs only the outstanding migrations when a file is partway through the history", () => { + const db = freshStore(); + // A prior release shipped one migration → file is at version 2. + applySchemaMigrations(db, [(d) => d.exec("ALTER TABLE t ADD COLUMN a TEXT")]); + expect(readSchemaVersion(db)).toBe(2); + const ran: string[] = []; + const migrations: Migration[] = [ + () => ran.push("0"), // already applied — must NOT run + (d) => { + d.exec("ALTER TABLE t ADD COLUMN b TEXT"); + ran.push("1"); + }, + ]; + expect(applySchemaMigrations(db, migrations)).toBe(3); + expect(ran).toEqual(["1"]); + db.close(); + }); + + it("never downgrades a file written by newer code (current > target): no migrations run, version unchanged", () => { + const db = freshStore(); + // Newer code (2 migrations) stamped this file at version 3. + applySchemaMigrations(db, [ + (d) => d.exec("ALTER TABLE t ADD COLUMN a TEXT"), + (d) => d.exec("ALTER TABLE t ADD COLUMN b TEXT"), + ]); + expect(readSchemaVersion(db)).toBe(3); + // Older code that only knows one migration opens the same file: it must run nothing and must NOT stamp the + // version back down to its own target of 2. + let ran = 0; + const resulting = applySchemaMigrations(db, [ + () => { + ran += 1; + }, + ]); + expect(ran).toBe(0); + expect(readSchemaVersion(db)).toBe(3); // left at the newer version, not downgraded to 2 + expect(resulting).toBe(3); // reports the file's actual (higher) version + db.close(); + }); + + it("coerces an absent, non-integer, or negative user_version to 0", () => { + const absent = { prepare: () => ({ get: () => undefined }) } as unknown as DatabaseSync; + expect(readSchemaVersion(absent)).toBe(0); + const nonInteger = { prepare: () => ({ get: () => ({ user_version: "oops" }) }) } as unknown as DatabaseSync; + expect(readSchemaVersion(nonInteger)).toBe(0); + const negative = { prepare: () => ({ get: () => ({ user_version: -3 }) }) } as unknown as DatabaseSync; + expect(readSchemaVersion(negative)).toBe(0); + }); +});