From 88130dc62f86e1147d67d1f4444dc0e996d30ffa Mon Sep 17 00:00:00 2001
From: Elis Jackson <elisjackson@icloud.com>
Date: Wed, 16 Sep 2026 15:57:26 -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

With transactions disabled for SQLite, each DDL statement autocommits while
knex records the migration in knex_migrations only after the whole file's up()
resolves. A process killed mid-file — or between a file's last statement and
the journal insert — leaves objects the journal never recorded, and every later
migrate.latest() re-runs that file from its first statement and fails on
"table ... already exists" or "duplicate column name", permanently, with no
in-package recovery path. For a wallet with a local SQLite store, 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 its journal insert atomic with its
DDL. This also covers the alterTable and index-adding migrations, which
statement-level idempotence guards cannot: knex exposes no portable hasIndex,
and guards never close the journal-write window.

PRAGMA foreign_keys is still issued outside migrate.latest(). SQLite's
single-connection pool means the migration transaction inherits it, and knex's
own alter-table rebuild deliberately leaves an ambient pragma alone while
transacting (sqlite3/schema/ddl.js: enforceForeignCheck = transacting ? null :
false), so the knex#4155 constraint the previous comment describes still holds.

MySQL behaviour is unchanged: isSQLite was already false there, so
disableTransactions was already false. dropAllData()'s own setting is
deliberately untouched.

Closes #538
---
 .../wallet-toolbox/src/storage/StorageKnex.ts | 22 ++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/packages/wallet/wallet-toolbox/src/storage/StorageKnex.ts b/packages/wallet/wallet-toolbox/src/storage/StorageKnex.ts
index 73b8c1ea4..186713710 100644
--- a/packages/wallet/wallet-toolbox/src/storage/StorageKnex.ts
+++ b/packages/wallet/wallet-toolbox/src/storage/StorageKnex.ts
@@ -1538,17 +1538,29 @@ 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 foreign keys off 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's single-connection pool means
+    // knex's per-migration transaction runs on this same connection and
+    // inherits the setting, and knex's own SQLite alter-table rebuild leaves an
+    // ambient pragma alone while transacting (sqlite3/schema/ddl.js: alter()
+    // uses `enforceForeignCheck = this.client.transacting ? null : false`).
     if (isSQLite) {
       await this.knex.raw('PRAGMA foreign_keys = OFF;')
     }
 
     const config = {
       migrationSource: new KnexMigrations(this.chain, storageName, storageIdentityKey, 1024),
-      disableTransactions: isSQLite
+      // Let knex wrap each migration file in a transaction on every engine.
+      // SQLite DDL is transactional, so an interrupted migration rolls back
+      // whole and its knex_migrations insert stays atomic with its DDL. With
+      // transactions disabled, a process killed between two statements of one
+      // file — or between its last statement and the journal insert — left
+      // objects the journal never recorded, and every later migrate.latest()
+      // re-ran the file from its first statement and failed on
+      // "table ... already exists", permanently, with no recovery path.
+      disableTransactions: false
     }
     await this.knex.migrate.latest(config)
     const version = await this.knex.migrate.currentVersion(config)