fix(driver-sql): pre-create autonumber sequences table to avoid batch/tx deadlock

.changeset/fix-autonumber-batch-deadlock.md

@@ -0,0 +1,29 @@
+---
+'@objectstack/driver-sql': patch
+---
+
+Fix a connection-pool deadlock when the first `auto_number` write after process
+start goes through a transaction (e.g. `POST /api/v1/batch`, which wraps every
+operation in one `ql.transaction(...)`).
+
+The sequence-counter table (`_objectstack_sequences`) was created lazily on the
+first autonumber INSERT via a bare `this.knex.schema.*` call that asks the pool
+for a second connection. On SQLite (better-sqlite3, pool max=1) the open batch
+transaction already holds the only connection, so the acquire blocked until
+`Knex: Timeout acquiring a connection`. Postgres/MySQL are exposed to the same
+pool-exhaustion deadlock under concurrent cold first-writes.
+
+Fixes:
+- `initObjects` now pre-creates the counter table up front, outside any data
+  transaction, so the first write never runs DDL (primary fix).
+- The lazy fallback (`ensureSequencesTable`) now runs its DDL on the caller's own
+  transaction on SQLite instead of grabbing a second connection. It deliberately
+  does not route DDL through the caller's transaction on MySQL, where DDL would
+  implicitly commit the caller's in-flight transaction.
+- Added a dev/test guard (`assertBareKnexSafe`): on SQLite, issuing a bare
+  `this.knex` query while a transaction holds the single pooled connection now
+  fails fast with an actionable error instead of hanging until the opaque
+  `Knex: Timeout acquiring a connection`. No-op in production and on non-SQLite
+  dialects, so it adds no runtime cost on the hot path — it just turns this whole
+  class of "forgot to thread the transaction through" bug into an immediate,
+  self-explaining failure at the call site.

packages/plugins/driver-sql/src/sql-driver-autonumber-tokens.test.ts

@@ -209,7 +209,8 @@ describe('SqlDriver auto_number format tokens', () => {
     await driver.initObjects([
       { name: 'invoice', fields: { invoice_number: { type: 'autonumber', format: 'INV-{0000}' } } },
     ]);
-    // First create triggers the lazy table-ensure → in-place migration.
+    // initObjects pre-ensures the table → the in-place migration runs at init,
+    // before any write; the first create then reads the already-migrated counter.
     const r = await driver.create('invoice', {});
 
     const cols = await k('_objectstack_sequences').columnInfo();

packages/plugins/driver-sql/src/sql-driver-autonumber-tx.test.ts

@@ -0,0 +1,119 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// Regression guard for the auto_number-in-transaction deadlock.
+//
+// Reported: on SQLite (better-sqlite3, pool max=1), the first autonumber write
+// after process start that goes through a transaction — e.g. POST /api/v1/batch,
+// which wraps every operation in one `ql.transaction(...)` — dead-locked with
+// "Knex: Timeout acquiring a connection. The pool is probably full."
+//
+// Root cause: the sequence-counter table (`_objectstack_sequences`) was created
+// lazily on the first autonumber INSERT, via a bare `this.knex.schema.*` call
+// that asks the pool for a SECOND connection. Inside a batch transaction the
+// only pooled connection is already held, so the acquire blocked until timeout.
+//
+// Fixes under test:
+//   1. `initObjects` pre-creates the table outside any data transaction, so the
+//      first write never runs DDL (primary fix — covers the real batch path).
+//   2. The lazy fallback (`ensureSequencesTable`) runs its DDL on the caller's
+//      own transaction on SQLite instead of grabbing a second connection, so
+//      even a cold-cache in-transaction first write cannot deadlock.
+
+import { describe, it, expect, afterEach } from 'vitest';
+import { SqlDriver } from '../src/index.js';
+
+const SEQUENCES_TABLE = '_objectstack_sequences';
+
+function withTimeout<T>(p: Promise<T>, ms: number, label: string): Promise<T> {
+  return Promise.race([
+    p,
+    new Promise<T>((_, rej) => setTimeout(() => rej(new Error(`TIMEOUT: ${label} (deadlock)`)), ms)),
+  ]);
+}
+
+describe('SqlDriver auto_number in-transaction (deadlock regression)', () => {
+  let driver: SqlDriver | undefined;
+
+  afterEach(async () => {
+    if (driver) await driver.disconnect();
+    driver = undefined;
+  });
+
+  async function setup() {
+    driver = new SqlDriver({
+      client: 'better-sqlite3',
+      connection: { filename: ':memory:' },
+      useNullAsDefault: true,
+    });
+    await driver.initObjects([
+      {
+        name: 'contract',
+        fields: {
+          organization_id: { type: 'string' },
+          contract_number: { type: 'autonumber', format: 'CTR-{0000}' },
+          name: { type: 'string' },
+        },
+      },
+    ]);
+    return (driver as any).knex;
+  }
+
+  it('pre-creates the sequences table during initObjects (before any write)', async () => {
+    const k = await setup();
+    // The counter table must exist immediately after initObjects, without a
+    // single create() — this is what keeps the first write off the DDL path.
+    expect(await k.schema.hasTable(SEQUENCES_TABLE)).toBe(true);
+    expect((driver as any).sequencesTableReady).toBe(true);
+  });
+
+  it('commits a batch-style transaction whose FIRST write fills an autonumber', async () => {
+    const k = await setup();
+
+    // Two autonumber creates inside a single transaction, mirroring how the REST
+    // /batch endpoint wraps operations. Must commit, not hang.
+    const trx = await driver!.beginTransaction();
+    const r1 = await withTimeout(
+      driver!.create('contract', { organization_id: 'org_x', name: 'A' }, { transaction: trx } as any),
+      6000,
+      'batch create #1 (autonumber)',
+    );
+    const r2 = await withTimeout(
+      driver!.create('contract', { organization_id: 'org_x', name: 'B' }, { transaction: trx } as any),
+      6000,
+      'batch create #2 (autonumber)',
+    );
+    await withTimeout(driver!.commit(trx), 6000, 'commit');
+
+    expect(r1.contract_number).toBe('CTR-0001');
+    expect(r2.contract_number).toBe('CTR-0002');
+
+    const rows = await k('contract').select('contract_number').orderBy('contract_number');
+    expect(rows.map((r: any) => r.contract_number)).toEqual(['CTR-0001', 'CTR-0002']);
+  });
+
+  it('does not deadlock even with a COLD cache inside the transaction (lazy fallback)', async () => {
+    const k = await setup();
+
+    // Simulate the path where the table was NOT pre-created (an external object,
+    // or a consumer that writes without initObjects): drop it and clear the
+    // process cache, then take the single connection with a transaction and let
+    // the first autonumber write hit the lazy ensure. On the old code this asked
+    // the pool for a second connection and blocked until acquire-timeout.
+    await k.schema.dropTableIfExists(SEQUENCES_TABLE);
+    (driver as any).sequencesTableReady = false;
+    (driver as any).sequencesHasKeyHash = false;
+    (driver as any).sequencesTableEnsurePromise = null;
+
+    const trx = await driver!.beginTransaction();
+    const r1 = await withTimeout(
+      driver!.create('contract', { organization_id: 'org_cold', name: 'C' }, { transaction: trx } as any),
+      6000,
+      'cold-cache create inside tx',
+    );
+    await withTimeout(driver!.commit(trx), 6000, 'commit');
+
+    expect(r1.contract_number).toBe('CTR-0001');
+    // The table the fallback created on the transaction survives the commit.
+    expect(await k.schema.hasTable(SEQUENCES_TABLE)).toBe(true);
+  });
+});

packages/plugins/driver-sql/src/sql-driver-sqlite-tx-guard.test.ts

@@ -0,0 +1,196 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// Dev/test guard against the SQLite single-connection dead-lock.
+//
+// SQLite's connection pool hands out exactly one connection. Issuing a bare
+// `this.knex` query while a transaction holds that connection blocks forever
+// acquiring a second one and finally fails with the opaque "Knex: Timeout
+// acquiring a connection" — the reported /api/v1/batch autonumber dead-lock.
+//
+// `assertBareKnexSafe` turns that latent, timeout-delayed hang into an
+// immediate, actionable error at the call site. This is the regression tripwire
+// for "a caller opened a transaction but forgot to thread it through": the
+// sequence-table ensure would otherwise silently fall back to `this.knex` and
+// dead-lock. The guard is a no-op in production and on non-SQLite dialects.
+
+import { describe, it, expect, afterEach, beforeEach } from 'vitest';
+import { SqlDriver } from '../src/index.js';
+
+const SEQUENCES_TABLE = '_objectstack_sequences';
+
+function withTimeout<T>(p: Promise<T>, ms: number, label: string): Promise<T> {
+  return Promise.race([
+    p,
+    new Promise<T>((_, rej) => setTimeout(() => rej(new Error(`TIMEOUT: ${label}`)), ms)),
+  ]);
+}
+
+describe('SqlDriver SQLite single-connection tx guard', () => {
+  let driver: SqlDriver | undefined;
+  const savedEnv = process.env.NODE_ENV;
+
+  afterEach(async () => {
+    if (savedEnv === undefined) delete process.env.NODE_ENV;
+    else process.env.NODE_ENV = savedEnv;
+    if (driver) await driver.disconnect();
+    driver = undefined;
+  });
+
+  async function setup() {
+    driver = new SqlDriver({
+      client: 'better-sqlite3',
+      connection: { filename: ':memory:' },
+      useNullAsDefault: true,
+    });
+    await driver.initObjects([
+      {
+        name: 'contract',
+        fields: {
+          contract_number: { type: 'autonumber', format: 'CTR-{0000}' },
+          name: { type: 'string' },
+        },
+      },
+    ]);
+    return (driver as any).knex;
+  }
+
+  /** Force the lazy fallback: drop the pre-created table and clear the cache. */
+  function coldStart() {
+    (driver as any).sequencesTableReady = false;
+    (driver as any).sequencesHasKeyHash = false;
+    (driver as any).sequencesTableEnsurePromise = null;
+  }
+
+  describe('beginTransaction bookkeeping', () => {
+    it('counts an open transaction and releases it on commit', async () => {
+      await setup();
+      expect((driver as any).activeTransactions).toBe(0);
+      const trx = await driver!.beginTransaction();
+      expect((driver as any).activeTransactions).toBe(1);
+      await driver!.commit(trx);
+      expect((driver as any).activeTransactions).toBe(0);
+    });
+
+    it('releases the count on rollback too', async () => {
+      await setup();
+      const trx = await driver!.beginTransaction();
+      expect((driver as any).activeTransactions).toBe(1);
+      await driver!.rollback(trx);
+      expect((driver as any).activeTransactions).toBe(0);
+    });
+
+    it('releases via the deprecated commitTransaction/rollbackTransaction aliases', async () => {
+      await setup();
+      const t1 = await driver!.beginTransaction();
+      await driver!.commitTransaction(t1);
+      expect((driver as any).activeTransactions).toBe(0);
+      const t2 = await driver!.beginTransaction();
+      await driver!.rollbackTransaction(t2);
+      expect((driver as any).activeTransactions).toBe(0);
+    });
+
+    it('does not double-decrement when a transaction is closed twice', async () => {
+      await setup();
+      const trx = await driver!.beginTransaction();
+      expect((driver as any).activeTransactions).toBe(1);
+      await driver!.commit(trx);
+      expect((driver as any).activeTransactions).toBe(0);
+      // A redundant second close must not drive the count negative.
+      await driver!.commit(trx).catch(() => {});
+      expect((driver as any).activeTransactions).toBe(0);
+    });
+  });
+
+  describe('guard fires on the dead-lock shape', () => {
+    it('fails fast (not a 6s timeout) when a cold ensure runs bare-knex mid-transaction', async () => {
+      const k = await setup();
+      await k.schema.dropTableIfExists(SEQUENCES_TABLE);
+      coldStart();
+
+      // A transaction now owns the single connection. A create WITHOUT threading
+      // that transaction through hits the lazy ensure on `this.knex` — the guard
+      // must reject immediately instead of blocking on connection acquisition.
+      const trx = await driver!.beginTransaction();
+      try {
+        await withTimeout(
+          (driver as any).ensureSequencesTable(undefined),
+          2000,
+          'ensureSequencesTable should have thrown, not hung',
+        );
+        throw new Error('expected the guard to throw');
+      } catch (err: any) {
+        expect(err.message).toMatch(/transaction is open|dead-lock|Timeout acquiring/i);
+        // Specifically the guard's message, not a real acquire-timeout.
+        expect(err.message).not.toMatch(/^TIMEOUT:/);
+      } finally {
+        await driver!.rollback(trx);
+      }
+    });
+
+    it('surfaces the guard through a real create() that forgot to pass the tx', async () => {
+      const k = await setup();
+      await k.schema.dropTableIfExists(SEQUENCES_TABLE);
+      coldStart();
+
+      const trx = await driver!.beginTransaction();
+      try {
+        await expect(
+          withTimeout(
+            driver!.create('contract', { name: 'A' }), // no { transaction: trx } — the bug shape
+            2000,
+            'create should have failed fast',
+          ),
+        ).rejects.toThrow(/transaction is open|dead-lock/i);
+      } finally {
+        await driver!.rollback(trx);
+      }
+    });
+  });
+
+  describe('guard does NOT fire on legitimate paths', () => {
+    it('allows a cold ensure that correctly rides the caller transaction', async () => {
+      const k = await setup();
+      await k.schema.dropTableIfExists(SEQUENCES_TABLE);
+      coldStart();
+
+      const trx = await driver!.beginTransaction();
+      // parentTrx passed → runs on the transaction, no bare-knex, no throw.
+      await expect((driver as any).ensureSequencesTable(trx)).resolves.toBeUndefined();
+      await driver!.commit(trx);
+      expect(await k.schema.hasTable(SEQUENCES_TABLE)).toBe(true);
+    });
+
+    it('allows bare-knex ensure when NO transaction is open (the initObjects path)', async () => {
+      const k = await setup();
+      await k.schema.dropTableIfExists(SEQUENCES_TABLE);
+      coldStart();
+      // activeTransactions === 0 → bare knex is safe.
+      await expect((driver as any).ensureSequencesTable(undefined)).resolves.toBeUndefined();
+      expect(await k.schema.hasTable(SEQUENCES_TABLE)).toBe(true);
+    });
+  });
+
+  describe('assertBareKnexSafe branch matrix', () => {
+    beforeEach(async () => {
+      await setup();
+    });
+
+    it('throws only in the danger combination: non-prod + sqlite + open tx', () => {
+      (driver as any).activeTransactions = 1;
+      delete process.env.NODE_ENV;
+      expect(() => (driver as any).assertBareKnexSafe('x')).toThrow(/transaction is open/i);
+    });
+
+    it('is a no-op in production (guard must never break real deployments)', () => {
+      (driver as any).activeTransactions = 1;
+      process.env.NODE_ENV = 'production';
+      expect(() => (driver as any).assertBareKnexSafe('x')).not.toThrow();
+    });
+
+    it('is a no-op when no transaction is open', () => {
+      (driver as any).activeTransactions = 0;
+      delete process.env.NODE_ENV;
+      expect(() => (driver as any).assertBareKnexSafe('x')).not.toThrow();
+    });
+  });
+});