From b17465bf085a28d8d653c1813276cf444533a9cd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 03:06:45 +0000 Subject: [PATCH 1/9] wip(objectql): envelope driver unique violations at the insert door (#14095) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- .../objectql/src/duplicate-record-error.ts | 133 ++++++++++++++++++ packages/objectql/src/engine.ts | 89 +++++++++--- packages/objectql/src/index.ts | 8 ++ 3 files changed, 212 insertions(+), 18 deletions(-) create mode 100644 packages/objectql/src/duplicate-record-error.ts diff --git a/packages/objectql/src/duplicate-record-error.ts b/packages/objectql/src/duplicate-record-error.ts new file mode 100644 index 0000000000..c3c6b3980d --- /dev/null +++ b/packages/objectql/src/duplicate-record-error.ts @@ -0,0 +1,133 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/types'; + +/** + * The ADR-0112 envelope `engine.insert` raises when a driver refuses a row as a + * unique-constraint violation (#14095). + * + * ## The defect this retires + * + * The platform RECOMMENDS "declare a unique index, attempt the insert, swallow + * the violation" — `createWithAutonumberResync`'s own doc argues at length + * against the read-then-write alternative ("a probe costs a query on every + * insert … and is still racy"). An application could not complete that pattern, + * because the insert door rethrew the DRIVER's error verbatim and left the app + * three bad options: + * + * 1. test `err.code === 'SQLITE_CONSTRAINT_UNIQUE'` — couples the app to one + * dialect, and it silently stops being idempotent the day it is deployed on + * Postgres (`23505`), MySQL (`ER_DUP_ENTRY`) or Mongo (`E11000`); + * 2. pattern-match the message — which on the measured SQLite path is the + * whole compiled INSERT statement; + * 3. use the platform's own `isUniqueViolationError`, which is correct and + * dialect-independent and lives in `@objectstack/types` — a package an + * application cannot reach (`ERR_MODULE_NOT_FOUND`). + * + * Triage ruled (2026-09-01) for wrapping in ObjectQL rather than re-exporting + * the predicate: 「抛一个带既有词表码(`DUPLICATE_RECORD` 已在 ADR-0112 台账里) + * 的平台错误,原驱动错误作 `cause` ⇒ `insert` 在每个驱动上有同一份契约」. + * + * ## The shape, and why each half is here + * + * - **`code: 'DUPLICATE_RECORD'`** — already a member of `StandardErrorCode` + * (the 409 conflict group), so nothing in `packages/spec` had to grow a + * member for this. It is what an application branches on, on every driver. + * - **`status: 409`** — the conflict status the engine's sibling refusals + * already declare (`DELETE_RESTRICTED`, `CONCURRENT_UPDATE`), so REST's + * declared-status passthrough answers 409 instead of the sanitised 500 an + * undeclared status would have earned. + * - **`cause`** — the driver's own error, WHOLE and unmodified. Nothing is + * copied out of it into the message: `isUniqueViolationError` and + * `uniqueViolationColumn` both walk a `cause` chain, so every existing + * consumer of the raw error keeps its answer by asking the envelope. + * - **`field`** — the conflicting column, and ONLY when + * {@link uniqueViolationColumn} determinably named one. An index name is + * never reported as a column (MySQL's `for key 'idx_email_unique'`, + * Postgres' constraint name, a composite key): that function's contract is + * that a wrong field name is worse than none, and this envelope does not + * widen it. + * - **`developerMessage`** — the remedy half, split off exactly as the + * engine's `DELETE_RESTRICTED` refusal splits it: `message` is the sentence + * a user-facing surface renders, `developerMessage` is the one addressed to + * the application author. + * + * ## ⛔ What this deliberately does NOT do + * + * It does not copy the driver's prose into `message`. The measured SQLite + * message IS the compiled statement with its bound values, and REST's declared + * 4xx arm ships `message` to the client verbatim — so quoting the driver here + * would move #8682's leak from the log onto the wire. The driver's own + * diagnosis stays reachable, in one place, on `cause`. + */ +export const DUPLICATE_RECORD_CODE = 'DUPLICATE_RECORD' as const; + +/** The conflict status the engine's sibling 409 refusals declare. */ +const DUPLICATE_RECORD_STATUS = 409 as const; + +export class DuplicateRecordError extends Error { + readonly code = DUPLICATE_RECORD_CODE; + readonly status = DUPLICATE_RECORD_STATUS; + /** The remedy half — see the module header on why it is not in `message`. */ + readonly developerMessage: string; + + constructor( + /** The object the refused insert targeted. */ + public readonly object: string, + /** The driver's own error, whole. */ + cause: unknown, + /** The conflicting column, when the dialect determinably named one. */ + public readonly field?: string, + ) { + super(buildDuplicateMessage(object, field)); + this.name = 'DuplicateRecordError'; + // Assigned rather than passed as `new Error(msg, { cause })`: this repo + // compiles against `lib: ES2020`, where `ErrorOptions` does not exist — + // the same reason `ERR_AUTONUMBER_COLLISION` one file over assigns it. + (this as { cause?: unknown }).cause = cause; + this.developerMessage = + `The driver refused this insert as a unique-constraint violation. Its own error is attached ` + + `as \`cause\` — branch on \`code === '${DUPLICATE_RECORD_CODE}'\` (ADR-0112) rather than on a ` + + `dialect's code or message, so the handling survives a change of store. To make the write ` + + `idempotent, catch this code and treat the row as already present.`; + } +} + +/** + * The user-facing sentence. + * + * ⛔ It must not begin with a SQL verb. `@objectstack/rest`'s importer runs + * every row error through `sanitizeRowError`, whose SQL backstop replaces any + * message STARTING with `insert`/`update`/`delete`/… with generic text — so a + * message opening "Insert on 'x' …" would be thrown away by a guard written + * against leaked driver statements. Measured, not guessed. + */ +function buildDuplicateMessage(object: string, field?: string): string { + return ( + `Duplicate record refused on '${object}': ` + + (field + ? `a unique constraint on '${field}' already holds this value. ` + : 'a unique constraint already holds these values. ') + + 'No record was written.' + ); +} + +/** + * The insert door's driver-error exit: the platform envelope for a unique + * violation, or the caller's own error unchanged for anything else. + * + * **Unrecognised is passed through untouched**, which is the whole of the + * negative contract: a NOT NULL violation, a deadlock, a missing table and an + * unreachable store all leave the door exactly as they did before. The verdict + * is {@link isUniqueViolationError} — never a dialect sniff and never a message + * match written here. + * + * Idempotent: an error that is already this envelope is returned as-is, so a + * seam that wraps a value another seam already wrapped cannot bury the driver's + * error one `cause` step deeper. + */ +export function envelopeUniqueViolation(error: unknown, object: string): unknown { + if (error instanceof DuplicateRecordError) return error; + if (!isUniqueViolationError(error)) return error; + return new DuplicateRecordError(object, error, uniqueViolationColumn(error)); +} diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 3864e5b226..a99557d92e 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -119,6 +119,7 @@ import { isMissingTableError } from '@objectstack/metadata/errors'; // engine is the consumer-side tolerant parsing PD #12 forbids and precedent // #5841 retired. import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/types'; +import { envelopeUniqueViolation } from './duplicate-record-error.js'; // [#8682] The write-path loggers' redaction — bound values never reach the log. import { redactBoundStatement } from './driver-fault-redaction.js'; // [#8844] The runtime half of #8686's ruling: a system-context write on a @@ -4472,8 +4473,12 @@ export class ObjectQL implements IObjectQLEngine { * (#6250) — never a word-list of this method's own. * - When the dialect names the conflicting COLUMN (`uniqueViolationColumn`, * #6544), it must be one of the fields the engine issued. A conflict on - * some other unique field is the caller's business error and is rethrown - * untouched, exactly as #5495's disposition ruled («非本字段的冲突原样上抛»). + * some other unique field is the caller's business error and is NOT + * re-issued, exactly as #5495's disposition ruled («非本字段的冲突原样上抛»); + * since #14095 it leaves the door as the `DUPLICATE_RECORD` envelope + * (`DuplicateRecordError`) carrying that same driver error as `cause` — + * the disposition is about not RE-ISSUING, and the envelope changes + * nothing about which failures qualify. * When the dialect names no determinable column the attribution falls back * to "the engine issued a number on this row, and the row was refused as a * duplicate" — deliberately, because `uniqueViolationColumn` answers @@ -4565,7 +4570,14 @@ export class ObjectQL implements IObjectQLEngine { try { return await driver.create(object, row, driverOptions); } catch (error) { - if (!this.isIssuedAutonumberCollision(error, issued)) throw error; + // [#14095] Not OUR collision to re-issue — so this is where a driver's + // refusal leaves the single-row door, and where it stops being the + // driver's error. `envelopeUniqueViolation` returns everything that is + // not a unique violation untouched (a deadlock, a NOT NULL, an + // unreachable store), so only the recognised conflict changes shape. + if (!this.isIssuedAutonumberCollision(error, issued)) { + throw envelopeUniqueViolation(error, object); + } // Whatever happens next, the stale counter must not survive this call: // leaving it in place is what turned one collision into a storm. for (const one of issued) this.autonumberCounters.delete(one.counterKey); @@ -4590,8 +4602,17 @@ export class ObjectQL implements IObjectQLEngine { for (const one of issued) delete row[one.field]; issued = await this.applyAutonumbers(object, row, execCtx, driverOwnsAutonumber); // Nothing left to re-issue (the field vanished from the schema - // mid-flight) — the next failure is the caller's to see. - if (issued.length === 0) return await driver.create(object, row, driverOptions); + // mid-flight) — the next failure is the caller's to see, enveloped on + // the same terms as the exit above (#14095): this call is OUTSIDE the + // loop's own `try`, so its rejection is a second driver-error exit and + // not a path the branch above covers. + if (issued.length === 0) { + try { + return await driver.create(object, row, driverOptions); + } catch (retryError) { + throw envelopeUniqueViolation(retryError, object); + } + } } } } @@ -9459,6 +9480,30 @@ export class ObjectQL implements IObjectQLEngine { }; } + /** + * Create one record, or a batch of them. + * + * # The error contract on a unique violation (#14095) + * + * A driver's unique-constraint refusal leaves this door as the ADR-0112 + * envelope `DuplicateRecordError` — `code: 'DUPLICATE_RECORD'`, `status: 409`, + * the driver's own error whole on `cause`, and `field` when the dialect + * determinably named the conflicting COLUMN. Identically on every driver, in + * the single-row and batch paths alike, so the platform's own recommended + * idiom ("declare a unique index, attempt the insert, swallow the violation") + * is expressible by an application that knows nothing about SQLite's + * `SQLITE_CONSTRAINT_UNIQUE`, Postgres' `23505` or MySQL's `ER_DUP_ENTRY`. + * + * ⛔ Every other driver failure is rethrown UNCHANGED — a NOT NULL violation, + * a deadlock, a missing table, an unreachable store. The verdict is + * `isUniqueViolationError` (`@objectstack/types`), the one predicate the repo + * has for the question; this door adds no dialect knowledge of its own. + * + * One conflict keeps a narrower identity: an autonumber the engine ITSELF + * issued and re-issued to exhaustion is `ERR_AUTONUMBER_COLLISION`, which + * says something `DUPLICATE_RECORD` cannot ("re-seeded, re-issued, still + * refused"). It carries the driver error as `cause` exactly as before. + */ async insert(object: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise { object = this.resolveObjectName(object); this.logger.debug('Insert operation starting', { object, isBatch: Array.isArray(data) }); @@ -9891,20 +9936,28 @@ export class ObjectQL implements IObjectQLEngine { // strictly worse than the collision. What must not survive is the // stale counter: leaving it is what makes the very next insert // collide too, one number at a time, which is the storm. So the - // counters this batch drew on are dropped and the driver's error is - // rethrown UNCHANGED (a batch caller — bulkWrite's per-row - // degradation — reads these errors, and this is not the place to - // change what it reads). + // counters this batch drew on are dropped and the batch is NOT + // re-issued. + // + // [#14095] What is rethrown is no longer the driver's raw error. + // The batch door answers the same contract as the single-row one: + // a recognised unique violation leaves as the `DUPLICATE_RECORD` + // envelope carrying the driver's error as `cause`, and everything + // else leaves untouched. A batch caller reading these errors — + // bulkWrite's per-row degradation, the import runner — reads a + // BETTER answer than before, because the row report's `code` was + // previously whatever dialect token the driver happened to use + // (`SQLITE_CONSTRAINT_UNIQUE`, `11000`), which is precisely the + // coupling this card exists to remove. // // What an author gets, stated plainly: `insert(object, rows[])` and - // `insertMany` both REJECT with the driver's own duplicate-key - // error — never `ERR_AUTONUMBER_COLLISION`, which is the - // single-row path's identity for "re-issued and still refused". - // Whether any row was written is the driver's answer, not this - // method's. The one thing the engine guarantees is that the NEXT - // write re-seeds instead of walking into the same collision, so a - // retry by the caller converges. Pinned in - // engine-autonumber-resync.test.ts. + // `insertMany` both REJECT with `DUPLICATE_RECORD` — never + // `ERR_AUTONUMBER_COLLISION`, which remains the single-row path's + // identity for "re-issued and still refused". Whether any row was + // written is the driver's answer, not this method's. The one thing + // the engine guarantees is that the NEXT write re-seeds instead of + // walking into the same collision, so a retry by the caller + // converges. Pinned in engine-autonumber-resync.test.ts. try { if (driver.bulkCreate) { result = await driver.bulkCreate(object, liveRows, driverOptions); @@ -9920,7 +9973,7 @@ export class ObjectQL implements IObjectQLEngine { object, fields: [...new Set(batchIssued.map((one) => one.field))], }); } - throw error; + throw envelopeUniqueViolation(error, object); } } } else { diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 2c21499463..9d4a354b8a 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -101,6 +101,14 @@ export type { SummaryRecomputeFailure } from './summary-errors.js'; // caller (a cron / server-side plugin) can narrow on the class; the `code` is // the boundary-crossing identity. export { ReadonlyFieldRejectedError } from './readonly-strict-errors.js'; +// [#14095] Thrown by `engine.insert` when a driver refuses a row as a unique +// violation. Exported so an application implementing the platform's own +// "declare a unique index, attempt the insert, swallow the violation" idiom can +// name the condition WITHOUT reaching for a dialect's code or +// `@objectstack/types` (which an app cannot resolve); `code === +// 'DUPLICATE_RECORD'` is the boundary-crossing identity, the class is the +// in-process convenience. +export { DuplicateRecordError, DUPLICATE_RECORD_CODE } from './duplicate-record-error.js'; // Boot guard: thrown by `ObjectQL.init()` when a registered driver's connect() // fails (framework#3741). Hosts that boot the engine themselves can catch it to // render their own "database unreachable" message. From 7bf9fc9d3a008f08c8d53d462d56f908c5cb0d87 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 03:14:43 +0000 Subject: [PATCH 2/9] test(objectql): pin the DUPLICATE_RECORD insert contract and retriage the resync pins (#14095) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- .../src/engine-autonumber-resync.test.ts | 75 ++- .../engine-insert-duplicate-record.test.ts | 476 ++++++++++++++++++ 2 files changed, 535 insertions(+), 16 deletions(-) create mode 100644 packages/objectql/src/engine-insert-duplicate-record.test.ts diff --git a/packages/objectql/src/engine-autonumber-resync.test.ts b/packages/objectql/src/engine-autonumber-resync.test.ts index 1a2e0671b3..ef6e196f56 100644 --- a/packages/objectql/src/engine-autonumber-resync.test.ts +++ b/packages/objectql/src/engine-autonumber-resync.test.ts @@ -80,12 +80,23 @@ * - **#6114 / #5979 read-failure discrimination.** A missing table still seeds * from 0; every other read failure still propagates and writes NOTHING — * including on a RE-seed, which is the new call site. Pinned below. - * - **A conflict on some OTHER unique field** is rethrown untouched (#5495's + * - **A conflict on some OTHER unique field is not re-issued** (#5495's * disposition: «非本字段的冲突原样上抛»). * - **The batch path is re-seeded but never re-issued.** `bulkCreate` may be * partially applied, so re-writing a batch could duplicate the rows that did - * land. The stale counter is dropped and the driver's error is rethrown as - * before. + * land. The stale counter is dropped and the batch is not re-written. + * + * ⚠️ [#14095] What those two cases REPORT did change, and the assertions below + * moved with it: the door no longer hands the driver's raw error to the caller. + * A recognised unique violation leaves `engine.insert` as the ADR-0112 + * `DUPLICATE_RECORD` envelope with that driver error whole on `cause`, on every + * driver and on the batch path too. So the pins here assert the same FACTS + * through `cause` — `driver.create` call counts, the dialect's own `code` and + * `detail`, the converging next number — rather than being re-baselined. The + * one identity that does NOT move is `ERR_AUTONUMBER_COLLISION`: it says + * something `DUPLICATE_RECORD` cannot ("re-seeded, re-issued, still refused"), + * so the engine raises it instead of the envelope, and its `cause` is still the + * driver's error at one step, not two. * * The unique-violation questions are asked of `@objectstack/types`' * `isUniqueViolationError` / `uniqueViolationColumn` (#6250 / #6544) — never a @@ -542,10 +553,16 @@ describe('ObjectQL autonumber resync (#6806)', () => { expect((await engine.insert('doc', { title: 'second' })).doc_no).toBe('D-0006'); }); - it('a conflict on a DIFFERENT column is rethrown untouched, with no re-issue', async () => { + it('a conflict on a DIFFERENT column is NOT re-issued, and keeps the driver diagnosis', async () => { // «非本字段的冲突原样上抛» — #5495's disposition. A duplicate email is the // caller's business error; re-issuing a record number cannot fix it, and - // swallowing it into an engine error would hide what actually failed. + // swallowing what actually failed would hide it. + // + // [#14095] "Not re-issued" is the disposition, and it is unchanged: ONE + // create, no second attempt. What the caller receives is now the + // `DUPLICATE_RECORD` envelope, and the driver's own diagnosis — the + // SQLSTATE and the DETAIL line naming the real column — is preserved + // whole on `cause` rather than replaced. const { engine, driver } = makeRig(SCHEMA, storedRows('doc_no', ['D-0003']), { uniqueOn: 'doc_no', alwaysDuplicate: true, @@ -553,7 +570,15 @@ describe('ObjectQL autonumber resync (#6806)', () => { }); await engine.init(); - await expect(engine.insert('doc', { title: 'first' })).rejects.toMatchObject({ + const failure = await engine.insert('doc', { title: 'first' }).then( + () => { throw new Error('expected the insert to be refused'); }, + (e) => e as any, + ); + + expect(failure.code).toBe('DUPLICATE_RECORD'); + expect(failure.status).toBe(409); + expect(failure.field).toBe('email'); + expect(failure.cause).toMatchObject({ code: '23505', detail: 'Key (email)=(a@b.example) already exists.', }); @@ -624,9 +649,18 @@ describe('ObjectQL autonumber resync (#6806)', () => { // An exempt writer replaying a number that is already taken. The engine // issued nothing on this row, so there is nothing of its own to re-issue // and the collision is the writer's to see. - await expect( - engine.insert('doc', { title: 'replay', doc_no: 'D-0003' }, { context: { isSystem: true } } as any), - ).rejects.toThrow(/E11000/); + const failure = await engine + .insert('doc', { title: 'replay', doc_no: 'D-0003' }, { context: { isSystem: true } } as any) + .then( + () => { throw new Error('expected the insert to be refused'); }, + (e) => e as any, + ); + + // [#14095] Nothing of the engine's is re-issued, and the collision is the + // writer's to see — through the envelope, with MongoDB's own `E11000` + // sentence intact one step down. + expect(failure.code).toBe('DUPLICATE_RECORD'); + expect(String((failure.cause as Error).message)).toMatch(/E11000/); expect(driver.create).toHaveBeenCalledTimes(1); }); @@ -642,16 +676,21 @@ describe('ObjectQL autonumber resync (#6806)', () => { // indistinguishable here, and only the drop reaches the real max. rows.push(...storedRows('doc_no', ['D-0005', 'D-0006', 'D-0007', 'D-0008', 'D-0009']).map((r, i) => ({ ...r, id: `x${i}` }))); - // What an author actually gets on a batch: the DRIVER's own error, not - // the single-row path's `ERR_AUTONUMBER_COLLISION` — because nothing was - // re-issued, so "re-issued and still refused" would be a false statement. + // What an author actually gets on a batch: the `DUPLICATE_RECORD` + // envelope (#14095), never the single-row path's + // `ERR_AUTONUMBER_COLLISION` — because nothing was re-issued, so + // "re-issued and still refused" would be a false statement. The driver's + // own `E11000` is on `cause`, at one step: the batch seam does not stack + // an envelope on top of another engine error. const failure = await engine.insert('doc', [{ title: 'a' }]).then( () => { throw new Error('expected the batch to be refused'); }, (e) => e as any, ); - expect(failure.message).toMatch(/E11000/); - expect(failure.code).toBe(11000); + expect(failure.code).toBe('DUPLICATE_RECORD'); + expect(failure.status).toBe(409); expect(failure.code).not.toBe('ERR_AUTONUMBER_COLLISION'); + expect(String((failure.cause as Error).message)).toMatch(/E11000/); + expect((failure.cause as any).code).toBe(11000); const createsAfterBatch = (driver.create as any).mock.calls.length; // The counter was dropped, so the next single insert RE-SEEDS and lands @@ -662,7 +701,7 @@ describe('ObjectQL autonumber resync (#6806)', () => { expect((driver.create as any).mock.calls.length).toBe(createsAfterBatch + 1); }); - it('insertMany reports the same way — driver error, counter dropped', async () => { + it('insertMany reports the same way — DUPLICATE_RECORD, counter dropped', async () => { const rows = storedRows('doc_no', ['D-0003']); const { engine } = makeRig(SCHEMA, rows, { uniqueOn: 'doc_no' }); await engine.init(); @@ -671,7 +710,11 @@ describe('ObjectQL autonumber resync (#6806)', () => { // Partial-row mode culls rows that fail PREPARATION; a driver write that // fails is still a whole-call rejection, so this is the same contract. - await expect(engine.insertMany('doc', [{ title: 'a' }])).rejects.toMatchObject({ code: 11000 }); + await expect(engine.insertMany('doc', [{ title: 'a' }])).rejects.toMatchObject({ + code: 'DUPLICATE_RECORD', + status: 409, + cause: { code: 11000 }, + }); expect((await engine.insert('doc', { title: 'after' })).doc_no).toBe('D-0010'); }); diff --git a/packages/objectql/src/engine-insert-duplicate-record.test.ts b/packages/objectql/src/engine-insert-duplicate-record.test.ts new file mode 100644 index 0000000000..a3985f5e10 --- /dev/null +++ b/packages/objectql/src/engine-insert-duplicate-record.test.ts @@ -0,0 +1,476 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #14095 — the insert door's ONE error contract for a driver's unique-constraint + * refusal. + * + * ## What was measured, and why it is a defect rather than a preference + * + * The platform recommends "declare a unique index, attempt the insert, swallow + * the violation" — `createWithAutonumberResync`'s own doc argues against the + * read-then-write alternative. A real application could not complete that + * pattern: on the driver that enforces the index, the RAW driver error + * propagated out of `engine.insert` (`name=SqliteError`, + * `code='SQLITE_CONSTRAINT_UNIQUE'`, `Object.keys(err) = ['code']`, message = + * the whole compiled INSERT), so the app's only readings were a dialect code + * (which stops being idempotent the day the store changes) or the message text. + * The platform's own dialect-independent predicate lives in + * `@objectstack/types`, which an application cannot resolve. + * + * Triage ruled (2026-09-01) for wrapping in ObjectQL: 「抛一个带既有词表码 + * (`DUPLICATE_RECORD` 已在 ADR-0112 台账里)的平台错误,原驱动错误作 `cause` + * ⇒ `insert` 在每个驱动上有同一份契约」. + * + * ## What this file pins + * + * Both directions, because only the pair is a contract: + * + * - every driver-error EXIT of the insert door turns a recognised unique + * violation into the envelope — single row, batch via `bulkCreate`, batch + * via the per-row fallback loop, `insertMany`'s partial mode, and the + * last-chance create the autonumber resync issues when the field vanished + * mid-flight; and + * - **nothing else moves**: a NOT NULL violation, a deadlock, a missing table + * and an unreachable store leave the door as the very object the driver + * threw — asserted on IDENTITY, not on a message match, so a future + * "helpful" re-wrap cannot pass this file. + * + * Refusal cases assert `code` AND `status` (ADR-0112), never `toThrow()` alone: + * a bare `toThrow` is green both when the door envelopes correctly and when a + * driver throws a raw error, which is the whole distinction under test. + * + * The driver fixtures are the dialect shapes `engine-autonumber-resync.test.ts` + * measured; they are restated here rather than shared because this file asks a + * different question of them (what the DOOR raises, not whether the resync + * re-issues) and a shared fixture module would couple the two files' futures. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/types'; +import { ObjectQL, ScopedContext } from './engine'; +import { DuplicateRecordError, DUPLICATE_RECORD_CODE } from './duplicate-record-error'; +import { SchemaRegistry } from './registry'; +import type { IDataDriver } from '@objectstack/spec/contracts'; + +vi.mock('./registry', async () => { + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); +}); + +type Row = Record; + +/* -------------------------------------------------------------------------- + * Driver fixtures — the shapes the supported dialects actually raise. + * ----------------------------------------------------------------------- */ + +/** better-sqlite3: names the COLUMN, and buries it behind the compiled statement. */ +const sqliteDuplicate = () => + Object.assign( + new Error( + 'insert into `doc` (`email`, `id`, `title`) values (?, ?, ?) - ' + + 'UNIQUE constraint failed: doc.email', + ), + { code: 'SQLITE_CONSTRAINT_UNIQUE' }, + ); + +/** node-postgres: the column is in the DETAIL line, not the message. */ +const postgresDuplicate = () => + Object.assign(new Error('duplicate key value violates unique constraint "doc_email_key"'), { + code: '23505', + detail: 'Key (email)=(a@b.example) already exists.', + }); + +/** mysql2: names the INDEX. `uniqueViolationColumn` refuses to read it as a column. */ +const mysqlDuplicate = () => + Object.assign(new Error("Duplicate entry 'a@b.example' for key 'idx_doc_email'"), { + code: 'ER_DUP_ENTRY', + errno: 1062, + }); + +/** driver-memory (#13197): already an ADR-0112 envelope, in the platform's own vocabulary. */ +const memoryDuplicate = () => + Object.assign( + new Error('Unique constraint violated on `doc.email`: a record with that value already exists.'), + { code: 'UNIQUE_VIOLATION', status: 409 }, + ); + +/* -------- the negative side: failures that must NOT change shape --------- */ + +const notNullViolation = () => + Object.assign(new Error('NOT NULL constraint failed: doc.title'), { + code: 'SQLITE_CONSTRAINT_NOTNULL', + }); + +const missingTable = () => + Object.assign(new Error('SQLITE_ERROR: no such table: doc'), { code: 'SQLITE_ERROR' }); + +const deadlock = () => Object.assign(new Error('deadlock detected'), { code: '40P01' }); + +const unreachableStore = () => + Object.assign(new Error('connect ECONNREFUSED 127.0.0.1:5432'), { code: 'ECONNREFUSED' }); + +/* -------------------------------------------------------------------------- + * Rig + * ----------------------------------------------------------------------- */ + +const SCHEMA = { + name: 'doc', + fields: { + title: { type: 'text' }, + email: { type: 'text' }, + }, + indexes: [{ name: 'idx_doc_email', fields: ['email'], unique: true }], +}; + +interface DriverOpts { + /** What `create` / `bulkCreate` reject with. `null` = accept everything. */ + refuse?: (() => unknown) | null; + /** Omit `bulkCreate` so a batch takes the engine's per-row fallback loop. */ + noBulkCreate?: boolean; +} + +function makeDriver(opts: DriverOpts = {}) { + const refuse = opts.refuse ?? null; + const stored: Row[] = []; + const driver: any = { + name: 'fake', + version: '0.0.0', + supports: {}, + connect: vi.fn().mockResolvedValue(undefined), + disconnect: vi.fn().mockResolvedValue(undefined), + checkHealth: vi.fn().mockResolvedValue(true), + execute: vi.fn(), + find: vi.fn(async () => []), + findOne: vi.fn(), + create: vi.fn(async (_obj: string, row: Row) => { + if (refuse) throw refuse(); + const written = { id: `new${stored.length + 1}`, ...row }; + stored.push(written); + return written; + }), + update: vi.fn(), + delete: vi.fn(), + count: vi.fn(), + }; + if (!opts.noBulkCreate) { + driver.bulkCreate = vi.fn(async (_obj: string, rows: Row[]) => { + if (refuse) throw refuse(); + return rows.map((row, i) => ({ id: `new${stored.length + i + 1}`, ...row })); + }); + } + return driver as IDataDriver & { create: any; bulkCreate?: any }; +} + +function makeRig(opts: DriverOpts = {}, schema: unknown = SCHEMA) { + vi.mocked(SchemaRegistry.getObject).mockReturnValue(schema as any); + const driver = makeDriver(opts); + const engine = new ObjectQL(); + engine.registerDriver(driver, true); + return { engine, driver }; +} + +/** The rejection, as the value the caller actually receives. */ +async function refusalOf(run: () => Promise): Promise { + return run().then( + () => { + throw new Error('expected the insert to be refused'); + }, + (e) => e as any, + ); +} + +describe('engine.insert — a driver unique violation is a DUPLICATE_RECORD envelope (#14095)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + /* ====================================================================== * + * (1) The envelope itself + * ==================================================================== */ + + describe('the envelope, on the single-row door', () => { + it('carries the ADR-0112 code AND status, with the driver error whole on `cause`', async () => { + const raw = sqliteDuplicate(); + const { engine } = makeRig({ refuse: () => raw }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', { title: 't', email: 'a@b.example' })); + + // The two halves a refusal test must assert — never `toThrow()` alone. + expect(failure.code).toBe(DUPLICATE_RECORD_CODE); + expect(failure.code).toBe('DUPLICATE_RECORD'); + expect(failure.status).toBe(409); + // The driver's own diagnosis is preserved rather than replaced — and it is + // the SAME object, not a copy, so nothing about it was lost in transit. + expect(failure.cause).toBe(raw); + expect(failure).toBeInstanceOf(DuplicateRecordError); + expect(failure.name).toBe('DuplicateRecordError'); + }); + + it('names the object, and the COLUMN when the dialect determinably named one', async () => { + const { engine } = makeRig({ refuse: sqliteDuplicate }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', { title: 't', email: 'a@b.example' })); + + expect(failure.object).toBe('doc'); + expect(failure.field).toBe('email'); + expect(failure.message).toContain("'doc'"); + expect(failure.message).toContain("'email'"); + expect(failure.message).toContain('No record was written'); + }); + + it('reads the column out of Postgres DETAIL, which is not on the message at all', async () => { + const { engine } = makeRig({ refuse: postgresDuplicate }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', { title: 't', email: 'a@b.example' })); + + expect(failure.code).toBe('DUPLICATE_RECORD'); + expect(failure.status).toBe(409); + expect(failure.field).toBe('email'); + }); + + it('names NO field when the dialect named an INDEX — never the index as a column', async () => { + // MySQL's `for key 'idx_doc_email'` is an index name. `uniqueViolationColumn` + // refuses it on the maintainer's 2026-08-08 ruling (#6544): a wrong field + // name is worse than none, because it sends the author to correct an input + // that was never the problem. This door does not widen that contract. + const { engine } = makeRig({ refuse: mysqlDuplicate }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', { title: 't', email: 'a@b.example' })); + + expect(failure.code).toBe('DUPLICATE_RECORD'); + expect(failure.status).toBe(409); + expect(failure.field).toBeUndefined(); + expect('field' in failure).toBe(true); // the class declares it; the VALUE is absent + expect(failure.message).toContain("'doc'"); + expect(failure.message).not.toContain('idx_doc_email'); + }); + + it('normalises a driver that already speaks an envelope — one code, not two', async () => { + // driver-memory raises `UNIQUE_VIOLATION` / 409 (#13197). It is a platform + // envelope, but it is a DIFFERENT one, so an application branching on the + // insert door would still need two spellings. The door answers one. + const raw = memoryDuplicate(); + const { engine } = makeRig({ refuse: () => raw }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', { title: 't', email: 'a@b.example' })); + + expect(failure.code).toBe('DUPLICATE_RECORD'); + expect(failure.status).toBe(409); + expect(failure.cause).toBe(raw); + expect((failure.cause as any).code).toBe('UNIQUE_VIOLATION'); + }); + + it('is idempotent — an envelope reaching a seam twice does not nest', async () => { + const inner = sqliteDuplicate(); + const already = new DuplicateRecordError('doc', inner, 'email'); + const { engine } = makeRig({ refuse: () => already }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', { title: 't' })); + + expect(failure).toBe(already); + expect(failure.cause).toBe(inner); + }); + }); + + /* ====================================================================== * + * (2) Every driver-error exit of the door, not just the easy one + * ==================================================================== */ + + describe('the same contract on every path a driver create failure leaves by', () => { + it('batch insert through `bulkCreate`', async () => { + const raw = sqliteDuplicate(); + const { engine, driver } = makeRig({ refuse: () => raw }); + await engine.init(); + + const failure = await refusalOf(() => + engine.insert('doc', [{ title: 'a', email: 'a@b.example' }, { title: 'b', email: 'a@b.example' }]), + ); + + expect((driver as any).bulkCreate).toHaveBeenCalledTimes(1); + expect(failure.code).toBe('DUPLICATE_RECORD'); + expect(failure.status).toBe(409); + expect(failure.cause).toBe(raw); + expect(failure.field).toBe('email'); + }); + + it('batch insert through the per-row fallback loop (a driver with no `bulkCreate`)', async () => { + const raw = postgresDuplicate(); + const { engine, driver } = makeRig({ refuse: () => raw, noBulkCreate: true }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', [{ title: 'a', email: 'a@b.example' }])); + + expect((driver as any).bulkCreate).toBeUndefined(); + expect(failure.code).toBe('DUPLICATE_RECORD'); + expect(failure.status).toBe(409); + expect(failure.cause).toBe(raw); + }); + + it("insertMany's partial-row mode — a driver write failure is still a whole-call rejection", async () => { + const raw = sqliteDuplicate(); + const { engine } = makeRig({ refuse: () => raw }); + await engine.init(); + + const failure = await refusalOf(() => engine.insertMany('doc', [{ title: 'a', email: 'a@b.example' }])); + + expect(failure.code).toBe('DUPLICATE_RECORD'); + expect(failure.status).toBe(409); + expect(failure.cause).toBe(raw); + }); + + it('the scoped-repository facade reaches the same envelope', async () => { + // `ScopedContext.object(name).insert(data)` is what a hook reaches as + // `ctx.api.object(name)` — it delegates to this same door, so it inherits + // the contract rather than declaring a second one. + const raw = sqliteDuplicate(); + const { engine } = makeRig({ refuse: () => raw }); + await engine.init(); + + const repo = new ScopedContext({} as any, engine as any).object('doc'); + const failure = await refusalOf(() => repo.insert({ title: 't' })); + + expect(failure.code).toBe('DUPLICATE_RECORD'); + expect(failure.cause).toBe(raw); + }); + + it('the autonumber resync\'s LAST-CHANCE create, after the field vanished mid-flight', async () => { + // The one exit that sits OUTSIDE the resync loop's own `try`: the engine + // re-seeds, finds nothing left to re-issue because the field is gone from + // the schema, and issues one final create. Its rejection is a second + // driver-error exit, and it is enveloped on the same terms as the first. + const numbered = { + name: 'doc', + fields: { title: { type: 'text' }, doc_no: { type: 'autonumber', required: true, format: 'D-{0000}' } }, + }; + const raw = mysqlDuplicate(); + const { engine } = makeRig({ refuse: () => raw }, numbered); + await engine.init(); + + // The first refusal is attributed to the number this insert issued, so the + // engine re-seeds; the schema it re-reads no longer declares the field, so + // `applyAutonumbers` issues nothing and the last-chance create runs. + vi.mocked(SchemaRegistry.getObject).mockReturnValue({ + name: 'doc', + fields: { title: { type: 'text' } }, + } as any); + + const failure = await refusalOf(() => engine.insert('doc', { title: 't' })); + + expect(failure.code).toBe('DUPLICATE_RECORD'); + expect(failure.status).toBe(409); + expect(failure.cause).toBe(raw); + }); + }); + + /* ====================================================================== * + * (3) The negative side — the positive controls + * ==================================================================== */ + + describe('nothing that is not a unique violation changes shape', () => { + const controls: Array<[string, () => unknown]> = [ + ['a NOT NULL violation', notNullViolation], + ['a missing table', missingTable], + ['a deadlock', deadlock], + ['an unreachable store', unreachableStore], + ]; + + for (const [label, make] of controls) { + it(`${label} leaves the single-row door as the very object the driver threw`, async () => { + const raw = make(); + const { engine } = makeRig({ refuse: () => raw }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', { title: 't' })); + + // Identity, not a message match: this is the assertion a future + // "helpful" re-wrap of every driver error would have to break. + expect(failure).toBe(raw); + expect(failure.code).not.toBe('DUPLICATE_RECORD'); + expect(failure).not.toBeInstanceOf(DuplicateRecordError); + expect(failure.status).toBeUndefined(); + }); + + it(`${label} leaves the BATCH door unchanged too`, async () => { + const raw = make(); + const { engine } = makeRig({ refuse: () => raw }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', [{ title: 't' }])); + + expect(failure).toBe(raw); + expect(failure).not.toBeInstanceOf(DuplicateRecordError); + }); + } + + it('a NOT NULL violation is refused as a NOT NULL violation, not as a conflict', async () => { + // SQLite spells NOT NULL and UNIQUE with the same `… constraint failed: + // t.c` shape, so this is the case a message-matching wrap gets wrong. The + // verdict comes from the shared predicate, which deliberately excludes the + // bare `constraint failed` word pair. + expect(isUniqueViolationError(notNullViolation())).toBe(false); + }); + }); + + /* ====================================================================== * + * (4) The envelope does not break the consumers of the raw error + * ==================================================================== */ + + describe('every existing reader of the raw error keeps its answer', () => { + it('the shared predicate still says yes, through the `cause` chain', async () => { + const { engine } = makeRig({ refuse: sqliteDuplicate }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', { title: 't' })); + + // `isUniqueViolationError` walks `cause`, so a consumer holding the + // envelope gets the same verdict it got from the raw error — which is what + // makes REST's 409 arm and the import runner survive this change. + expect(isUniqueViolationError(failure)).toBe(true); + expect(uniqueViolationColumn(failure)).toBe('email'); + }); + + it('the message never opens with a SQL verb', async () => { + // `@objectstack/rest`'s importer runs every row error through + // `sanitizeRowError`, whose backstop DISCARDS any message starting with + // `insert`/`update`/`delete`/`select`/`with`/`replace` as a leaked + // statement. A message opening "Insert on 'doc' …" would be replaced by + // generic text — measured, which is why the wording is pinned here. + const { engine } = makeRig({ refuse: sqliteDuplicate }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', { title: 't' })); + + expect(/^\s*(insert|update|delete|select|with|replace)\s/i.test(failure.message)).toBe(false); + }); + + it('carries none of the driver statement or its bound values', async () => { + // #8682's discipline, one layer out: the compiled statement stays where it + // was, on `cause`. REST's declared-4xx arm ships `message` to the client + // verbatim, so quoting the driver here would move the leak onto the wire. + const { engine } = makeRig({ refuse: sqliteDuplicate }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', { title: 't' })); + + expect(failure.message).not.toMatch(/insert into/i); + expect(failure.message).not.toContain('values (?'); + expect(String((failure.cause as Error).message)).toMatch(/insert into/i); + }); + + it('addresses the application author on `developerMessage`, as DELETE_RESTRICTED does', async () => { + const { engine } = makeRig({ refuse: sqliteDuplicate }); + await engine.init(); + + const failure = await refusalOf(() => engine.insert('doc', { title: 't' })); + + expect(failure.developerMessage).toContain('DUPLICATE_RECORD'); + expect(failure.developerMessage).toContain('cause'); + }); + }); +}); From 52c7e3762699da9f8e2478e5eeebf984f09d6caa Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 03:19:08 +0000 Subject: [PATCH 3/9] fix(objectql): keep the driver diagnosis on the operator log line (#14095) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- .../src/driver-fault-redaction.test.ts | 41 +++++++++++++++---- .../objectql/src/duplicate-record-error.ts | 16 +++++--- packages/objectql/src/engine.ts | 14 ++++++- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/packages/objectql/src/driver-fault-redaction.test.ts b/packages/objectql/src/driver-fault-redaction.test.ts index 11b32018a8..5ac343d0dc 100644 --- a/packages/objectql/src/driver-fault-redaction.test.ts +++ b/packages/objectql/src/driver-fault-redaction.test.ts @@ -23,6 +23,7 @@ // the tolerant-fallback direction, not the loud one. import { describe, it, expect } from 'vitest'; +import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/types'; import { ObjectQL } from './engine.js'; import { VALUE_BEARING_TEMPLATES, @@ -863,6 +864,11 @@ describe('#8682 half B — the write-path loggers', () => { }; it('MySQL duplicate entry — the entry survives and still names the index', async () => { + // [#14095] Load-bearing after the envelope: the caller's error changed, the + // OPERATOR's line did not. The engine logs the driver's own error (the + // envelope's `cause`) precisely because the platform logger serializes only + // `message` and `stack` — logging the envelope instead would drop the index + // name below, which is the operator's answer to "which constraint?". const { line } = await insertAgainstADriftedColumn(MYSQL_DUPLICATE_ENTRY); expect(line).toBeDefined(); @@ -883,15 +889,34 @@ describe('#8682 half B — the write-path loggers', () => { } }); - it('MySQL duplicate entry — the RETHROWN error is still untouched', async () => { - // Same boundary as above: the log narrows, the caller's answer does not - // move. `isUniqueViolationError` and `uniqueViolationColumn` read this - // message downstream and must keep seeing the driver's own text. + it('MySQL duplicate entry — the driver error reaches the caller UNTOUCHED, on `cause`', async () => { + // Same boundary as above: the log narrows, and what the driver said is not + // rewritten anywhere. `isUniqueViolationError` and `uniqueViolationColumn` + // read this text downstream and must keep seeing it. + // + // ⚠️ [#14095] WHERE the caller finds it moved by one step, and only for a + // recognised unique violation: the insert door now answers the + // `DUPLICATE_RECORD` envelope so an application can name the condition + // without knowing `ER_DUP_ENTRY` from `23505`, and attaches the driver's + // error whole as `cause`. Both predicates walk a `cause` chain, so every + // downstream reading this pin protects is unchanged — asserted here rather + // than assumed. The non-unique fault one test up is the control: it is not + // enveloped at all and still arrives as the driver threw it. const { thrown } = await insertAgainstADriftedColumn(MYSQL_DUPLICATE_ENTRY); - expect(String(thrown?.message)).toContain('insert into'); - expect(String(thrown?.message)).toContain(SECRET); - expect(String(thrown?.message)).toContain('Duplicate entry'); - expect((thrown as any)?.code).toBe('ER_DUP_ENTRY'); + expect((thrown as any)?.code).toBe('DUPLICATE_RECORD'); + expect((thrown as any)?.status).toBe(409); + + const cause = (thrown as any)?.cause; + expect(String(cause?.message)).toContain('insert into'); + expect(String(cause?.message)).toContain(SECRET); + expect(String(cause?.message)).toContain('Duplicate entry'); + expect(cause?.code).toBe('ER_DUP_ENTRY'); + + // The verdicts the downstream consumers ask of it, asked of the envelope. + expect(isUniqueViolationError(thrown)).toBe(true); + // MySQL names an INDEX, so the column answer is `undefined` — before and + // after, for the same reason (#6544). + expect(uniqueViolationColumn(thrown)).toBeUndefined(); }); }); diff --git a/packages/objectql/src/duplicate-record-error.ts b/packages/objectql/src/duplicate-record-error.ts index c3c6b3980d..abc40bd325 100644 --- a/packages/objectql/src/duplicate-record-error.ts +++ b/packages/objectql/src/duplicate-record-error.ts @@ -68,23 +68,29 @@ const DUPLICATE_RECORD_STATUS = 409 as const; export class DuplicateRecordError extends Error { readonly code = DUPLICATE_RECORD_CODE; readonly status = DUPLICATE_RECORD_STATUS; + /** + * The driver's own error, whole. + * + * DECLARED on the class rather than merely assigned, because this repo + * compiles against `lib: ES2020`, where `Error` has no `cause` member and no + * `ErrorOptions` to pass one through the constructor — so an undeclared + * assignment would be invisible to every TypeScript consumer of the very + * field this envelope's contract rests on. + */ + readonly cause: unknown; /** The remedy half — see the module header on why it is not in `message`. */ readonly developerMessage: string; constructor( /** The object the refused insert targeted. */ public readonly object: string, - /** The driver's own error, whole. */ cause: unknown, /** The conflicting column, when the dialect determinably named one. */ public readonly field?: string, ) { super(buildDuplicateMessage(object, field)); this.name = 'DuplicateRecordError'; - // Assigned rather than passed as `new Error(msg, { cause })`: this repo - // compiles against `lib: ES2020`, where `ErrorOptions` does not exist — - // the same reason `ERR_AUTONUMBER_COLLISION` one file over assigns it. - (this as { cause?: unknown }).cause = cause; + this.cause = cause; this.developerMessage = `The driver refused this insert as a unique-constraint violation. Its own error is attached ` + `as \`cause\` — branch on \`code === '${DUPLICATE_RECORD_CODE}'\` (ADR-0112) rather than on a ` + diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index a99557d92e..7f7b6ddde1 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -119,7 +119,7 @@ import { isMissingTableError } from '@objectstack/metadata/errors'; // engine is the consumer-side tolerant parsing PD #12 forbids and precedent // #5841 retired. import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/types'; -import { envelopeUniqueViolation } from './duplicate-record-error.js'; +import { DuplicateRecordError, envelopeUniqueViolation } from './duplicate-record-error.js'; // [#8682] The write-path loggers' redaction — bound values never reach the log. import { redactBoundStatement } from './driver-fault-redaction.js'; // [#8844] The runtime half of #8686's ruling: a system-context write on a @@ -10102,7 +10102,17 @@ export class ObjectQL implements IObjectQLEngine { // database itself said, including the failing column, is kept; the // error rethrown below is untouched, so the caller's answer does not // move. See `redactBoundStatement`. - this.logger.error('Insert operation failed', redactBoundStatement(e) as Error, { object }); + // + // [#14095] …and the line still carries what the DATABASE said, even now + // that the door hands the CALLER an envelope instead. The platform + // logger serializes an error's `message` and `stack` and nothing else, + // so logging the envelope in the driver error's place would silently + // drop exactly the diagnosis this line exists to carry — the failing + // column, MySQL's index name, the driver's own frames. So the log takes + // the `cause`; the caller's answer does not move, because `e` is what + // is rethrown one line down, with that same error still on it. + const logged = e instanceof DuplicateRecordError ? e.cause : e; + this.logger.error('Insert operation failed', redactBoundStatement(logged) as Error, { object }); throw e; } }); From 75c7c81c2f36bac654d48b9d456d792466dd053d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 03:32:35 +0000 Subject: [PATCH 4/9] chore(objectql): changeset + regenerated system-context census (#14095) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- ...que-violation-duplicate-record-envelope.md | 61 +++++++++++++++++++ content/docs/permissions/system-context.mdx | 24 ++++---- 2 files changed, 73 insertions(+), 12 deletions(-) create mode 100644 .changeset/insert-unique-violation-duplicate-record-envelope.md diff --git a/.changeset/insert-unique-violation-duplicate-record-envelope.md b/.changeset/insert-unique-violation-duplicate-record-envelope.md new file mode 100644 index 0000000000..b73f8346a5 --- /dev/null +++ b/.changeset/insert-unique-violation-duplicate-record-envelope.md @@ -0,0 +1,61 @@ +--- +"@objectstack/objectql": minor +--- + +fix(objectql): `insert` answers a driver unique violation with the `DUPLICATE_RECORD` envelope, on every driver (#14095) + +The platform recommends "declare a unique index, attempt the insert, swallow the +violation" — it is what lets an idempotent writer be an ordinary job instead of +needing a distributed lock, and `packages/objectql`'s own autonumber-resync doc +argues at length against the read-then-write alternative ("a probe costs a query +on every insert … and is still racy"). **An application could not complete that +pattern**, because the insert door rethrew the DRIVER's error verbatim and left +three bad options: branch on `SQLITE_CONSTRAINT_UNIQUE` (and silently stop being +idempotent the day the deployment moves to Postgres' `23505`, MySQL's +`ER_DUP_ENTRY` or Mongo's `E11000`); pattern-match a message that on the measured +SQLite path is the whole compiled INSERT statement; or use the platform's own +`isUniqueViolationError`, which is correct, dialect-independent — and lives in +`@objectstack/types`, a package an application cannot resolve. + +Triage ruling 2026-09-01, verbatim: 「抛一个带既有词表码(`DUPLICATE_RECORD` 已在 +ADR-0112 台账里)的平台错误,原驱动错误作 `cause` ⇒ `insert` 在每个驱动上有同一份 +契约」. + +**What `engine.insert` now raises** for a recognised unique violation, identically +on every driver and on every path a driver create failure leaves the door by +(single row, `bulkCreate` batch, the per-row fallback loop, `insertMany`'s partial +mode, the scoped-repository facade, and the resync's last-chance create): +`DuplicateRecordError` — `code: 'DUPLICATE_RECORD'` (already a member of +`StandardErrorCode`; no `packages/spec` change was needed), `status: 409` (the +conflict status its sibling refusals `DELETE_RESTRICTED` / `CONCURRENT_UPDATE` +declare), the driver's own error WHOLE on `cause`, `object`, a `developerMessage` +carrying the remedy, and `field` when — and only when — `uniqueViolationColumn` +determinably named the conflicting COLUMN (an index name is never reported as a +column; #6544's contract is not widened here). + +**Nothing else moves.** A NOT NULL violation, a deadlock, a missing table and an +unreachable store all leave the door as the very object the driver threw — pinned +on identity, in both the single-row and batch paths. The verdict is the shared +`isUniqueViolationError` predicate; this door adds no dialect knowledge of its own. +`ERR_AUTONUMBER_COLLISION` keeps its narrower identity, because "re-seeded, +re-issued, still refused" says something `DUPLICATE_RECORD` cannot. + +Shipped as `minor` rather than a patch because callers observe a different error +object on a public data-API door. Measured consequences, end to end on real +drivers: + +- **HTTP status is unchanged at 409** on `driver-sqlite-wasm` and `driver-memory`, + single-column and composite declared indexes alike: REST's declared-status + passthrough honours the envelope's `status`. +- **The wire `code` changes from `UNIQUE_VIOLATION` to `DUPLICATE_RECORD`** (both + registered), and the flat body no longer carries the `field` key on the dialects + that name a column, because the passthrough arm ships neither. Restoring it is a + dedicated `mapDataError` arm in `@objectstack/rest` — another lane, filed + separately, not a rider here. +- **Import row reports improve**: the row `code` was previously whatever dialect + token the driver used (`SQLITE_CONSTRAINT_UNIQUE`, `11000`) and is now + `DUPLICATE_RECORD`. +- **The operator log is unchanged**: the engine logs the driver's own error (the + envelope's `cause`), because the platform logger serializes only `message` and + `stack` — so #8682's "what the database said, including the failing column, is + kept" still holds. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index d5f735c117..183d6ec83d 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -109,18 +109,18 @@ that silently does not happen. | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:10981` | -| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11149` | -| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9777` | +| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11044` | +| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11212` | +| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9822` | | 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1746` | -| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9814`, `readonly-strict-errors.ts:66` | -| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5735` | -| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3604`, `:3614`, `:3641` | +| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9859`, `readonly-strict-errors.ts:66` | +| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5756` | +| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3605`, `:3615`, `:3642` | | 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` | | 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:98` | -| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6433` | -| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11742` | -| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11671` | +| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6454` | +| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11805` | +| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11734` | ### 3. Sharing (`plugin-sharing`) @@ -179,8 +179,8 @@ a reader tracing where elevation travels needs them. | # | Site | Package | What it does | |:--|:---|:---|:---| -| 62 | `objectql/src/engine.ts:3411` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | -| 63 | `objectql/src/engine.ts:14091` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | +| 62 | `objectql/src/engine.ts:3412` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | +| 63 | `objectql/src/engine.ts:14154` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | | 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report | | 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across | @@ -195,7 +195,7 @@ assuming `isSystem` covers it is a documented source of bugs. |:---|:---|:---| | "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1909` (rationale at `:1819`–`1821`, #3760), `flow.zod.ts:685` | | "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) | -| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9760`–`9777` | +| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9805`–`9822` | | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:286` | From c9ebe6422266d921d93b3cc1b96b2da42e403e6b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 03:45:52 +0000 Subject: [PATCH 5/9] test(objectql): type the registry double so tsconfig.test.json compiles the new suite (#14095) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- .../engine-insert-duplicate-record.test.ts | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/objectql/src/engine-insert-duplicate-record.test.ts b/packages/objectql/src/engine-insert-duplicate-record.test.ts index a3985f5e10..ad3934a5a3 100644 --- a/packages/objectql/src/engine-insert-duplicate-record.test.ts +++ b/packages/objectql/src/engine-insert-duplicate-record.test.ts @@ -57,6 +57,20 @@ vi.mock('./registry', async () => { return createRegistryModuleMock(); }); +/** + * The double's `getObject`, typed. + * + * `createRegistryModuleMock` hands back a FUNCTION with the instance members + * assigned onto it (`Object.assign(SchemaRegistry, instance)`), so the mocked + * `getObject` is reachable off the imported binding at run time — but the + * binding's STATIC type is the real class, which declares `getObject` on + * instances only. Narrowed once here rather than cast at each call site, so + * `tsconfig.test.json` (which does compile this file — the package's plain + * `typecheck` excludes tests and would have said nothing) stays satisfied + * without an assertion in the middle of a test body. + */ +const registryDouble = SchemaRegistry as unknown as { getObject: ReturnType }; + type Row = Record; /* -------------------------------------------------------------------------- @@ -162,7 +176,7 @@ function makeDriver(opts: DriverOpts = {}) { } function makeRig(opts: DriverOpts = {}, schema: unknown = SCHEMA) { - vi.mocked(SchemaRegistry.getObject).mockReturnValue(schema as any); + registryDouble.getObject.mockReturnValue(schema); const driver = makeDriver(opts); const engine = new ObjectQL(); engine.registerDriver(driver, true); @@ -355,10 +369,10 @@ describe('engine.insert — a driver unique violation is a DUPLICATE_RECORD enve // The first refusal is attributed to the number this insert issued, so the // engine re-seeds; the schema it re-reads no longer declares the field, so // `applyAutonumbers` issues nothing and the last-chance create runs. - vi.mocked(SchemaRegistry.getObject).mockReturnValue({ + registryDouble.getObject.mockReturnValue({ name: 'doc', fields: { title: { type: 'text' } }, - } as any); + }); const failure = await refusalOf(() => engine.insert('doc', { title: 't' })); From dbb4c7c59e0aeed9247d409ec954666c4acc7aea Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 04:35:26 +0000 Subject: [PATCH 6/9] chore: regenerate the system-context census on the merged tree (#14095) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- content/docs/permissions/system-context.mdx | 24 ++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 183d6ec83d..3d9aa7eebe 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -97,7 +97,7 @@ that silently does not happen. | 8 | `explain()` may target a principal other than the caller | plugin-security | Get: no `manage_users` / delegated-admin check | `security-plugin.ts:3857` | | 9 | Anonymous-deny treats the caller as authenticated | core | Get: passes the 401 seam with no `userId` | `anonymous-deny.ts:154` | | 10 | Permission-set projection middleware skipped | plugin-security | Lose: projection of permission-set-derived columns | `permission-set-projection.ts:1015` | -| 11 | Session-resolution middleware skipped | plugin-auth | Get: no session lookup attempted | `auth-plugin.ts:1296` | +| 11 | Session-resolution middleware skipped | plugin-auth | Get: no session lookup attempted | `auth-plugin.ts:1301` | | 12 | Per-request performance timings disclosed | observability | Get: timing headers a normal caller cannot pull | `perf-timing.ts:474` | | 13 | Permission-set **overlay discard** skips the tenant-admin assertion | plugin-security | Get: an overlay can be discarded with no authenticated tenant administrator | `permission-set-overlay-discard.ts:142` | | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:246` | @@ -109,18 +109,18 @@ that silently does not happen. | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11044` | -| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11212` | -| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9822` | +| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11049` | +| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11217` | +| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9827` | | 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1746` | -| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9859`, `readonly-strict-errors.ts:66` | -| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5756` | +| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9864`, `readonly-strict-errors.ts:66` | +| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5761` | | 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3605`, `:3615`, `:3642` | | 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` | | 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:98` | -| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6454` | -| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11805` | -| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11734` | +| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6459` | +| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11810` | +| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11739` | ### 3. Sharing (`plugin-sharing`) @@ -180,7 +180,7 @@ a reader tracing where elevation travels needs them. | # | Site | Package | What it does | |:--|:---|:---|:---| | 62 | `objectql/src/engine.ts:3412` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | -| 63 | `objectql/src/engine.ts:14154` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | +| 63 | `objectql/src/engine.ts:14159` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | | 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report | | 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across | @@ -195,7 +195,7 @@ assuming `isSystem` covers it is a documented source of bugs. |:---|:---|:---| | "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1909` (rationale at `:1819`–`1821`, #3760), `flow.zod.ts:685` | | "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) | -| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9805`–`9822` | +| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9810`–`9827` | | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:286` | @@ -235,7 +235,7 @@ should recognise it instead of re-deriving it. rule-materialised grant that the next reconcile silently restores. 5. **`applySystemFields` does not read this flag.** It is named as if it did. - `packages/objectql/src/registry.ts:459` is **schema-side column + `packages/objectql/src/registry.ts:464` is **schema-side column provisioning** — which columns an object carries — and consumes `ExecutionContext.isSystem` zero times. The write-time ownership behaviour people attribute to it is row 2, in `plugin-security`. From d9ce0043f9dcedb4737fa7ba42ca5fc2640d617b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 04:45:58 +0000 Subject: [PATCH 7/9] fix(metadata-protocol): seed-loader operator line reaches through cause; retriage the two runtime disclosure pins (#14095) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- .../seed-loader-cause-hop-operator-line.md | 15 ++++ packages/metadata-protocol/src/seed-loader.ts | 80 ++++++++++++++++--- ...river-text-real-driver.integration.test.ts | 53 +++++++++++- ...river-text-real-driver.integration.test.ts | 52 ++++++++++-- 4 files changed, 184 insertions(+), 16 deletions(-) create mode 100644 .changeset/seed-loader-cause-hop-operator-line.md diff --git a/.changeset/seed-loader-cause-hop-operator-line.md b/.changeset/seed-loader-cause-hop-operator-line.md new file mode 100644 index 0000000000..0867faa456 --- /dev/null +++ b/.changeset/seed-loader-cause-hop-operator-line.md @@ -0,0 +1,15 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): the seed loader's operator line reaches through `cause`, so an enveloped driver fault still names what the database said (#14095) + +The seed channel has two halves by design: the payload quotes a caught sentence only when the producer DECLARES a client refusal, and the log carries the caught sentence ALWAYS — because withholding text that nothing else records is indistinguishable from deleting the diagnostic, which is what makes a disclosure fix a net loss for whoever has to fix the database. + +`seedFailureCause` read `err.message` and nothing else. That was complete while every producer put its whole diagnosis there. It stopped being complete the moment one of them started ENVELOPING: `engine.insert` now answers a driver unique violation with `DUPLICATE_RECORD` / `status: 409` and keeps the driver's own error whole on `cause`, so the platform sentence sits on `message` and `UNIQUE constraint failed: dt_acct.email` sits one hop down. Read off `message` alone, the operator line printed the platform sentence and the driver's words reached **neither the response nor the log** — the exact loss the two-halves design exists to prevent, arriving through a producer doing the right thing. + +So the log follows the hop: `seedFailureCause` now walks the `cause` chain (bounded at 4, the depth `@objectstack/types`' unique-violation predicate walks) and prints the DEEPEST non-empty sentence — the one no wrapper above it restates. The walk is structural, never a type check: this package must not import `@objectstack/objectql`, and an envelope from any producer earns the same treatment. + +`seedCauseLabel` moves with it, because the marker would otherwise go false. It used to ask "was this ERROR's text withheld from the payload?", which was the same question while the printed sentence was always `err.message`. Now the two differ: an enveloped fault has its PLATFORM sentence quoted to the caller and its DRIVER sentence printed to the operator, and the old question answered `Cause` — telling an operator the reporter saw words the reporter never saw. It now compares the sentence about to be printed against the one the payload actually quoted, so `Cause` means "these are the same words". All three populations stay correct: withheld outright, enveloped, and plainly declared. + +No behaviour changes for a producer that carries no `cause` — the walk finds nothing and returns `err.message`, byte for byte as before. diff --git a/packages/metadata-protocol/src/seed-loader.ts b/packages/metadata-protocol/src/seed-loader.ts index b8cdc77a5f..c9b945e811 100644 --- a/packages/metadata-protocol/src/seed-loader.ts +++ b/packages/metadata-protocol/src/seed-loader.ts @@ -101,10 +101,54 @@ function quotableSeedFailureDetail(err: unknown): string | undefined { return typeof declared === 'string' && declared.length > 0 ? declared : undefined; } -/** The caught sentence, whole — for the LOG, which never withholds. */ +/** + * How far to follow an `error.cause` chain. Producers wrap, but not deeply — + * the same bound `@objectstack/types`' unique-violation predicate walks, chosen + * for the same reason rather than copied: a chain longer than this is a bug in + * the producer, not a diagnostic worth mining. + */ +const MAX_CAUSE_DEPTH = 4; + +/** + * The caught sentence, whole — for the LOG, which never withholds. + * + * ## [#14095] It reaches through `cause`, and that is the whole point + * + * A producer that ENVELOPES a lower-level failure keeps its own diagnosis on + * `cause` and puts a platform sentence on `message` — which is exactly what + * `engine.insert` now does for a driver's unique violation (`DUPLICATE_RECORD`, + * `status: 409`, the driver error whole on `cause`). Read off `message` alone, + * this function would hand the operator the platform sentence and the driver's + * own words — `UNIQUE constraint failed: dt_acct.email`, MySQL's index name, + * Postgres' DETAIL line — would reach NEITHER the response nor the log. + * + * That is the failure this file's own header calls the one that makes a + * disclosure fix "a net loss for whoever has to fix the database": withholding + * text becomes indistinguishable from DELETING the diagnostic. The envelope + * moved the diagnostic one hop rather than deleting it, so the log follows the + * hop. The insert door met the identical problem at its own logging seam and + * took the identical remedy (it logs the envelope's `cause`); this is that + * remedy at the seam a seed write reaches instead. + * + * The DEEPEST non-empty sentence wins, because that is the one nothing else + * restates: every wrapper above it has already put its own summary on the + * payload. Structural, never a type check — this package must not import + * `@objectstack/objectql` (that is the dependency direction, backwards), and + * an envelope from any producer deserves the same treatment as one from that + * one. + */ function seedFailureCause(err: unknown): string { - const message = (err as { message?: unknown } | null | undefined)?.message; - return typeof message === 'string' && message.length > 0 ? message : String(err); + const own = (err as { message?: unknown } | null | undefined)?.message; + let deepest = typeof own === 'string' && own.length > 0 ? own : undefined; + + let node: unknown = (err as { cause?: unknown } | null | undefined)?.cause; + for (let depth = 0; node !== null && node !== undefined && depth < MAX_CAUSE_DEPTH; depth += 1) { + const message = (node as { message?: unknown }).message; + if (typeof message === 'string' && message.length > 0) deepest = message; + node = (node as { cause?: unknown }).cause; + } + + return deepest ?? String(err); } /** @@ -118,11 +162,23 @@ function seedFailureCause(err: unknown): string { * saw it, so "what did the response say?" has a different answer than the log * suggests. Both passes share this one vocabulary so the two halves of the file * cannot drift into answering that question differently. + * + * ## [#14095] The question is asked about the SENTENCE, not about the error + * + * It used to ask "was this ERROR's text withheld?" — fine while the printed + * sentence was always `err.message`. Now that {@link seedFailureCause} reaches + * through `cause`, the two can differ: an enveloped driver fault has its + * PLATFORM sentence quoted to the caller and its DRIVER sentence printed here, + * and the old question answers `Cause` — telling an operator the reporter saw + * a sentence the reporter never saw. So the label compares the sentence about + * to be printed against the one the payload actually quoted; `Cause` means + * "these are the same words", which is the only reading that stays true for + * all three populations (withheld outright, enveloped, plainly declared). */ -function seedCauseLabel(err: unknown): string { - return quotableSeedFailureDetail(err) === undefined - ? 'Cause (withheld from the seed response)' - : 'Cause'; +function seedCauseLabel(err: unknown, printed: string): string { + return quotableSeedFailureDetail(err) === printed + ? 'Cause' + : 'Cause (withheld from the seed response)'; } /** @@ -135,7 +191,7 @@ function seedFailureLogLine(payloadMessage: string, err: unknown): string { const cause = seedFailureCause(err); return payloadMessage.includes(cause) ? `[SeedLoader] ${payloadMessage}` - : `[SeedLoader] ${payloadMessage} ${seedCauseLabel(err)}: ${cause}`; + : `[SeedLoader] ${payloadMessage} ${seedCauseLabel(err, cause)}: ${cause}`; } /** The environments a seed dataset can be scoped to — mirrors `SeedSchema.env`. */ @@ -1665,6 +1721,12 @@ export class SeedLoaderService implements ISeedLoaderService { // now agree, and the line owes the two things AGENTS.md // ("Degradation log levels") requires of an `error`: the // CONSEQUENCE and the FIX. + // [#14095] Derived ONCE and shared by both halves below: the label + // asks its question ABOUT the sentence being printed, and + // `seedFailureCause` now reaches through `cause`, so a second + // derivation is a chance for the marker to describe a sentence this + // line is not printing. + const causeSentence = seedFailureCause(err); this.logger.error( `[SeedLoader] Deferred reference back-fill FAILED — ${deferred.objectName}.${deferred.field} stays NULL ` + `on record '${deferred.recordExternalId}'. The row itself was seeded, so every row counter looks healthy ` + @@ -1675,7 +1737,7 @@ export class SeedLoaderService implements ISeedLoaderService { // [#8442] Same cause vocabulary as the pass-1 write sites: the // raw sentence always, MARKED when the payload half withheld it // so an operator can see the reporter did not receive this line. - `${seedCauseLabel(err)}: ${seedFailureCause(err)}`, + `${seedCauseLabel(err, causeSentence)}: ${causeSentence}`, err instanceof Error ? err : undefined, { object: deferred.objectName, diff --git a/packages/runtime/src/batch-row-driver-text-real-driver.integration.test.ts b/packages/runtime/src/batch-row-driver-text-real-driver.integration.test.ts index f3eb268f2b..bcec31f793 100644 --- a/packages/runtime/src/batch-row-driver-text-real-driver.integration.test.ts +++ b/packages/runtime/src/batch-row-driver-text-real-driver.integration.test.ts @@ -208,10 +208,61 @@ describe('[#8502] a REAL driver fault is withheld from every batch row', () => { expect(raw.message).toContain('dup@example.com'); const payload = JSON.stringify(res); - expect(res.results[0].errors[0].message).toBe('The create of this record failed. The reason is in the server log.'); + + // ── The row's SENTENCE moved populations (#14095) ────────────────── + // ⚠️ RETRIAGED, not re-baselined. `clientFacingRowFailureText` is a + // POSITIVE list: it quotes a caught sentence exactly when the producer + // DECLARES a client refusal (a 4xx status/statusCode, or the + // VALIDATION_FAILED shape) and withholds it otherwise. Nothing about + // that rule moved. What moved is which side of it THIS error is on: + // since #14095 the insert door answers a driver unique violation with + // the ADR-0112 envelope `DUPLICATE_RECORD` / `status: 409`, so the row + // is now a DECLARED refusal and the sink quotes it — which is the + // remedy this file's own sink documents ("declaring is cheaper than + // the workaround"), taken by the producer. + // + // The withheld population is NOT vacated: the `deleteManyData` case + // above is the live control. Its FK fault declares no status, still + // takes the withheld branch, and still says the generic sentence — so + // a regression that stopped withholding UNDECLARED faults reddens + // here, in this same file, on the very next test. + expect(res.results[0].errors[0].message).toBe( + "Duplicate record refused on 'bd_note': a unique constraint on 'email' already holds this value. " + + 'No record was written.', + ); + // The row now carries the machine-readable half too, which is what an + // idempotent batch writer branches on — it was `INTERNAL_ERROR` with no + // status while the sentence was withheld. + expect(res.results[0].errors[0].code).toBe('DUPLICATE_RECORD'); + expect(res.results[0].errors[0].httpStatus).toBe(409); + + // ── …and NOT ONE leak assertion moved ────────────────────────────── + // These are what this file exists for, and they hold against the new + // sentence for the reason the envelope was built that way: the + // platform sentence carries no statement, no bound value and no + // dialect text — the driver's error is preserved WHOLE on `cause`, + // which never reaches response data. expect(payload).not.toContain('insert into'); expect(payload).not.toContain('dup@example.com'); expect(payload).not.toContain('UNIQUE constraint failed'); + expect(payload).not.toContain('SQLITE_CONSTRAINT'); + + // ── ⚠️ The OPERATOR half of this row is a KNOWN RESIDUAL, not a pin ── + // Measured on this exact rig: with the row disclosed, the sink returns + // before its `console.warn`, so the warn fires ZERO times and the + // driver's own sentence — `UNIQUE constraint failed: bd_note.email` — + // reaches neither the response nor the console. Withholding used to be + // what carried it to an operator; disclosure removed the carrier + // without replacing it. + // + // ⛔ Deliberately NOT asserted either way here: asserting the zero + // would PIN the loss as correct, and the remedy is one file over in + // `metadata-protocol/src/protocol.ts`, which is another card's surface. + // The seed loader's twin of this defect IS fixed (`seedFailureCause` + // now reaches through `cause`) and is pinned in + // `seed-loader-driver-text-real-driver.integration.test.ts`; this one + // is tracked as the residual on #14403. When it is taken, the pin + // belongs right here. }); it('a stopped batch does not re-publish the withheld text through its NOT_ATTEMPTED rows', async () => { diff --git a/packages/runtime/src/seed-loader-driver-text-real-driver.integration.test.ts b/packages/runtime/src/seed-loader-driver-text-real-driver.integration.test.ts index e5172461a7..fa6b79ab6e 100644 --- a/packages/runtime/src/seed-loader-driver-text-real-driver.integration.test.ts +++ b/packages/runtime/src/seed-loader-driver-text-real-driver.integration.test.ts @@ -179,24 +179,64 @@ describe('[#8442] a REAL driver constraint violation is withheld from the seed r expect(validationFailureDetails(raw)).toBeUndefined(); expect(raw.status).toBeUndefined(); - // ── The withhold ─────────────────────────────────────────────────────── + // ── The row's SENTENCE moved populations (#14095) ────────────────────── + // ⚠️ RETRIAGED, not re-baselined. `declaresSeedClientRefusal` is a POSITIVE + // list — a 4xx status, or the VALIDATION_FAILED shape — and it did not + // move. What moved is which side of it this error is on: since #14095 the + // insert door answers a driver unique violation with the ADR-0112 envelope + // `DUPLICATE_RECORD` / `status: 409`, so the seed row is a DECLARED refusal + // and `quotableSeedFailureDetail` quotes it. The rejection is still + // located the same way ("record #1 (name=second)"); only the tail changed, + // from the generic reason to the platform's own sentence. + // + // ⛔ The withheld population is NOT vacated. `raw.status` is asserted + // `undefined` twelve lines up: the DRIVER's error still declares nothing + // and would still be withheld — what the sink now sees is the engine's + // envelope, not that error. A regression that started quoting undeclared + // driver faults would redden the `seed-loader-driver-text.test.ts` cases + // that drive this sink with a bare driver throw. expect(result.success).toBe(false); const failed = result.errors.find((e: any) => e.recordIndex === 1); expect(failed, 'the duplicate row was not reported').toBeDefined(); - expect(failed!.message).toContain('the data engine rejected the write; the reason is in the server log'); - - // Nothing of the driver's sentence reaches the caller — asserted over the - // WHOLE payload, because this message carries the statement AND the values. + expect(failed!.message).toContain("Duplicate record refused on 'dt_acct'"); + expect(failed!.message).toContain("a unique constraint on 'email' already holds this value"); + expect(failed!.message).toContain('record #1 (name=second)'); + + // Nothing of the DRIVER's sentence reaches the caller — asserted over the + // WHOLE payload, and every one of these is unchanged. They hold against + // the new sentence for the reason the envelope was built that way: the + // platform sentence carries no statement, no bound value and no dialect + // text, because the driver's error is preserved WHOLE on `cause` and + // `cause` never reaches the wire. const wire = JSON.stringify(result); expect(wire).not.toContain('UNIQUE constraint failed'); expect(wire).not.toContain('SQLITE_CONSTRAINT'); expect(wire).not.toContain('insert into'); expect(wire).not.toContain('dup@example.com'); - // ── …and the operator still gets it ──────────────────────────────────── + // ── …and the operator still gets it — the half #14095 had to REPAIR ──── + // Both assertions are the ORIGINAL ones, byte for byte, and that is the + // point: the envelope nearly cost them. `seedFailureCause` read + // `err.message` alone, so with a platform sentence on `message` the + // driver's own words would have reached NEITHER the response nor the log — + // withholding becoming indistinguishable from deleting the diagnostic, + // which is the exact failure this file's sink was built against. It now + // reaches through `cause` to the deepest sentence, so the operator keeps + // `UNIQUE constraint failed: dt_acct.email`. + // + // And the MARKER stays true for a newly subtle reason: the payload quoted + // the PLATFORM sentence while this line prints the DRIVER sentence, so the + // reporter genuinely did not see these words. `seedCauseLabel` now asks its + // question about the sentence being printed rather than about the error, + // which is what keeps `Cause (withheld from the seed response)` honest + // here while a plainly-declared refusal (whose printed and quoted sentence + // are the same string) still reads `Cause`. const logged = ((logger.error as any).calls as string[]).join('\n'); expect(logged).toContain('UNIQUE constraint failed'); expect(logged).toContain('Cause (withheld from the seed response)'); + // The platform sentence is on the payload; the driver's is on the log. + // Asserted together so a future edit cannot quietly collapse them into one. + expect(logged).toContain('dt_acct'); // The structured authoring feedback survives: which row failed. expect(failed!.sourceObject).toBe('dt_acct'); From e8131d679f744d42aefc2149f3cea785106e42f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 05:07:09 +0000 Subject: [PATCH 8/9] chore: re-anchor the system-context census after the seed-loader edit (#14095) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- content/docs/permissions/system-context.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 3d9aa7eebe..a6fd7f3811 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -193,7 +193,7 @@ assuming `isSystem` covers it is a documented source of bugs. | Assumption | Reality | Anchor | |:---|:---|:---| -| "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1909` (rationale at `:1819`–`1821`, #3760), `flow.zod.ts:685` | +| "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1971` (rationale at `:1881`–`1883`, #3760), `flow.zod.ts:685` | | "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) | | "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9810`–`9827` | | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) | From 683eeedf3327055d6459b917be06e607bac1bcb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 05:34:17 +0000 Subject: [PATCH 9/9] chore: ratchet the error-status unpinned baseline down for DUPLICATE_RECORD (#14095) `DuplicateRecordError` is the first producer to declare this code's status, so the code leaves the unpinned census. Baseline written by `check-error-status-conformance.mjs --update`; shrink-only, one line. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- scripts/error-status-unpinned-baseline.json | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/error-status-unpinned-baseline.json b/scripts/error-status-unpinned-baseline.json index 1ebc816916..ac8e1f7031 100644 --- a/scripts/error-status-unpinned-baseline.json +++ b/scripts/error-status-unpinned-baseline.json @@ -3,7 +3,6 @@ "unpinned": [ "CONCURRENT_LIMIT_EXCEEDED", "CONCURRENT_MODIFICATION", - "DUPLICATE_RECORD", "DUPLICATE_VALUE", "EMAIL_NOT_VERIFIED", "ENDPOINT_NOT_FOUND",