From 60feaa8651b4e8a8a66597d4fce49c9ec7bd1b9b Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Tue, 18 Aug 2026 10:38:01 -0700 Subject: [PATCH] fix(sqlite): only reach for an immutable URI where node:sqlite honours one node:sqlite enables SQLITE_OPEN_URI from Node 22.15 on. Below that -- 22.13 is the package floor -- a `file:...` location is a literal filename, so the immutable open failed as CANTOPEN and the copy quietly stood in for it. That was the right outcome by accident; the test asserted the newer behaviour and failed on the floor. The support question is now asked once per process, with an in-memory URI that touches no filesystem whichever answer comes back, and the immutable open is attempted only when the answer is yes. The test asks the same question rather than skipping, so both CI lines assert something: rows are correct either way, in place where URI filenames work and from a copy where they do not. --- src/sqlite.ts | 27 +++++++++++++++++++++++++-- tests/sqlite-readonly-parent.test.ts | 10 ++++++---- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/sqlite.ts b/src/sqlite.ts index 935a42f5..7c107c0d 100644 --- a/src/sqlite.ts +++ b/src/sqlite.ts @@ -153,6 +153,29 @@ function isSqliteSidecarError(err: unknown): boolean { return typeof errcode === 'number' && (errcode & 0xff) === 14 } +let uriFilenamesSupported: boolean | null = null + +/// node:sqlite only enables SQLITE_OPEN_URI from Node 22.15 on (measured: 22.13 +/// and 22.14 fail, 22.15 and later work). Below that a `file:...` location is +/// taken literally and fails as CANTOPEN, so the immutable open is not attempted +/// there. The probe is an in-memory URI rather than a version comparison: it +/// answers the question directly and touches no filesystem either way. +export function sqliteSupportsUriFilenames(): boolean { + if (uriFilenamesSupported !== null) return uriFilenamesSupported + uriFilenamesSupported = false + const Driver = loadDriver() ? DatabaseSync : null + if (Driver !== null) { + try { + new Driver('file:codeburn-uri-probe?mode=memory', { readOnly: true }).close() + uriFilenamesSupported = true + } catch { + // An older build: locations are plain paths, and the copy fallback covers + // exactly the case the immutable open would have. + } + } + return uriFilenamesSupported +} + type DatabaseFingerprint = { dev: number ino: number @@ -362,11 +385,11 @@ function openReadonlyCache(path: string, originalError: unknown): DatabaseSyncIn // An absent or empty -wal holds no frames, so there is nothing to go stale and // nothing worth copying: immutable lets SQLite skip the -shm it cannot create // and read the source in place. - if (fingerprint.walBytes === 0) { + if (fingerprint.walBytes === 0 && sqliteSupportsUriFilenames()) { try { return new Driver(`${pathToFileURL(path).href}?immutable=1`, { readOnly: true }) } catch { - // Older node:sqlite builds may not enable URI filenames. Copy instead. + // Understood but refused: the copy covers it. } } diff --git a/tests/sqlite-readonly-parent.test.ts b/tests/sqlite-readonly-parent.test.ts index 1fc5ffd8..c227a1c7 100644 --- a/tests/sqlite-readonly-parent.test.ts +++ b/tests/sqlite-readonly-parent.test.ts @@ -9,6 +9,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { isSqliteReadonlyError, openDatabase, + sqliteSupportsUriFilenames, } from '../src/sqlite.js' import { discoverSqliteSessions, @@ -158,7 +159,7 @@ describe('SQLite read-only parent fallback', () => { expect(cachedDatabaseFiles()).toEqual([]) }) - it('reads a read-only parent with no -wal in place, without copying it', ({ skip }) => { + it('reads a read-only parent with no -wal, in place where it can and by copy where it cannot', ({ skip }) => { const dbPath = join(sourceRoot, 'state.vscdb') createClosedWalDatabase(dbPath) expect(existsSync(dbPath + '-wal')).toBe(false) @@ -169,9 +170,10 @@ describe('SQLite read-only parent fallback', () => { expect(existsSync(dbPath + '-wal')).toBe(false) expect(existsSync(dbPath + '-shm')).toBe(false) - // No WAL frames exist, so immutable reads the source in place: nothing to go - // stale, nothing to copy. - expect(cachedDatabaseFiles()).toEqual([]) + // No WAL frames exist, so there is nothing to go stale and nothing worth + // copying: immutable reads the source in place. node:sqlite only honours + // URI filenames on newer builds, and on the 22.13 floor the copy stands in. + expect(cachedDatabaseFiles()).toHaveLength(sqliteSupportsUriFilenames() ? 0 : 1) }) it('opens directly when a read-only parent already has WAL sidecars', ({ skip }) => {