fix(driver-sql): a merge-path upsert no longer rewrites an existing row's autonumber (#7011)

.changeset/driver-sql-upsert-autonumber-immutable.md

@@ -0,0 +1,41 @@
+---
+"@objectstack/driver-sql": patch
+---
+
+fix(driver-sql): a merge-path `upsert` no longer rewrites an existing row's autonumber (#7011)
+
+Measured on a completely healthy counter, single row throughout:
+
+```
+create                      → CASE-00001    last_value 1
+upsert same id (1st time)   → CASE-00002    last_value 2
+upsert same id (2nd time)   → CASE-00003    last_value 3
+```
+
+`fillAutoNumberFields` reserves a number before the statement knows whether it
+will insert or merge, and the autonumber column sat in `mergeColumns` — so
+every `ON CONFLICT … DO UPDATE` wrote the freshly reserved number over the
+row's existing one, silently replacing an externally visible business
+identifier the caller never asked to change.
+
+Per the triage ruling on the card: an autonumber is an **immutable business
+identifier once assigned**. `auto_number` columns are now excluded from the
+merge column list, exactly like `created_at` (both are insert-only facts about
+the row's birth). After the fix the same sequence keeps `CASE-00001` through
+both upserts. The exclusion is unconditional — an explicit autonumber value in
+the upsert payload does not renumber an existing row on the merge branch
+either; `update()` writes what it is given and remains the deliberate
+renumbering path. Insert-path upserts still assign fresh numbers, and every
+non-autonumber column (including `updated_at`) merges as before.
+
+Deliberately out of scope (#6943's reseed family): the reservation itself still
+happens before insert-vs-merge is known, so a merge-only upsert still consumes
+one sequence value per call — now a permanent gap in the sequence rather than a
+rewrite of the row (measured post-fix: row keeps `CASE-00001`, `last_value`
+walks 1 → 2 → 3, the next inserted row gets `CASE-00004`).
+
+Covered faces: `SqliteWasmDriver` inherits `upsert` unchanged; `TursoDriver`
+local/replica routes its override to `super` — both pinned by their own tests.
+Turso remote (`RemoteTransport.upsert`) never enters `fillAutoNumberFields` and
+has neither the defect nor the fix. Rows already renumbered by past merges
+cannot be restored from the driver side.

packages/drivers/driver-sql/src/sql-driver-upsert-autonumber-immutable.test.ts

@@ -0,0 +1,167 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#7011] A merge-path upsert must never rewrite an existing row's autonumber.
+ *
+ * # The defect, measured on a HEALTHY counter (no staleness involved)
+ *
+ * ```
+ * create                      → CASE-00001    last_value 1
+ * upsert same id (1st time)   → CASE-00002    last_value 2
+ * upsert same id (2nd time)   → CASE-00003    last_value 3
+ * ```
+ *
+ * One row throughout — and its `case_number` was rewritten twice. The cause:
+ * `fillAutoNumberFields` reserves a number before the statement knows whether
+ * it will insert or merge, and the autonumber column sat in `mergeColumns`, so
+ * the `ON CONFLICT … DO UPDATE` branch wrote the freshly reserved number over
+ * the row's existing one.
+ *
+ * # The ruling this file pins (triage, 2026-08-09, on the card)
+ *
+ * An autonumber is an **immutable business identifier once assigned** — it is
+ * usually an externally visible document number (`CASE-00001`), and a caller
+ * asking to update a row did not ask for a new one. The fix excludes
+ * `auto_number` columns from `mergeColumns`, exactly like `created_at` (both
+ * are insert-only facts about the row's birth). The exclusion is unconditional:
+ * even an EXPLICIT autonumber value in the upsert payload does not rewrite an
+ * existing row's number on the merge branch — `update()` writes whatever it is
+ * given and remains the deliberate renumbering path.
+ *
+ * # Out of scope here, deliberately
+ *
+ * The reservation itself still happens before insert-vs-merge is known, so a
+ * merge-only upsert still consumes a sequence value (leaves a gap). That
+ * pre-burn half belongs to #6943's reseed family and is NOT pinned by this
+ * file — no test here asserts `last_value` on the merge path, so deferring the
+ * reservation later cannot turn this file red.
+ *
+ * # Reverse verification (direction predicted before running)
+ *
+ * Restoring the deleted limb — removing the autonumber exclusion from
+ * `mergeColumns` — turns exactly the merge-path pins below red, with the
+ * filing's own values (received `CASE-00002` / `CASE-00003` where `CASE-00001`
+ * was asserted). The insert-path and non-autonumber-merge cases stay green
+ * either way; they are here to pin that the exclusion does not over-reach.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { SqlDriver } from './index.js';
+
+const CRM_CASE = {
+  name: 'crm_case',
+  fields: {
+    organization_id: { type: 'string' },
+    case_number: { type: 'autonumber', format: 'CASE-{00000}', unique: true },
+    title: { type: 'string' },
+    status: { type: 'string' },
+  },
+} as any;
+
+describe('[#7011] merge-path upsert keeps the assigned autonumber', () => {
+  let driver: SqlDriver;
+
+  const knex = () => (driver as any).knex;
+
+  const rowCount = async () => Number((await knex()('crm_case').count({ c: '*' }).first()).c);
+
+  beforeEach(async () => {
+    driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
+    await driver.initObjects([CRM_CASE]);
+  });
+
+  afterEach(async () => {
+    await driver.disconnect();
+  });
+
+  it('keeps the existing number across repeated merges to the same row (the filing repro)', async () => {
+    const created = await driver.create('crm_case', { organization_id: 'orgA', title: 'original' });
+    expect(created.case_number).toBe('CASE-00001');
+
+    const first = await driver.upsert('crm_case', { id: created.id, organization_id: 'orgA', title: 'edit 1' });
+    expect(first.case_number).toBe('CASE-00001'); // was CASE-00002 before the fix
+    expect(first.title).toBe('edit 1');
+
+    const second = await driver.upsert('crm_case', { id: created.id, organization_id: 'orgA', title: 'edit 2' });
+    expect(second.case_number).toBe('CASE-00001'); // was CASE-00003 before the fix
+    expect(second.title).toBe('edit 2');
+
+    // One row throughout, and the stored value agrees with the returned one.
+    expect(await rowCount()).toBe(1);
+    const stored = await knex()('crm_case').where({ id: created.id }).first();
+    expect(stored.case_number).toBe('CASE-00001');
+  });
+
+  it('does not rewrite the number even when the payload carries an explicit one', async () => {
+    const created = await driver.create('crm_case', { organization_id: 'orgA', title: 'original' });
+    expect(created.case_number).toBe('CASE-00001');
+
+    // An explicit value is skipped by fillAutoNumberFields (caller-supplied),
+    // but it still sits in the INSERT's column list — the exclusion is what
+    // keeps it off the merge branch. `update()` remains the deliberate
+    // renumbering path for the caller who really means it.
+    const merged = await driver.upsert('crm_case', {
+      id: created.id,
+      organization_id: 'orgA',
+      case_number: 'CASE-99999',
+      title: 'tried to renumber',
+    });
+    expect(merged.case_number).toBe('CASE-00001');
+    expect(merged.title).toBe('tried to renumber');
+  });
+
+  it('insert-path upsert still assigns a fresh number', async () => {
+    const first = await driver.upsert('crm_case', { organization_id: 'orgA', title: 'new row' });
+    expect(first.case_number).toBe('CASE-00001');
+
+    const second = await driver.upsert('crm_case', { organization_id: 'orgA', title: 'another new row' });
+    expect(second.case_number).toBe('CASE-00002');
+
+    expect(await rowCount()).toBe(2);
+  });
+
+  it('still merges every non-autonumber column normally', async () => {
+    const created = await driver.create('crm_case', { organization_id: 'orgA', title: 'original', status: 'open' });
+
+    const merged = await driver.upsert('crm_case', {
+      id: created.id,
+      organization_id: 'orgA',
+      title: 'renamed',
+      status: 'closed',
+    });
+
+    expect(merged.title).toBe('renamed');
+    expect(merged.status).toBe('closed');
+    expect(merged.case_number).toBe('CASE-00001');
+    expect(await rowCount()).toBe(1);
+  });
+
+  it('a merge on a non-id conflict key keeps the number too', async () => {
+    // The exclusion is per-column, not per-conflict-target: merging on a
+    // business key instead of `id` must protect the number the same way.
+    await driver.initObjects([
+      {
+        name: 'crm_ticket',
+        fields: {
+          organization_id: { type: 'string' },
+          ticket_number: { type: 'autonumber', format: 'TKT-{0000}', unique: true },
+          external_ref: { type: 'string', unique: 'global' },
+          title: { type: 'string' },
+        },
+      } as any,
+    ]);
+
+    const created = await driver.create('crm_ticket', { organization_id: 'orgA', external_ref: 'ext-1', title: 'first' });
+    expect(created.ticket_number).toBe('TKT-0001');
+
+    const merged = await driver.upsert(
+      'crm_ticket',
+      { id: created.id, organization_id: 'orgA', external_ref: 'ext-1', title: 'merged by ref' },
+      ['external_ref'],
+    );
+    expect(merged.title).toBe('merged by ref');
+
+    const stored = await knex()('crm_ticket').where({ external_ref: 'ext-1' }).first();
+    expect(stored.ticket_number).toBe('TKT-0001');
+  });
+});

packages/drivers/driver-sql/src/sql-driver.ts

@@ -3983,6 +3983,27 @@ export class SqlDriver implements IDataDriver {
     return this.formatOutput(object, updated) || null;
   }
 
+  /**
+   * Columns `upsert`'s merge branch must never write ([#7011]): `created_at`
+   * (the row's birth timestamp belongs to the original insert) and every
+   * `auto_number` column — an autonumber is an immutable business identifier
+   * once assigned, so an upsert that lands on an existing row keeps that row's
+   * number (see the fuller rationale at the merge site). Autonumber columns
+   * are returned under their PHYSICAL names (an external object can remap
+   * logical fields via `external.columnMap`), matching the
+   * `applyWriteColumnMap`-processed row the merge column list is derived from;
+   * `created_at` stays the literal post-map key it has always been filtered as.
+   */
+  protected insertOnlyUpsertColumns(object: string): Set<string> {
+    // Same config resolution as `fillAutoNumberFields`: object name first,
+    // then the storage-mapped table name.
+    const tableName = this.physicalTableByObject[object] ?? StorageNameMapping.resolveTableName({ name: object } as any);
+    const cfgs = this.autoNumberFields[object] || this.autoNumberFields[tableName] || [];
+    const columns = new Set<string>(['created_at']);
+    for (const cfg of cfgs) columns.add(this.remoteColumn(object, cfg.name, cfg.name));
+    return columns;
+  }
+
   async upsert(object: string, data: Record<string, any>, conflictKeys?: string[], options?: DriverOptions): Promise<Record<string, any>> {
     const { _id, ...rest } = data;
     const toUpsert = { ...rest };
@@ -4022,9 +4043,22 @@ export class SqlDriver implements IDataDriver {
       const builder = this.getBuilder(this.rotationWriteTarget(object) ?? object, options);
       // `created_at` is insert-only — never overwrite it when an existing row is
       // merged on conflict (the stamped/seeded value belongs to the original
-      // insert). Everything else (incl. `updated_at`) merges as before, so an
-      // upsert that updates a row still advances `updated_at`.
-      const mergeColumns = Object.keys(formatted).filter((c) => c !== 'created_at');
+      // insert). [#7011] `auto_number` columns are insert-only for the same
+      // reason, and a stronger one: the number is an externally visible
+      // business identifier once assigned (`CASE-00001`), and
+      // `fillAutoNumberFields` above reserved a FRESH value before the
+      // statement could know it would merge — leaving these columns in the
+      // merge set rewrote the existing row's number on every merge-path upsert
+      // (measured on a healthy counter: create → CASE-00001, two upserts of
+      // the same id → CASE-00002 then CASE-00003, one row throughout). The
+      // exclusion is unconditional — an explicit payload value does not
+      // renumber on merge either; `update()` is the deliberate renumbering
+      // path. The reservation itself still happens on the merge path (a gap,
+      // not a rewrite) — that pre-burn half is #6943's reseed family, not
+      // this exclusion's. Everything else (incl. `updated_at`) merges as
+      // before, so an upsert that updates a row still advances `updated_at`.
+      const insertOnlyColumns = this.insertOnlyUpsertColumns(object);
+      const mergeColumns = Object.keys(formatted).filter((c) => !insertOnlyColumns.has(c));
       const insertion = builder.insert(formatted).onConflict(mergeKeys);
 
       try {

packages/drivers/driver-sqlite-wasm/src/sqlite-wasm-autonumber-batch-resync.test.ts

@@ -76,4 +76,16 @@ describe('[#6943] driver-sqlite-wasm inherits the batch/upsert autonumber re-see
     const upserted = await driver.upsert('crm_case', { organization_id: 'orgA', title: 'u1' });
     expect(upserted.case_number).toBe('CASE-00031');
   });
+
+  it('[#7011] a merge-path upsert keeps the existing autonumber on this transport too', async () => {
+    // Inherited from `SqlDriver.upsert` (no override) — pinned here because the
+    // exclusion is applied to the ON CONFLICT merge column list, and this
+    // driver swaps the transport under that statement.
+    const created = await driver.create('crm_case', { organization_id: 'orgA', title: 'original' });
+    expect(created.case_number).toBe('CASE-00001');
+
+    const merged = await driver.upsert('crm_case', { id: created.id, organization_id: 'orgA', title: 'edited' });
+    expect(merged.case_number).toBe('CASE-00001');
+    expect(merged.title).toBe('edited');
+  });
 });

packages/drivers/driver-turso/src/turso-autonumber-batch-resync.test.ts

@@ -89,6 +89,27 @@ describe('[#6943] TursoDriver batch/upsert autonumber re-seed', () => {
     expect(upserted.case_number).toBe('CASE-00031');
   });
 
+  it('LOCAL: [#7011] a merge-path upsert keeps the existing autonumber', async () => {
+    // Turso OVERRIDES `upsert` (remote → RemoteTransport, else `super`), so
+    // "the base class was fixed" is not on its own an answer about this face —
+    // the local route through `super.upsert` is pinned here.
+    const created = await driver.create(
+      'crm_case',
+      { organization_id: 'orgA', title: 'original' },
+      { bypassTenantAudit: true },
+    );
+    expect(created.case_number).toBe('CASE-00001');
+
+    const merged = await driver.upsert(
+      'crm_case',
+      { id: created.id, organization_id: 'orgA', title: 'edited' },
+      undefined,
+      { bypassTenantAudit: true } as any,
+    );
+    expect(merged.case_number).toBe('CASE-00001');
+    expect(merged.title).toBe('edited');
+  });
+
   it('REMOTE: the transport that bypasses this path has no autonumber machinery to re-seed', async () => {
     const remote = new TursoDriver({ url: 'libsql://example.turso.io', authToken: 'placeholder' });
     expect(remote.transportMode).toBe('remote');