From a0e50c31db66821e45d542df8f3ad2abf9142ff6 Mon Sep 17 00:00:00 2001
From: Elis Jackson <elisjackson@icloud.com>
Date: Wed, 16 Sep 2026 15:41:25 -0500
Subject: [PATCH] fix(wallet-toolbox): run SQLite migrations inside knex's
 per-file transaction
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

An interrupted migration currently leaves a store that can never migrate
forward. With transactions disabled for SQLite, each DDL statement autocommits
while knex records the migration only after the whole file's up() resolves, so a
process killed mid-file — or between a file's last statement and the
knex_migrations insert — leaves tables the journal never recorded. Every later
migrate.latest() re-runs that file from its first statement and dies on
"table ... already exists" (or "duplicate column name"), permanently, with no
in-package recovery path. For a desktop wallet a force-quit during first-run
bring-up is enough.

SQLite DDL is transactional, so letting knex wrap each migration file rolls an
interrupted migration back whole and keeps the journal insert atomic with its
DDL. PRAGMA foreign_keys is still issued outside migrate.latest(): SQLite's
forced single-connection pool means the migration transaction runs on that same
connection and inherits the setting. Measured by reading PRAGMA foreign_keys
from inside a migration under better-sqlite3 — 0 (OFF) with transactions both
enabled and disabled — so the knex#4155 constraint the old comment describes is
not lost.

MySQL behaviour is unchanged: isSQLite was already false there, so
disableTransactions was already false.

Guarding each statement instead was considered and rejected: a complete guard
fix is ~46 call sites across three operation classes, knex has no portable
hasIndex for the 13 index-adding migrations, and guards never close the
journal-write window.

Verified on this change: full suite 216/216 suites, 2023 passed, 1 skipped,
jest exit 0; an interrupted first migration now rolls back and the next
migrate() converges to the same 24 tables and 21 journal rows as an
uninterrupted run.

Refs project-at-las/Atlas-Documentation#167
---
 .../wallet-toolbox/src/storage/StorageKnex.ts | 23 +++++++++++++++----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/packages/wallet/wallet-toolbox/src/storage/StorageKnex.ts b/packages/wallet/wallet-toolbox/src/storage/StorageKnex.ts
index cab11752d..1597ddb80 100644
--- a/packages/wallet/wallet-toolbox/src/storage/StorageKnex.ts
+++ b/packages/wallet/wallet-toolbox/src/storage/StorageKnex.ts
@@ -1206,17 +1206,30 @@ export class StorageKnex extends StorageProvider implements WalletStorageProvide
     const clientName = (this.knex.client as { config?: { client?: string } }).config?.client ?? ''
     const isSQLite = clientName.includes('sqlite')
 
-    // For SQLite, disable transactions during migrations and turn off foreign keys.
-    // PRAGMA foreign_keys is silently ignored inside transactions, so we must
-    // disable transactions for the migration to allow the PRAGMA to take effect.
-    // See: https://github.com/knex/knex/issues/4155
+    // For SQLite, turn off foreign keys for the duration of the migration.
+    // PRAGMA foreign_keys is silently ignored *when executed inside* a
+    // transaction (https://github.com/knex/knex/issues/4155), so it is issued
+    // here, outside migrate.latest(). SQLite forces a {min:1,max:1} pool, so
+    // knex's per-migration transaction runs on this same connection and
+    // inherits the PRAGMA; measured directly by reading `PRAGMA foreign_keys`
+    // from inside a migration under better-sqlite3: 0 (OFF) with transactions
+    // both enabled and disabled.
     if (isSQLite) {
       await this.knex.raw('PRAGMA foreign_keys = OFF;')
     }
 
     const config = {
       migrationSource: new KnexMigrations(this.chain, storageName, storageIdentityKey, 1024),
-      disableTransactions: isSQLite
+      // Migrations run inside knex's per-file transaction on every engine.
+      // SQLite DDL is transactional, so an interrupted migration rolls back
+      // whole instead of leaving statements the journal never recorded. With
+      // transactions disabled, a process killed between two statements of one
+      // file — or between its last statement and the journal insert — left a
+      // store that could never migrate again: every later migrate.latest()
+      // re-ran the file from its first statement and died on
+      // "table ... already exists" / "duplicate column name", permanently,
+      // with no in-package recovery path.
+      disableTransactions: false
     }
     await this.knex.migrate.latest(config)
     const version = await this.knex.migrate.currentVersion(config)