fix(objectql): assign autonumbers after validation; document at-least-once hooks (#3152)

In the batch insert path, beforeInsert hooks and autonumber assignment
ran before per-row validation. When bulkWrite degrades a batch to per-row
(or retries it), the whole engine.insert re-runs, so a batch that dies in
validation had already consumed an autonumber sequence value for every
good row — leaving number-range gaps — and re-fired side-effecting
beforeInsert hooks on the retry.

Move autonumber assignment to after validation: a doomed attempt now
consumes no sequence value (required-validation already exempts
autonumber fields, and a driver-owned autonumber assigns nothing here, so
no validation rule can depend on the value — the reorder is safe). The
remaining beforeInsert double-execution on a degraded batch is documented
as at-least-once hook semantics on engine.insert (hooks must be
idempotent); root-causing it needs a partial-success engine API, out of
scope here.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga

Author: claude

packages/objectql/src/engine-autonumber-batch.test.ts

@@ -0,0 +1,90 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// framework#3152: a batch insert that dies in validation must not have already
+// consumed autonumber sequence values for its good rows. Autonumbers are now
+// assigned AFTER validation, so a doomed attempt (which bulkWrite then degrades
+// to per-row) leaves no gaps in the number range.
+
+import { describe, it, expect } from 'vitest';
+import { ObjectQL } from './engine.js';
+
+function makeDriver() {
+  const stores = new Map<string, Map<string, any>>();
+  const storeFor = (o: string) => {
+    let s = stores.get(o);
+    if (!s) { s = new Map(); stores.set(o, s); }
+    return s;
+  };
+  let n = 0;
+  const driver: any = {
+    name: 'memory', version: '0.0.0', supports: {},
+    async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
+    async find(object: string) { return Array.from(storeFor(object).values()); },
+    findStream() { throw new Error('ni'); },
+    async findOne() { return null; },
+    async create(object: string, data: Record<string, unknown>) {
+      n += 1;
+      const id = (data.id as string) ?? `r_${n}`;
+      const row = { ...data, id };
+      storeFor(object).set(id, row);
+      return row;
+    },
+    async update(object: string, id: string, data: Record<string, unknown>) {
+      const s = storeFor(object);
+      const row = { ...s.get(id), ...data, id };
+      s.set(id, row);
+      return row;
+    },
+    async delete(object: string, id: string) { return storeFor(object).delete(id); },
+    async count() { return 0; },
+    async bulkCreate(object: string, rows: Record<string, unknown>[]) {
+      const out: any[] = [];
+      for (const r of rows) out.push(await this.create(object, r));
+      return out;
+    },
+    async bulkUpdate() { return []; }, async bulkDelete() {},
+    async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; },
+    async commit() {}, async rollback() {},
+  };
+  return { driver, storeFor };
+}
+
+async function makeEngine() {
+  const engine = new ObjectQL();
+  const d = makeDriver();
+  engine.registerDriver(d.driver, true);
+  await engine.init();
+  engine.registry.registerObject({
+    name: 'task',
+    fields: {
+      name: { type: 'text', required: true },
+      code: { type: 'autonumber', format: 'T-{000}' },
+    },
+  } as any);
+  return { engine, storeFor: d.storeFor };
+}
+
+describe('batch insert — autonumber assigned after validation (framework#3152)', () => {
+  it('a validation-failed batch consumes no autonumber, leaving no gap for the degraded per-row retry', async () => {
+    const { engine } = await makeEngine();
+
+    // A batch with a bad (name-less) middle row fails in validation before any
+    // driver write. Previously the good rows had already been numbered.
+    await expect(engine.insert('task', [{ name: 'a' }, { slug: 'no-name' }, { name: 'b' }]))
+      .rejects.toThrow();
+
+    // bulkWrite would now degrade to per-row; simulate that by inserting the
+    // good rows individually. Their numbers must be contiguous from 1.
+    const a = await engine.insert('task', { name: 'a' });
+    const b = await engine.insert('task', { name: 'b' });
+
+    expect(a.code).toBe('T-001');
+    expect(b.code).toBe('T-002'); // no gap — the failed batch consumed nothing
+  });
+
+  it('a fully-valid batch still numbers every row contiguously', async () => {
+    const { engine } = await makeEngine();
+    const rows = await engine.insert('task', [{ name: 'a' }, { name: 'b' }, { name: 'c' }]);
+    expect((rows as any[]).map((r) => r.code)).toEqual(['T-001', 'T-002', 'T-003']);
+  });
+});

packages/objectql/src/engine.ts

@@ -2200,6 +2200,20 @@ export class ObjectQL implements IDataEngine {
     return opCtx.result;
   }
 
+  /**
+   * Insert one record or an array of records.
+   *
+   * At-least-once hook semantics (framework#3152): when this call is driven by
+   * `bulkWrite` (seed / import), a transient batch retry or a per-row
+   * degradation re-runs the whole insert, so a `beforeInsert` hook may fire
+   * MORE THAN ONCE for the same input row (a first attempt that failed in
+   * validation, then the degraded retry). Side-effecting `beforeInsert` hooks
+   * (notifications, external calls, counters) must therefore be idempotent.
+   * `afterInsert` hooks fire only on a successful write, so they are not
+   * re-run by a validation failure. Autonumbers are assigned only after
+   * validation passes, so a doomed attempt no longer consumes a sequence value
+   * (no number-range gaps from a rejected batch).
+   */
   async insert(object: string, data: any | any[], options?: DataEngineInsertOptions): Promise<any> {
     object = this.resolveObjectName(object);
     this.logger.debug('Insert operation starting', { object, isBatch: Array.isArray(data) });
@@ -2271,9 +2285,6 @@ export class ObjectQL implements IDataEngine {
         // Defaults are already resolved above (pre-hook, #2703); a hook may
         // have overridden fields or replaced `input.data` — take its data as-is.
         const rows = rowHookContexts.map((rowCtx) => rowCtx.input.data as Record<string, unknown>);
-        for (const r of rows) {
-          await this.applyAutonumbers(object, r, opCtx.context, driverOwnsAutonumber);
-        }
         for (const r of rows) {
           await this.encryptSecretFields(object, r, opCtx.context, driverOptions);
         }
@@ -2282,6 +2293,17 @@ export class ObjectQL implements IDataEngine {
           validateRecord(schemaForValidation, r, 'insert');
           evaluateValidationRules(schemaForValidation as any, r, 'insert', { logger: this.logger, currentUser: this.buildEvalUser(opCtx.context) });
         }
+        // Autonumbers are assigned AFTER validation (framework#3152): in the
+        // batch path a bulkWrite retry / per-row degradation re-runs the whole
+        // engine.insert, and a batch that dies in validation would otherwise
+        // have already consumed a sequence value for every good row on the
+        // failed attempt, leaving gaps. Required-validation exempts autonumber
+        // fields either way (they are engine-assigned), and a driver that owns
+        // autonumber assigns nothing here — so no validation rule can depend on
+        // the value, making this reorder safe.
+        for (const r of rows) {
+          await this.applyAutonumbers(object, r, opCtx.context, driverOwnsAutonumber);
+        }
         if (isBatch) {
           if (driver.bulkCreate) {
                result = await driver.bulkCreate(object, rows, driverOptions);