From 663c4fba10536a7148749f2b35fb5af6d54d3cb7 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 25 Jul 2026 04:18:06 +0800 Subject: [PATCH] fix(sqlite): repair canonical indexes before database use (#113390) * fix(sqlite): recover canonical indexes before open * chore(sqlite): keep index contract types internal * test(sqlite): keep unsafe index corruption fail closed --- config/env-var-count-budget.txt | 2 +- .../doctor-session-sqlite-recover-report.ts | 60 +++++- src/commands/doctor-session-sqlite.test.ts | 73 +++++++ src/infra/sqlite-index-schema.test.ts | 203 +++++++++++++++--- src/infra/sqlite-index-schema.ts | 170 +++++++++++---- src/infra/sqlite-schema-contract.ts | 92 +++++++- src/state/openclaw-agent-db-maintenance.ts | 12 +- src/state/openclaw-agent-db-schema.ts | 87 ++------ src/state/openclaw-agent-db.test.ts | 125 +++++++++-- src/state/openclaw-database-verify.test.ts | 14 +- src/state/openclaw-state-db.test.ts | 79 ++++++- src/state/openclaw-state-db.ts | 75 +++---- src/state/sqlite-schema-shape.test-support.ts | 20 +- 13 files changed, 768 insertions(+), 244 deletions(-) diff --git a/config/env-var-count-budget.txt b/config/env-var-count-budget.txt index ed0c89a8e515..84d268f02b52 100644 --- a/config/env-var-count-budget.txt +++ b/config/env-var-count-budget.txt @@ -1,3 +1,3 @@ # Distinct OPENCLAW_* names in production source under src, packages, and extensions. # Ratchet: lower this number when cleanup removes names; never raise it. -523 +522 diff --git a/src/commands/doctor-session-sqlite-recover-report.ts b/src/commands/doctor-session-sqlite-recover-report.ts index 7a8537bd39aa..3374accfef7e 100644 --- a/src/commands/doctor-session-sqlite-recover-report.ts +++ b/src/commands/doctor-session-sqlite-recover-report.ts @@ -7,6 +7,12 @@ import type { SessionStoreTarget } from "../config/sessions/targets.js"; import { requireNodeSqlite, resolveNodeSqliteLocation } from "../infra/node-sqlite.js"; import { resolveSqliteDatabaseFilePaths } from "../infra/sqlite-files.js"; import { assertSqliteIntegrity } from "../infra/sqlite-integrity.js"; +import { getCanonicalSqliteNamedIndexContracts } from "../infra/sqlite-schema-contract.js"; +import { + clearOpenClawAgentDatabaseOpenFailure, + migrateOpenClawAgentDatabaseForMaintenance, +} from "../state/openclaw-agent-db.js"; +import { OPENCLAW_AGENT_SCHEMA_SQL } from "../state/openclaw-agent-schema.generated.js"; import { OPENCLAW_SQLITE_BUSY_TIMEOUT_MS } from "../state/openclaw-state-db.js"; import { createSessionSqliteMigrationFailureIssue, @@ -27,6 +33,10 @@ type SessionSqliteRecoverTargetValidator = ( target: SessionStoreTarget, ) => Promise; +const CANONICAL_AGENT_INDEX_NAMES = getCanonicalSqliteNamedIndexContracts( + OPENCLAW_AGENT_SCHEMA_SQL, +).map((index) => index.name); + /** Restores the latest failed migration run and validates only selected manifest targets. */ export async function recoverDoctorSessionSqliteTargets(params: { env: NodeJS.ProcessEnv; @@ -37,7 +47,7 @@ export async function recoverDoctorSessionSqliteTargets(params: { const trustedTargets = resolveRecoverTargets(params.targets); const failedRun = findLatestFailedSessionSqliteMigrationManifest(params.env, trustedTargets); if (!failedRun) { - const recoveredCorruptTargets = recoverCorruptSqliteTargets(params.targets); + const recoveredCorruptTargets = recoverCorruptSqliteTargets(params.targets, params.env); if (recoveredCorruptTargets.length > 0) { return summarizeRecoverReport(recoveredCorruptTargets); } @@ -89,6 +99,7 @@ export async function recoverDoctorSessionSqliteTargets(params: { function recoverCorruptSqliteTargets( targets: readonly SessionStoreTarget[], + env: NodeJS.ProcessEnv, ): DoctorSessionSqliteTargetReport[] { return targets.flatMap((target) => { const sqlitePath = resolveTargetSqlitePath(target); @@ -117,10 +128,50 @@ function recoverCorruptSqliteTargets( if (!isSqliteCorruptionError(inspection.error)) { return [createRecoverInspectionFailureTargetReport(target, sqlitePath, inspection.error)]; } + if (!isCanonicalAgentIndexCorruptionError(inspection.error)) { + return [recoverCorruptSqliteTarget(target, sqlitePath, inspection.error)]; + } + const repair = repairCanonicalIndexesForRecovery(target, sqlitePath, env); + if (repair.ok) { + return [createEmptyRecoverTargetReport(target, sqlitePath)]; + } + if (repair.preserveOriginal) { + return [createRecoverInspectionFailureTargetReport(target, sqlitePath, repair.error)]; + } return [recoverCorruptSqliteTarget(target, sqlitePath, inspection.error)]; }); } +function repairCanonicalIndexesForRecovery( + target: SessionStoreTarget, + sqlitePath: string, + env: NodeJS.ProcessEnv, +): { ok: true } | { error: unknown; ok: false; preserveOriginal: boolean } { + try { + migrateOpenClawAgentDatabaseForMaintenance({ + agentId: target.agentId, + pathname: sqlitePath, + }); + } catch (error) { + return { error, ok: false, preserveOriginal: false }; + } + const sourcePaths = inspectSqliteRecoveryFiles(sqlitePath).existing; + const inspection = inspectSqliteForRecovery(sqlitePath, sourcePaths); + if (!inspection.ok) { + return { error: inspection.error, ok: false, preserveOriginal: false }; + } + if (!clearOpenClawAgentDatabaseOpenFailure(sqlitePath, { env })) { + return { + error: new Error( + `Repaired canonical SQLite indexes, but could not clear the quarantine for ${sqlitePath}.`, + ), + ok: false, + preserveOriginal: true, + }; + } + return { ok: true }; +} + function inspectSqliteForRecovery( sqlitePath: string, sourcePaths: readonly string[], @@ -313,6 +364,13 @@ function isSqliteCorruptionError(error: unknown): boolean { ); } +function isCanonicalAgentIndexCorruptionError(error: unknown): boolean { + if (!(error instanceof Error) || error.name !== "SqliteIntegrityError") { + return false; + } + return CANONICAL_AGENT_INDEX_NAMES.some((indexName) => error.message.includes(indexName)); +} + function resolveRecoverTargets( targets: readonly SessionStoreTarget[], ): SessionSqliteMigrationTargetInput[] { diff --git a/src/commands/doctor-session-sqlite.test.ts b/src/commands/doctor-session-sqlite.test.ts index a44bb7a16f43..22b5ff9c5fcc 100644 --- a/src/commands/doctor-session-sqlite.test.ts +++ b/src/commands/doctor-session-sqlite.test.ts @@ -850,6 +850,46 @@ describe("runDoctorSessionSqlite", () => { expect(openOpenClawAgentDatabase({ agentId: "main", env: store.env }).db.isOpen).toBe(true); }); + it("repairs canonical index corruption in place during recovery", async () => { + const { sqlitePath, store } = await createImportedStoreForCompaction(); + createCanonicalCacheIndexDrift(sqlitePath); + expect( + recordOpenClawDatabaseQuarantine({ + env: store.env, + kind: "agent", + path: sqlitePath, + reason: "canonical cache index drift", + }), + ).toBe(true); + + const report = await runDoctorSessionSqlite({ + env: store.env, + mode: "recover", + store: store.storePath, + }); + + expect(report.totals.issues).toBe(0); + expect(report.targets[0]?.corruptRecovery).toBeUndefined(); + expect(fs.existsSync(sqlitePath)).toBe(true); + expect(readOpenClawDatabaseQuarantine(sqlitePath, { env: store.env })).toBeUndefined(); + + const sqlite = nodeSqlite.requireNodeSqlite(); + const database = new sqlite.DatabaseSync(sqlitePath, { readOnly: true }); + try { + expect(database.prepare("PRAGMA integrity_check").get()).toEqual({ + integrity_check: "ok", + }); + expect( + database + .prepare("SELECT value_json FROM cache_entries WHERE scope = ? AND key = ?") + .get("doctor", "canonical-index"), + ).toEqual({ value_json: '{"ok":true}' }); + } finally { + database.close(); + } + expect(openOpenClawAgentDatabase({ agentId: "main", env: store.env }).db.isOpen).toBe(true); + }); + it.skipIf(process.platform === "win32")( "reapplies owner-only permissions after compaction", async () => { @@ -2841,6 +2881,39 @@ function createUnsafeIndexDrift(sqlitePath: string): void { } } +function createCanonicalCacheIndexDrift(sqlitePath: string): void { + const sqlite = nodeSqlite.requireNodeSqlite(); + const database = new sqlite.DatabaseSync(sqlitePath); + try { + database.exec(` + INSERT INTO cache_entries (scope, key, value_json, expires_at, updated_at) + VALUES ('doctor', 'canonical-index', '{"ok":true}', 100, 1); + DROP INDEX idx_agent_cache_expiry; + CREATE INDEX idx_agent_cache_expiry ON cache_entries(key); + `); + database.enableDefensive?.(false); + database.exec("PRAGMA writable_schema = ON;"); + database + .prepare( + `UPDATE sqlite_schema + SET sql = 'CREATE INDEX idx_agent_cache_expiry ON cache_entries(scope, expires_at, key) WHERE expires_at IS NOT NULL' + WHERE name = 'idx_agent_cache_expiry'`, + ) + .run(); + database.exec("PRAGMA writable_schema = OFF;"); + const schemaVersionRow = database.prepare("PRAGMA schema_version;").get() as + | Record + | undefined; + const schemaVersion = Number( + schemaVersionRow?.schema_version ?? + (schemaVersionRow ? Object.values(schemaVersionRow)[0] : undefined), + ); + database.exec(`PRAGMA schema_version = ${schemaVersion + 1};`); + } finally { + database.close(); + } +} + function createLegacyStore( params: { agentDirName?: string; diff --git a/src/infra/sqlite-index-schema.test.ts b/src/infra/sqlite-index-schema.test.ts index 08cd3dc7c757..c2555eefd56d 100644 --- a/src/infra/sqlite-index-schema.test.ts +++ b/src/infra/sqlite-index-schema.test.ts @@ -1,48 +1,37 @@ import { DatabaseSync } from "node:sqlite"; import { describe, expect, it } from "vitest"; -import { - repairCanonicalSqliteUniqueIndexes, - type CanonicalSqliteUniqueIndex, -} from "./sqlite-index-schema.js"; +import { repairCanonicalSqliteIndexes } from "./sqlite-index-schema.js"; -const CANONICAL_INDEX: CanonicalSqliteUniqueIndex = { - name: "idx_records_identity", - tableName: "records", - definition: ` +const CANONICAL_SCHEMA = ` + CREATE TABLE records ( + id INTEGER PRIMARY KEY, + tenant_id TEXT NOT NULL, + external_id TEXT, + active INTEGER NOT NULL + ); + CREATE UNIQUE INDEX IF NOT EXISTS idx_records_identity ON records( tenant_id COLLATE NOCASE, IFNULL(external_id, '') ) - WHERE active = 1 - `, -}; + WHERE active = 1; + CREATE INDEX IF NOT EXISTS idx_records_active_lookup + ON records(active, tenant_id); +`; function createDatabase(): DatabaseSync { const db = new DatabaseSync(":memory:"); - db.exec(` - CREATE TABLE records ( - id INTEGER PRIMARY KEY, - tenant_id TEXT NOT NULL, - external_id TEXT, - active INTEGER NOT NULL - ); - CREATE UNIQUE INDEX IF NOT EXISTS idx_records_identity - ON records( - tenant_id COLLATE NOCASE, - IFNULL(external_id, '') - ) - WHERE active = 1; - `); + db.exec(CANONICAL_SCHEMA); return db; } -describe("repairCanonicalSqliteUniqueIndexes", () => { +describe("repairCanonicalSqliteIndexes", () => { it("does not rewrite an already canonical index", () => { const db = createDatabase(); try { const before = db.prepare("PRAGMA schema_version").get(); - repairCanonicalSqliteUniqueIndexes(db, "test database", [CANONICAL_INDEX]); + repairCanonicalSqliteIndexes(db, "test database", CANONICAL_SCHEMA); expect(db.prepare("PRAGMA schema_version").get()).toEqual(before); } finally { @@ -73,7 +62,7 @@ describe("repairCanonicalSqliteUniqueIndexes", () => { try { db.exec(`DROP INDEX idx_records_identity; ${driftedSql};`); - repairCanonicalSqliteUniqueIndexes(db, "test database", [CANONICAL_INDEX]); + repairCanonicalSqliteIndexes(db, "test database", CANONICAL_SCHEMA); const row = db .prepare("SELECT sql FROM sqlite_schema WHERE name = 'idx_records_identity'") @@ -103,9 +92,9 @@ describe("repairCanonicalSqliteUniqueIndexes", () => { (2, 'tenant', NULL, 1); `); - expect(() => - repairCanonicalSqliteUniqueIndexes(db, "test database", [CANONICAL_INDEX]), - ).toThrow(/canonical unique index idx_records_identity failed.*UNIQUE constraint failed/iu); + expect(() => repairCanonicalSqliteIndexes(db, "test database", CANONICAL_SCHEMA)).toThrow( + /canonical index idx_records_identity failed.*UNIQUE constraint failed/iu, + ); expect( db.prepare("SELECT sql FROM sqlite_schema WHERE name = 'idx_records_identity'").get(), @@ -170,7 +159,7 @@ describe("repairCanonicalSqliteUniqueIndexes", () => { .all(), ).toEqual([]); - repairCanonicalSqliteUniqueIndexes(db, "test database", [CANONICAL_INDEX]); + repairCanonicalSqliteIndexes(db, "test database", CANONICAL_SCHEMA); expect(db.prepare("PRAGMA integrity_check").get()).toEqual({ integrity_check: "ok" }); expect( @@ -192,6 +181,154 @@ describe("repairCanonicalSqliteUniqueIndexes", () => { } }); + it("repairs same-name ordinary index definition drift", () => { + const db = createDatabase(); + try { + db.exec(` + DROP INDEX idx_records_active_lookup; + CREATE INDEX idx_records_active_lookup ON records(tenant_id, active); + `); + + repairCanonicalSqliteIndexes(db, "test database", CANONICAL_SCHEMA); + + expect( + db.prepare("SELECT sql FROM sqlite_schema WHERE name = 'idx_records_active_lookup'").get(), + ).toEqual({ + sql: "CREATE INDEX idx_records_active_lookup ON records(active, tenant_id)", + }); + } finally { + db.close(); + } + }); + + it("removes bogus uniqueness from a canonical ordinary index", () => { + const db = createDatabase(); + try { + db.exec(` + DROP INDEX idx_records_active_lookup; + CREATE UNIQUE INDEX idx_records_active_lookup ON records(active, tenant_id); + `); + + repairCanonicalSqliteIndexes(db, "test database", CANONICAL_SCHEMA); + + expect( + ( + db.prepare("PRAGMA index_list(records)").all() as Array<{ + name: string; + unique: number; + }> + ).find((index) => index.name === "idx_records_active_lookup"), + ).toMatchObject({ unique: 0 }); + expect(() => + db.exec(` + INSERT INTO records VALUES (1, 'Tenant', 'one', 0); + INSERT INTO records VALUES (2, 'Tenant', 'two', 0); + `), + ).not.toThrow(); + } finally { + db.close(); + } + }); + + it("repairs physical ordinary-index drift hidden behind canonical schema text", () => { + const db = createDatabase(); + try { + db.exec(` + DROP INDEX idx_records_active_lookup; + CREATE INDEX idx_records_active_lookup ON records(id); + INSERT INTO records VALUES + (1, 'Tenant', NULL, 1), + (2, 'Other', NULL, 1); + `); + db.enableDefensive?.(false); + db.exec("PRAGMA writable_schema = ON;"); + db.prepare("UPDATE sqlite_schema SET sql = ? WHERE name = 'idx_records_active_lookup'").run( + "CREATE INDEX idx_records_active_lookup ON records(active, tenant_id)", + ); + db.exec("PRAGMA writable_schema = OFF;"); + const schemaVersion = db.prepare("PRAGMA schema_version").get() as { + schema_version?: unknown; + }; + db.exec(`PRAGMA schema_version = ${Number(schemaVersion.schema_version) + 1};`); + + expect(db.prepare("PRAGMA integrity_check('records')").all()).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + integrity_check: expect.stringMatching(/idx_records_active_lookup/), + }), + ]), + ); + expect( + db + .prepare( + `SELECT id + FROM records INDEXED BY idx_records_active_lookup + WHERE active = 1 AND tenant_id = 'Tenant'`, + ) + .all(), + ).toEqual([]); + + repairCanonicalSqliteIndexes(db, "test database", CANONICAL_SCHEMA); + + expect(db.prepare("PRAGMA integrity_check").get()).toEqual({ integrity_check: "ok" }); + expect( + db + .prepare( + `SELECT id + FROM records INDEXED BY idx_records_active_lookup + WHERE active = 1 AND tenant_id = 'Tenant'`, + ) + .all(), + ).toEqual([{ id: 1 }]); + } finally { + db.close(); + } + }); + + it("rejects an unexpected named unique index", () => { + const db = createDatabase(); + try { + db.exec("CREATE UNIQUE INDEX idx_records_unexpected_unique ON records(active, id);"); + + expect(() => repairCanonicalSqliteIndexes(db, "test database", CANONICAL_SCHEMA)).toThrow( + "unexpected unique index idx_records_unexpected_unique", + ); + } finally { + db.close(); + } + }); + + it("defers only indexes whose columns are owned by a pending migration", () => { + const db = new DatabaseSync(":memory:"); + try { + db.exec(` + CREATE TABLE records ( + id INTEGER PRIMARY KEY, + tenant_id TEXT NOT NULL, + external_id TEXT + ); + `); + + expect(() => repairCanonicalSqliteIndexes(db, "test database", CANONICAL_SCHEMA)).toThrow( + /no such column: active/iu, + ); + expect( + repairCanonicalSqliteIndexes(db, "test database", CANONICAL_SCHEMA, { + allowMissingColumns: true, + }), + ).toEqual([]); + + db.exec("ALTER TABLE records ADD COLUMN active INTEGER NOT NULL DEFAULT 0;"); + expect( + repairCanonicalSqliteIndexes(db, "test database", CANONICAL_SCHEMA, { + allowMissingColumns: true, + }), + ).toEqual(["idx_records_active_lookup", "idx_records_identity"]); + } finally { + db.close(); + } + }); + it("repairs only the main schema when a temporary index has the same name", () => { const db = createDatabase(); try { @@ -202,7 +339,7 @@ describe("repairCanonicalSqliteUniqueIndexes", () => { CREATE UNIQUE INDEX main.idx_records_identity ON records(id); `); - repairCanonicalSqliteUniqueIndexes(db, "test database", [CANONICAL_INDEX]); + repairCanonicalSqliteIndexes(db, "test database", CANONICAL_SCHEMA); expect( db.prepare("SELECT sql FROM main.sqlite_schema WHERE name = 'idx_records_identity'").get(), diff --git a/src/infra/sqlite-index-schema.ts b/src/infra/sqlite-index-schema.ts index 9031277b2302..96ee36f90e33 100644 --- a/src/infra/sqlite-index-schema.ts +++ b/src/infra/sqlite-index-schema.ts @@ -1,62 +1,84 @@ import type { DatabaseSync } from "node:sqlite"; -import { assertSqliteIntegrity, assertSqliteTableIntegrity } from "./sqlite-integrity.js"; - -export type CanonicalSqliteUniqueIndex = { - name: string; - tableName: string; - definition: string; -}; - -type SqliteSchemaRow = { - sql?: unknown; -}; +import { + assertSqliteIntegrity, + assertSqliteTableIntegrity, + isTerminalSqliteIntegrityError, +} from "./sqlite-integrity.js"; +import { + collectSqliteNamedIndexContract, + getCanonicalSqliteNamedIndexContracts, + getCanonicalSqliteTableNames, + type CanonicalSqliteNamedIndexContract, +} from "./sqlite-schema-contract.js"; const SQLITE_IDENTIFIER_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/u; +type SqliteIndexListRow = { + name: string; + origin: string; + unique: number; +}; + /** - * Restore named unique indexes when SQLite's IF NOT EXISTS semantics preserve - * a same-name definition that no longer enforces the canonical constraint. + * Restore every named index when SQLite's IF NOT EXISTS semantics preserve a + * same-name definition or b-tree that no longer matches the committed schema. */ -export function repairCanonicalSqliteUniqueIndexes( +export function repairCanonicalSqliteIndexes( db: DatabaseSync, databaseLabel: string, - indexes: readonly CanonicalSqliteUniqueIndex[], -): void { - const indexesByTable = new Map(); - const repairIndexes = new Set(); + schemaSql: string, + options: { + /** + * A recognized schema migration may add a column before recreating its + * canonical index. No other repair failure is deferred. + */ + allowMissingColumns?: boolean; + verifyPhysicalIntegrity?: boolean; + } = {}, +): string[] { + const indexes = getCanonicalSqliteNamedIndexContracts(schemaSql); + const indexesByTable = new Map(); + const integrityFailuresByTable = new Map(); + const repairIndexes = new Set(); for (const index of indexes) { assertSqliteIdentifier(index.name); assertSqliteIdentifier(index.tableName); + const tableExists = db + .prepare("SELECT 1 FROM main.sqlite_schema WHERE type = 'table' AND name = ?") + .get(index.tableName); + if (!tableExists) { + continue; + } const tableIndexes = indexesByTable.get(index.tableName) ?? []; tableIndexes.push(index); indexesByTable.set(index.tableName, tableIndexes); - const row = db - .prepare("SELECT sql FROM main.sqlite_schema WHERE type = 'index' AND name = ?") - .get(index.name) as SqliteSchemaRow | undefined; - if ( - typeof row?.sql !== "string" || - normalizeCreateIndexSql(row.sql) !== - normalizeCreateIndexSql(createIndexSql(index, index.name, false)) - ) { + const actual = collectSqliteNamedIndexContract(db, index.name); + if (!isEqual(actual, index.fingerprint)) { repairIndexes.add(index); } } + assertNoUnexpectedUniqueIndexes(db, databaseLabel, schemaSql, indexesByTable); - for (const [tableName, tableIndexes] of indexesByTable) { - try { - assertSqliteTableIntegrity(db, databaseLabel, tableName); - } catch { - for (const index of tableIndexes) { - repairIndexes.add(index); + if (options.verifyPhysicalIntegrity !== false) { + for (const [tableName, tableIndexes] of indexesByTable) { + try { + assertSqliteTableIntegrity(db, databaseLabel, tableName); + } catch (error) { + if (error instanceof Error) { + integrityFailuresByTable.set(tableName, error); + } + for (const index of tableIndexes) { + repairIndexes.add(index); + } } } } if (repairIndexes.size === 0) { - return; + return []; } - const savepoint = "repair_canonical_unique_indexes"; - let activeIndex: CanonicalSqliteUniqueIndex | undefined; + const savepoint = "repair_canonical_indexes"; + let activeIndex: CanonicalSqliteNamedIndexContract | undefined; db.exec(`SAVEPOINT ${savepoint};`); try { for (const index of repairIndexes) { @@ -64,11 +86,23 @@ export function repairCanonicalSqliteUniqueIndexes( const probeName = findUnusedProbeIndexName(db, index.name); // Build the canonical constraint first. If existing rows conflict, the // wrong same-name index remains in place and the whole repair rolls back. - db.exec(createIndexSql(index, probeName, true)); + try { + db.exec(createIndexSql(index, probeName, true)); + } catch (error) { + if (options.allowMissingColumns && isMissingColumnError(error)) { + repairIndexes.delete(index); + continue; + } + throw error; + } db.exec(`DROP INDEX IF EXISTS main.${index.name};`); db.exec(createIndexSql(index, index.name, true)); db.exec(`DROP INDEX main.${probeName};`); } + if (repairIndexes.size === 0) { + db.exec(`RELEASE SAVEPOINT ${savepoint};`); + return []; + } for (const tableName of indexesByTable.keys()) { assertSqliteTableIntegrity(db, databaseLabel, tableName); } @@ -80,21 +114,62 @@ export function repairCanonicalSqliteUniqueIndexes( } finally { db.exec(`RELEASE SAVEPOINT ${savepoint};`); } + if (error instanceof Error && isTerminalSqliteIntegrityError(error)) { + throw error; + } + const tableIntegrityFailure = activeIndex + ? integrityFailuresByTable.get(activeIndex.tableName) + : undefined; + if (tableIntegrityFailure && isTerminalSqliteIntegrityError(tableIntegrityFailure)) { + throw tableIntegrityFailure; + } const detail = error instanceof Error ? error.message : String(error); throw new Error( - `SQLite canonical unique index ${activeIndex?.name ?? "repair"} failed for ${databaseLabel}: ${detail}`, + `SQLite canonical index ${activeIndex?.name ?? "repair"} failed for ${databaseLabel}: ${detail}`, { cause: error }, ); } + return [...repairIndexes].map((index) => index.name).toSorted(); +} + +function assertNoUnexpectedUniqueIndexes( + db: DatabaseSync, + databaseLabel: string, + schemaSql: string, + indexesByTable: ReadonlyMap, +): void { + for (const tableName of getCanonicalSqliteTableNames(schemaSql)) { + assertSqliteIdentifier(tableName); + const tableExists = db + .prepare("SELECT 1 FROM main.sqlite_schema WHERE type = 'table' AND name = ?") + .get(tableName); + if (!tableExists) { + continue; + } + const canonicalIndexNames = new Set( + (indexesByTable.get(tableName) ?? []).map((index) => index.name), + ); + const unexpected = ( + db.prepare(`PRAGMA main.index_list(${tableName})`).all() as SqliteIndexListRow[] + ).find( + (index) => index.unique === 1 && index.origin === "c" && !canonicalIndexNames.has(index.name), + ); + if (unexpected) { + throw new Error( + `SQLite schema is incomplete or noncanonical for ${databaseLabel}: unexpected unique index ${unexpected.name}`, + ); + } + } } function createIndexSql( - index: CanonicalSqliteUniqueIndex, + index: CanonicalSqliteNamedIndexContract, name: string, qualifyMain: boolean, ): string { assertSqliteIdentifier(name); - return `CREATE UNIQUE INDEX ${qualifyMain ? `main.${name}` : name} ${index.definition};`; + const create = index.unique ? "CREATE UNIQUE INDEX" : "CREATE INDEX"; + return `${create} ${qualifyMain ? `main.${name}` : name} ${index.definition};`; } function findUnusedProbeIndexName(db: DatabaseSync, canonicalName: string): string { @@ -117,11 +192,14 @@ function assertSqliteIdentifier(identifier: string): void { } } -function normalizeCreateIndexSql(sql: string): string { - return sql - .trim() - .replace(/;\s*$/u, "") - .replace(/^CREATE\s+UNIQUE\s+INDEX\s+(?:IF\s+NOT\s+EXISTS\s+)?/iu, "CREATE UNIQUE INDEX ") - .replace(/\s+/gu, " ") - .trim(); +function isMissingColumnError(error: unknown): boolean { + return ( + error instanceof Error && + (error as NodeJS.ErrnoException).code === "ERR_SQLITE_ERROR" && + /^no such column:/iu.test(error.message) + ); +} + +function isEqual(left: unknown, right: unknown): boolean { + return JSON.stringify(left) === JSON.stringify(right); } diff --git a/src/infra/sqlite-schema-contract.ts b/src/infra/sqlite-schema-contract.ts index 190774f71cb5..a3ab134b1060 100644 --- a/src/infra/sqlite-schema-contract.ts +++ b/src/infra/sqlite-schema-contract.ts @@ -24,6 +24,7 @@ type SqliteIndexTermContract = Omit & { type SqliteSchemaRow = { name: string; sql: string | null; + tbl_name?: string; }; type SqliteTableListRow = { @@ -57,6 +58,14 @@ type SqliteTableContract = { type SqliteSchemaContract = Map; +export type CanonicalSqliteNamedIndexContract = { + definition: string; + fingerprint: SqliteIndexContract; + name: string; + tableName: string; + unique: boolean; +}; + export type SqliteSchemaCompatibility = { /** * Exact definitions produced by supported additive migrations when SQLite @@ -89,11 +98,7 @@ export function assertSqliteSchemaContains( schemaSql: string, compatibility: SqliteSchemaCompatibility = {}, ): void { - let expected = schemaContractCache.get(schemaSql); - if (!expected) { - expected = buildSqliteSchemaContract(schemaSql); - schemaContractCache.set(schemaSql, expected); - } + const expected = getSqliteSchemaContract(schemaSql); const mismatches: string[] = []; for (const [tableName, expectedTable] of expected) { @@ -184,6 +189,53 @@ export function assertSqliteSchemaContains( } } +/** Return every explicit named index owned by one committed schema. */ +export function getCanonicalSqliteNamedIndexContracts( + schemaSql: string, +): CanonicalSqliteNamedIndexContract[] { + const schema = getSqliteSchemaContract(schemaSql); + const indexes: CanonicalSqliteNamedIndexContract[] = []; + for (const [tableName, table] of schema) { + for (const fingerprint of table.indexes) { + if (fingerprint.name === null || fingerprint.sql === null || fingerprint.origin !== "c") { + continue; + } + indexes.push({ + definition: readCanonicalIndexDefinition(fingerprint), + fingerprint, + name: fingerprint.name, + tableName, + unique: fingerprint.unique === 1, + }); + } + } + return indexes; +} + +/** Return every table owned by one committed schema. */ +export function getCanonicalSqliteTableNames(schemaSql: string): string[] { + return [...getSqliteSchemaContract(schemaSql).keys()]; +} + +/** Inspect one explicit main-schema index using the canonical schema fingerprint shape. */ +export function collectSqliteNamedIndexContract( + database: DatabaseSync, + indexName: string, +): SqliteIndexContract | undefined { + const row = database + .prepare("SELECT name, sql, tbl_name FROM main.sqlite_schema WHERE type = 'index' AND name = ?") + .get(indexName) as SqliteSchemaRow | undefined; + if (!row || typeof row.tbl_name !== "string") { + return undefined; + } + const index = ( + database.prepare(`PRAGMA main.index_list(${quoteSqliteIdentifier(row.tbl_name)})`).all() as + | SqliteIndexListRow[] + | undefined + )?.find((candidate) => candidate.name === indexName); + return index ? collectSqliteIndexContract(database, index) : undefined; +} + function collectOptionalCanonicalTriggerGroups( compatibility: SqliteSchemaCompatibility, tableName: string, @@ -203,6 +255,15 @@ function normalizeOptionalCanonicalTriggerSql(sql: string): string | null { return normalizeSchemaSql(sql)?.replace(/^(CREATE TRIGGER) main\./iu, "$1 ") ?? null; } +function getSqliteSchemaContract(schemaSql: string): SqliteSchemaContract { + let expected = schemaContractCache.get(schemaSql); + if (!expected) { + expected = buildSqliteSchemaContract(schemaSql); + schemaContractCache.set(schemaSql, expected); + } + return expected; +} + function buildSqliteSchemaContract(schemaSql: string): SqliteSchemaContract { const sqlite = requireNodeSqlite(); const database = new sqlite.DatabaseSync(":memory:"); @@ -233,6 +294,27 @@ function buildSqliteSchemaContract(schemaSql: string): SqliteSchemaContract { } } +function readCanonicalIndexDefinition(index: SqliteIndexContract): string { + if (index.name === null || index.sql === null) { + throw new Error("Canonical SQLite named index is missing its schema definition."); + } + const createPrefix = + index.unique === 1 ? /^CREATE\s+UNIQUE\s+INDEX\s+/iu : /^CREATE\s+INDEX\s+/iu; + const prefix = createPrefix.exec(index.sql); + if (!prefix) { + throw new Error(`Canonical SQLite index ${index.name} has an unreadable definition.`); + } + const name = readSqlToken(index.sql, prefix[0].length); + if (!name || normalizeSqlIdentifier(name.raw) !== index.name.toLowerCase()) { + throw new Error(`Canonical SQLite index ${index.name} has an unexpected schema name.`); + } + const definition = index.sql.slice(name.end).trim(); + if (!/^ON\s+/iu.test(definition)) { + throw new Error(`Canonical SQLite index ${index.name} has an unreadable target.`); + } + return definition; +} + function collectSqliteTableContract( database: DatabaseSync, tableName: string, diff --git a/src/state/openclaw-agent-db-maintenance.ts b/src/state/openclaw-agent-db-maintenance.ts index a5ea8d9885eb..f0bcfdcfb369 100644 --- a/src/state/openclaw-agent-db-maintenance.ts +++ b/src/state/openclaw-agent-db-maintenance.ts @@ -5,6 +5,7 @@ import { } from "../../packages/memory-host-sdk/src/host/memory-schema.js"; import { clearNodeSqliteKyselyCacheForDatabase } from "../infra/kysely-sync.js"; import { requireNodeSqlite, resolveNodeSqliteLocation } from "../infra/node-sqlite.js"; +import { repairCanonicalSqliteIndexes } from "../infra/sqlite-index-schema.js"; import { assertSqliteSchemaContains, type SqliteSchemaCompatibility, @@ -91,7 +92,7 @@ export function assertOpenClawAgentDatabaseForMaintenance( ); } -/** Upgrade a supported older owned schema before strict offline maintenance. */ +/** Upgrade or repair a supported owned schema before strict offline maintenance. */ export function migrateOpenClawAgentDatabaseForMaintenance(options: { agentId: string; pathname: string; @@ -109,6 +110,9 @@ export function migrateOpenClawAgentDatabaseForMaintenance(options: { assertSupportedAgentSchemaVersion(database, options.pathname); const userVersion = readSqliteUserVersion(database); const metadataVersion = metadata.schemaVersion; + const hasCurrentVersion = + userVersion === OPENCLAW_AGENT_SCHEMA_VERSION && + metadataVersion === OPENCLAW_AGENT_SCHEMA_VERSION; const hasSupportedOlderVersion = userVersion >= 1 && userVersion < OPENCLAW_AGENT_SCHEMA_VERSION && @@ -116,7 +120,11 @@ export function migrateOpenClawAgentDatabaseForMaintenance(options: { metadataVersion === userVersion && metadataVersion >= 1 && metadataVersion < OPENCLAW_AGENT_SCHEMA_VERSION; - if (!hasSupportedOlderVersion) { + if (!hasCurrentVersion && !hasSupportedOlderVersion) { + return; + } + if (hasCurrentVersion) { + repairCanonicalSqliteIndexes(database, options.pathname, OPENCLAW_AGENT_SCHEMA_SQL); return; } ensureOpenClawAgentDatabaseSchema(database, { diff --git a/src/state/openclaw-agent-db-schema.ts b/src/state/openclaw-agent-db-schema.ts index 05c09903e584..2ab511dc1a44 100644 --- a/src/state/openclaw-agent-db-schema.ts +++ b/src/state/openclaw-agent-db-schema.ts @@ -1,11 +1,8 @@ import type { DatabaseSync } from "node:sqlite"; import { migrateMemoryIndexSourcesIdentity } from "../../packages/memory-host-sdk/src/host/memory-schema.js"; import { executeSqliteQuerySync, getNodeSqliteKysely } from "../infra/kysely-sync.js"; -import { - repairCanonicalSqliteUniqueIndexes, - type CanonicalSqliteUniqueIndex, -} from "../infra/sqlite-index-schema.js"; -import { assertSqliteIntegrity, assertSqliteTableIntegrity } from "../infra/sqlite-integrity.js"; +import { repairCanonicalSqliteIndexes } from "../infra/sqlite-index-schema.js"; +import { assertSqliteIntegrity } from "../infra/sqlite-integrity.js"; import { migrateSqliteSchemaToStrictInTransaction } from "../infra/sqlite-strict.js"; import { runSqliteImmediateTransactionSync } from "../infra/sqlite-transaction.js"; import { readSqliteUserVersion } from "../infra/sqlite-user-version.js"; @@ -42,52 +39,6 @@ import { OPENCLAW_AGENT_SCHEMA_SQL } from "./openclaw-agent-schema.generated.js" import { AGENT_SCHEMA_WITHOUT_LAZY_SURFACES_SQL } from "./openclaw-agent-session-sharing-schema.js"; import { OPENCLAW_SQLITE_BUSY_TIMEOUT_MS } from "./openclaw-state-db.js"; -const OPENCLAW_AGENT_CANONICAL_UNIQUE_INDEXES = [ - { - name: "idx_agent_conversations_identity", - tableName: "conversations", - definition: ` - ON conversations( - channel, - account_id, - kind, - peer_id, - IFNULL(parent_conversation_id, ''), - IFNULL(thread_id, '') - ) - `, - }, - { - name: "idx_agent_session_conversations_primary", - tableName: "session_conversations", - definition: ` - ON session_conversations(session_id) - WHERE role = 'primary' - `, - }, - { - name: "idx_agent_transcript_message_idempotency", - tableName: "transcript_event_identities", - definition: ` - ON transcript_event_identities(session_id, message_idempotency_key) - WHERE message_idempotency_key IS NOT NULL - `, - }, - { - name: "idx_agent_transcript_active_event_seq", - tableName: "session_transcript_active_events", - definition: "ON session_transcript_active_events(session_id, event_seq)", - }, - { - name: "idx_agent_transcript_active_messages", - tableName: "session_transcript_active_events", - definition: ` - ON session_transcript_active_events(session_id, message_position) - WHERE message_position IS NOT NULL - `, - }, -] as const satisfies readonly CanonicalSqliteUniqueIndex[]; - type OpenClawAgentMetadataDatabase = Pick; type MigratedSessionEntry = Record; @@ -503,25 +454,25 @@ export function assertAgentDatabaseIntegrityBeforeMutation( const hasApplicationSchema = database .prepare("SELECT 1 FROM sqlite_master WHERE name NOT LIKE 'sqlite_%' LIMIT 1") .get(); - if ( + const migrationPending = (userVersion === 0 && hasApplicationSchema) || - (userVersion > 0 && userVersion < OPENCLAW_AGENT_SCHEMA_VERSION) - ) { - // Migration rewrites the schema; prove the whole file before that mutation. - // Only a truly empty v0 file may skip; legacy v0 files need the same proof. + (userVersion > 0 && userVersion < OPENCLAW_AGENT_SCHEMA_VERSION); + if (migrationPending) { agentDbLog.info("agent database schema migration pending; verifying integrity first", { fromVersion: userVersion, path: pathname, toVersion: OPENCLAW_AGENT_SCHEMA_VERSION, }); - assertSqliteIntegrity(database, pathname); - return; } - const schemaMetaExists = database - .prepare("SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'schema_meta'") - .get(); - if (schemaMetaExists) { - assertSqliteTableIntegrity(database, pathname, "schema_meta"); + const rebuiltIndexes = + userVersion === OPENCLAW_AGENT_SCHEMA_VERSION + ? repairCanonicalSqliteIndexes(database, pathname, OPENCLAW_AGENT_SCHEMA_SQL, { + allowMissingColumns: true, + }) + : []; + if (rebuiltIndexes.length === 0) { + // Every physical open proves the full file before schema mutation or exposure. + assertSqliteIntegrity(database, pathname); } } @@ -555,11 +506,11 @@ function ensureAgentSchema(db: DatabaseSync, agentId: string, pathname: string): } backfillSessionEntryProvenance(db, previousVersion); migrateSessionNodesAndWindows(db, previousVersion); - db.exec( + const schemaSql = previousVersion === OPENCLAW_AGENT_SCHEMA_VERSION ? AGENT_SCHEMA_WITHOUT_LAZY_SURFACES_SQL - : OPENCLAW_AGENT_SCHEMA_SQL, - ); + : OPENCLAW_AGENT_SCHEMA_SQL; + db.exec(schemaSql); migrateSessionTranscriptGenerations(db, previousVersion); migrateSessionTranscriptActiveProjection(db, previousVersion); if (previousVersion < 11) { @@ -567,7 +518,9 @@ function ensureAgentSchema(db: DatabaseSync, agentId: string, pathname: string): databaseLabel: pathname, }); } - repairCanonicalSqliteUniqueIndexes(db, pathname, OPENCLAW_AGENT_CANONICAL_UNIQUE_INDEXES); + repairCanonicalSqliteIndexes(db, pathname, schemaSql, { + verifyPhysicalIntegrity: false, + }); const kysely = getNodeSqliteKysely(db); db.exec(`PRAGMA user_version = ${OPENCLAW_AGENT_SCHEMA_VERSION};`); const now = Date.now(); diff --git a/src/state/openclaw-agent-db.test.ts b/src/state/openclaw-agent-db.test.ts index 543f6fd38be8..e7f59fc77a4b 100644 --- a/src/state/openclaw-agent-db.test.ts +++ b/src/state/openclaw-agent-db.test.ts @@ -58,7 +58,7 @@ import { collectSqliteSchemaShape, createSqliteSchemaShapeFromSql, normalizeSqliteSchemaShapeSql, - replaceNamedUniqueIndexesWithOrdinaryIndexes, + replaceNamedIndexesWithNoncanonicalIndexes, } from "./sqlite-schema-shape.test-support.js"; type AgentDbTestDatabase = Pick< @@ -345,6 +345,39 @@ function createUnsafeIndexDrift(databasePath: string): void { } } +function createCacheExpiryIndexPhysicalDrift(databasePath: string): void { + const { DatabaseSync } = requireNodeSqlite(); + const database = new DatabaseSync(databasePath); + try { + database.exec(` + INSERT INTO cache_entries (scope, key, value_json, expires_at, updated_at) + VALUES ('scope-a', 'key-a', '{}', 100, 1); + DROP INDEX idx_agent_cache_expiry; + CREATE INDEX idx_agent_cache_expiry ON cache_entries(key); + `); + database.enableDefensive?.(false); + database.exec("PRAGMA writable_schema = ON;"); + database + .prepare( + `UPDATE sqlite_schema + SET sql = 'CREATE INDEX idx_agent_cache_expiry ON cache_entries(scope, expires_at, key) WHERE expires_at IS NOT NULL' + WHERE name = 'idx_agent_cache_expiry'`, + ) + .run(); + const schemaVersion = readSqliteNumberPragma(database, "schema_version"); + database.exec(`PRAGMA writable_schema = OFF; PRAGMA schema_version = ${schemaVersion + 1};`); + expect(database.prepare("PRAGMA integrity_check('cache_entries')").all()).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + integrity_check: expect.stringMatching(/idx_agent_cache_expiry/), + }), + ]), + ); + } finally { + database.close(); + } +} + function createUnsafeSchemaMetaIndexDrift(databasePath: string): void { const { DatabaseSync } = requireNodeSqlite(); const database = new DatabaseSync(databasePath); @@ -2350,7 +2383,7 @@ describe("openclaw agent database", () => { ).toThrow(/UNIQUE constraint failed/iu); }); - it("repairs every canonical agent-state named unique index", () => { + it("repairs every canonical agent-state named index", () => { const stateDir = createTempStateDir(); const env = { OPENCLAW_STATE_DIR: stateDir }; const created = openOpenClawAgentDatabase({ agentId: "worker-1", env }); @@ -2362,7 +2395,7 @@ describe("openclaw agent database", () => { const { DatabaseSync } = requireNodeSqlite(); const drifted = new DatabaseSync(databasePath); try { - expect(replaceNamedUniqueIndexesWithOrdinaryIndexes(drifted)).toHaveLength(5); + expect(replaceNamedIndexesWithNoncanonicalIndexes(drifted).length).toBeGreaterThan(25); expect(drifted.prepare("PRAGMA integrity_check").get()).toEqual({ integrity_check: "ok", }); @@ -2377,6 +2410,29 @@ describe("openclaw agent database", () => { ); }); + it("repairs physical ordinary-index drift before cold-open reads", () => { + const stateDir = createTempStateDir(); + const env = { OPENCLAW_STATE_DIR: stateDir }; + const databasePath = openOpenClawAgentDatabase({ agentId: "worker-1", env }).path; + closeOpenClawAgentDatabasesForTest(); + closeOpenClawStateDatabaseForTest(); + createCacheExpiryIndexPhysicalDrift(databasePath); + + const reopened = openOpenClawAgentDatabase({ agentId: "worker-1", env }); + expect(reopened.db.prepare("PRAGMA integrity_check").get()).toEqual({ + integrity_check: "ok", + }); + expect( + reopened.db + .prepare( + `SELECT key + FROM cache_entries INDEXED BY idx_agent_cache_expiry + WHERE scope = 'scope-a' AND expires_at = 100 AND key = 'key-a'`, + ) + .all(), + ).toEqual([{ key: "key-a" }]); + }); + it("rejects same-name transcript index drift when duplicate rows block repair", () => { const stateDir = createTempStateDir(); const env = { OPENCLAW_STATE_DIR: stateDir }; @@ -2387,7 +2443,7 @@ describe("openclaw agent database", () => { createTranscriptIdempotencyIndexDrift(databasePath, { duplicateRows: true }); expect(() => openOpenClawAgentDatabase({ agentId: "worker-1", env })).toThrow( - /canonical unique index idx_agent_transcript_message_idempotency failed.*UNIQUE constraint failed/iu, + /canonical index idx_agent_transcript_message_idempotency failed.*UNIQUE constraint failed/iu, ); const { DatabaseSync } = requireNodeSqlite(); @@ -2952,7 +3008,30 @@ describe("openclaw agent database", () => { } }); - it("defers unrelated current-schema index corruption to background verification", () => { + it("rejects unexpected unique indexes before writable initialization", () => { + const stateDir = createTempStateDir(); + const env = { OPENCLAW_STATE_DIR: stateDir }; + const databasePath = openOpenClawAgentDatabase({ agentId: "worker-1", env }).path; + closeOpenClawAgentDatabasesForTest(); + closeOpenClawStateDatabaseForTest(); + + const { DatabaseSync } = requireNodeSqlite(); + const drifted = new DatabaseSync(databasePath); + try { + drifted.exec("CREATE UNIQUE INDEX unsafe_cache_key_unique ON cache_entries(key);"); + expect(drifted.prepare("PRAGMA integrity_check").get()).toEqual({ + integrity_check: "ok", + }); + } finally { + drifted.close(); + } + + expect(() => openOpenClawAgentDatabase({ agentId: "worker-1", env })).toThrow( + /unexpected unique index unsafe_cache_key_unique/iu, + ); + }); + + it("rejects unrelated current-schema index corruption before exposure", () => { const stateDir = createTempStateDir(); const env = { OPENCLAW_STATE_DIR: stateDir }; const databasePath = openOpenClawAgentDatabase({ agentId: "worker-1", env }).path; @@ -2960,7 +3039,22 @@ describe("openclaw agent database", () => { closeOpenClawStateDatabaseForTest(); createUnsafeIndexDrift(databasePath); - expect(openOpenClawAgentDatabase({ agentId: "worker-1", env }).db.isOpen).toBe(true); + expect(() => openOpenClawAgentDatabase({ agentId: "worker-1", env })).toThrow( + /integrity_check failed.*missing from index unsafe_index_records_value/iu, + ); + }); + + it("rechecks integrity after a validated handle is physically reopened", () => { + const stateDir = createTempStateDir(); + const env = { OPENCLAW_STATE_DIR: stateDir }; + const databasePath = openOpenClawAgentDatabase({ agentId: "worker-1", env }).path; + expect(closeOpenClawAgentDatabaseByPath(databasePath)).toBe(true); + closeOpenClawStateDatabaseForTest(); + createUnsafeIndexDrift(databasePath); + + expect(() => openOpenClawAgentDatabase({ agentId: "worker-1", env })).toThrow( + /integrity_check failed.*missing from index unsafe_index_records_value/iu, + ); }); it("runs full integrity before a pending agent schema migration", () => { @@ -3005,7 +3099,7 @@ describe("openclaw agent database", () => { ); }); - it("defers current-schema foreign-key violations to background verification", () => { + it("rejects current-schema foreign-key violations before exposure", () => { const stateDir = createTempStateDir(); const env = { OPENCLAW_STATE_DIR: stateDir }; const created = openOpenClawAgentDatabase({ agentId: "worker-1", env }); @@ -3036,20 +3130,9 @@ describe("openclaw agent database", () => { corrupted.close(); } - expect(openOpenClawAgentDatabase({ agentId: "worker-1", env }).db.isOpen).toBe(true); - closeOpenClawAgentDatabasesForTest(); - - const after = new DatabaseSync(databasePath, { readOnly: true }); - try { - expect(after.prepare("PRAGMA foreign_key_check").get()).toEqual({ - table: "session_windows", - rowid: 1, - parent: "session_nodes", - fkid: 1, - }); - } finally { - after.close(); - } + expect(() => openOpenClawAgentDatabase({ agentId: "worker-1", env })).toThrow( + /foreign_key_check failed.*session_windows row 1 references session_nodes \(foreign key 1\)/iu, + ); }); it("latches newer per-agent schema failures before integrity scans", () => { diff --git a/src/state/openclaw-database-verify.test.ts b/src/state/openclaw-database-verify.test.ts index 2bba54eb7b32..0e7539faa66e 100644 --- a/src/state/openclaw-database-verify.test.ts +++ b/src/state/openclaw-database-verify.test.ts @@ -93,17 +93,17 @@ describe("OpenClaw database integrity verifier", () => { ]); await expect(runDatabaseVerifyWorker(targets)).resolves.toEqual(directResults); - // The drift lives outside schema_meta, so the rescoped open still succeeds; - // the recorder must then quarantine this live handle, not just future opens. - const liveHandle = openOpenClawAgentDatabase({ agentId: "worker-1", env }); - expect(liveHandle.db.isOpen).toBe(true); + // The drift is not a committed canonical index, so open must fail closed + // instead of guessing a replacement definition. + expect(() => openOpenClawAgentDatabase({ agentId: "worker-1", env })).toThrow( + expect.objectContaining({ name: "SqliteIntegrityError" }), + ); applyOpenClawDatabaseVerificationResults({ env, results: directResults, targets, }); - expect(liveHandle.db.isOpen).toBe(false); expect(readOpenClawDatabaseQuarantine(agentPath, { env })).toEqual({ kind: "agent", quarantinedAt: expect.any(Number), @@ -123,7 +123,9 @@ describe("OpenClaw database integrity verifier", () => { }), ); clearOpenClawAgentDatabaseOpenFailure(agentPath, { env }); - expect(openOpenClawAgentDatabase({ agentId: "worker-1", env }).db.isOpen).toBe(true); + expect(() => openOpenClawAgentDatabase({ agentId: "worker-1", env })).toThrow( + expect.objectContaining({ name: "SqliteIntegrityError" }), + ); }); it("reports an uncleared quarantine row instead of claiming repair success", () => { diff --git a/src/state/openclaw-state-db.test.ts b/src/state/openclaw-state-db.test.ts index 504bc7396dbd..d845eeb76da3 100644 --- a/src/state/openclaw-state-db.test.ts +++ b/src/state/openclaw-state-db.test.ts @@ -36,7 +36,7 @@ import { collectSqliteSchemaShape, createSqliteSchemaShapeFromSql, normalizeSqliteSchemaShapeSql, - replaceNamedUniqueIndexesWithOrdinaryIndexes, + replaceNamedIndexesWithNoncanonicalIndexes, } from "./sqlite-schema-shape.test-support.js"; type StateDbTestDatabase = Pick< @@ -480,6 +480,42 @@ function createUnsafeIndexDrift(databasePath: string): void { } } +function createTaskRunStatusIndexPhysicalDrift(databasePath: string): void { + const { DatabaseSync } = requireNodeSqlite(); + const database = new DatabaseSync(databasePath); + try { + database.exec(` + INSERT INTO task_runs ( + task_id, runtime, owner_key, scope_kind, task, status, + delivery_status, notify_policy, created_at + ) VALUES ( + 'task-index-repair', 'subagent', 'owner', 'session', 'repair index', + 'running', 'pending', 'summary', 1 + ); + DROP INDEX idx_task_runs_status; + CREATE INDEX idx_task_runs_status ON task_runs(task_id); + `); + database.enableDefensive?.(false); + database.exec("PRAGMA writable_schema = ON;"); + database + .prepare( + "UPDATE sqlite_schema SET sql = 'CREATE INDEX idx_task_runs_status ON task_runs(status)' WHERE name = 'idx_task_runs_status'", + ) + .run(); + const schemaVersion = readSqliteNumberPragma(database, "schema_version"); + database.exec(`PRAGMA writable_schema = OFF; PRAGMA schema_version = ${schemaVersion + 1};`); + expect(database.prepare("PRAGMA integrity_check('task_runs')").all()).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + integrity_check: expect.stringMatching(/idx_task_runs_status/), + }), + ]), + ); + } finally { + database.close(); + } +} + function createUnsafeSchemaMetaIndexDrift(databasePath: string): void { const { DatabaseSync } = requireNodeSqlite(); const database = new DatabaseSync(databasePath); @@ -1624,7 +1660,7 @@ INSERT INTO macos_port_guardian_records VALUES (4242, 18789, '/usr/bin/ssh', 're ).not.toThrow(); }); - it("repairs every canonical shared-state named unique index", () => { + it("repairs every canonical shared-state named index", () => { const stateDir = createTempStateDir(); const env = { OPENCLAW_STATE_DIR: stateDir }; const created = openOpenClawStateDatabase({ env }); @@ -1635,7 +1671,7 @@ INSERT INTO macos_port_guardian_records VALUES (4242, 18789, '/usr/bin/ssh', 're const { DatabaseSync } = requireNodeSqlite(); const drifted = new DatabaseSync(databasePath); try { - expect(replaceNamedUniqueIndexesWithOrdinaryIndexes(drifted)).toHaveLength(3); + expect(replaceNamedIndexesWithNoncanonicalIndexes(drifted).length).toBeGreaterThan(100); expect(drifted.prepare("PRAGMA integrity_check").get()).toEqual({ integrity_check: "ok", }); @@ -1649,6 +1685,26 @@ INSERT INTO macos_port_guardian_records VALUES (4242, 18789, '/usr/bin/ssh', 're ); }); + it("repairs physical ordinary-index drift before cold-open reads", () => { + const stateDir = createTempStateDir(); + const env = { OPENCLAW_STATE_DIR: stateDir }; + const databasePath = openOpenClawStateDatabase({ env }).path; + closeOpenClawStateDatabaseForTest(); + createTaskRunStatusIndexPhysicalDrift(databasePath); + + const reopened = openOpenClawStateDatabase({ env }); + expect(reopened.db.prepare("PRAGMA integrity_check").get()).toEqual({ + integrity_check: "ok", + }); + expect( + reopened.db + .prepare( + "SELECT task_id FROM task_runs INDEXED BY idx_task_runs_status WHERE status = 'running'", + ) + .all(), + ).toEqual([{ task_id: "task-index-repair" }]); + }); + it("migrates the released audit ledger to message-compatible attribution exactly once", () => { const stateDir = createTempStateDir(); const databasePath = createLegacyAuditStateDatabase(stateDir); @@ -2172,14 +2228,15 @@ INSERT INTO macos_port_guardian_records VALUES (4242, 18789, '/usr/bin/ssh', 're ); }); - it("defers unrelated current-schema index corruption but keeps doctor scans full", () => { + it("rejects unrelated current-schema index corruption before exposure", () => { const stateDir = createTempStateDir(); const databasePath = createCanonicalAuditStateDatabase(stateDir); const options = { env: { OPENCLAW_STATE_DIR: stateDir } }; createUnsafeIndexDrift(databasePath); - expect(openOpenClawStateDatabase(options).db.isOpen).toBe(true); - closeOpenClawStateDatabaseForTest(); + expect(() => openOpenClawStateDatabase(options)).toThrow( + /integrity_check failed.*missing from index unsafe_index_records_value/iu, + ); expect(repairOpenClawStateDatabaseSchema(options)).toEqual({ changes: [], warnings: [ @@ -2233,7 +2290,7 @@ INSERT INTO macos_port_guardian_records VALUES (4242, 18789, '/usr/bin/ssh', 're ); }); - it("defers current-schema foreign-key violations but keeps doctor scans full", () => { + it("rejects current-schema foreign-key violations before exposure", () => { const stateDir = createTempStateDir(); const databasePath = createCanonicalAuditStateDatabase(stateDir); const options = { env: { OPENCLAW_STATE_DIR: stateDir } }; @@ -2258,8 +2315,7 @@ INSERT INTO macos_port_guardian_records VALUES (4242, 18789, '/usr/bin/ssh', 're const failure = /foreign_key_check failed.*task_delivery_state row 1 references task_runs \(foreign key 0\)/iu; - expect(openOpenClawStateDatabase(options).db.isOpen).toBe(true); - closeOpenClawStateDatabaseForTest(); + expect(() => openOpenClawStateDatabase(options)).toThrow(failure); expect(repairOpenClawStateDatabaseSchema(options)).toEqual({ changes: [], warnings: [expect.stringMatching(failure)], @@ -2551,7 +2607,10 @@ INSERT INTO macos_port_guardian_records VALUES (4242, 18789, '/usr/bin/ssh', 're path: databasePath, }); expect(repairOpenClawStateDatabaseSchema(options)).toEqual({ - changes: ["Migrated shared state operator approvals → OpenClaw system changes"], + changes: [ + "Migrated shared state operator approvals → OpenClaw system changes", + expect.stringMatching(/^Rebuilt canonical shared-state SQLite indexes \(\d+\)$/u), + ], warnings: [], }); diff --git a/src/state/openclaw-state-db.ts b/src/state/openclaw-state-db.ts index f7c92515b32b..a6761b83183a 100644 --- a/src/state/openclaw-state-db.ts +++ b/src/state/openclaw-state-db.ts @@ -12,13 +12,9 @@ import { resolveNodeSqliteLocation, resolveNodeSqliteReadOnlyLocation, } from "../infra/node-sqlite.js"; -import { - repairCanonicalSqliteUniqueIndexes, - type CanonicalSqliteUniqueIndex, -} from "../infra/sqlite-index-schema.js"; +import { repairCanonicalSqliteIndexes } from "../infra/sqlite-index-schema.js"; import { assertSqliteIntegrity, - assertSqliteTableIntegrity, isTerminalSqliteIntegrityError, } from "../infra/sqlite-integrity.js"; import { migrateSqliteSchemaToStrictInTransaction } from "../infra/sqlite-strict.js"; @@ -94,30 +90,6 @@ export { withOpenClawStateStartupMigrationCheckpointDatabase } from "./openclaw- * tables, private file permissions, cached handles, and audit rows for * migrations/backups that operate on local state. */ -const OPENCLAW_STATE_CANONICAL_UNIQUE_INDEXES = [ - { - name: "idx_operator_approvals_resolution_ref", - tableName: "operator_approvals", - definition: "ON operator_approvals(resolution_ref)", - }, - { - name: "idx_worker_environments_provider_lease", - tableName: "worker_environments", - definition: ` - ON worker_environments(provider_id, lease_id) - WHERE lease_id IS NOT NULL - `, - }, - { - name: "idx_worker_inference_turns_pending_run", - tableName: "worker_inference_turns", - definition: ` - ON worker_inference_turns(session_id, run_epoch, run_id) - WHERE state = 'pending' - `, - }, -] as const satisfies readonly CanonicalSqliteUniqueIndex[]; - const cachedDatabases = new Map(); const terminalOpenLatch = createSqliteTerminalOpenLatch({ closeByPath: (pathname) => { @@ -159,9 +131,19 @@ export function repairOpenClawStateDatabaseSchema(options: OpenClawStateDatabase ensureOpenClawStatePermissions(pathname, env); const sqlite = requireNodeSqlite(); const db = new sqlite.DatabaseSync(resolveNodeSqliteLocation(pathname)); + const rebuiltIndexNames = new Set(); try { - assertSqliteIntegrity(db, pathname); assertSupportedSchemaVersion(db, pathname); + if (readSqliteUserVersion(db) === OPENCLAW_STATE_SCHEMA_VERSION) { + for (const name of repairCanonicalSqliteIndexes(db, pathname, OPENCLAW_STATE_SCHEMA_SQL, { + allowMissingColumns: true, + })) { + rebuiltIndexNames.add(name); + } + } + if (rebuiltIndexNames.size === 0) { + assertSqliteIntegrity(db, pathname); + } db.exec("PRAGMA foreign_keys = OFF;"); const changes = runSqliteImmediateTransactionSync( db, @@ -203,8 +185,16 @@ export function repairOpenClawStateDatabaseSchema(options: OpenClawStateDatabase `Migrated shared state tables to SQLite STRICT typing (${strictMigration.migratedTables.length})`, ); } + for (const name of repairCanonicalSqliteIndexes(db, pathname, OPENCLAW_STATE_SCHEMA_SQL, { + verifyPhysicalIntegrity: false, + })) { + rebuiltIndexNames.add(name); + } } markCurrentStateSchemaVersion(db); + if (rebuiltIndexNames.size > 0) { + applied.push(`Rebuilt canonical shared-state SQLite indexes (${rebuiltIndexNames.size})`); + } return applied; }, { @@ -266,7 +256,9 @@ function ensureSchema(db: DatabaseSync, pathname: string): void { databaseLabel: pathname, }); } - repairCanonicalSqliteUniqueIndexes(db, pathname, OPENCLAW_STATE_CANONICAL_UNIQUE_INDEXES); + repairCanonicalSqliteIndexes(db, pathname, OPENCLAW_STATE_SCHEMA_SQL, { + verifyPhysicalIntegrity: false, + }); db.exec(`PRAGMA user_version = ${OPENCLAW_STATE_SCHEMA_VERSION};`); executeSqliteQuerySync( db, @@ -346,22 +338,25 @@ function assertStateDatabaseIntegrityBeforeMutation( const hasApplicationSchema = database .prepare("SELECT 1 FROM sqlite_master WHERE name NOT LIKE 'sqlite_%' LIMIT 1") .get(); - if ( + const migrationPending = (userVersion === 0 && hasApplicationSchema) || - (userVersion > 0 && userVersion < OPENCLAW_STATE_SCHEMA_VERSION) - ) { - // Migration rewrites the schema; prove the whole file before that mutation. - // Only a truly empty v0 file may skip; legacy v0 files need the same proof. + (userVersion > 0 && userVersion < OPENCLAW_STATE_SCHEMA_VERSION); + if (migrationPending) { stateDbLog.info("state database schema migration pending; verifying integrity first", { fromVersion: userVersion, path: pathname, toVersion: OPENCLAW_STATE_SCHEMA_VERSION, }); - assertSqliteIntegrity(database, pathname); - return; } - if (tableExists(database, "schema_meta")) { - assertSqliteTableIntegrity(database, pathname, "schema_meta"); + const rebuiltIndexes = + userVersion === OPENCLAW_STATE_SCHEMA_VERSION + ? repairCanonicalSqliteIndexes(database, pathname, OPENCLAW_STATE_SCHEMA_SQL, { + allowMissingColumns: true, + }) + : []; + if (rebuiltIndexes.length === 0) { + // Every physical open proves the full file before schema mutation or exposure. + assertSqliteIntegrity(database, pathname); } } diff --git a/src/state/sqlite-schema-shape.test-support.ts b/src/state/sqlite-schema-shape.test-support.ts index ed7e0afeadb3..59d17c5fd6fb 100644 --- a/src/state/sqlite-schema-shape.test-support.ts +++ b/src/state/sqlite-schema-shape.test-support.ts @@ -128,12 +128,12 @@ export function normalizeSqliteSchemaShapeSql(shape: SqliteSchemaShape): SqliteS } /** - * Replace every explicit named UNIQUE index with a same-name ordinary index. + * Replace every explicit named index with a same-name noncanonical index. * * Startup repair tests use this to prove the repair registry covers the whole * canonical schema rather than only a hand-picked index. */ -export function replaceNamedUniqueIndexesWithOrdinaryIndexes(db: DatabaseSync): string[] { +export function replaceNamedIndexesWithNoncanonicalIndexes(db: DatabaseSync): string[] { const indexes = db .prepare( ` @@ -145,15 +145,8 @@ export function replaceNamedUniqueIndexesWithOrdinaryIndexes(db: DatabaseSync): `, ) .all() as NamedIndexRow[]; - const uniqueIndexes = indexes.filter((index) => - ( - db - .prepare(`PRAGMA index_list(${quoteSqliteIdentifier(index.tbl_name)})`) - .all() as IndexListRow[] - ).some((candidate) => candidate.name === index.name && candidate.unique === 1), - ); - for (const index of uniqueIndexes) { + for (const index of indexes) { const firstColumn = ( db .prepare(`PRAGMA table_info(${quoteSqliteIdentifier(index.tbl_name)})`) @@ -165,11 +158,14 @@ export function replaceNamedUniqueIndexesWithOrdinaryIndexes(db: DatabaseSync): db.exec(` DROP INDEX main.${quoteSqliteIdentifier(index.name)}; CREATE INDEX main.${quoteSqliteIdentifier(index.name)} - ON ${quoteSqliteIdentifier(index.tbl_name)}(${quoteSqliteIdentifier(firstColumn.name)}); + ON ${quoteSqliteIdentifier(index.tbl_name)}( + ${quoteSqliteIdentifier(firstColumn.name)}, + ${quoteSqliteIdentifier(firstColumn.name)} + ); `); } - return uniqueIndexes.map((index) => index.name); + return indexes.map((index) => index.name); } function collectStrictFlag(db: DatabaseSync, tableName: string): number {