From 687923189eae20166cbe82759ceecfdcf77b6490 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 25 Jul 2026 21:50:27 +0800 Subject: [PATCH] fix(sqlite): make reliability proofs cross-platform safe (#113652) * test(sqlite): canonicalize reliability crash barriers * fix(sqlite): normalize reliability database paths * test(sqlite): allow Windows reliability proof to finish --- .../sqlite-reliability-index-repair-worker.ts | 5 +- .../lib/sqlite-reliability-index-repair.ts | 8 ++-- .../sqlite-reliability-repository-worker.ts | 19 +++----- .../lib/sqlite-reliability-restore-worker.ts | 11 +++-- scripts/lib/sqlite-reliability-runner.ts | 25 ++++------ .../lib/sqlite-reliability-worker-paths.ts | 18 +++++++ scripts/lib/sqlite-reliability-writer.ts | 5 +- test/scripts/bench-sqlite-reliability.test.ts | 47 ++++++++++++++++--- 8 files changed, 89 insertions(+), 49 deletions(-) create mode 100644 scripts/lib/sqlite-reliability-worker-paths.ts diff --git a/scripts/lib/sqlite-reliability-index-repair-worker.ts b/scripts/lib/sqlite-reliability-index-repair-worker.ts index d8536f9eda37..164259cbb0d3 100644 --- a/scripts/lib/sqlite-reliability-index-repair-worker.ts +++ b/scripts/lib/sqlite-reliability-index-repair-worker.ts @@ -1,5 +1,5 @@ import { pathToFileURL } from "node:url"; -import { requireNodeSqlite } from "../../src/infra/node-sqlite.js"; +import { openNodeSqliteDatabase } from "../../src/infra/node-sqlite.js"; import { repairCanonicalSqliteIndexes } from "../../src/infra/sqlite-index-schema.js"; import { INDEX_REPAIR_SCHEMA_SQL, @@ -36,8 +36,7 @@ async function main(argv: string[]): Promise { throw new Error("invalid SQLite index repair worker arguments"); } const journalMode = parseJournalMode(journalModeValue); - const { DatabaseSync } = requireNodeSqlite(); - const database = new DatabaseSync(databasePath); + const database = openNodeSqliteDatabase(databasePath); try { database.exec(` PRAGMA busy_timeout = 30000; diff --git a/scripts/lib/sqlite-reliability-index-repair.ts b/scripts/lib/sqlite-reliability-index-repair.ts index 46cff64ab487..7b7a819a0e38 100644 --- a/scripts/lib/sqlite-reliability-index-repair.ts +++ b/scripts/lib/sqlite-reliability-index-repair.ts @@ -5,7 +5,7 @@ import path from "node:path"; import type { DatabaseSync } from "node:sqlite"; import { setTimeout as delay } from "node:timers/promises"; import { fileURLToPath } from "node:url"; -import { requireNodeSqlite } from "../../src/infra/node-sqlite.js"; +import { openNodeSqliteDatabase } from "../../src/infra/node-sqlite.js"; import { repairCanonicalSqliteIndexes } from "../../src/infra/sqlite-index-schema.js"; import { assertSqliteIntegrity } from "../../src/infra/sqlite-integrity.js"; import { @@ -71,8 +71,7 @@ function prepareIndexRepairDatabase( databasePath: string, journalMode: IndexRepairJournalMode, ): IndexRepairState { - const { DatabaseSync } = requireNodeSqlite(); - const database = new DatabaseSync(databasePath); + const database = openNodeSqliteDatabase(databasePath); try { database.exec(` PRAGMA synchronous = FULL; @@ -237,8 +236,7 @@ function assertForcedExit(exit: WorkerExit): void { } function recoverAndRepair(databasePath: string, expectedState: IndexRepairState): string[] { - const { DatabaseSync } = requireNodeSqlite(); - const database = new DatabaseSync(databasePath); + const database = openNodeSqliteDatabase(databasePath); try { assertSqliteIntegrity(database, databasePath); assertSameState(readIndexRepairState(database), expectedState); diff --git a/scripts/lib/sqlite-reliability-repository-worker.ts b/scripts/lib/sqlite-reliability-repository-worker.ts index b34f0ee5c273..e186f94eb1f3 100644 --- a/scripts/lib/sqlite-reliability-repository-worker.ts +++ b/scripts/lib/sqlite-reliability-repository-worker.ts @@ -1,8 +1,11 @@ import fsSync, { type PathLike } from "node:fs"; import fs from "node:fs/promises"; -import path from "node:path"; import { pathToFileURL } from "node:url"; import type { SnapshotDatabaseIdentity } from "../../src/snapshot/snapshot-provider.js"; +import { + canonicalPathWithExistingParent, + isPendingPathInRepository, +} from "./sqlite-reliability-worker-paths.js"; type RepositoryCrashPoint = "after-commit" | "before-pending" | "pending"; @@ -40,14 +43,6 @@ function holdAtCrashPoint(crashPoint: RepositoryCrashPoint): never { } } -function isPendingPath(filePath: unknown, repositoryPath: string): boolean { - const resolvedPath = path.resolve(String(filePath)); - return ( - path.basename(resolvedPath) === ".pending" && - path.dirname(path.dirname(resolvedPath)) === repositoryPath - ); -} - function installCrashBarrier(crashPoint: RepositoryCrashPoint, repositoryPath: string): void { if (crashPoint === "before-pending") { const originalOpen = fs.open.bind(fs); @@ -55,7 +50,7 @@ function installCrashBarrier(crashPoint: RepositoryCrashPoint, repositoryPath: s configurable: true, enumerable: true, value: (async (...args: unknown[]) => { - if (args[1] === "wx+" && isPendingPath(args[0], repositoryPath)) { + if (args[1] === "wx+" && isPendingPathInRepository(args[0], repositoryPath)) { holdAtCrashPoint(crashPoint); } return await (originalOpen as (...openArgs: unknown[]) => Promise)(...args); @@ -70,7 +65,7 @@ function installCrashBarrier(crashPoint: RepositoryCrashPoint, repositoryPath: s configurable: true, enumerable: true, value: ((filePath: PathLike) => { - if (!isPendingPath(filePath, repositoryPath)) { + if (!isPendingPathInRepository(filePath, repositoryPath)) { return originalUnlinkSync(filePath); } if (crashPoint === "after-commit") { @@ -101,7 +96,7 @@ async function main(argv: string[]): Promise { throw new Error("invalid SQLite repository interruption worker arguments"); } const crashPoint = parseCrashPoint(crashPointValue); - const repositoryPath = path.resolve(repositoryPathValue); + const repositoryPath = canonicalPathWithExistingParent(repositoryPathValue); const identity = parseIdentity(identityValue); installCrashBarrier(crashPoint, repositoryPath); diff --git a/scripts/lib/sqlite-reliability-restore-worker.ts b/scripts/lib/sqlite-reliability-restore-worker.ts index 98eec28f8a78..3dbef8845c15 100644 --- a/scripts/lib/sqlite-reliability-restore-worker.ts +++ b/scripts/lib/sqlite-reliability-restore-worker.ts @@ -1,6 +1,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import { pathToFileURL } from "node:url"; +import { canonicalPathWithExistingParent } from "./sqlite-reliability-worker-paths.js"; type RestoreCrashPoint = "after-publish" | "before-publish"; @@ -27,7 +28,7 @@ async function main(argv: string[]): Promise { throw new Error("invalid SQLite restore interruption worker arguments"); } const crashPoint = parseCrashPoint(crashPointValue); - const resolvedTargetPath = path.resolve(targetPath); + const resolvedTargetPath = canonicalPathWithExistingParent(targetPath); if (crashPoint === "before-publish") { const originalLink = fs.link.bind(fs); @@ -37,7 +38,7 @@ async function main(argv: string[]): Promise { value: async (sourcePath: string, publishedPath: string) => { const resolvedSourcePath = path.resolve(sourcePath); if ( - path.resolve(publishedPath) === resolvedTargetPath && + canonicalPathWithExistingParent(publishedPath) === resolvedTargetPath && path.basename(resolvedSourcePath) === "database.sqlite" && path.basename(path.dirname(resolvedSourcePath)).startsWith(".sqlite-publish-") ) { @@ -49,12 +50,12 @@ async function main(argv: string[]): Promise { }); } else { const originalRmdir = fs.rmdir.bind(fs); - const restoreParentPath = path.resolve(path.dirname(targetPath)); + const restoreParentPath = path.dirname(resolvedTargetPath); Object.defineProperty(fs, "rmdir", { configurable: true, enumerable: true, value: async (directoryPath: string) => { - const resolvedDirectoryPath = path.resolve(directoryPath); + const resolvedDirectoryPath = canonicalPathWithExistingParent(directoryPath); if ( path.dirname(resolvedDirectoryPath) === restoreParentPath && path.basename(resolvedDirectoryPath).startsWith(".tmp-restore-") @@ -74,7 +75,7 @@ async function main(argv: string[]): Promise { validationRootPath, }); process.send?.({ kind: "ready" }); - await provider.restoreFresh({ path: snapshotPath }, targetPath); + await provider.restoreFresh({ path: snapshotPath }, resolvedTargetPath); throw new Error(`SQLite restore worker passed ${crashPoint} without being terminated.`); } diff --git a/scripts/lib/sqlite-reliability-runner.ts b/scripts/lib/sqlite-reliability-runner.ts index 4dc1743d6f02..7e31c722e0d8 100644 --- a/scripts/lib/sqlite-reliability-runner.ts +++ b/scripts/lib/sqlite-reliability-runner.ts @@ -6,7 +6,7 @@ import path from "node:path"; import type { DatabaseSync } from "node:sqlite"; import { compactDoctorSessionSqliteTarget } from "../../src/commands/doctor-session-sqlite-compact.js"; import { runDoctorStateSqliteCompact } from "../../src/commands/doctor-state-sqlite-compact.js"; -import { requireNodeSqlite } from "../../src/infra/node-sqlite.js"; +import { openNodeSqliteDatabase } from "../../src/infra/node-sqlite.js"; import { createLocalSqliteSnapshotProvider } from "../../src/snapshot/local-repository.js"; import type { SnapshotDatabaseIdentity } from "../../src/snapshot/snapshot-provider.js"; import { @@ -104,8 +104,7 @@ function resolveTargetDatabase(options: CliOptions, env: NodeJS.ProcessEnv): Tar } function setupStressTable(databasePath: string): void { - const { DatabaseSync } = requireNodeSqlite(); - const database = new DatabaseSync(databasePath); + const database = openNodeSqliteDatabase(databasePath); try { database.exec("PRAGMA journal_mode = WAL;"); database.exec("PRAGMA busy_timeout = 30000;"); @@ -213,8 +212,9 @@ function verifyRestoredDatabase(params: { rowsPerBatch: number; uncommittedBatch: number | null; }): ReliabilityStateProof { - const { DatabaseSync } = requireNodeSqlite(); - const database = new DatabaseSync(params.path, { readOnly: params.readOnly ?? true }); + const database = openNodeSqliteDatabase(params.path, { + readOnly: params.readOnly ?? true, + }); try { database.exec("PRAGMA trusted_schema = OFF;"); assertPragmaOk(database, "quick_check"); @@ -258,8 +258,7 @@ function verifyRestoredDatabase(params: { } function createCompactionBloat(databasePath: string): number { - const { DatabaseSync } = requireNodeSqlite(); - const database = new DatabaseSync(databasePath); + const database = openNodeSqliteDatabase(databasePath); const payload = "b".repeat(COMPACTION_BLOAT_PAYLOAD_BYTES); try { database.exec("PRAGMA journal_mode = WAL;"); @@ -297,8 +296,7 @@ function readCompactionPayload(databasePath: string): { idSum: number; rows: number; } { - const { DatabaseSync } = requireNodeSqlite(); - const database = new DatabaseSync(databasePath, { readOnly: true }); + const database = openNodeSqliteDatabase(databasePath, { readOnly: true }); try { const row = database .prepare( @@ -320,8 +318,7 @@ function readCompactionPayload(databasePath: string): { } function deleteCompactionBloat(databasePath: string): void { - const { DatabaseSync } = requireNodeSqlite(); - const database = new DatabaseSync(databasePath); + const database = openNodeSqliteDatabase(databasePath); try { database.exec(` DELETE FROM openclaw_reliability_compaction_bloat; @@ -333,8 +330,7 @@ function deleteCompactionBloat(databasePath: string): void { } function readAutoVacuum(databasePath: string): number { - const { DatabaseSync } = requireNodeSqlite(); - const database = new DatabaseSync(databasePath, { readOnly: true }); + const database = openNodeSqliteDatabase(databasePath, { readOnly: true }); try { const row = database.prepare("PRAGMA auto_vacuum;").get() as | Record @@ -349,8 +345,7 @@ function readAutoVacuum(databasePath: string): number { } function prepareVacuumRollbackSentinel(databasePath: string): number { - const { DatabaseSync } = requireNodeSqlite(); - const database = new DatabaseSync(databasePath); + const database = openNodeSqliteDatabase(databasePath); try { database.exec(` PRAGMA busy_timeout = 30000; diff --git a/scripts/lib/sqlite-reliability-worker-paths.ts b/scripts/lib/sqlite-reliability-worker-paths.ts new file mode 100644 index 000000000000..ec6cdccd241a --- /dev/null +++ b/scripts/lib/sqlite-reliability-worker-paths.ts @@ -0,0 +1,18 @@ +import fs from "node:fs"; +import path from "node:path"; + +export function canonicalPathWithExistingParent(filePath: string): string { + const resolvedPath = path.resolve(filePath); + return path.join(fs.realpathSync.native(path.dirname(resolvedPath)), path.basename(resolvedPath)); +} + +export function isPendingPathInRepository(filePath: unknown, repositoryPath: string): boolean { + const resolvedPath = path.resolve(String(filePath)); + if (path.basename(resolvedPath) !== ".pending") { + return false; + } + const candidateRepositoryPath = canonicalPathWithExistingParent( + path.dirname(path.dirname(resolvedPath)), + ); + return candidateRepositoryPath === repositoryPath; +} diff --git a/scripts/lib/sqlite-reliability-writer.ts b/scripts/lib/sqlite-reliability-writer.ts index 8a8be3f06f78..20376e39220c 100644 --- a/scripts/lib/sqlite-reliability-writer.ts +++ b/scripts/lib/sqlite-reliability-writer.ts @@ -1,7 +1,7 @@ import { fork, type ChildProcess } from "node:child_process"; import { setImmediate as delayImmediate, setTimeout as delay } from "node:timers/promises"; import { fileURLToPath, pathToFileURL } from "node:url"; -import { requireNodeSqlite } from "../../src/infra/node-sqlite.js"; +import { openNodeSqliteDatabase } from "../../src/infra/node-sqlite.js"; import { COMMITTED_WAL_SENTINEL, STRESS_TABLE_SQL, @@ -255,8 +255,7 @@ function parseWriterChildArgs(argv: string[]): { async function runWriterChild(argv: string[]): Promise { const options = parseWriterChildArgs(argv); - const { DatabaseSync } = requireNodeSqlite(); - const database = new DatabaseSync(options.databasePath); + const database = openNodeSqliteDatabase(options.databasePath); let nextBatch = 0; let batchesCommitted = 0; let rowsCommitted = 0; diff --git a/test/scripts/bench-sqlite-reliability.test.ts b/test/scripts/bench-sqlite-reliability.test.ts index ff5fca9b12c5..6faa208b3a7d 100644 --- a/test/scripts/bench-sqlite-reliability.test.ts +++ b/test/scripts/bench-sqlite-reliability.test.ts @@ -3,15 +3,20 @@ import { fork, spawnSync, type ChildProcess } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { DatabaseSync } from "node:sqlite"; import { afterEach, describe, expect, it } from "vitest"; import { parseSqliteReliabilityCli } from "../../scripts/lib/sqlite-reliability-cli.js"; import type { ReliabilityReport } from "../../scripts/lib/sqlite-reliability-contract.js"; import { monitorSqliteWalDuring } from "../../scripts/lib/sqlite-reliability-wal-monitor.js"; +import { + canonicalPathWithExistingParent, + isPendingPathInRepository, +} from "../../scripts/lib/sqlite-reliability-worker-paths.js"; +import { openNodeSqliteDatabase } from "../../src/infra/node-sqlite.js"; const tempDirs: string[] = []; -// The real smoke proof runs twice and can exceed Vitest's 120s default on fork CI runners. -const RELIABILITY_SMOKE_TEST_TIMEOUT_MS = 300_000; +// Windows repeats ACL checks and >64 MiB crash/restore copies across two full runs. +const RELIABILITY_PROOF_TIMEOUT_MS = process.platform === "win32" ? 480_000 : 240_000; +const RELIABILITY_SMOKE_TEST_TIMEOUT_MS = process.platform === "win32" ? 1_200_000 : 300_000; function reliabilitySmokeTest(name: string, test: () => void): void { it(name, test, RELIABILITY_SMOKE_TEST_TIMEOUT_MS); @@ -24,15 +29,19 @@ function makeTempDir(): string { } function runProof(args: string[]) { - return spawnSync( + const result = spawnSync( process.execPath, ["--import", "tsx", "scripts/bench-sqlite-reliability.ts", ...args], { cwd: process.cwd(), encoding: "utf8", - timeout: 240_000, + timeout: RELIABILITY_PROOF_TIMEOUT_MS, }, ); + if (result.error) { + throw result.error; + } + return result; } async function waitForChildReady(child: ChildProcess): Promise { @@ -373,7 +382,7 @@ describe("scripts/bench-sqlite-reliability", () => { expect(firstReport.walBytes.peak).toBeGreaterThan(0); expect(firstReport.walBytes.peak).toBeLessThanOrEqual(firstReport.walBytes.limit); - const database = new DatabaseSync(firstReport.paths.sourceDatabase); + const database = openNodeSqliteDatabase(firstReport.paths.sourceDatabase); try { database .prepare( @@ -402,6 +411,32 @@ describe("scripts/bench-sqlite-reliability", () => { expect(secondReport.paths.syncedRepository).not.toBe(firstReport.paths.syncedRepository); }); + it("matches crash barriers across filesystem path aliases", () => { + const realRoot = makeTempDir(); + const aliasRoot = path.join(makeTempDir(), "alias"); + fs.symlinkSync(realRoot, aliasRoot, process.platform === "win32" ? "junction" : "dir"); + const repositoryPath = path.join(realRoot, "snapshots"); + const snapshotPath = path.join(repositoryPath, "snapshot"); + fs.mkdirSync(snapshotPath, { recursive: true }); + + const canonicalRepositoryPath = canonicalPathWithExistingParent( + path.join(aliasRoot, "snapshots"), + ); + expect(canonicalRepositoryPath).toBe(path.join(fs.realpathSync.native(realRoot), "snapshots")); + expect( + isPendingPathInRepository( + path.join(aliasRoot, "snapshots", "snapshot", ".pending"), + canonicalRepositoryPath, + ), + ).toBe(true); + + const finalAlias = path.join(realRoot, "final-alias"); + fs.symlinkSync(snapshotPath, finalAlias, process.platform === "win32" ? "junction" : "dir"); + expect(canonicalPathWithExistingParent(finalAlias)).toBe( + path.join(fs.realpathSync.native(realRoot), "final-alias"), + ); + }); + it("stops the writer when its parent IPC channel disconnects", async () => { const databasePath = path.join(makeTempDir(), "writer.sqlite"); const child = fork(