From 3209711ddb8c97496d18fa54d54945937db40ea5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 15:25:46 +0000 Subject: [PATCH 1/2] refactor(metadata-protocol): move the bulk data writes out of protocol.ts into protocol-batch-data.ts Sixteen ObjectStackProtocolImplementation method bodies (batchData and its atomic path, createManyData, insertManyData, updateManyData, deleteManyData, the three per-row loops and the response builders) move unedited into protocol-batch-data.ts, in the host shape rest-server.ts's split set. The class keeps every method, docblock and signature; each body becomes a literal of the members it reads plus a .call. mergeDroppedFieldEvents and rowOperationFailureFallback, read only by the moved bodies, move with them; byIdUpdateRecord, rowRequiredIdError, toRowApiError, BatchDataRowResult and BatchDataLoopOutcome gain `export` in place (index.ts exports from protocol.ts by name). Move-only: no behaviour change. Claude-Session: https://claude.ai/code/session_01J8pUyVs5eas1AjMdReJM7C Co-authored-by: Claude --- .../src/protocol-batch-data.ts | 944 ++++++++++++++++++ packages/metadata-protocol/src/protocol.ts | 873 +++------------- 2 files changed, 1082 insertions(+), 735 deletions(-) create mode 100644 packages/metadata-protocol/src/protocol-batch-data.ts diff --git a/packages/metadata-protocol/src/protocol-batch-data.ts b/packages/metadata-protocol/src/protocol-batch-data.ts new file mode 100644 index 00000000000..2b8aca7e2f2 --- /dev/null +++ b/packages/metadata-protocol/src/protocol-batch-data.ts @@ -0,0 +1,944 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25467 cut 1] The bulk data writes of `ObjectStackProtocolImplementation` — + * `batchData` (with its ADR-0119 atomic path), `createManyData`, + * `insertManyData`, `updateManyData` and `deleteManyData`, the three per-row + * loops behind them, and the builders that turn a loop's outcome into the + * `BatchUpdateResponse` a caller reconciles against — moved out of + * `protocol.ts` whole. + * + * A MOVE, not a redesign. Each function below is the body of the + * `ObjectStackProtocolImplementation` method of the same name as it stood, + * re-indented one level and otherwise unedited: the same engine calls, the + * same per-row outcomes, the same rollback marking, refusing exactly where + * they did. The two module-level declarations only these bodies read moved + * with them, also unedited: `mergeDroppedFieldEvents` and + * `rowOperationFailureFallback`. + * + * ## How the moved bodies reach the class + * + * The shape #23050 set for `rest-server.ts` (its `data-action-routes.ts` + * header carries the full reasoning), applied per method. The class keeps + * every method — the published `dist/*.d.ts` lists each one's name, docblock + * and position — and each body is one literal of exactly the + * {@link BatchDataHost} members that method reads, plus a `.call`; a body that + * reads no member takes no host and is called directly. The `async` methods + * are no longer `async`: each returns the moved function's own promise, so a + * caller awaits the same promise the body settles, with no extra wrapper. A + * method is handed over as a closure that calls it on the protocol at call + * time, so a test double or spy set on the instance is still the one the body + * reaches — the moved bodies that call one another (`batchData` → its atomic + * path → `runAtomicBatch` → the loop → the builders) do so through the + * protocol, exactly as before. + * + * The one field the bodies read, `engine`, goes as a GETTER, so each read + * happens when the body makes it: tests replace it on a constructed protocol. + * + * The other module-level declarations the bodies read stay in `protocol.ts`, + * where code that did not move reads them too, and are imported from there: + * the module cycle is evaluation-safe because every one of them is read only + * inside a function body, never while the two modules are first evaluated. + * `index.ts` names what it publishes from `protocol.ts` one by one, so the + * `export` they gained for this keeps them off the published surface, and + * nothing re-exports this module, so the published surface does not move. + */ + +import type { ObjectStackProtocolImplementation, BatchDataLoopOutcome, BatchDataRowResult } from './protocol.js'; +import { byIdUpdateRecord, rowRequiredIdError, toRowApiError } from './protocol.js'; +import type { BatchUpdateRequest, BatchUpdateResponse, DeleteManyDataRequest, UpdateManyDataRequest } from '@objectstack/spec/api'; +import type { DroppedFieldsEvent } from '@objectstack/spec/data'; +import { engineCanRollBack, recordNotFoundError } from '@objectstack/core'; +import { omitInternalFieldsFromWriteResponse } from './write-response-internal-fields.js'; + +/** + * [#3455] Collapse a batch's per-row `DroppedFieldsEvent`s into one event per + * `(object, reason)` with the UNION of dropped field names. + * + * Used by both bulk-create surfaces at BATCH level. `createManyData`'s + * `{ object, records, count }` response has no per-row slot to hang a + * `droppedFields` on — a union is the only view that response can represent, + * which is the whole reason this collapse exists. `insertManyData` has a + * per-row slot and reports here anyway: the slot is real, a per-row answer is + * not (its docblock states why), so it reports the union where the union is + * true. (Since #14147 that strip is the ENGINE's, which reports one event per + * CALL for it, so the aggregation is over the runtime-owned per-row events.) + * + * ⚠️ So read a name in a merged event as "AT LEAST ONE row dropped this field", + * never "every row dropped the same set". Maintainer ruling C (#14147) put the + * static-`readonly` strip INSIDE `engine.insert`, AFTER the `beforeInsert` + * hooks, where it exempts keys a hook itself assigned — recorded PER ROW and + * indexed per row at the call: `packages/objectql/src/engine.ts` hands + * `stripReadonlyFields` the option `hookWrittenKeys: rowHookWrittenKeys[i]`, + * and that option's only power is to turn a STRIP into a KEEP. A hook that + * stamps a protected key on some rows and not others therefore makes those rows + * drop DIFFERENT sets, so the union is faithful to the BATCH without being + * faithful to any one row. + * + * Returns `[]` when nothing was dropped so callers can spread + * `...(x.length ? { droppedFields: x } : {})` and keep the omit-when-empty shape. + * The paths that DO keep row precision — `updateManyData` and `batchData` — + * earn it mechanically rather than by inference: each row is its own + * `engine.update` / `engine.insert` call, so that call's events are that row's. + */ +function mergeDroppedFieldEvents(events: DroppedFieldsEvent[]): DroppedFieldsEvent[] { + if (events.length === 0) return []; + const byKey = new Map }>(); + for (const ev of events) { + const key = `${ev.object}|${ev.reason}`; + let bucket = byKey.get(key); + if (!bucket) { bucket = { object: ev.object, reason: ev.reason, fields: new Set() }; byKey.set(key, bucket); } + for (const f of ev.fields) bucket.fields.add(f); + } + return Array.from(byKey.values()).map((b) => ({ object: b.object, fields: Array.from(b.fields), reason: b.reason })); +} + +/** + * [#8502] The stable sentence a failed batch row says when nothing may be + * quoted — the operation, named, and no interpolation of any kind. + * + * One vocabulary for all three loops, so a caller reconciling a mixed batch is + * not reading three phrasings of one condition. Deliberately says where the + * reason IS (the server log), because the row itself is the caller's only + * channel here: a batch row rides response DATA on a 200, so there is no `cause` + * for a boundary to print and no 5xx withhold anywhere above it. + */ +function rowOperationFailureFallback(operation: string): string { + return `The ${operation} of this record failed. The reason is in the server log.`; +} + +/** + * The `ObjectStackProtocolImplementation` members the functions below read — + * every one of them, and no other; each function's `this` is the `Pick` of the + * members IT reads. Each is typed AS the class member (an indexed access on + * the class), so a change to a member's signature on the class is a change + * here too, and naming a member the class does not have is a compile error. + * See the module header for which members are getters. + */ +export interface BatchDataHost { + assertObjectRegistered: ObjectStackProtocolImplementation['assertObjectRegistered']; + assertRecordExists: ObjectStackProtocolImplementation['assertRecordExists']; + buildBatchDataResponse: ObjectStackProtocolImplementation['buildBatchDataResponse']; + buildDeleteManyResponse: ObjectStackProtocolImplementation['buildDeleteManyResponse']; + buildRolledBackBatchResponse: ObjectStackProtocolImplementation['buildRolledBackBatchResponse']; + buildUpdateManyResponse: ObjectStackProtocolImplementation['buildUpdateManyResponse']; + readonly engine: ObjectStackProtocolImplementation['engine']; + locateBatchCause: ObjectStackProtocolImplementation['locateBatchCause']; + probeRecord: ObjectStackProtocolImplementation['probeRecord']; + reconcileStoppedBatch: ObjectStackProtocolImplementation['reconcileStoppedBatch']; + runAtomicBatch: ObjectStackProtocolImplementation['runAtomicBatch']; + runAtomicBatchData: ObjectStackProtocolImplementation['runAtomicBatchData']; + runBatchDataLoop: ObjectStackProtocolImplementation['runBatchDataLoop']; + runDeleteManyLoop: ObjectStackProtocolImplementation['runDeleteManyLoop']; + runUpdateManyLoop: ObjectStackProtocolImplementation['runUpdateManyLoop']; +} + +/** The body of `ObjectStackProtocolImplementation#batchData`; the method's docblock there is its contract. */ +export async function batchData( + this: Pick, + request: { object: string, request: BatchUpdateRequest, context?: any }): Promise { + const { object, request: batchReq, context } = request; + this.assertObjectRegistered(object); // [#3770] + const { operation, records, options } = batchReq; + + // [#3043 → #14147] The batch endpoint is an external ingress, and it + // used to strip forged read-only columns on create HERE; that strip is + // `engine.insert`'s since the 2026-09-03 ruling, reported back per row + // through `onFieldsDropped`. [#3455] It DOES resolve an execution + // context (threaded by REST); thread it to every engine call so RLS/FLS, + // `readonlyWhen` AND the create-side readonly strip run under the + // caller, so a system caller is correctly exempt (the pre-#3455 code + // hard-coded the strip context to `undefined`, treating every batch + // create as non-system). + const batchSchema = this.engine.registry?.getObject(object); + + // ADR-0119 D4 — `atomic` is REAL or REFUSED, never silent best-effort. + // This flag used to only `break` the loop: every write before the + // failure stayed COMMITTED while the response called itself atomic and + // reported those rows `success: true`. Same class as #4346 — a + // write-path guarantee declared but not enforced, silent and + // destructive exactly when it matters. + // + // Opt-in is an explicit `=== true`. `BatchOptionsSchema` declared + // `.default(true)` while no enforcement site ever delivered atomicity + // (REST forwards the original body, so the parsed default never reached + // this loop), so treating "absent" as atomic would silently flip every + // existing caller's failure semantics. The declaration is aligned to + // the enforced value instead; see the schema's note. + if (options?.atomic === true) { + return await this.runAtomicBatchData({ object, operation, records, options, batchSchema, context }); + } + + const outcome = await this.runBatchDataLoop({ object, operation, records, options, batchSchema, context, atomic: false }); + return this.buildBatchDataResponse(operation, records, options, outcome); +} + +/** The body of `ObjectStackProtocolImplementation#runAtomicBatchData`; the method's docblock there is its contract. */ +export async function runAtomicBatchData( + this: Pick, + args: { + object: string; + operation: BatchUpdateRequest['operation']; + records: BatchUpdateRequest['records']; + options: BatchUpdateRequest['options']; + batchSchema: any; + context: any; +}): Promise { + const { object, operation, records, options, batchSchema, context } = args; + return await this.runAtomicBatch({ + object, + context, + runLoop: (trxCtx) => this.runBatchDataLoop({ object, operation, records, options, batchSchema, context: trxCtx, atomic: true }), + onCommit: (outcome) => this.buildBatchDataResponse(operation, records, options, outcome), + onRollback: (outcome) => this.buildRolledBackBatchResponse(operation, records, outcome), + }); +} + +/** The body of `ObjectStackProtocolImplementation#runAtomicBatch`; the method's docblock there is its contract. */ +export async function runAtomicBatch( + this: Pick, + args: { + object: string; + context: any; + runLoop: (trxCtx: any) => Promise; + onCommit: (outcome: BatchDataLoopOutcome) => BatchUpdateResponse; + onRollback: (outcome: BatchDataLoopOutcome) => BatchUpdateResponse; +}): Promise { + const { object, context, runLoop, onCommit, onRollback } = args; + + // Two-level probe, shared with the ADR-0119 D2 migration-journal runner + // as `engineCanRollBack` (#4617). `engine.transaction()` runs the + // callback with NO transaction and NO rollback when the default driver + // lacks `beginTransaction` — a declared caveat of the contract member + // (ADR-0119 D1), and one that would turn "atomic" back into a lie + // precisely where it matters. So where the driver registry is + // inspectable, the driver is checked too; where it is not (test + // doubles), the engine-level probe is all there is. + // + // Shared rather than restated: this and the runner's gate were the same + // condition written twice, and two copies of "can this runtime actually + // roll back?" drift by one clause and leave one caller believing it has + // atomicity it does not have. + const engineTx = engineCanRollBack(this.engine) + ? this.engine.transaction.bind(this.engine) + : undefined; + + if (!engineTx) { + // REFUSE, do not degrade. A caller that asked for atomicity is + // exactly the caller who must not silently receive best-effort — + // silent degradation is how this flag came to lie in the first + // place. Mirrors the cross-object /batch route's refusal; the + // condition is generic, so it uses the standard error catalog + // rather than registering an ADR-0112 synonym. + const err: any = new Error( + `Atomic batch on '${object}' requires engine transaction support; this runtime cannot roll back. ` + + `Retry without options.atomic, or probe capabilities.transactionalBatch on /discovery first.`, + ); + err.status = 501; + err.code = 'NOT_IMPLEMENTED'; + throw err; + } + + // Identity-checked sentinel: aborting the transaction is how a rollback + // is requested, but the abort itself is not an error to propagate. + const ABORT = new Error('atomic batch aborted — rolled back'); + let aborted: BatchDataLoopOutcome | undefined; + try { + return await engineTx(async (trxCtx: any) => { + const outcome = await runLoop(trxCtx); + if (outcome.failed > 0) { + aborted = outcome; + throw ABORT; + } + return onCommit(outcome); + }, context); + } catch (err) { + if (err === ABORT && aborted) { + return onRollback(aborted); + } + throw err; + } +} + +/** The body of `ObjectStackProtocolImplementation#runBatchDataLoop`; the method's docblock there is its contract. */ +export async function runBatchDataLoop( + this: Pick, + args: { + object: string; + operation: BatchUpdateRequest['operation']; + records: BatchUpdateRequest['records']; + options: BatchUpdateRequest['options']; + batchSchema: any; + context: any; + atomic: boolean; +}): Promise { + const { object, operation, records, options, batchSchema, context, atomic } = args; + const results: BatchDataRowResult[] = []; + let succeeded = 0; + let failed = 0; + + // Spread form for options objects that already carry `where`/`onFieldsDropped` + // (`{}` spread is a safe no-op). [#14147] EVERY `engine.insert` in this + // loop — `create`, and both arms of `upsert` that create — builds an + // options object: the readonly strip they report moved into + // `engine.insert` and reaches this seam only through `onFieldsDropped`, + // so the arg form (`undefined` when there is no context) is gone from + // here. A create-shaped arm that passed no listener reported a silent + // drop — the upsert arms did exactly that until the patch round. + const ctxOpt = context !== undefined ? { context } : {}; + + // [#4793] `index` is the row's position in the REQUEST array — the + // correlation a caller needs for failure rows that carry no id. + for (const [index, record] of records.entries()) { + try { + switch (operation) { + case 'create': { + // [#3455/#14147] A batch-create caller sees the same + // `droppedFields` a single-write create surfaces (#3431) — + // read off the ENGINE's listener now that the readonly + // create strip lives there, instead of reconstructed from a + // before/after payload diff at this seam. + const rowDropped: DroppedFieldsEvent[] = []; + const created = await this.engine.insert(object, record.data || record, { + ...ctxOpt, + onFieldsDropped: (e: DroppedFieldsEvent) => { rowDropped.push(e); }, + } as any); + omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823] + const ev = mergeDroppedFieldEvents(rowDropped); + results.push({ id: created.id, success: true, data: created, index, ...(ev.length > 0 ? { droppedFields: ev } : {}) }); + succeeded++; + break; + } + case 'update': { + if (!record.id) throw rowRequiredIdError('update'); + // [#5088] Same existence gate as `updateMany` and the + // single-record PATCH — a row naming no record must not + // enter the write pipeline, where #4770's stored ⊕ + // payload merge has no stored side and #4775 blames the + // hook for the resulting gap. + await this.assertRecordExists(object, record.id); + // [#3455] Collect the engine's LEGAL write strips per row. + const dropped: DroppedFieldsEvent[] = []; + const updated = byIdUpdateRecord(await this.engine.update(object, record.data || {}, { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, ...ctxOpt } as any)); + omitInternalFieldsFromWriteResponse(batchSchema, updated); // [#7823] + results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) }); + succeeded++; + break; + } + case 'upsert': { + if (record.id) { + // [#5099] The update-or-insert fork asks EXISTENCE, + // not visibility. `findOne` under the CALLER's + // context is the read RLS/sharing narrows (#3455), + // so an existing row outside the caller's scope + // answered null, took the insert arm, and either + // duplicate-keyed — an authorization/update + // scenario reported as a key collision — or, on a + // store without a unique id constraint, wrote a + // second row. Same probe as the update branch above + // and the single-record path (#4620): it asks the + // database a fact, and whether the caller may WRITE + // the row it proves stays #1994's decision inside + // `engine.update`. + // + // The old fallback (update threw → blind insert) + // is gone with the fork's visibility read: with + // existence decided BEFORE the write, a fallback + // insert could only bury a real update failure + // under the duplicate-key error of inserting a row + // just proven to exist — the same masking ADR-0119 + // D4 forbade inside the atomic arm. + const existing = await this.probeRecord(object, record.id); + if (existing) { + const dropped: DroppedFieldsEvent[] = []; + const updated = byIdUpdateRecord(await this.engine.update(object, record.data || {}, { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, ...ctxOpt } as any)); + omitInternalFieldsFromWriteResponse(batchSchema, updated); // [#7823] + results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) }); + } else { + // [#14147] The upsert's CREATE arm is a create face + // too: same listener, same per-row `droppedFields`, + // as `case 'create'` above and the update arm just up. + const rowDropped: DroppedFieldsEvent[] = []; + const created = await this.engine.insert(object, { id: record.id, ...(record.data || {}) }, { + ...ctxOpt, + onFieldsDropped: (e: DroppedFieldsEvent) => { rowDropped.push(e); }, + } as any); + omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823] + const ev = mergeDroppedFieldEvents(rowDropped); + results.push({ id: created.id, success: true, data: created, index, ...(ev.length > 0 ? { droppedFields: ev } : {}) }); + } + } else { + // [#14147] Same: an id-less upsert row IS a create. + const rowDropped: DroppedFieldsEvent[] = []; + const created = await this.engine.insert(object, record.data || record, { + ...ctxOpt, + onFieldsDropped: (e: DroppedFieldsEvent) => { rowDropped.push(e); }, + } as any); + omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823] + const ev = mergeDroppedFieldEvents(rowDropped); + results.push({ id: created.id, success: true, data: created, index, ...(ev.length > 0 ? { droppedFields: ev } : {}) }); + } + succeeded++; + break; + } + case 'delete': { + if (!record.id) throw rowRequiredIdError('delete'); + // [#5088] `deleteManyData` learned this in #4435; this + // branch — the OTHER by-id bulk delete, ten lines from + // it — kept discarding the driver's return and pushing + // `success: true` unconditionally, so a batch of typo'd + // ids reported every one of them deleted. Same `=== false` + // reading as both fixed faces: the contract's positive + // not-found value (`IDataDriver.delete`), never an + // inference from a falsy return, so a third-party driver + // that answers with the deleted row is not turned into a + // spurious 404. + const deleted = await this.engine.delete(object, { where: { id: record.id }, ...ctxOpt } as any); + if (deleted === false) throw recordNotFoundError(object, record.id); + // [#19433] The SECOND half of this site, and the THIRD and + // last of the by-id delete doors to learn it — the + // single-record face (commit f9e16d856) and `deleteManyData` (#19412) + // both already read the engine's answer. The paragraph + // above fixed "no match"; this is "matched, and + // deliberately NOT removed", where `success` was still a + // LITERAL for every result that was not the contract's + // `false`, so that row was reported as a deletion too. + // + // `sys_permission_set` was the shipped shape of it until C3 + // stage 8 retired that object: deleting a package-declared + // set was an ADR-0005 RESET — the overlay tombstoned and the + // record re-projected to the declared body instead of + // vanishing — so the row MATCHED, the write ran, and the + // record was still there. On a security-configuration + // write this envelope told an operator a permission set was + // gone while it was still being enforced. + // + // The defect rests on the CONTRACT, not on any shipped + // handler: `IDataEngine.delete` declares + // `Promise` — the driver's boolean for a + // by-id write, a COUNT of rows removed otherwise — and + // `isDeleteResultShape` admits the number arm at the + // ADR-0112 hook gate, so an `afterDelete` handler or a + // non-ObjectQL engine may legally answer `0` today. + // + // ⛔ Not the `false` arm: that is spoken for by the 404 + // above, about a record this caller can still GET, and + // answering it here would trade one wrong answer for a + // louder one. Everything else keeps its #4435 reading, an + // off-contract `undefined` from a third-party driver + // included: only a POSITIVE zero is read as "not removed". + // + // ⛔ And the row gets NO `errors[]` entry. A surviving + // record is an OUTCOME, not a fault — the single-record door + // answers the same case with a bare `success: false` on a + // 200 — and the two per-row codes this envelope owns + // (`ROLLED_BACK`, `NOT_ATTEMPTED`) both describe a row that + // never ran. Minting one for this ending is an + // ERROR_CODE_LEDGER widening in `packages/spec`, and belongs + // to the spec lane. + // + // This envelope's own consequences, measured here rather + // than inherited from `deleteManyData`: the row is counted + // in `failed` because `succeeded` and `failed` PARTITION + // `results` (#7539, `reconcileStoppedBatch`, shared by all + // three bulk faces), which makes the request-level `success` + // false and, on the `atomic` arm, aborts the batch through + // `runAtomicBatch`'s `failed > 0` — so an atomic batch + // holding a package-declared set now rolls back instead of + // committing under a response that called every row deleted. + // It does NOT stop a non-atomic run: the `continueOnError` + // stop belongs to the catch below and nothing was thrown, so + // every remaining record is still attempted. `returnRecords: + // false` keeps `success`, so the honest value survives that + // projection too. Only this `case` is touched — the sibling + // arms of this shared loop keep their own counters. + const removed = deleted !== 0; + results.push({ id: record.id, success: removed, index }); + if (removed) succeeded++; + else failed++; + break; + } + default: + results.push({ id: record.id, success: false, index, errors: [{ code: 'VALIDATION_FAILED', message: `Unknown operation: ${operation}`, httpStatus: 400 }] }); + failed++; + } + } catch (err: any) { + // [#8502] `operation` is the request's own verb, so a mixed + // batch's withheld rows each name what THEY were doing. + results.push({ id: record.id, success: false, index, errors: [toRowApiError(err, rowOperationFailureFallback(operation))] }); + failed++; + if (atomic) { + // Abort on the first failure; the caller rolls back. Atomic + // outranks `continueOnError` — there is nothing to continue + // toward when every write so far is about to be undone. + break; + } + if (!options?.continueOnError) { + break; + } + } + } + + return { results, succeeded, failed }; +} + +/** The body of `ObjectStackProtocolImplementation#reconcileStoppedBatch`; the method's docblock there is its contract. */ +export function reconcileStoppedBatch( + this: Pick, + records: ReadonlyArray<{ id?: string }>, + outcome: BatchDataLoopOutcome, +): BatchDataLoopOutcome { + if (outcome.results.length >= records.length) return outcome; + + const cause = this.locateBatchCause(outcome.results); + + const results = outcome.results.slice(); + for (let index = results.length; index < records.length; index++) { + results.push({ + // Echoed back so a caller can retry exactly the skipped rows. + id: records[index]?.id, + success: false, + index, + errors: [{ + code: 'NOT_ATTEMPTED' as const, + message: (cause ? `${cause.clause}; the batch stopped there. ` : 'the batch stopped before this record. ') + + 'Set options.continueOnError to process the remaining records.', + }], + }); + } + + // Every padded row is a non-success, so this stays a PARTITION of + // `results` rather than a second tally free to drift from it. + return { results, succeeded: outcome.succeeded, failed: results.length - outcome.succeeded }; +} + +/** The body of `ObjectStackProtocolImplementation#locateBatchCause`; the method's docblock there is its contract. */ +export function locateBatchCause( + rows: ReadonlyArray, +): { index: number; clause: string } | undefined { + for (let index = rows.length - 1; index >= 0; index--) { + const fault = rows[index]?.errors?.[0]; + if (fault) return { index, clause: `record ${index} failed — ${fault.message}` }; + } + const stalled = rows.findIndex(r => !r.success); + return stalled >= 0 ? { index: stalled, clause: `record ${stalled} did not succeed` } : undefined; +} + +/** The body of `ObjectStackProtocolImplementation#buildBatchDataResponse`; the method's docblock there is its contract. */ +export function buildBatchDataResponse( + this: Pick, + operation: BatchUpdateRequest['operation'], + records: BatchUpdateRequest['records'], + options: BatchUpdateRequest['options'], + outcome: BatchDataLoopOutcome, +): BatchUpdateResponse { + const { results, succeeded, failed } = this.reconcileStoppedBatch(records, outcome); // [#7539] + return { + success: failed === 0, + operation, + total: records.length, + succeeded, + failed, + // [#3455] `returnRecords: false` drops the record payload (`data`) + // but KEEPS `errors`, `index` and `droppedFields` — the last is a + // small write-observability warning, not the record data the flag + // suppresses. + results: options?.returnRecords !== false ? results : results.map(r => ({ id: r.id, success: r.success, index: r.index, ...(r.errors ? { errors: r.errors } : {}), ...(r.droppedFields ? { droppedFields: r.droppedFields } : {}) })), + } as BatchUpdateResponse; +} + +/** The body of `ObjectStackProtocolImplementation#buildRolledBackBatchResponse`; the method's docblock there is its contract. */ +export function buildRolledBackBatchResponse( + this: Pick, + operation: BatchUpdateRequest['operation'], + // Only `length` and `id` are read, so `updateManyData`'s rows and the + // id list `deleteManyData` rolls back reuse this verbatim (#4620) — + // the marking a client reconciles against must be one implementation, + // not three that agree today. + records: ReadonlyArray<{ id?: string }>, + outcome: BatchDataLoopOutcome, +): BatchUpdateResponse { + const attempted = outcome.results; + // [#19452] Same locator as the stopped-batch arm, for the same reason: + // `!success` stopped meaning "this row failed" when #19412 widened it. + // Both this builder's interpolations of the causal index read it, so + // neither the ROLLED_BACK message nor the NOT_ATTEMPTED one can name a + // row that merely survived — the two could not be fixed apart. + const cause = this.locateBatchCause(attempted); + + const results: BatchDataRowResult[] = records.map((record, index) => { + const attempt = attempted[index]; + if (!attempt) { + return { + id: record.id, success: false, index, + errors: [{ code: 'NOT_ATTEMPTED' as const, message: cause ? `atomic batch aborted by record ${cause.index}` : 'atomic batch aborted' }], + }; + } + if (attempt.success) { + return { + id: attempt.id ?? record.id, success: false, index, + errors: [{ code: 'ROLLED_BACK' as const, message: cause ? cause.clause : 'the atomic batch rolled back' }], + }; + } + return { id: attempt.id ?? record.id, success: false, index, errors: attempt.errors }; + }); + + return { + success: false, + operation, + total: records.length, + succeeded: 0, + failed: records.length, + results, + } as BatchUpdateResponse; +} + +/** The body of `ObjectStackProtocolImplementation#createManyData`; the method's docblock there is its contract. */ +export async function createManyData( + this: Pick, + request: { object: string, records: any[], context?: any }): Promise { + this.assertObjectRegistered(request.object); // [#3770] + // [#3455/#5503/#14147] Both create-side strips are the engine's now — + // runtime-owned `autonumber` (#5503) and, since #14147, static + // author-declared `readonly` — so ONE listener carries both, and this + // seam no longer diffs payloads to recover a strip it performed itself. + // AGGREGATED: the `{ records, count }` response has no per-row slot, so + // a union is the only representable view here. (`insertManyData` HAS a + // per-row slot and still reports at the top level — the union cannot be + // resolved to rows at either seam; see its own docblock.) + const dropped: DroppedFieldsEvent[] = []; + const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } }; + if (request.context !== undefined) opts.context = request.context; + const records = await this.engine.insert(request.object, request.records, opts); + // [#7823] Bulk create is the same generic-data-path surface as the + // single-record 201 — one strip over the returned rows, at the + // ingress. (Today's `internal`-flagged objects grant no `bulk` + // apiMethod, so this face cannot reach one over REST yet; the strip is + // here so the flag's guarantee does not depend on that staying true.) + omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(request.object), records); + const merged = mergeDroppedFieldEvents(dropped); + return { + object: request.object, + records, + count: records.length, + ...(merged.length > 0 ? { droppedFields: merged } : {}), + }; +} + +/** The body of `ObjectStackProtocolImplementation#insertManyData`; the method's docblock there is its contract. */ +export async function insertManyData( + this: Pick, + request: { object: string, records: any[], context?: any }): Promise<{ object: string; outcomes: Array<{ ok: boolean; record?: any; error?: unknown; droppedFields?: DroppedFieldsEvent[] }>; droppedFields?: DroppedFieldsEvent[] }> { + this.assertObjectRegistered(request.object); // [#3770] + const engineInsertMany = (this.engine as any)?.insertMany; + if (typeof engineInsertMany !== 'function') { + throw new Error('insertManyData requires an engine with insertMany: the partial-success batch insert, which reports an outcome per row so a bad row neither fails the whole batch nor makes the good rows run their beforeInsert hooks twice'); + } + // [#5503/#14147] The engine's events are collected WHOLE and merged — + // the same handling `createManyData` gives them, and for the reason + // spelled out on this method: the union is reportable, a row set is not. + // Keeping each event also keeps its own `reason`, which the old + // flatten-into-a-Set-and-relabel could not. + const dropped: DroppedFieldsEvent[] = []; + const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } }; + if (request.context !== undefined) opts.context = request.context; + // [#20922] Each `ok` outcome carries the ENGINE's per-row report + // (`outcomes[i].droppedFields`), recorded at its strips — a separate + // channel from the batch-level union below, which still names no row. + // Passed through as the engine answers it; ⛔ nothing here derives a + // row's drops from the union. + const outcomes: Array<{ ok: boolean; record?: any; error?: unknown; droppedFields?: DroppedFieldsEvent[] }> = await engineInsertMany.call( + this.engine, + request.object, + request.records, + opts, + ); + // [#7823] Per-outcome ingress strip — the partial-success face hands + // each written row back as `outcomes[i].record`, so each is the same + // generic-data-path surface as a single-record 201 body. + if (Array.isArray(outcomes)) { + const outcomeSchema = this.engine.registry?.getObject(request.object); + for (const o of outcomes) { + if (o?.record) omitInternalFieldsFromWriteResponse(outcomeSchema, o.record); + } + } + const merged = mergeDroppedFieldEvents(dropped); + return { + object: request.object, + outcomes, + ...(merged.length > 0 ? { droppedFields: merged } : {}), + }; +} + +/** The body of `ObjectStackProtocolImplementation#updateManyData`; the method's docblock there is its contract. */ +export async function updateManyData( + this: Pick, + request: UpdateManyDataRequest & { context?: any }): Promise { + const { object, records, options, context } = request; + this.assertObjectRegistered(object); // [#3770] + + // [#4620] `atomic` used not to appear ANYWHERE in this method: the + // option was accepted, declared in `BatchOptionsSchema` with a contract + // that promises all-or-nothing, and never read — a caller asking for + // atomicity silently got best-effort with no signal at all. Same + // enforcement shape as `batchData` (ADR-0119 D4), same runner, so the + // three bulk-write surfaces cannot drift apart again. `=== true` is the + // deliberate opt-in from D4: absent/false keeps today's semantics + // exactly. + if (options?.atomic === true) { + return await this.runAtomicBatch({ + object, + context, + runLoop: (trxCtx) => this.runUpdateManyLoop({ object, records, options, context: trxCtx, atomic: true }), + onCommit: (outcome) => this.buildUpdateManyResponse(records, outcome), + onRollback: (outcome) => this.buildRolledBackBatchResponse('update', records, outcome), + }); + } + + const outcome = await this.runUpdateManyLoop({ object, records, options, context, atomic: false }); + return this.buildUpdateManyResponse(records, outcome); +} + +/** The body of `ObjectStackProtocolImplementation#runUpdateManyLoop`; the method's docblock there is its contract. */ +export async function runUpdateManyLoop( + this: Pick, + args: { + object: string; + records: UpdateManyDataRequest['records']; + options: UpdateManyDataRequest['options']; + context: any; + atomic: boolean; +}): Promise { + const { object, records, options, context, atomic } = args; + const results: BatchDataRowResult[] = []; + let succeeded = 0; + let failed = 0; + // [#7823] Ingress strip over each row's `data` payload — the bulk + // update face is the same generic-data-path surface as the by-id + // PATCH body, one row at a time. Resolved once; the loop reuses it. + const updateManySchema = this.engine.registry?.getObject(object); + + for (const [index, record] of records.entries()) { + try { + // [#3455] Two gaps the pre-#3455 loop had, both fixed per row: + // 1. `context` was never threaded — bulk updates ran the engine + // context-less, so RLS/FLS and `readonlyWhen` evaluated without + // the caller's principal. Thread it like single-write updateData. + // 2. `onFieldsDropped` was never wired — the same static `readonly` + // (#2948) / `readonlyWhen` (#3042) strips that single-write now + // surfaces (#3431) happened silently here. Collect per row. + // + // [#5100] Same guard as `runBatchDataLoop`'s update branch + // (#4793), same classification: an id-less row is a CALLER + // error — VALIDATION_FAILED/400 — not a data-state one. The + // REST entrance already rejects it (`UpdateManyRecordSchema` + // requires `id`, #3939), but that invariant lives two packages + // away; unguarded, an in-process caller's malformed row + // reached the probe and the write as `{ id: undefined }`, + // whose reading is up to each driver's undefined-where-key + // handling — at best a 404 with `undefined` interpolated into + // the message, at worst a where-clause with no id at all. + if (!record.id) throw rowRequiredIdError('update'); + // [#5088] Third gap, the same shape: no existence gate. A row + // naming no record went straight into `engine.update`, so the + // hook pipeline ran over a payload-only record and the row came + // back `INTERNAL_ERROR` from #4775's condition abort — blaming + // the hook for a caller's stale id. Probe first, per row, so + // this face answers what the single-record PATCH answers. + await this.assertRecordExists(object, record.id); + const dropped: DroppedFieldsEvent[] = []; + const opts: any = { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } }; + if (context !== undefined) opts.context = context; + const updated = byIdUpdateRecord(await this.engine.update(object, record.data || {}, opts)); + omitInternalFieldsFromWriteResponse(updateManySchema, updated); // [#7823] + results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) }); + succeeded++; + } catch (err: any) { + results.push({ id: record.id, success: false, index, errors: [toRowApiError(err, rowOperationFailureFallback('update'))] }); + failed++; + if (atomic) { + // Abort on the first failure; the caller rolls back. + break; + } + if (!options?.continueOnError) { + break; + } + } + } + + return { results, succeeded, failed }; +} + +/** The body of `ObjectStackProtocolImplementation#buildUpdateManyResponse`; the method's docblock there is its contract. */ +export function buildUpdateManyResponse( + this: Pick, + records: UpdateManyDataRequest['records'], + outcome: BatchDataLoopOutcome, +): BatchUpdateResponse { + // [#7539] Same under-report as `batchData`, ten lines away — the card's + // "per-object bulk counters under-report whenever a row fails without + // `continueOnError`". Fixed through the one shared reconciler, because + // a second copy is how these three drifted apart before (#4620). + const { results, succeeded, failed } = this.reconcileStoppedBatch(records, outcome); + return { + success: failed === 0, + operation: 'update', + total: records.length, + succeeded, + failed, + results, + } as BatchUpdateResponse; +} + +/** The body of `ObjectStackProtocolImplementation#deleteManyData`; the method's docblock there is its contract. */ +export async function deleteManyData( + this: Pick, + request: DeleteManyDataRequest & { context?: any }): Promise { + const { object, options, context } = request; + this.assertObjectRegistered(object); // [#3770] + + // Fail CLOSED on anything that is not a list of scalar ids. A non-scalar + // entry (`{"ids":[{"$ne":null}]}`) must never reach `where.id` as an + // operator object — that is the same "predicate widening" this endpoint + // was just hardened against, one layer down. + const isScalarId = (v: unknown) => + (typeof v === 'string' && v.length > 0) || typeof v === 'number' || typeof v === 'bigint'; + const ids = request.ids as unknown; + if (!Array.isArray(ids) || ids.some((id) => !isScalarId(id))) { + const err: any = new Error( + `deleteMany on '${object}' requires 'ids' to be an array of record ids`, + ); + err.code = 'VALIDATION_FAILED'; + err.status = 400; + throw err; + } + + // [#4620] `atomic` here was the same fake-atomic `batchData` carried + // before ADR-0119 D4 — it only `break`-ed the loop, so every row deleted + // before the failure stayed DELETED while the response called itself + // atomic. Worse than the `batchData` case, because a partial delete has + // no natural undo: the caller cannot reconstruct the rows from the + // request. Same runner, same fail-closed gate, same row marking. + if (options?.atomic === true) { + const rows = ids.map((id) => ({ id: String(id) })); + return await this.runAtomicBatch({ + object, + context, + runLoop: (trxCtx) => this.runDeleteManyLoop({ object, ids, options, context: trxCtx, atomic: true }), + onCommit: (outcome) => this.buildDeleteManyResponse(ids, outcome), + onRollback: (outcome) => this.buildRolledBackBatchResponse('delete', rows, outcome), + }); + } + + const outcome = await this.runDeleteManyLoop({ object, ids, options, context, atomic: false }); + return this.buildDeleteManyResponse(ids, outcome); +} + +/** The body of `ObjectStackProtocolImplementation#runDeleteManyLoop`; the method's docblock there is its contract. */ +export async function runDeleteManyLoop( + this: Pick, + args: { + object: string; + ids: unknown[]; + options: DeleteManyDataRequest['options']; + context: any; + atomic: boolean; +}): Promise { + const { object, ids, options, context, atomic } = args; + const results: BatchDataRowResult[] = []; + let succeeded = 0; + let failed = 0; + const ctxOpt = context !== undefined ? { context } : {}; + + for (const [index, id] of ids.entries()) { + try { + // [#4435] Per-row honesty on the bulk path. This discarded the + // driver's return and pushed `success: true` unconditionally, so + // `{"ids":["nonexistent_1"]}` answered `succeeded: 1` — a batch + // of typo'd ids reported every one of them deleted. A caller + // reconciling "which of my 200 ids were real" got a list that + // agreed with whatever it sent. Same `=== false` reading as the + // single-record path: the contract's positive not-found value, + // never an inference from a falsy return. + const deleted = await this.engine.delete(object, { where: { id }, ...ctxOpt } as any); + // `id` is `unknown` to this helper only because the caller's + // fail-closed `isScalarId` guard is what proves it scalar. + if (deleted === false) throw recordNotFoundError(object, id as string | number); + // [#19412] The SECOND half of the same site. The paragraph above + // fixed "no match"; this is "matched, and deliberately NOT + // removed" — and until now `success` was still a LITERAL for + // every answer that was not the contract's `false`, so that row + // was reported as a deletion too. + // + // `sys_permission_set` was the shipped case until C3 stage 8 + // retired that object: a package-declared set's delete was an + // ADR-0005 RESET — plugin-security's write-through tombstoned the + // overlay and the record re-projected to the declared body + // instead of vanishing — so the row MATCHED, the write ran, and + // the record was still there. The + // batch envelope said `success: true` and counted it in + // `succeeded`, which on a security-configuration write tells an + // operator a permission set is gone while it is still enforced. + // + // `IDataEngine.delete` declares `Promise` — + // the driver's boolean for a by-id write, a COUNT of rows + // removed otherwise — so a numeric zero is the one value that + // positively means "it is still there". ⛔ Not `false`: that + // arm is spoken for by the 404 above, about a record this caller + // can still GET, and answering it here would trade one wrong + // answer for a louder one. Everything else keeps its #4435 + // reading, an off-contract `undefined` from a third-party driver + // included: only a POSITIVE zero is read as "not removed". + // + // ⛔ The row is NOT given an `errors[]` entry. A surviving record + // is an OUTCOME, not a fault — the single-record door answers the + // same case with a bare `success: false` on a 200 — and the two + // per-row codes this envelope owns (`ROLLED_BACK`, + // `NOT_ATTEMPTED`) both describe a row that never ran. Whether + // this ending deserves a per-row code of its own is a + // `packages/spec` widening (ERROR_CODE_LEDGER) and belongs to the + // spec lane, not here. + // + // It is counted in `failed` because that is the envelope's ONE + // declared reading — `succeeded` and `failed` PARTITION `results` + // (#7539, `reconcileStoppedBatch`) — which also makes the + // request-level `success` false and, on the `atomic` arm, aborts + // the batch through `runAtomicBatch`'s `failed > 0`. It does NOT + // stop a non-atomic run: the `continueOnError` stop belongs to + // the catch below, and nothing was thrown. + const removed = deleted !== 0; + results.push({ id: String(id), success: removed, index }); + if (removed) succeeded++; + else failed++; + } catch (err: any) { + results.push({ id: String(id), success: false, index, errors: [toRowApiError(err, rowOperationFailureFallback('delete'))] }); + failed++; + // Same stop semantics as `batchData`: `atomic` aborts the rest on + // the first failure (the caller rolls back), and without + // `continueOnError` a failure ends the run rather than silently + // ploughing on. + if (atomic) break; + if (!options?.continueOnError) break; + } + } + + return { results, succeeded, failed }; +} + +/** The body of `ObjectStackProtocolImplementation#buildDeleteManyResponse`; the method's docblock there is its contract. */ +export function buildDeleteManyResponse( + this: Pick, + ids: unknown[], outcome: BatchDataLoopOutcome): BatchUpdateResponse { + // [#7539] Same reconciliation as the other two faces; `ids` are mapped + // to the `{ id }` row shape the atomic arm already hands the rollback + // builder, so a skipped id comes back echoed and retryable. + const { results, succeeded, failed } = this.reconcileStoppedBatch(ids.map((id) => ({ id: String(id) })), outcome); + return { + success: failed === 0, + operation: 'delete', + total: ids.length, + succeeded, + failed, + results, + } as BatchUpdateResponse; +} diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 0b8aec20ff1..9be5f1ec505 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -280,6 +280,27 @@ import type { StoredMigrationTodo, } from './stored-migration.js'; import { collectDecisionModeReview } from './stored-migration.js'; +// [#25467 cut 1] The bulk data writes (`batchData`, the `*ManyData` verbs, their +// loops and response builders); the methods of the same names below hand each +// body the members it reads (`BatchDataHost`). +import { + batchData, + buildBatchDataResponse, + buildDeleteManyResponse, + buildRolledBackBatchResponse, + buildUpdateManyResponse, + createManyData, + deleteManyData, + insertManyData, + locateBatchCause, + reconcileStoppedBatch, + runAtomicBatch, + runAtomicBatchData, + runBatchDataLoop, + runDeleteManyLoop, + runUpdateManyLoop, + updateManyData, +} from './protocol-batch-data.js'; /** * Canonical Zod schema per metadata type lives in @@ -1773,7 +1794,7 @@ export { recordNotFoundError }; * and this file preserves it rather than widening a shipped response shape as * a rider; the assertion below is the one place it is written down. */ -function byIdUpdateRecord(result: Record | number | null): Record { +export function byIdUpdateRecord(result: Record | number | null): Record { if (typeof result === 'number') { throw new Error( `A by-id update resolved an affected-row count (${result}) instead of a record. ` + @@ -3476,48 +3497,6 @@ const CLONE_STRIP_FIELDS: readonly string[] = [ 'id', 'created_at', 'created_by', 'updated_at', 'updated_by', ]; -/** - * [#3455] Collapse a batch's per-row `DroppedFieldsEvent`s into one event per - * `(object, reason)` with the UNION of dropped field names. - * - * Used by both bulk-create surfaces at BATCH level. `createManyData`'s - * `{ object, records, count }` response has no per-row slot to hang a - * `droppedFields` on — a union is the only view that response can represent, - * which is the whole reason this collapse exists. `insertManyData` has a - * per-row slot and reports here anyway: the slot is real, a per-row answer is - * not (its docblock states why), so it reports the union where the union is - * true. (Since #14147 that strip is the ENGINE's, which reports one event per - * CALL for it, so the aggregation is over the runtime-owned per-row events.) - * - * ⚠️ So read a name in a merged event as "AT LEAST ONE row dropped this field", - * never "every row dropped the same set". Maintainer ruling C (#14147) put the - * static-`readonly` strip INSIDE `engine.insert`, AFTER the `beforeInsert` - * hooks, where it exempts keys a hook itself assigned — recorded PER ROW and - * indexed per row at the call: `packages/objectql/src/engine.ts` hands - * `stripReadonlyFields` the option `hookWrittenKeys: rowHookWrittenKeys[i]`, - * and that option's only power is to turn a STRIP into a KEEP. A hook that - * stamps a protected key on some rows and not others therefore makes those rows - * drop DIFFERENT sets, so the union is faithful to the BATCH without being - * faithful to any one row. - * - * Returns `[]` when nothing was dropped so callers can spread - * `...(x.length ? { droppedFields: x } : {})` and keep the omit-when-empty shape. - * The paths that DO keep row precision — `updateManyData` and `batchData` — - * earn it mechanically rather than by inference: each row is its own - * `engine.update` / `engine.insert` call, so that call's events are that row's. - */ -function mergeDroppedFieldEvents(events: DroppedFieldsEvent[]): DroppedFieldsEvent[] { - if (events.length === 0) return []; - const byKey = new Map }>(); - for (const ev of events) { - const key = `${ev.object}|${ev.reason}`; - let bucket = byKey.get(key); - if (!bucket) { bucket = { object: ev.object, reason: ev.reason, fields: new Set() }; byKey.set(key, bucket); } - for (const f of ev.fields) bucket.fields.add(f); - } - return Array.from(byKey.values()).map((b) => ({ object: b.object, fields: Array.from(b.fields), reason: b.reason })); -} - /** * One row of a bulk-write result — exactly the `BatchOperationResult` the spec * declares (`BatchOperationResultSchema`, spec/api/batch.zod.ts). Aliased to @@ -3532,7 +3511,7 @@ function mergeDroppedFieldEvents(events: DroppedFieldsEvent[]): DroppedFieldsEve * marking (#4620) is structured with it: `ROLLED_BACK` / `NOT_ATTEMPTED` are * `ApiError.code` values, not message-string prefixes. */ -type BatchDataRowResult = BatchOperationResult; +export type BatchDataRowResult = BatchOperationResult; /** * Map a thrown per-row batch error into the wire's `ApiError` (#4793). @@ -3648,7 +3627,7 @@ type BatchDataRowResult = BatchOperationResult; * ⛔ `httpStatus` is NOT stamped for it. That limb carries only a status the * producer declared (above), and this body declared none. */ -function toRowApiError(err: any, fallback: string): ApiError { +export function toRowApiError(err: any, fallback: string): ApiError { const thrown = isEngineDuplicateRecordEnvelope(err) ? 'UNIQUE_VIOLATION' : typeof err?.code === 'string' && ErrorCode.safeParse(err.code).success @@ -3682,20 +3661,6 @@ function isEngineDuplicateRecordEnvelope(err: unknown): boolean { return e?.code === 'DUPLICATE_RECORD' && e?.name === 'DuplicateRecordError'; } -/** - * [#8502] The stable sentence a failed batch row says when nothing may be - * quoted — the operation, named, and no interpolation of any kind. - * - * One vocabulary for all three loops, so a caller reconciling a mixed batch is - * not reading three phrasings of one condition. Deliberately says where the - * reason IS (the server log), because the row itself is the caller's only - * channel here: a batch row rides response DATA on a 200, so there is no `cause` - * for a boundary to print and no 5xx withhold anywhere above it. - */ -function rowOperationFailureFallback(operation: string): string { - return `The ${operation} of this record failed. The reason is in the server log.`; -} - /** * [#7426] Carry a wrapped error's `code` onto its re-wrap — but only when that * code is part of the DECLARED vocabulary. @@ -4290,7 +4255,7 @@ function specValidationFindings( * caller error, so it carries VALIDATION_FAILED / 400 rather than falling * through {@link toRowApiError}'s unclassified-throw default (#4793). */ -function rowRequiredIdError(operation: 'update' | 'delete'): Error { +export function rowRequiredIdError(operation: 'update' | 'delete'): Error { const err = new Error(`Record id is required for ${operation}`) as Error & { code?: string; status?: number }; err.code = 'VALIDATION_FAILED'; err.status = 400; @@ -4481,7 +4446,7 @@ function metadataItemNotFoundError(type: string, name: string): Error { } /** What one pass of the `batchData` record loop produced (ADR-0119 D4). */ -type BatchDataLoopOutcome = { results: BatchDataRowResult[]; succeeded: number; failed: number }; +export type BatchDataLoopOutcome = { results: BatchDataRowResult[]; succeeded: number; failed: number }; /** * The canonical `QueryAST` surface (`spec/data/query.zod.ts`), enumerated. @@ -16693,41 +16658,17 @@ export class ObjectStackProtocolImplementation implements // Batch Operations // ========================================== - async batchData(request: { object: string, request: BatchUpdateRequest, context?: any }): Promise { - const { object, request: batchReq, context } = request; - this.assertObjectRegistered(object); // [#3770] - const { operation, records, options } = batchReq; - - // [#3043 → #14147] The batch endpoint is an external ingress, and it - // used to strip forged read-only columns on create HERE; that strip is - // `engine.insert`'s since the 2026-09-03 ruling, reported back per row - // through `onFieldsDropped`. [#3455] It DOES resolve an execution - // context (threaded by REST); thread it to every engine call so RLS/FLS, - // `readonlyWhen` AND the create-side readonly strip run under the - // caller, so a system caller is correctly exempt (the pre-#3455 code - // hard-coded the strip context to `undefined`, treating every batch - // create as non-system). - const batchSchema = this.engine.registry?.getObject(object); - - // ADR-0119 D4 — `atomic` is REAL or REFUSED, never silent best-effort. - // This flag used to only `break` the loop: every write before the - // failure stayed COMMITTED while the response called itself atomic and - // reported those rows `success: true`. Same class as #4346 — a - // write-path guarantee declared but not enforced, silent and - // destructive exactly when it matters. - // - // Opt-in is an explicit `=== true`. `BatchOptionsSchema` declared - // `.default(true)` while no enforcement site ever delivered atomicity - // (REST forwards the original body, so the parsed default never reached - // this loop), so treating "absent" as atomic would silently flip every - // existing caller's failure semantics. The declaration is aligned to - // the enforced value instead; see the schema's note. - if (options?.atomic === true) { - return await this.runAtomicBatchData({ object, operation, records, options, batchSchema, context }); - } - - const outcome = await this.runBatchDataLoop({ object, operation, records, options, batchSchema, context, atomic: false }); - return this.buildBatchDataResponse(operation, records, options, outcome); + batchData(request: { object: string, request: BatchUpdateRequest, context?: any }): Promise { + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const readEngine = () => this.engine; + const host: ThisParameterType = { + assertObjectRegistered: (...a) => this.assertObjectRegistered(...a), + buildBatchDataResponse: (...a) => this.buildBatchDataResponse(...a), + get engine() { return readEngine(); }, + runAtomicBatchData: (...a) => this.runAtomicBatchData(...a), + runBatchDataLoop: (...a) => this.runBatchDataLoop(...a), + }; + return batchData.call(host, request); } /** @@ -16736,7 +16677,7 @@ export class ObjectStackProtocolImplementation implements * prior write — and the response says so, rather than reporting rows that * no longer exist as successes. */ - private async runAtomicBatchData(args: { + private runAtomicBatchData(args: { object: string; operation: BatchUpdateRequest['operation']; records: BatchUpdateRequest['records']; @@ -16744,14 +16685,14 @@ export class ObjectStackProtocolImplementation implements batchSchema: any; context: any; }): Promise { - const { object, operation, records, options, batchSchema, context } = args; - return await this.runAtomicBatch({ - object, - context, - runLoop: (trxCtx) => this.runBatchDataLoop({ object, operation, records, options, batchSchema, context: trxCtx, atomic: true }), - onCommit: (outcome) => this.buildBatchDataResponse(operation, records, options, outcome), - onRollback: (outcome) => this.buildRolledBackBatchResponse(operation, records, outcome), - }); + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const host: ThisParameterType = { + buildBatchDataResponse: (...a) => this.buildBatchDataResponse(...a), + buildRolledBackBatchResponse: (...a) => this.buildRolledBackBatchResponse(...a), + runAtomicBatch: (...a) => this.runAtomicBatch(...a), + runBatchDataLoop: (...a) => this.runBatchDataLoop(...a), + }; + return runAtomicBatchData.call(host, args); } /** @@ -16772,67 +16713,19 @@ export class ObjectStackProtocolImplementation implements * fail-closed capability gate, the single `engine.transaction()`, the abort * sentinel, the zero-successes rollback response — lives here, once. */ - private async runAtomicBatch(args: { + private runAtomicBatch(args: { object: string; context: any; runLoop: (trxCtx: any) => Promise; onCommit: (outcome: BatchDataLoopOutcome) => BatchUpdateResponse; onRollback: (outcome: BatchDataLoopOutcome) => BatchUpdateResponse; }): Promise { - const { object, context, runLoop, onCommit, onRollback } = args; - - // Two-level probe, shared with the ADR-0119 D2 migration-journal runner - // as `engineCanRollBack` (#4617). `engine.transaction()` runs the - // callback with NO transaction and NO rollback when the default driver - // lacks `beginTransaction` — a declared caveat of the contract member - // (ADR-0119 D1), and one that would turn "atomic" back into a lie - // precisely where it matters. So where the driver registry is - // inspectable, the driver is checked too; where it is not (test - // doubles), the engine-level probe is all there is. - // - // Shared rather than restated: this and the runner's gate were the same - // condition written twice, and two copies of "can this runtime actually - // roll back?" drift by one clause and leave one caller believing it has - // atomicity it does not have. - const engineTx = engineCanRollBack(this.engine) - ? this.engine.transaction.bind(this.engine) - : undefined; - - if (!engineTx) { - // REFUSE, do not degrade. A caller that asked for atomicity is - // exactly the caller who must not silently receive best-effort — - // silent degradation is how this flag came to lie in the first - // place. Mirrors the cross-object /batch route's refusal; the - // condition is generic, so it uses the standard error catalog - // rather than registering an ADR-0112 synonym. - const err: any = new Error( - `Atomic batch on '${object}' requires engine transaction support; this runtime cannot roll back. ` + - `Retry without options.atomic, or probe capabilities.transactionalBatch on /discovery first.`, - ); - err.status = 501; - err.code = 'NOT_IMPLEMENTED'; - throw err; - } - - // Identity-checked sentinel: aborting the transaction is how a rollback - // is requested, but the abort itself is not an error to propagate. - const ABORT = new Error('atomic batch aborted — rolled back'); - let aborted: BatchDataLoopOutcome | undefined; - try { - return await engineTx(async (trxCtx: any) => { - const outcome = await runLoop(trxCtx); - if (outcome.failed > 0) { - aborted = outcome; - throw ABORT; - } - return onCommit(outcome); - }, context); - } catch (err) { - if (err === ABORT && aborted) { - return onRollback(aborted); - } - throw err; - } + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const readEngine = () => this.engine; + const host: ThisParameterType = { + get engine() { return readEngine(); }, + }; + return runAtomicBatch.call(host, args); } /** @@ -16847,7 +16740,7 @@ export class ObjectStackProtocolImplementation implements * fallback insert could only bury a real update failure under the * duplicate-key error of inserting a row just proven to exist. */ - private async runBatchDataLoop(args: { + private runBatchDataLoop(args: { object: string; operation: BatchUpdateRequest['operation']; records: BatchUpdateRequest['records']; @@ -16856,214 +16749,14 @@ export class ObjectStackProtocolImplementation implements context: any; atomic: boolean; }): Promise { - const { object, operation, records, options, batchSchema, context, atomic } = args; - const results: BatchDataRowResult[] = []; - let succeeded = 0; - let failed = 0; - - // Spread form for options objects that already carry `where`/`onFieldsDropped` - // (`{}` spread is a safe no-op). [#14147] EVERY `engine.insert` in this - // loop — `create`, and both arms of `upsert` that create — builds an - // options object: the readonly strip they report moved into - // `engine.insert` and reaches this seam only through `onFieldsDropped`, - // so the arg form (`undefined` when there is no context) is gone from - // here. A create-shaped arm that passed no listener reported a silent - // drop — the upsert arms did exactly that until the patch round. - const ctxOpt = context !== undefined ? { context } : {}; - - // [#4793] `index` is the row's position in the REQUEST array — the - // correlation a caller needs for failure rows that carry no id. - for (const [index, record] of records.entries()) { - try { - switch (operation) { - case 'create': { - // [#3455/#14147] A batch-create caller sees the same - // `droppedFields` a single-write create surfaces (#3431) — - // read off the ENGINE's listener now that the readonly - // create strip lives there, instead of reconstructed from a - // before/after payload diff at this seam. - const rowDropped: DroppedFieldsEvent[] = []; - const created = await this.engine.insert(object, record.data || record, { - ...ctxOpt, - onFieldsDropped: (e: DroppedFieldsEvent) => { rowDropped.push(e); }, - } as any); - omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823] - const ev = mergeDroppedFieldEvents(rowDropped); - results.push({ id: created.id, success: true, data: created, index, ...(ev.length > 0 ? { droppedFields: ev } : {}) }); - succeeded++; - break; - } - case 'update': { - if (!record.id) throw rowRequiredIdError('update'); - // [#5088] Same existence gate as `updateMany` and the - // single-record PATCH — a row naming no record must not - // enter the write pipeline, where #4770's stored ⊕ - // payload merge has no stored side and #4775 blames the - // hook for the resulting gap. - await this.assertRecordExists(object, record.id); - // [#3455] Collect the engine's LEGAL write strips per row. - const dropped: DroppedFieldsEvent[] = []; - const updated = byIdUpdateRecord(await this.engine.update(object, record.data || {}, { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, ...ctxOpt } as any)); - omitInternalFieldsFromWriteResponse(batchSchema, updated); // [#7823] - results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) }); - succeeded++; - break; - } - case 'upsert': { - if (record.id) { - // [#5099] The update-or-insert fork asks EXISTENCE, - // not visibility. `findOne` under the CALLER's - // context is the read RLS/sharing narrows (#3455), - // so an existing row outside the caller's scope - // answered null, took the insert arm, and either - // duplicate-keyed — an authorization/update - // scenario reported as a key collision — or, on a - // store without a unique id constraint, wrote a - // second row. Same probe as the update branch above - // and the single-record path (#4620): it asks the - // database a fact, and whether the caller may WRITE - // the row it proves stays #1994's decision inside - // `engine.update`. - // - // The old fallback (update threw → blind insert) - // is gone with the fork's visibility read: with - // existence decided BEFORE the write, a fallback - // insert could only bury a real update failure - // under the duplicate-key error of inserting a row - // just proven to exist — the same masking ADR-0119 - // D4 forbade inside the atomic arm. - const existing = await this.probeRecord(object, record.id); - if (existing) { - const dropped: DroppedFieldsEvent[] = []; - const updated = byIdUpdateRecord(await this.engine.update(object, record.data || {}, { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, ...ctxOpt } as any)); - omitInternalFieldsFromWriteResponse(batchSchema, updated); // [#7823] - results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) }); - } else { - // [#14147] The upsert's CREATE arm is a create face - // too: same listener, same per-row `droppedFields`, - // as `case 'create'` above and the update arm just up. - const rowDropped: DroppedFieldsEvent[] = []; - const created = await this.engine.insert(object, { id: record.id, ...(record.data || {}) }, { - ...ctxOpt, - onFieldsDropped: (e: DroppedFieldsEvent) => { rowDropped.push(e); }, - } as any); - omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823] - const ev = mergeDroppedFieldEvents(rowDropped); - results.push({ id: created.id, success: true, data: created, index, ...(ev.length > 0 ? { droppedFields: ev } : {}) }); - } - } else { - // [#14147] Same: an id-less upsert row IS a create. - const rowDropped: DroppedFieldsEvent[] = []; - const created = await this.engine.insert(object, record.data || record, { - ...ctxOpt, - onFieldsDropped: (e: DroppedFieldsEvent) => { rowDropped.push(e); }, - } as any); - omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823] - const ev = mergeDroppedFieldEvents(rowDropped); - results.push({ id: created.id, success: true, data: created, index, ...(ev.length > 0 ? { droppedFields: ev } : {}) }); - } - succeeded++; - break; - } - case 'delete': { - if (!record.id) throw rowRequiredIdError('delete'); - // [#5088] `deleteManyData` learned this in #4435; this - // branch — the OTHER by-id bulk delete, ten lines from - // it — kept discarding the driver's return and pushing - // `success: true` unconditionally, so a batch of typo'd - // ids reported every one of them deleted. Same `=== false` - // reading as both fixed faces: the contract's positive - // not-found value (`IDataDriver.delete`), never an - // inference from a falsy return, so a third-party driver - // that answers with the deleted row is not turned into a - // spurious 404. - const deleted = await this.engine.delete(object, { where: { id: record.id }, ...ctxOpt } as any); - if (deleted === false) throw recordNotFoundError(object, record.id); - // [#19433] The SECOND half of this site, and the THIRD and - // last of the by-id delete doors to learn it — the - // single-record face (commit f9e16d856) and `deleteManyData` (#19412) - // both already read the engine's answer. The paragraph - // above fixed "no match"; this is "matched, and - // deliberately NOT removed", where `success` was still a - // LITERAL for every result that was not the contract's - // `false`, so that row was reported as a deletion too. - // - // `sys_permission_set` was the shipped shape of it until C3 - // stage 8 retired that object: deleting a package-declared - // set was an ADR-0005 RESET — the overlay tombstoned and the - // record re-projected to the declared body instead of - // vanishing — so the row MATCHED, the write ran, and the - // record was still there. On a security-configuration - // write this envelope told an operator a permission set was - // gone while it was still being enforced. - // - // The defect rests on the CONTRACT, not on any shipped - // handler: `IDataEngine.delete` declares - // `Promise` — the driver's boolean for a - // by-id write, a COUNT of rows removed otherwise — and - // `isDeleteResultShape` admits the number arm at the - // ADR-0112 hook gate, so an `afterDelete` handler or a - // non-ObjectQL engine may legally answer `0` today. - // - // ⛔ Not the `false` arm: that is spoken for by the 404 - // above, about a record this caller can still GET, and - // answering it here would trade one wrong answer for a - // louder one. Everything else keeps its #4435 reading, an - // off-contract `undefined` from a third-party driver - // included: only a POSITIVE zero is read as "not removed". - // - // ⛔ And the row gets NO `errors[]` entry. A surviving - // record is an OUTCOME, not a fault — the single-record door - // answers the same case with a bare `success: false` on a - // 200 — and the two per-row codes this envelope owns - // (`ROLLED_BACK`, `NOT_ATTEMPTED`) both describe a row that - // never ran. Minting one for this ending is an - // ERROR_CODE_LEDGER widening in `packages/spec`, and belongs - // to the spec lane. - // - // This envelope's own consequences, measured here rather - // than inherited from `deleteManyData`: the row is counted - // in `failed` because `succeeded` and `failed` PARTITION - // `results` (#7539, `reconcileStoppedBatch`, shared by all - // three bulk faces), which makes the request-level `success` - // false and, on the `atomic` arm, aborts the batch through - // `runAtomicBatch`'s `failed > 0` — so an atomic batch - // holding a package-declared set now rolls back instead of - // committing under a response that called every row deleted. - // It does NOT stop a non-atomic run: the `continueOnError` - // stop belongs to the catch below and nothing was thrown, so - // every remaining record is still attempted. `returnRecords: - // false` keeps `success`, so the honest value survives that - // projection too. Only this `case` is touched — the sibling - // arms of this shared loop keep their own counters. - const removed = deleted !== 0; - results.push({ id: record.id, success: removed, index }); - if (removed) succeeded++; - else failed++; - break; - } - default: - results.push({ id: record.id, success: false, index, errors: [{ code: 'VALIDATION_FAILED', message: `Unknown operation: ${operation}`, httpStatus: 400 }] }); - failed++; - } - } catch (err: any) { - // [#8502] `operation` is the request's own verb, so a mixed - // batch's withheld rows each name what THEY were doing. - results.push({ id: record.id, success: false, index, errors: [toRowApiError(err, rowOperationFailureFallback(operation))] }); - failed++; - if (atomic) { - // Abort on the first failure; the caller rolls back. Atomic - // outranks `continueOnError` — there is nothing to continue - // toward when every write so far is about to be undone. - break; - } - if (!options?.continueOnError) { - break; - } - } - } - - return { results, succeeded, failed }; + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const readEngine = () => this.engine; + const host: ThisParameterType = { + assertRecordExists: (...a) => this.assertRecordExists(...a), + get engine() { return readEngine(); }, + probeRecord: (...a) => this.probeRecord(...a), + }; + return runBatchDataLoop.call(host, args); } /** @@ -17109,28 +16802,11 @@ export class ObjectStackProtocolImplementation implements records: ReadonlyArray<{ id?: string }>, outcome: BatchDataLoopOutcome, ): BatchDataLoopOutcome { - if (outcome.results.length >= records.length) return outcome; - - const cause = this.locateBatchCause(outcome.results); - - const results = outcome.results.slice(); - for (let index = results.length; index < records.length; index++) { - results.push({ - // Echoed back so a caller can retry exactly the skipped rows. - id: records[index]?.id, - success: false, - index, - errors: [{ - code: 'NOT_ATTEMPTED' as const, - message: (cause ? `${cause.clause}; the batch stopped there. ` : 'the batch stopped before this record. ') - + 'Set options.continueOnError to process the remaining records.', - }], - }); - } - - // Every padded row is a non-success, so this stays a PARTITION of - // `results` rather than a second tally free to drift from it. - return { results, succeeded: outcome.succeeded, failed: results.length - outcome.succeeded }; + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const host: ThisParameterType = { + locateBatchCause: (...a) => this.locateBatchCause(...a), + }; + return reconcileStoppedBatch.call(host, records, outcome); } /** @@ -17170,12 +16846,8 @@ export class ObjectStackProtocolImplementation implements private locateBatchCause( rows: ReadonlyArray, ): { index: number; clause: string } | undefined { - for (let index = rows.length - 1; index >= 0; index--) { - const fault = rows[index]?.errors?.[0]; - if (fault) return { index, clause: `record ${index} failed — ${fault.message}` }; - } - const stalled = rows.findIndex(r => !r.success); - return stalled >= 0 ? { index: stalled, clause: `record ${stalled} did not succeed` } : undefined; + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + return locateBatchCause(rows); } /** The ordinary (committed) batch response — every row reports what it did. */ @@ -17185,19 +16857,11 @@ export class ObjectStackProtocolImplementation implements options: BatchUpdateRequest['options'], outcome: BatchDataLoopOutcome, ): BatchUpdateResponse { - const { results, succeeded, failed } = this.reconcileStoppedBatch(records, outcome); // [#7539] - return { - success: failed === 0, - operation, - total: records.length, - succeeded, - failed, - // [#3455] `returnRecords: false` drops the record payload (`data`) - // but KEEPS `errors`, `index` and `droppedFields` — the last is a - // small write-observability warning, not the record data the flag - // suppresses. - results: options?.returnRecords !== false ? results : results.map(r => ({ id: r.id, success: r.success, index: r.index, ...(r.errors ? { errors: r.errors } : {}), ...(r.droppedFields ? { droppedFields: r.droppedFields } : {}) })), - } as BatchUpdateResponse; + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const host: ThisParameterType = { + reconcileStoppedBatch: (...a) => this.reconcileStoppedBatch(...a), + }; + return buildBatchDataResponse.call(host, operation, records, options, outcome); } /** @@ -17225,68 +16889,21 @@ export class ObjectStackProtocolImplementation implements records: ReadonlyArray<{ id?: string }>, outcome: BatchDataLoopOutcome, ): BatchUpdateResponse { - const attempted = outcome.results; - // [#19452] Same locator as the stopped-batch arm, for the same reason: - // `!success` stopped meaning "this row failed" when #19412 widened it. - // Both this builder's interpolations of the causal index read it, so - // neither the ROLLED_BACK message nor the NOT_ATTEMPTED one can name a - // row that merely survived — the two could not be fixed apart. - const cause = this.locateBatchCause(attempted); - - const results: BatchDataRowResult[] = records.map((record, index) => { - const attempt = attempted[index]; - if (!attempt) { - return { - id: record.id, success: false, index, - errors: [{ code: 'NOT_ATTEMPTED' as const, message: cause ? `atomic batch aborted by record ${cause.index}` : 'atomic batch aborted' }], - }; - } - if (attempt.success) { - return { - id: attempt.id ?? record.id, success: false, index, - errors: [{ code: 'ROLLED_BACK' as const, message: cause ? cause.clause : 'the atomic batch rolled back' }], - }; - } - return { id: attempt.id ?? record.id, success: false, index, errors: attempt.errors }; - }); - - return { - success: false, - operation, - total: records.length, - succeeded: 0, - failed: records.length, - results, - } as BatchUpdateResponse; + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const host: ThisParameterType = { + locateBatchCause: (...a) => this.locateBatchCause(...a), + }; + return buildRolledBackBatchResponse.call(host, operation, records, outcome); } - async createManyData(request: { object: string, records: any[], context?: any }): Promise { - this.assertObjectRegistered(request.object); // [#3770] - // [#3455/#5503/#14147] Both create-side strips are the engine's now — - // runtime-owned `autonumber` (#5503) and, since #14147, static - // author-declared `readonly` — so ONE listener carries both, and this - // seam no longer diffs payloads to recover a strip it performed itself. - // AGGREGATED: the `{ records, count }` response has no per-row slot, so - // a union is the only representable view here. (`insertManyData` HAS a - // per-row slot and still reports at the top level — the union cannot be - // resolved to rows at either seam; see its own docblock.) - const dropped: DroppedFieldsEvent[] = []; - const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } }; - if (request.context !== undefined) opts.context = request.context; - const records = await this.engine.insert(request.object, request.records, opts); - // [#7823] Bulk create is the same generic-data-path surface as the - // single-record 201 — one strip over the returned rows, at the - // ingress. (Today's `internal`-flagged objects grant no `bulk` - // apiMethod, so this face cannot reach one over REST yet; the strip is - // here so the flag's guarantee does not depend on that staying true.) - omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(request.object), records); - const merged = mergeDroppedFieldEvents(dropped); - return { - object: request.object, - records, - count: records.length, - ...(merged.length > 0 ? { droppedFields: merged } : {}), + createManyData(request: { object: string, records: any[], context?: any }): Promise { + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const readEngine = () => this.engine; + const host: ThisParameterType = { + assertObjectRegistered: (...a) => this.assertObjectRegistered(...a), + get engine() { return readEngine(); }, }; + return createManyData.call(host, request); } /** @@ -17345,72 +16962,26 @@ export class ObjectStackProtocolImplementation implements * channel (an `onFieldsDropped` signature that carries the row), never a * reconstruction at this call site. */ - async insertManyData(request: { object: string, records: any[], context?: any }): Promise<{ object: string; outcomes: Array<{ ok: boolean; record?: any; error?: unknown; droppedFields?: DroppedFieldsEvent[] }>; droppedFields?: DroppedFieldsEvent[] }> { - this.assertObjectRegistered(request.object); // [#3770] - const engineInsertMany = (this.engine as any)?.insertMany; - if (typeof engineInsertMany !== 'function') { - throw new Error('insertManyData requires an engine with insertMany: the partial-success batch insert, which reports an outcome per row so a bad row neither fails the whole batch nor makes the good rows run their beforeInsert hooks twice'); - } - // [#5503/#14147] The engine's events are collected WHOLE and merged — - // the same handling `createManyData` gives them, and for the reason - // spelled out on this method: the union is reportable, a row set is not. - // Keeping each event also keeps its own `reason`, which the old - // flatten-into-a-Set-and-relabel could not. - const dropped: DroppedFieldsEvent[] = []; - const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } }; - if (request.context !== undefined) opts.context = request.context; - // [#20922] Each `ok` outcome carries the ENGINE's per-row report - // (`outcomes[i].droppedFields`), recorded at its strips — a separate - // channel from the batch-level union below, which still names no row. - // Passed through as the engine answers it; ⛔ nothing here derives a - // row's drops from the union. - const outcomes: Array<{ ok: boolean; record?: any; error?: unknown; droppedFields?: DroppedFieldsEvent[] }> = await engineInsertMany.call( - this.engine, - request.object, - request.records, - opts, - ); - // [#7823] Per-outcome ingress strip — the partial-success face hands - // each written row back as `outcomes[i].record`, so each is the same - // generic-data-path surface as a single-record 201 body. - if (Array.isArray(outcomes)) { - const outcomeSchema = this.engine.registry?.getObject(request.object); - for (const o of outcomes) { - if (o?.record) omitInternalFieldsFromWriteResponse(outcomeSchema, o.record); - } - } - const merged = mergeDroppedFieldEvents(dropped); - return { - object: request.object, - outcomes, - ...(merged.length > 0 ? { droppedFields: merged } : {}), + insertManyData(request: { object: string, records: any[], context?: any }): Promise<{ object: string; outcomes: Array<{ ok: boolean; record?: any; error?: unknown; droppedFields?: DroppedFieldsEvent[] }>; droppedFields?: DroppedFieldsEvent[] }> { + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const readEngine = () => this.engine; + const host: ThisParameterType = { + assertObjectRegistered: (...a) => this.assertObjectRegistered(...a), + get engine() { return readEngine(); }, }; + return insertManyData.call(host, request); } - async updateManyData(request: UpdateManyDataRequest & { context?: any }): Promise { - const { object, records, options, context } = request; - this.assertObjectRegistered(object); // [#3770] - - // [#4620] `atomic` used not to appear ANYWHERE in this method: the - // option was accepted, declared in `BatchOptionsSchema` with a contract - // that promises all-or-nothing, and never read — a caller asking for - // atomicity silently got best-effort with no signal at all. Same - // enforcement shape as `batchData` (ADR-0119 D4), same runner, so the - // three bulk-write surfaces cannot drift apart again. `=== true` is the - // deliberate opt-in from D4: absent/false keeps today's semantics - // exactly. - if (options?.atomic === true) { - return await this.runAtomicBatch({ - object, - context, - runLoop: (trxCtx) => this.runUpdateManyLoop({ object, records, options, context: trxCtx, atomic: true }), - onCommit: (outcome) => this.buildUpdateManyResponse(records, outcome), - onRollback: (outcome) => this.buildRolledBackBatchResponse('update', records, outcome), - }); - } - - const outcome = await this.runUpdateManyLoop({ object, records, options, context, atomic: false }); - return this.buildUpdateManyResponse(records, outcome); + updateManyData(request: UpdateManyDataRequest & { context?: any }): Promise { + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const host: ThisParameterType = { + assertObjectRegistered: (...a) => this.assertObjectRegistered(...a), + buildRolledBackBatchResponse: (...a) => this.buildRolledBackBatchResponse(...a), + buildUpdateManyResponse: (...a) => this.buildUpdateManyResponse(...a), + runAtomicBatch: (...a) => this.runAtomicBatch(...a), + runUpdateManyLoop: (...a) => this.runUpdateManyLoop(...a), + }; + return updateManyData.call(host, request); } /** @@ -17421,71 +16992,20 @@ export class ObjectStackProtocolImplementation implements * `atomic=false`, and which has nothing to continue toward when every write * so far is about to be undone. */ - private async runUpdateManyLoop(args: { + private runUpdateManyLoop(args: { object: string; records: UpdateManyDataRequest['records']; options: UpdateManyDataRequest['options']; context: any; atomic: boolean; }): Promise { - const { object, records, options, context, atomic } = args; - const results: BatchDataRowResult[] = []; - let succeeded = 0; - let failed = 0; - // [#7823] Ingress strip over each row's `data` payload — the bulk - // update face is the same generic-data-path surface as the by-id - // PATCH body, one row at a time. Resolved once; the loop reuses it. - const updateManySchema = this.engine.registry?.getObject(object); - - for (const [index, record] of records.entries()) { - try { - // [#3455] Two gaps the pre-#3455 loop had, both fixed per row: - // 1. `context` was never threaded — bulk updates ran the engine - // context-less, so RLS/FLS and `readonlyWhen` evaluated without - // the caller's principal. Thread it like single-write updateData. - // 2. `onFieldsDropped` was never wired — the same static `readonly` - // (#2948) / `readonlyWhen` (#3042) strips that single-write now - // surfaces (#3431) happened silently here. Collect per row. - // - // [#5100] Same guard as `runBatchDataLoop`'s update branch - // (#4793), same classification: an id-less row is a CALLER - // error — VALIDATION_FAILED/400 — not a data-state one. The - // REST entrance already rejects it (`UpdateManyRecordSchema` - // requires `id`, #3939), but that invariant lives two packages - // away; unguarded, an in-process caller's malformed row - // reached the probe and the write as `{ id: undefined }`, - // whose reading is up to each driver's undefined-where-key - // handling — at best a 404 with `undefined` interpolated into - // the message, at worst a where-clause with no id at all. - if (!record.id) throw rowRequiredIdError('update'); - // [#5088] Third gap, the same shape: no existence gate. A row - // naming no record went straight into `engine.update`, so the - // hook pipeline ran over a payload-only record and the row came - // back `INTERNAL_ERROR` from #4775's condition abort — blaming - // the hook for a caller's stale id. Probe first, per row, so - // this face answers what the single-record PATCH answers. - await this.assertRecordExists(object, record.id); - const dropped: DroppedFieldsEvent[] = []; - const opts: any = { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } }; - if (context !== undefined) opts.context = context; - const updated = byIdUpdateRecord(await this.engine.update(object, record.data || {}, opts)); - omitInternalFieldsFromWriteResponse(updateManySchema, updated); // [#7823] - results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) }); - succeeded++; - } catch (err: any) { - results.push({ id: record.id, success: false, index, errors: [toRowApiError(err, rowOperationFailureFallback('update'))] }); - failed++; - if (atomic) { - // Abort on the first failure; the caller rolls back. - break; - } - if (!options?.continueOnError) { - break; - } - } - } - - return { results, succeeded, failed }; + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const readEngine = () => this.engine; + const host: ThisParameterType = { + assertRecordExists: (...a) => this.assertRecordExists(...a), + get engine() { return readEngine(); }, + }; + return runUpdateManyLoop.call(host, args); } /** @@ -17498,19 +17018,11 @@ export class ObjectStackProtocolImplementation implements records: UpdateManyDataRequest['records'], outcome: BatchDataLoopOutcome, ): BatchUpdateResponse { - // [#7539] Same under-report as `batchData`, ten lines away — the card's - // "per-object bulk counters under-report whenever a row fails without - // `continueOnError`". Fixed through the one shared reconciler, because - // a second copy is how these three drifted apart before (#4620). - const { results, succeeded, failed } = this.reconcileStoppedBatch(records, outcome); - return { - success: failed === 0, - operation: 'update', - total: records.length, - succeeded, - failed, - results, - } as BatchUpdateResponse; + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const host: ThisParameterType = { + reconcileStoppedBatch: (...a) => this.reconcileStoppedBatch(...a), + }; + return buildUpdateManyResponse.call(host, records, outcome); } // `analyticsQuery` / `getAnalyticsMeta` were retired with the degraded @@ -17571,153 +17083,44 @@ export class ObjectStackProtocolImplementation implements * delivered for real now, through the shared {@link runAtomicBatch}, and * refused (501 `NOT_IMPLEMENTED`) on a runtime that cannot roll back. */ - async deleteManyData(request: DeleteManyDataRequest & { context?: any }): Promise { - const { object, options, context } = request; - this.assertObjectRegistered(object); // [#3770] - - // Fail CLOSED on anything that is not a list of scalar ids. A non-scalar - // entry (`{"ids":[{"$ne":null}]}`) must never reach `where.id` as an - // operator object — that is the same "predicate widening" this endpoint - // was just hardened against, one layer down. - const isScalarId = (v: unknown) => - (typeof v === 'string' && v.length > 0) || typeof v === 'number' || typeof v === 'bigint'; - const ids = request.ids as unknown; - if (!Array.isArray(ids) || ids.some((id) => !isScalarId(id))) { - const err: any = new Error( - `deleteMany on '${object}' requires 'ids' to be an array of record ids`, - ); - err.code = 'VALIDATION_FAILED'; - err.status = 400; - throw err; - } - - // [#4620] `atomic` here was the same fake-atomic `batchData` carried - // before ADR-0119 D4 — it only `break`-ed the loop, so every row deleted - // before the failure stayed DELETED while the response called itself - // atomic. Worse than the `batchData` case, because a partial delete has - // no natural undo: the caller cannot reconstruct the rows from the - // request. Same runner, same fail-closed gate, same row marking. - if (options?.atomic === true) { - const rows = ids.map((id) => ({ id: String(id) })); - return await this.runAtomicBatch({ - object, - context, - runLoop: (trxCtx) => this.runDeleteManyLoop({ object, ids, options, context: trxCtx, atomic: true }), - onCommit: (outcome) => this.buildDeleteManyResponse(ids, outcome), - onRollback: (outcome) => this.buildRolledBackBatchResponse('delete', rows, outcome), - }); - } - - const outcome = await this.runDeleteManyLoop({ object, ids, options, context, atomic: false }); - return this.buildDeleteManyResponse(ids, outcome); + deleteManyData(request: DeleteManyDataRequest & { context?: any }): Promise { + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const host: ThisParameterType = { + assertObjectRegistered: (...a) => this.assertObjectRegistered(...a), + buildDeleteManyResponse: (...a) => this.buildDeleteManyResponse(...a), + buildRolledBackBatchResponse: (...a) => this.buildRolledBackBatchResponse(...a), + runAtomicBatch: (...a) => this.runAtomicBatch(...a), + runDeleteManyLoop: (...a) => this.runDeleteManyLoop(...a), + }; + return deleteManyData.call(host, request); } /** * The per-id loop of {@link deleteManyData}, shared by both arms (#4620) — * see {@link runUpdateManyLoop} for why `atomic` outranks `continueOnError`. */ - private async runDeleteManyLoop(args: { + private runDeleteManyLoop(args: { object: string; ids: unknown[]; options: DeleteManyDataRequest['options']; context: any; atomic: boolean; }): Promise { - const { object, ids, options, context, atomic } = args; - const results: BatchDataRowResult[] = []; - let succeeded = 0; - let failed = 0; - const ctxOpt = context !== undefined ? { context } : {}; - - for (const [index, id] of ids.entries()) { - try { - // [#4435] Per-row honesty on the bulk path. This discarded the - // driver's return and pushed `success: true` unconditionally, so - // `{"ids":["nonexistent_1"]}` answered `succeeded: 1` — a batch - // of typo'd ids reported every one of them deleted. A caller - // reconciling "which of my 200 ids were real" got a list that - // agreed with whatever it sent. Same `=== false` reading as the - // single-record path: the contract's positive not-found value, - // never an inference from a falsy return. - const deleted = await this.engine.delete(object, { where: { id }, ...ctxOpt } as any); - // `id` is `unknown` to this helper only because the caller's - // fail-closed `isScalarId` guard is what proves it scalar. - if (deleted === false) throw recordNotFoundError(object, id as string | number); - // [#19412] The SECOND half of the same site. The paragraph above - // fixed "no match"; this is "matched, and deliberately NOT - // removed" — and until now `success` was still a LITERAL for - // every answer that was not the contract's `false`, so that row - // was reported as a deletion too. - // - // `sys_permission_set` was the shipped case until C3 stage 8 - // retired that object: a package-declared set's delete was an - // ADR-0005 RESET — plugin-security's write-through tombstoned the - // overlay and the record re-projected to the declared body - // instead of vanishing — so the row MATCHED, the write ran, and - // the record was still there. The - // batch envelope said `success: true` and counted it in - // `succeeded`, which on a security-configuration write tells an - // operator a permission set is gone while it is still enforced. - // - // `IDataEngine.delete` declares `Promise` — - // the driver's boolean for a by-id write, a COUNT of rows - // removed otherwise — so a numeric zero is the one value that - // positively means "it is still there". ⛔ Not `false`: that - // arm is spoken for by the 404 above, about a record this caller - // can still GET, and answering it here would trade one wrong - // answer for a louder one. Everything else keeps its #4435 - // reading, an off-contract `undefined` from a third-party driver - // included: only a POSITIVE zero is read as "not removed". - // - // ⛔ The row is NOT given an `errors[]` entry. A surviving record - // is an OUTCOME, not a fault — the single-record door answers the - // same case with a bare `success: false` on a 200 — and the two - // per-row codes this envelope owns (`ROLLED_BACK`, - // `NOT_ATTEMPTED`) both describe a row that never ran. Whether - // this ending deserves a per-row code of its own is a - // `packages/spec` widening (ERROR_CODE_LEDGER) and belongs to the - // spec lane, not here. - // - // It is counted in `failed` because that is the envelope's ONE - // declared reading — `succeeded` and `failed` PARTITION `results` - // (#7539, `reconcileStoppedBatch`) — which also makes the - // request-level `success` false and, on the `atomic` arm, aborts - // the batch through `runAtomicBatch`'s `failed > 0`. It does NOT - // stop a non-atomic run: the `continueOnError` stop belongs to - // the catch below, and nothing was thrown. - const removed = deleted !== 0; - results.push({ id: String(id), success: removed, index }); - if (removed) succeeded++; - else failed++; - } catch (err: any) { - results.push({ id: String(id), success: false, index, errors: [toRowApiError(err, rowOperationFailureFallback('delete'))] }); - failed++; - // Same stop semantics as `batchData`: `atomic` aborts the rest on - // the first failure (the caller rolls back), and without - // `continueOnError` a failure ends the run rather than silently - // ploughing on. - if (atomic) break; - if (!options?.continueOnError) break; - } - } - - return { results, succeeded, failed }; + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const readEngine = () => this.engine; + const host: ThisParameterType = { + get engine() { return readEngine(); }, + }; + return runDeleteManyLoop.call(host, args); } /** The ordinary (committed) `deleteMany` response — every id reports what it did. */ private buildDeleteManyResponse(ids: unknown[], outcome: BatchDataLoopOutcome): BatchUpdateResponse { - // [#7539] Same reconciliation as the other two faces; `ids` are mapped - // to the `{ id }` row shape the atomic arm already hands the rollback - // builder, so a skipped id comes back echoed and retryable. - const { results, succeeded, failed } = this.reconcileStoppedBatch(ids.map((id) => ({ id: String(id) })), outcome); - return { - success: failed === 0, - operation: 'delete', - total: ids.length, - succeeded, - failed, - results, - } as BatchUpdateResponse; + // [#25467 cut 1] The body moved to `protocol-batch-data.ts` (see `BatchDataHost`). + const host: ThisParameterType = { + reconcileStoppedBatch: (...a) => this.reconcileStoppedBatch(...a), + }; + return buildDeleteManyResponse.call(host, ids, outcome); } /** From 5fce393705e028cd3cb3d8831dbce300b5b45fc1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 15:26:56 +0000 Subject: [PATCH 2/2] chore(metadata-protocol): point the ADR-0119 anchor and the bracketed-refusal scan at protocol-batch-data.ts The batchData atomic path the protocol.ts ADR anchor entry's ADR-0119 paragraph describes now lives in protocol-batch-data.ts, so that module gets its own entry carrying the paragraph verbatim (the protocol.ts entry is unchanged; ADR-0119 is still cited there). The bracketed-refusal opener scan follows the moved throw sites with a per-file floor. Claude-Session: https://claude.ai/code/session_01J8pUyVs5eas1AjMdReJM7C Co-authored-by: Claude --- .../src/protocol.bracketed-refusal-opener-absence.test.ts | 4 ++++ ...es__metadata-protocol__src__protocol-batch-data.ts.json | 7 +++++++ 2 files changed, 11 insertions(+) create mode 100644 scripts/adr-anchors/packages__metadata-protocol__src__protocol-batch-data.ts.json diff --git a/packages/metadata-protocol/src/protocol.bracketed-refusal-opener-absence.test.ts b/packages/metadata-protocol/src/protocol.bracketed-refusal-opener-absence.test.ts index e6d23feb270..ce15397efad 100644 --- a/packages/metadata-protocol/src/protocol.bracketed-refusal-opener-absence.test.ts +++ b/packages/metadata-protocol/src/protocol.bracketed-refusal-opener-absence.test.ts @@ -71,6 +71,10 @@ const HERE = dirname(fileURLToPath(import.meta.url)); */ const PRODUCERS: Readonly> = { 'protocol.ts': 5, + // The bulk data writes, moved out of `protocol.ts` whole: their throw sites + // are still this family's, so the scan follows them. The floor is the + // module's own `new Error(` count when it was split out. + 'protocol-batch-data.ts': 4, 'sys-metadata-repository.ts': 5, 'runtime-authoring-gate.ts': 1, }; diff --git a/scripts/adr-anchors/packages__metadata-protocol__src__protocol-batch-data.ts.json b/scripts/adr-anchors/packages__metadata-protocol__src__protocol-batch-data.ts.json new file mode 100644 index 00000000000..ce8d767b013 --- /dev/null +++ b/scripts/adr-anchors/packages__metadata-protocol__src__protocol-batch-data.ts.json @@ -0,0 +1,7 @@ +{ + "file": "packages/metadata-protocol/src/protocol-batch-data.ts", + "adrs": [ + "ADR-0119" + ], + "invariant": "ADR-0119 — `batchData`'s `atomic` is REAL or REFUSED, never silent best-effort. An explicitly atomic batch runs inside ONE `engine.transaction()`, so a failure rolls back every prior write and the response reports zero successes (rows marked ROLLED_BACK / NOT_ATTEMPTED — reporting a rolled-back row as `success: true` was the original bug, not merely the missing transaction). A runtime that cannot roll back gets 501 NOT_IMPLEMENTED; degrading to best-effort is how the flag came to lie." +}