From 9f37fcdf706fed694ecb4ef9e374818c1b7ea75c Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 19 Oct 2026 02:56:37 -0700 Subject: [PATCH] fix(persistence): a commit that runs out of disk no longer stops all later saves (#30694) * fix(persistence): a commit that runs out of disk no longer retires the profile-state writer A WAL COMMIT on a full disk fails with SQLITE_FULL after SQLite rolls the transaction back. It was reported as indeterminate, which retires the writer, so every later write failed until a restart: on a managed SSH host, a disk that filled once left orcad unable to open a terminal even after the space was freed. SQLITE_FULL fails a frame write before the commit point, so it is now a known failure; every other failed commit stays indeterminate. Co-Authored-By: Claude * docs(persistence): say why SQLITE_FULL on COMMIT is safe to treat as not committed Opus review: the claim holds because powersafe overwrite (on by default) means no padding frames follow the WAL commit frame. Co-Authored-By: Claude --------- Co-authored-by: Claude --- .../profile-state-write-transaction.test.ts | 62 +++++++++++++++++++ .../profile-state-write-transaction.ts | 7 ++- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/main/persistence/profile-state/profile-state-write-transaction.test.ts b/src/main/persistence/profile-state/profile-state-write-transaction.test.ts index 2f4dade139c..c0f0d7cc852 100644 --- a/src/main/persistence/profile-state/profile-state-write-transaction.test.ts +++ b/src/main/persistence/profile-state/profile-state-write-transaction.test.ts @@ -4,6 +4,7 @@ import { ProfileStateIndeterminateWriteError, withProfileStateWriteTransaction } from './profile-state-write-transaction' +import { profileStateWriterFailureOutcome } from './profile-state-writer-errors' describe('profile state write transaction ownership', () => { it('preserves SQLITE_FULL after SQLite rolls back the transaction itself', () => { @@ -26,6 +27,67 @@ describe('profile state write transaction ownership', () => { } }) + // What a WAL commit on a full disk reports: node:sqlite's SQLITE_FULL after SQLite rolled back + // (reproduced on a 2 MB tmpfs: COMMIT throws errcode 13, isTransaction is false, rows unchanged). + function failCommitAfterAutoRollback(db: Database, failure: Error): () => void { + const exec = db.exec.bind(db) + const injected = vi.spyOn(db, 'exec').mockImplementation((sql) => { + if (sql === 'COMMIT') { + exec('ROLLBACK') + throw failure + } + exec(sql) + }) + return () => injected.mockRestore() + } + + it('reports a commit that ran out of disk as a failed write the next write can follow', () => { + const db = new Database(':memory:') + db.exec('CREATE TABLE writes (id INTEGER)') + const full = Object.assign(new Error('database or disk is full'), { + code: 'ERR_SQLITE_ERROR', + errcode: 13 + }) + const restore = failCommitAfterAutoRollback(db, full) + let thrown: unknown + try { + withProfileStateWriteTransaction(db, () => db.exec('INSERT INTO writes VALUES (1)')) + } catch (error) { + thrown = error + } finally { + restore() + } + try { + expect(thrown).toBe(full) + expect(profileStateWriterFailureOutcome(thrown)).toBe('known-failure') + expect(db.isTransaction).toBe(false) + expect(db.prepare('SELECT COUNT(*) AS count FROM writes').get()).toMatchObject({ count: 0 }) + withProfileStateWriteTransaction(db, () => db.exec('INSERT INTO writes VALUES (2)')) + expect(db.prepare('SELECT COUNT(*) AS count FROM writes').get()).toMatchObject({ count: 1 }) + } finally { + db.close() + } + }) + + it('keeps any other failed commit indeterminate even after SQLite rolled back', () => { + const db = new Database(':memory:') + db.exec('CREATE TABLE writes (id INTEGER)') + // e.g. a failed fsync: the commit frame may already be on disk. + const ioerr = Object.assign(new Error('disk I/O error'), { + code: 'ERR_SQLITE_ERROR', + errcode: 1034 + }) + const restore = failCommitAfterAutoRollback(db, ioerr) + try { + expect(() => + withProfileStateWriteTransaction(db, () => db.exec('INSERT INTO writes VALUES (1)')) + ).toThrow(ProfileStateIndeterminateWriteError) + } finally { + restore() + db.close() + } + }) + it('rolls back a failed deferred commit and leaves the connection usable', () => { const db = new Database(':memory:') try { diff --git a/src/main/persistence/profile-state/profile-state-write-transaction.ts b/src/main/persistence/profile-state/profile-state-write-transaction.ts index 1dbf10d5bed..93dcd3e6fc3 100644 --- a/src/main/persistence/profile-state/profile-state-write-transaction.ts +++ b/src/main/persistence/profile-state/profile-state-write-transaction.ts @@ -1,4 +1,5 @@ import type Database from '../../sqlite/sync-database' +import { storageCapacityErrorCode } from '../../../shared/storage-capacity-error' export class ProfileStateIndeterminateWriteError extends Error { readonly code = 'profile-state-write-indeterminate' as const @@ -26,8 +27,10 @@ export function withProfileStateWriteTransaction(db: Database.Database, write return result } catch (error) { if (!db.isTransaction) { - // SQLite can roll back a failed statement itself; a failed COMMIT is ambiguous. - if (committing) { + // SQLite can roll back a failed statement itself; a failed COMMIT is ambiguous, except + // SQLITE_FULL: with powersafe overwrite (the default VFS) the WAL commit frame is the last + // write, so a write that ran out of room means no commit frame landed. + if (committing && storageCapacityErrorCode(error) !== 'SQLITE_FULL') { throw new ProfileStateIndeterminateWriteError(error, undefined) } throw error