From bb81a312dee0df89db0fb091b8c5a199a7a91118 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 02:41:37 +0000 Subject: [PATCH 01/10] =?UTF-8?q?wip(engine):=20narrow=20findOne/update/de?= =?UTF-8?q?lete=20result=20declarations=20=E2=80=94=20census=20measurement?= =?UTF-8?q?=20leg?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- packages/objectql/src/engine.ts | 6 +++--- packages/spec/src/contracts/data-engine.ts | 6 +++--- packages/spec/src/contracts/scoped-context.ts | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 1cead9fce9..4171186960 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -9702,7 +9702,7 @@ export class ObjectQL implements IObjectQLEngine { * * Fires the same `beforeFind`/`afterFind` hooks as `find` (#3195). */ - async findOne(objectName: string, query?: EngineQueryOptions, options?: EngineReadOptions): Promise { + async findOne(objectName: string, query?: EngineQueryOptions, options?: EngineReadOptions): Promise | null> { objectName = this.resolveObjectName(objectName); // Same alias fold as find() (#4346). Without it, `findOne({ filter })` // matched the first row of the WHOLE table rather than the predicate. @@ -10868,7 +10868,7 @@ export class ObjectQL implements IObjectQLEngine { * `catch` also sees the `afterUpdate` dispatch and the roll-up recompute, and * a violation raised by a nested driver call in there is not this object's. */ - async update(object: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise { + async update(object: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise | number | null> { object = this.resolveObjectName(object); this.logger.debug('Update operation starting', { object }); this.assertWriteAllowed(object, 'update'); @@ -13086,7 +13086,7 @@ export class ObjectQL implements IObjectQLEngine { } } - async delete(object: string, options?: EngineDeleteOptions): Promise { + async delete(object: string, options?: EngineDeleteOptions): Promise { object = this.resolveObjectName(object); this.logger.debug('Delete operation starting', { object }); this.assertWriteAllowed(object, 'delete'); diff --git a/packages/spec/src/contracts/data-engine.ts b/packages/spec/src/contracts/data-engine.ts index 991aa2038e..70d8180928 100644 --- a/packages/spec/src/contracts/data-engine.ts +++ b/packages/spec/src/contracts/data-engine.ts @@ -273,10 +273,10 @@ export interface IDataEngine { * * [#6300] `query` is the author state (`z.input`), same as `find` above. */ - findOne(objectName: string, query?: EngineQueryOptions, options?: BaseEngineOptions): Promise; + findOne(objectName: string, query?: EngineQueryOptions, options?: BaseEngineOptions): Promise | null>; insert(objectName: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise; - update(objectName: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise; - delete(objectName: string, options?: EngineDeleteOptions): Promise; + update(objectName: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise | number | null>; + delete(objectName: string, options?: EngineDeleteOptions): Promise; count(objectName: string, query?: EngineCountOptions, options?: BaseEngineOptions): Promise; aggregate(objectName: string, query: EngineAggregateOptions, options?: BaseEngineOptions): Promise; diff --git a/packages/spec/src/contracts/scoped-context.ts b/packages/spec/src/contracts/scoped-context.ts index 5c1878edc0..a758e5493f 100644 --- a/packages/spec/src/contracts/scoped-context.ts +++ b/packages/spec/src/contracts/scoped-context.ts @@ -145,7 +145,7 @@ export interface IScopedObjectRepository { * nothing to do with what was asked, which no `if (!row)` can catch. When * any row genuinely will do, that is `find({ limit: 1 })`, which says so. */ - findOne(query?: Record): Promise; + findOne(query?: Record): Promise | null>; /** Count the records the query selects. */ count(query?: Record): Promise; @@ -161,7 +161,7 @@ export interface IScopedObjectRepository { * key out of the payload. The bulk form is `update(data, { where, multi: true })`; * there is no `updateMany`. */ - update(data: any, options?: Record): Promise; + update(data: any, options?: Record): Promise | number | null>; /** Update a single record by id — the id travels as the first argument. */ updateById(id: string | number, data: any): Promise; From 6f059312fc36316cf585ff161982501eb575521d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 03:04:04 +0000 Subject: [PATCH 02/10] feat(engine): declare findOne/update/delete result shapes and guard their hook seams Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- packages/metadata-protocol/src/protocol.ts | 44 ++- .../metadata/src/loaders/database-loader.ts | 11 +- packages/objectql/src/engine.ts | 70 +++- .../objectql/src/find-hook-result-shape.ts | 12 +- packages/objectql/src/index.ts | 21 ++ .../objectql/src/verb-hook-result-shape.ts | 340 ++++++++++++++++++ .../plugin-auth/src/objectql-adapter.ts | 14 +- .../spec/src/api/error-code-ledger.zod.ts | 42 ++- packages/spec/src/contracts/data-engine.ts | 4 +- packages/spec/src/contracts/scoped-context.ts | 9 +- 10 files changed, 543 insertions(+), 24 deletions(-) create mode 100644 packages/objectql/src/verb-hook-result-shape.ts diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 456a24e53f..b82ae107e7 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -869,6 +869,42 @@ export function zodIssuesToMetadataIssues(issues: unknown): MetadataIssueEntry[] */ export { recordNotFoundError }; +/** + * The RECORD limb of `IDataEngine.update`'s declared result, for the four + * ingresses in this file whose call is BY-ID by construction (#16231). + * + * `update()` used to declare `Promise` and now declares the union its two + * dispatch paths actually answer: a record (or `null`) from the by-id exit + * (`driver.update`), or the affected-row COUNT from the predicate exit + * (`driver.updateMany`, #4639). Every call site below hands the engine a + * `where` naming exactly one primary key — or folds the row's id into the + * payload — with no `multi`, which `resolveEngineUpdateDispatch` resolves + * `by-id`. So the count limb is unreachable from here, and it is REFUSED + * loudly rather than cast away: a dispatch change that started routing these + * calls through `updateMany` would otherwise drop an affected count into a + * per-row receipt's `record` / `data` slot, where every consumer reads it as a + * row. + * + * The `null` limb is NOT refused and NOT narrowed away — it is passed through + * exactly as it was while the declaration said `any`. `update()`'s by-id exit + * answers `null` when the post-write readback leaves the caller's row scope + * (see `updateData`'s own note on that), while `DataProtocol`'s row receipts + * declare `record` / `data` non-null. That disagreement pre-dates this change, + * 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 { + if (typeof result === 'number') { + throw new Error( + `A by-id update resolved an affected-row count (${result}) instead of a record. ` + + `This ingress addresses exactly one row, so the engine's predicate dispatch ` + + `(driver.updateMany) is not reachable from it — the dispatch ladder or this ` + + `call site changed.`, + ); + } + return result as Record; +} + /** * A 400 for a `$filter` ARRAY that looks like a filter AST but is not one. * @@ -11006,7 +11042,7 @@ export class ObjectStackProtocolImplementation implements ) ? { ...(request.data as Record), id: request.id } : request.data; - const result = await this.engine.update(request.object, writeData, opts); + const result = byIdUpdateRecord(await this.engine.update(request.object, writeData, opts)); // [#7823] The PATCH 200 body is the surface #7728's fourth measurement // caught: a client revoking a `sys_api_key` (apiMethods keeps `update` // open, #7727) got the stored hash back in this response. That closure @@ -12042,7 +12078,7 @@ export class ObjectStackProtocolImplementation implements await this.assertRecordExists(object, record.id); // [#3455] Collect the engine's LEGAL write strips per row. const dropped: DroppedFieldsEvent[] = []; - const updated = await this.engine.update(object, record.data || {}, { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, ...ctxOpt } as any); + 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++; @@ -12074,7 +12110,7 @@ export class ObjectStackProtocolImplementation implements const existing = await this.probeRecord(object, record.id); if (existing) { const dropped: DroppedFieldsEvent[] = []; - const updated = await this.engine.update(object, record.data || {}, { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, ...ctxOpt } as any); + 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 { @@ -12457,7 +12493,7 @@ export class ObjectStackProtocolImplementation implements const dropped: DroppedFieldsEvent[] = []; const opts: any = { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } }; if (context !== undefined) opts.context = context; - const updated = await this.engine.update(object, record.data || {}, opts); + 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++; diff --git a/packages/metadata/src/loaders/database-loader.ts b/packages/metadata/src/loaders/database-loader.ts index ab705f3c85..fd21213168 100644 --- a/packages/metadata/src/loaders/database-loader.ts +++ b/packages/metadata/src/loaders/database-loader.ts @@ -410,7 +410,16 @@ export class DatabaseLoader implements MetadataLoader { // nothing to compare against: a `data.id` spread over the id parameter // would retarget the write to a row no caller resolved. The separate // `id` parameter is the row address — do not let a payload outrank it. - return this.engine.update(table, { ...data, id }); + // [#16231] `IDataEngine.update` now declares its dispatch union + // (`record | affected-count | null`). This call passes NO `where`, so + // the payload id is the only address the dispatch ladder sees and it + // resolves `by-id` — the limb that answers a record or `null`. The + // `number` limb is the predicate path (`driver.updateMany`'s affected + // count, #4639), which this call cannot reach; it is narrowed away here + // rather than cast, so a future dispatch change surfaces at THIS line + // instead of as a wrong-shaped row at the caller. + const updated = await this.engine.update(table, { ...data, id }); + return typeof updated === 'number' ? null : updated; } return this.driver!.update(table, id, data); } diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 4171186960..d2d5f8251a 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -93,6 +93,14 @@ import { import { ReadonlyFieldRejectedError } from './readonly-strict-errors.js'; import { HookTargetRebindError } from './hook-target-rebind-errors.js'; import { FindHookResultNotArrayError } from './find-hook-result-shape.js'; +import { + isFindOneResultShape, + isUpdateResultShape, + isDeleteResultShape, + FindOneHookResultNotRecordError, + UpdateHookResultNotWriteShapeError, + DeleteHookResultNotWriteShapeError, +} from './verb-hook-result-shape.js'; import { DriverConnectError, DatasourceUnavailableError, @@ -9702,7 +9710,7 @@ export class ObjectQL implements IObjectQLEngine { * * Fires the same `beforeFind`/`afterFind` hooks as `find` (#3195). */ - async findOne(objectName: string, query?: EngineQueryOptions, options?: EngineReadOptions): Promise | null> { + async findOne(objectName: string, query?: EngineQueryOptions, options?: EngineReadOptions): Promise | null> { objectName = this.resolveObjectName(objectName); // Same alias fold as find() (#4346). Without it, `findOne({ filter })` // matched the first row of the WHOLE table rather than the predicate. @@ -9808,6 +9816,27 @@ export class ObjectQL implements IObjectQLEngine { hookContext.result = result; await this.triggerHooks('afterFind', hookContext); + // [#16231] `findOne()` now DECLARES what it answers — the one record the + // query selects, or `null` — so the seam that can break the declaration + // is closed here, on the terms #15823 set for `find()`. An `afterFind` + // handler may SHAPE the record (mutate it, drop keys, assign a different + // RECORD built from it); replacing it with something that is neither a + // record nor `null` is a hook-contract violation, refused loudly rather + // than returned as a value the caller's type says cannot occur. + // + // ⛔ Placement is load-bearing and not the `return`, exactly as on + // `find()`: `maskSecretFields` and `stripSearchCompanionFromRead` below + // both run on `hookContext.result`, so the check has to precede them — + // otherwise the first consumer to walk a replaced value is the one that + // reports the problem, from the wrong place. + if (!isFindOneResultShape(hookContext.result)) { + throw new FindOneHookResultNotRecordError({ + object: objectName, + event: 'afterFind', + result: hookContext.result, + }); + } + // Mask secret fields — plaintext never leaves through the read path. this.maskSecretFields(objectName, hookContext.result); // [#7642] Hidden `__search` companion — same door, same rule as `find`. @@ -10868,7 +10897,7 @@ export class ObjectQL implements IObjectQLEngine { * `catch` also sees the `afterUpdate` dispatch and the roll-up recompute, and * a violation raised by a nested driver call in there is not this object's. */ - async update(object: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise | number | null> { + async update(object: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise | number | null> { object = this.resolveObjectName(object); this.logger.debug('Update operation starting', { object }); this.assertWriteAllowed(object, 'update'); @@ -12073,6 +12102,28 @@ export class ObjectQL implements IObjectQLEngine { await this.triggerHooks('afterUpdate', hookContext); } + // [#16231] The `update()` twin of `find()`'s #15823 refusal, now + // that this verb declares the union its two dispatch paths answer: + // a post-write record (or `null`), or the affected-row COUNT a + // predicate write resolves. An `afterUpdate` handler may SHAPE what + // it is handed; replacing it with a shape outside the declaration + // is refused. + // + // ⛔ Ahead of `stripSearchCompanion` and the realtime publish for + // the same reason `find()`'s guard precedes its consumers: both + // read this value already assuming it is a record or a non-object + // they may skip, so a replaced container would be diagnosed from + // whichever of them tripped over it first. AFTER the per-row + // `afterUpdate` fan-out (#5038), because the batch context is the + // one this call returns and a per-row handler can reassign it. + if (!isUpdateResultShape(hookContext.result)) { + throw new UpdateHookResultNotWriteShapeError({ + object, + event: 'afterUpdate', + result: hookContext.result, + }); + } + // Roll-up: recompute parent summaries; pass priorRecord too so a child // that moved to a different parent updates BOTH old and new parent. const summaryFailures = await this.recomputeSummaries(object, result, priorRecord, opCtx.context); @@ -13531,6 +13582,21 @@ export class ObjectQL implements IObjectQLEngine { await this.triggerHooks('afterDelete', hookContext); } + // [#16231] The `delete()` twin, on its own declaration: whether the + // by-id row was there (`boolean`), or how many rows a predicate + // delete removed (`number`). ⚠️ `false` and `0` are the two most + // ordinary answers this verb gives, so `isDeleteResultShape` is a + // pair of `typeof` tests and never a truthiness check — a lenient + // guard here would refuse exactly the answers + // `metadata-protocol`'s `deleteData` turns into its 404. + if (!isDeleteResultShape(hookContext.result)) { + throw new DeleteHookResultNotWriteShapeError({ + object, + event: 'afterDelete', + result: hookContext.result, + }); + } + // Roll-up: recompute the parent summary now that the child is gone, // from the row's FK values captured BEFORE deletion. [#5272] That // capture is now the same single pre-image read `previous` rides on diff --git a/packages/objectql/src/find-hook-result-shape.ts b/packages/objectql/src/find-hook-result-shape.ts index ede0117732..89c7f11fcc 100644 --- a/packages/objectql/src/find-hook-result-shape.ts +++ b/packages/objectql/src/find-hook-result-shape.ts @@ -15,11 +15,13 @@ * diagnostic, no log. Measured on a real engine over a real SQL driver — * `ARRAY(len=1)` with no hooks, `OBJECT{records}` with the handler. * - * Of the four `return hookContext.result` sites in `engine.ts` this is the only - * one with a concrete declared shape to violate; `findOne`, `update` and - * `delete` all declare `Promise` and carry no enforceable declaration at - * all. That is a separate question about those declarations and is deliberately - * NOT answered here. + * Of the four `return hookContext.result` sites in `engine.ts` this was, when + * #15823 landed, the only one with a concrete declared shape to violate: + * `findOne`, `update` and `delete` all declared `Promise` and carried no + * enforceable declaration at all. #15823 fenced that out as a separate question + * about those declarations; #16231 answered it (maintainer ruling, option A, + * 2026-09-07). All four verbs declare now, and the other three seams are closed + * on these terms in `verb-hook-result-shape.ts`. * * ## Why a refusal rather than a wider declaration * diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 13f463116b..f5646c28d1 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -200,6 +200,27 @@ export { FIND_HOOK_RESULT_NOT_ARRAY_STATUS, describeFindHookResult, } from './find-hook-result-shape.js'; +// [#16231] The other three seams, closed on the same terms once their verbs +// had declarations worth guarding (maintainer ruling, option A, 2026-09-07). +// Exported for the same reason: the remedy belongs to the HOOK'S AUTHOR, who +// branches on the code. The three shape predicates ride along because a host +// that installs `after*` handlers can use them to check its own answer before +// the engine does — which is cheaper than reading the refusal. +export { + FindOneHookResultNotRecordError, + FIND_ONE_HOOK_RESULT_NOT_RECORD_CODE, + FIND_ONE_HOOK_RESULT_NOT_RECORD_STATUS, + isFindOneResultShape, + UpdateHookResultNotWriteShapeError, + UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE, + UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_STATUS, + isUpdateResultShape, + DeleteHookResultNotWriteShapeError, + DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE, + DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_STATUS, + isDeleteResultShape, + describeHookResult, +} from './verb-hook-result-shape.js'; // [#14010] `Hook.runAs` — the declared execution identity of a hook's `ctx.api` // data operations. The refusal a `runAs: 'user'` hook raises when its trigger // resolved no user (ADR-0112 code + status), the api that raises it, and the diff --git a/packages/objectql/src/verb-hook-result-shape.ts b/packages/objectql/src/verb-hook-result-shape.ts new file mode 100644 index 0000000000..eb59c5c9e8 --- /dev/null +++ b/packages/objectql/src/verb-hook-result-shape.ts @@ -0,0 +1,340 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#16231] The other three `return hookContext.result` seams — `findOne`, + * `update` and `delete` — closed on the same terms `find()`'s was, now that + * each of them has a declaration worth guarding. + * + * ## Why this module could not have been written before + * + * `engine.ts` has FOUR `return hookContext.result` sites, one per hook-bearing + * verb. #15823 closed the `find()` one, and its own module note recorded why it + * could only close that one: `find()` declared `Promise` — a concrete + * container to violate — while `findOne`, `update` and `delete` all declared + * `Promise` and so carried nothing an `after*` handler could break. + * + * A guard cannot exist before a declaration worth guarding does. That is the + * whole framing of #16231, and the maintainer ruled it (option A, 2026-09-07, + * director seat summon #17, decision batch #2): the three declarations move off + * `any` onto what the verbs actually answer, and each seam is then guarded + * exactly as `find()`'s is. Options B (declare only, no enforcement) and C + * (record `any` as intended) were refused. + * + * ## What each verb declares, and where the shape comes from + * + * Not invented here — read off the driver contract each engine exit delegates + * to (`IDataDriver`, `packages/spec/src/contracts/data-driver.ts`) and off the + * dispatch ladder that chooses between them + * (`resolveEngineUpdateDispatch` / `resolveEngineDeleteDispatch`): + * + * findOne → `Record | null` + * ONE exit: `driver.findOne`, declared + * `Promise | null>`. The engine's own + * docblock has said the same in prose since #4419 — "Read the ONE + * record the query selects, or `null`" — and callers already + * branch on `if (!row)`. + * + * update → `Record | number | null` + * TWO exits. By-id (`driver.update`, + * `Promise | null>`) answers the + * post-write readback — or `null` when that readback leaves the + * caller's row scope. Predicate (`driver.updateMany`, + * `Promise`) answers the affected-row COUNT and names no + * row (#4639); `engine.ts` says so at the publish branch it feeds. + * + * delete → `boolean | number` + * TWO exits, same fork. By-id (`driver.delete`, + * `Promise`) answers whether the row was there — + * `metadata-protocol`'s `deleteData` already turns `false` into a + * 404. Predicate (`driver.deleteMany`, `Promise`) answers + * the affected count. + * + * ⚠️ The row's FIELD values stay erased (`Record`, not + * `Record`), and that is the precedent being extended rather + * than a softening of it. `find()` declares `Promise`: the CONTAINER is + * the contract and the rows inside it are `any`. Declaring the container is + * what makes a normalizer limb dead by type; declaring every field's value + * type is a different, much larger change that no ruling has asked for, and it + * was measured on this card's census as breaking a materially larger consumer + * set (every `new Date(row.some_column)` in the repo) for no gain the ruling + * names. `Record | null` is also the only spelling that can say + * "record or null" at all — `any | null` collapses to `any`. + * + * ## One predicate per verb, on the CONTAINER, and nothing cleverer + * + * SHAPING STAYS LEGAL, exactly as #15823 holds for `find()`. An `afterFind` + * handler may mutate the row in place, drop keys, or assign a DIFFERENT record + * built from it; an `afterUpdate` handler may reshape the record it is handed. + * So each check is on the shape of the container the verb declares and never on + * identity — comparing against the value the engine put there, freezing it or + * cloning it would each refuse a legitimate reshaping. + * + * What is refused is exactly one thing per verb: the answer stops being one of + * the shapes the declaration admits. + * + * ## `undefined` is refused everywhere, and that is a decision, not an oversight + * + * Same call #15823 made for `find()`, for the same reason and with the same + * cost. `undefined` is not one of the declared limbs on any of the three, and + * admitting it would leave a second hole beside the one being closed, in the + * same slot, indistinguishable from the outside. A read that answers no record + * answers `null`; a handler that wants to REFUSE an operation throws from the + * handler, which is how every other hook guard says no. + * + * ⚠️ For `delete` this is load-bearing in a way the others are not: `boolean | + * number` admits `false` and `0`, so a guard written as a truthiness check + * would refuse the two most ordinary answers there are ("the row was not + * there", "no rows matched"). The predicates below are `typeof` tests for that + * reason. + * + * ## Why `500`, and why REGISTERED ADR-0112 codes + * + * Both answers are `find-hook-result-shape.ts`'s, unchanged: the request was + * well-formed and authorized and a hook this deployment installed broke the + * engine's declared contract (a 5xx by definition, nothing for the caller to + * retry), and the value of the refusal is that a host can RECOGNISE it to find + * its own misbehaving handler — which an unregistered spelling cannot do, + * because it is demoted off `error.code` at every door + * (`resolveThrownHttpError`). A declared 5xx has its prose withheld at the HTTP + * doors, so the wire carries the code and the message reaches the HOOK'S + * AUTHOR in-process and in the server log, which is who it is addressed to. + * + * THREE codes rather than one shared code: the three declarations are three + * different contracts, a host branching on one of them is branching on a + * specific verb's answer shape, and ADR-0112's closed vocabulary is where that + * distinction belongs. `observed` carries WHICH wrong shape it was, per D3/D4, + * rather than growing the `code` vocabulary per shape. + * + * @see find-hook-result-shape.ts — the #15823 original this mirrors. + */ + +import { describeFindHookResult } from './find-hook-result-shape.js'; + +/** + * One word for what the handler left behind, shared with the `find()` refusal + * so the three verbs and their elder sibling describe a shape identically. + * + * Re-exported under a verb-neutral name rather than re-implemented: a second + * copy of `typeof`-with-special-cases is exactly the kind of near-duplicate + * that drifts (one of them learns about `Date`, the other does not) and then + * makes two refusals disagree about what they saw. + */ +export const describeHookResult = describeFindHookResult; + +/** A record — an object that is neither `null` nor an array. */ +function isRecordShape(value: unknown): boolean { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +// --------------------------------------------------------------------------- +// findOne +// --------------------------------------------------------------------------- + +/** The wire code, registered in the spec's `ERROR_CODE_LEDGER`. */ +export const FIND_ONE_HOOK_RESULT_NOT_RECORD_CODE = 'FIND_ONE_HOOK_RESULT_NOT_RECORD' as const; + +/** `500` — a server-side handler broke a server-side contract. */ +export const FIND_ONE_HOOK_RESULT_NOT_RECORD_STATUS = 500 as const; + +/** Does this value satisfy `findOne`'s declared `Record | null`? */ +export function isFindOneResultShape(value: unknown): boolean { + return value === null || isRecordShape(value); +} + +/** + * The ADR-0112 envelope `findOne()` raises when its `afterFind` dispatch left + * `hookContext.result` outside `Record | null`. + * + * Thrown at the seam — immediately after `triggerHooks('afterFind', …)` and + * BEFORE `maskSecretFields` / `stripSearchCompanionFromRead`, both of which + * already assume a record or a nullish value — so the diagnosis names the + * handler that did it rather than surfacing as a `TypeError` at a call site + * that trusted the declaration. + */ +export class FindOneHookResultNotRecordError extends Error { + override readonly name = 'FindOneHookResultNotRecordError'; + readonly code = FIND_ONE_HOOK_RESULT_NOT_RECORD_CODE; + readonly status = FIND_ONE_HOOK_RESULT_NOT_RECORD_STATUS; + /** The object being read. */ + readonly object: string; + /** The hook event whose dispatch the replacement was observed after. */ + readonly event: string; + /** What the handler left behind — {@link describeHookResult}. */ + readonly observed: string; + /** The remedy half, addressed to the hook's author rather than to a user. */ + readonly developerMessage: string; + + constructor(info: { object: string; event: string; result: unknown }) { + const observed = describeHookResult(info.result); + super(refusalSentence(info.object, info.event, observed, 'findOne', 'a record or null')); + this.object = info.object; + this.event = info.event; + this.observed = observed; + this.developerMessage = + `'findOne()' declares 'Promise | null>' — the ONE record the query ` + + `selects, or 'null' — and its callers branch on 'if (!row)' rather than on a container ` + + `check. A '${info.event}' handler may SHAPE that record: mutate it in place, drop keys, ` + + `or assign a different RECORD built from it. Replacing it with something that is neither ` + + `a record nor 'null' is refused. To answer no record, assign 'null'. To REFUSE the read, ` + + `throw from the handler — that is the supported way for a '${info.event}' guard to say no. ` + + `To hand the caller a different structure, do it in the caller, not in the hook. Branch on ` + + `\`code === '${FIND_ONE_HOOK_RESULT_NOT_RECORD_CODE}'\` (ADR-0112) to detect this.`; + } +} + +// --------------------------------------------------------------------------- +// update +// --------------------------------------------------------------------------- + +/** The wire code, registered in the spec's `ERROR_CODE_LEDGER`. */ +export const UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE = 'UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE' as const; + +/** `500` — a server-side handler broke a server-side contract. */ +export const UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_STATUS = 500 as const; + +/** + * Does this value satisfy `update`'s declared + * `Record | number | null`? + * + * `typeof value === 'number'` admits `0` deliberately — "the predicate matched + * no rows" is an ordinary answer, not a failure. + */ +export function isUpdateResultShape(value: unknown): boolean { + return value === null || typeof value === 'number' || isRecordShape(value); +} + +/** + * The ADR-0112 envelope `update()` raises when its `afterUpdate` dispatch left + * `hookContext.result` outside `Record | number | null`. + * + * Thrown at the seam — after the `afterUpdate` dispatch (including the per-row + * fan-out) and BEFORE `stripSearchCompanion`, which already assumes it is + * looking at either a record or a non-object it can skip. + */ +export class UpdateHookResultNotWriteShapeError extends Error { + override readonly name = 'UpdateHookResultNotWriteShapeError'; + readonly code = UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE; + readonly status = UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_STATUS; + /** The object being written. */ + readonly object: string; + /** The hook event whose dispatch the replacement was observed after. */ + readonly event: string; + /** What the handler left behind — {@link describeHookResult}. */ + readonly observed: string; + /** The remedy half, addressed to the hook's author rather than to a user. */ + readonly developerMessage: string; + + constructor(info: { object: string; event: string; result: unknown }) { + const observed = describeHookResult(info.result); + super(refusalSentence( + info.object, info.event, observed, 'update', 'a record, an affected-row count or null', + )); + this.object = info.object; + this.event = info.event; + this.observed = observed; + this.developerMessage = + `'update()' declares 'Promise | number | null>' — the two answers its ` + + `two dispatch paths give. A BY-ID write resolves the post-write record (or 'null' when the ` + + `readback leaves the caller's row scope); a PREDICATE write resolves the affected-row COUNT ` + + `and names no row (#4639). A '${info.event}' handler may SHAPE what it is handed — mutate ` + + `the record in place, drop keys, assign a different RECORD — but replacing it with a shape ` + + `outside that union is refused, because the declaration is the contract. To REFUSE the ` + + `write, throw from the handler. Branch on ` + + `\`code === '${UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE}'\` (ADR-0112) to detect this.`; + } +} + +// --------------------------------------------------------------------------- +// delete +// --------------------------------------------------------------------------- + +/** The wire code, registered in the spec's `ERROR_CODE_LEDGER`. */ +export const DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE = 'DELETE_HOOK_RESULT_NOT_WRITE_SHAPE' as const; + +/** `500` — a server-side handler broke a server-side contract. */ +export const DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_STATUS = 500 as const; + +/** + * Does this value satisfy `delete`'s declared `boolean | number`? + * + * ⚠️ `false` and `0` are the two most ORDINARY answers here — "the row was not + * there" and "no rows matched" — so the predicate is a pair of `typeof` tests + * and never a truthiness check. `metadata-protocol`'s `deleteData` turns the + * `false` into its 404, and would lose that answer to a lenient guard. + */ +export function isDeleteResultShape(value: unknown): boolean { + return typeof value === 'boolean' || typeof value === 'number'; +} + +/** + * The ADR-0112 envelope `delete()` raises when its `afterDelete` dispatch left + * `hookContext.result` outside `boolean | number`. + */ +export class DeleteHookResultNotWriteShapeError extends Error { + override readonly name = 'DeleteHookResultNotWriteShapeError'; + readonly code = DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE; + readonly status = DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_STATUS; + /** The object being written. */ + readonly object: string; + /** The hook event whose dispatch the replacement was observed after. */ + readonly event: string; + /** What the handler left behind — {@link describeHookResult}. */ + readonly observed: string; + /** The remedy half, addressed to the hook's author rather than to a user. */ + readonly developerMessage: string; + + constructor(info: { object: string; event: string; result: unknown }) { + const observed = describeHookResult(info.result); + super(refusalSentence( + info.object, info.event, observed, 'delete', 'a boolean or an affected-row count', + )); + this.object = info.object; + this.event = info.event; + this.observed = observed; + this.developerMessage = + `'delete()' declares 'Promise' — the two answers its two dispatch paths ` + + `give. A BY-ID delete resolves whether the row was there ('false' is a real answer, and ` + + `'@objectstack/metadata-protocol' turns it into a 404); a PREDICATE delete resolves the ` + + `affected-row COUNT ('0' is a real answer). An '${info.event}' handler that wants to ` + + `REFUSE a delete throws from the handler; a delete has no post-state to reshape, so ` + + `replacing 'ctx.result' with a record or an envelope is refused. Branch on ` + + `\`code === '${DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE}'\` (ADR-0112) to detect this.`; + } +} + +// --------------------------------------------------------------------------- +// The shared user-facing sentence +// --------------------------------------------------------------------------- + +/** + * The user-facing sentence, one composer for all three refusals. + * + * ⛔ It must not begin with a SQL verb — `@objectstack/rest`'s importer runs row + * errors through `sanitizeRowError`, whose SQL backstop replaces any message + * STARTING with `insert`/`update`/`delete` with generic text. That constraint + * bites here harder than it did on `find()`: two of these three refusals are + * ABOUT `update` and `delete`, so a sentence naming the verb first would be + * silently rewritten on exactly the path a REST caller reads. `Refusing the …` + * keeps the verb out of first position, the same shape + * `FindHookResultNotArrayError`, `DuplicateRecordError` and + * `MultiUpdateHookKeyDivergenceError` all record. + */ +function refusalSentence( + object: string, + event: string, + observed: string, + verb: string, + declared: string, +): string { + // `undefined` / `null` name themselves; everything else takes an article, and + // `object` is the one that needs `an`. + const what = + observed === 'undefined' || observed === 'null' + ? observed + : `${'aeiou'.includes(observed[0]) ? 'an' : 'a'} ${observed}`; + return ( + `Refusing the '${verb}' on '${object}': its '${event}' handler replaced 'ctx.result' with ` + + `${what}, and '${verb}()' answers ${declared}. Shaping what it answers is supported; ` + + `replacing it with another shape is not.` + ); +} diff --git a/packages/plugins/plugin-auth/src/objectql-adapter.ts b/packages/plugins/plugin-auth/src/objectql-adapter.ts index 6be89f42f0..3107bd0c1a 100644 --- a/packages/plugins/plugin-auth/src/objectql-adapter.ts +++ b/packages/plugins/plugin-auth/src/objectql-adapter.ts @@ -981,7 +981,12 @@ export function createObjectQLAdapterFactory(rawDataEngine: IDataEngine) { // first config edit, leaving a column that only LOOKS protected. liftClientSecretForWrite(objectName, patch); const result = await dataEngine.update(objectName, { ...patch, id: record.id }); - if (!result) return null; + // [#16231] The payload carries the resolved `id` and no `where`, so + // the engine dispatches `by-id` and answers the record or `null`. The + // `number` limb of the declared union is the predicate path's affected + // count, unreachable from here — refused rather than coerced, so this + // adapter never hands `normaliseLegacyDates` a count shaped as a row. + if (!result || typeof result === 'number') return null; const norm = normaliseLegacyDates(model, result); return (bridged ? remapKeys(norm, snakeToCamel) : norm) as T; }, @@ -1085,7 +1090,12 @@ export function createObjectQLAdapterFactory(rawDataEngine: IDataEngine) { if (set) Object.assign(patch, bridged ? remapKeys(set, camelToSnake) : set); const result = await dataEngine.update(objectName, { ...patch, id: record.id }); - if (!result) return null; + // [#16231] The payload carries the resolved `id` and no `where`, so + // the engine dispatches `by-id` and answers the record or `null`. The + // `number` limb of the declared union is the predicate path's affected + // count, unreachable from here — refused rather than coerced, so this + // adapter never hands `normaliseLegacyDates` a count shaped as a row. + if (!result || typeof result === 'number') return null; const norm = normaliseLegacyDates(model, result); return (bridged ? remapKeys(norm, snakeToCamel) : norm) as T; }, diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index 05f48d47a0..78c31bc531 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -655,10 +655,12 @@ export const ERROR_CODE_LEDGER = { // ADR-0119 D1/D4 fail-closed posture). Same #8087-gate family. 'ERR_TRANSACTION_UNSUPPORTED', // [#15823] an `afterFind` handler REPLACED `ctx.result` with something that - // is not an array, breaking the one `return hookContext.result` site in - // `engine.ts` that has a concrete declared shape to violate - // (`find(): Promise`; `findOne`/`update`/`delete` all declare - // `Promise`). Refused at the seam — immediately after the dispatch and + // is not an array, breaking `find()`'s declared `Promise`. It was the + // FIRST of the four `return hookContext.result` sites in `engine.ts` to be + // closed, and for a while the only one that could be: `findOne`/`update`/ + // `delete` declared `Promise` and so had nothing to violate. #16231 + // ruled that gap shut — all four verbs declare now, and the three refusals + // below are this one's twins. Refused at the seam — immediately after the dispatch and // ahead of `maskSecretFields` / `stripSearchCompanionFromRead`, which both // already assume the array — rather than surfacing as a `TypeError` at one // of ~140 call sites. SHAPING stays legal: mutating rows, dropping keys, @@ -671,6 +673,19 @@ export const ERROR_CODE_LEDGER = { // and the fault is a server-side extension's, not the caller's. // `FindHookResultNotArrayError`, `find-hook-result-shape.ts`. 'FIND_HOOK_RESULT_NOT_ARRAY', + // [#16231] The `findOne` twin of the refusal above, admitted once that verb + // DECLARED what it answers. `findOne(): Promise | null>` + // — the one record the query selects, or `null`, which its own docblock has + // said in prose since #4419 and which callers already branch on with + // `if (!row)`. An `afterFind` handler that replaces `ctx.result` with + // anything else (an envelope, an array, `undefined`) is refused at the same + // position `find()`'s is: after the dispatch, ahead of `maskSecretFields` + // and `stripSearchCompanionFromRead`, both of which already assume it. + // SHAPING stays legal — mutating the record, dropping keys, assigning a + // different RECORD are all untouched. Registered for the same reason as its + // sibling: a host has to RECOGNISE this to find its own misbehaving + // handler. `FindOneHookResultNotRecordError`, `verb-hook-result-shape.ts`. + 'FIND_ONE_HOOK_RESULT_NOT_RECORD', // [#14010] a hook declared `runAs: 'user'` and its trigger resolved NO user // (an `isSystem` plugin/service write, a system-elevated flow node), so its // `ctx.api` data operation has no identity to scope to and is REFUSED @@ -691,6 +706,25 @@ export const ERROR_CODE_LEDGER = { // decision. `MultiUpdateHookKeyDivergenceError`, // `multi-update-hook-key-divergence.ts`. 'MULTI_UPDATE_HOOK_KEY_DIVERGENCE', + // [#16231] The `delete` twin. `delete(): Promise` — the + // two answers its two dispatch paths give: whether the by-id row was there + // (`driver.delete`), or how many rows a predicate delete removed + // (`driver.deleteMany`, #4639). An `afterDelete` handler that replaces + // `ctx.result` with anything outside that union is refused. ⚠️ `false` and + // `0` are ORDINARY answers and are not refused — `metadata-protocol`'s + // `deleteData` turns the `false` into its 404. A handler that wants to + // refuse a delete throws. `DeleteHookResultNotWriteShapeError`, + // `verb-hook-result-shape.ts`. + 'DELETE_HOOK_RESULT_NOT_WRITE_SHAPE', + // [#16231] The `update` twin. + // `update(): Promise | number | null>` — the post-write + // record (or `null` when the readback leaves the caller's row scope) from + // the by-id exit, or the affected-row COUNT from the predicate exit + // (#4639), which names no row. An `afterUpdate` handler may SHAPE what it + // is handed; replacing it with a shape outside the union is refused, at the + // seam and ahead of `stripSearchCompanion` and the realtime publish. + // `UpdateHookResultNotWriteShapeError`, `verb-hook-result-shape.ts`. + 'UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE', // [#14748] the ADR-0048 Phase 1 install-time namespace gate's refusal: a // package's `manifest.namespace` is already owned by an INSTALLED package // that is not a co-owner of it (ADR-0130 D1), so the install is refused up diff --git a/packages/spec/src/contracts/data-engine.ts b/packages/spec/src/contracts/data-engine.ts index 70d8180928..7907f0e294 100644 --- a/packages/spec/src/contracts/data-engine.ts +++ b/packages/spec/src/contracts/data-engine.ts @@ -273,9 +273,9 @@ export interface IDataEngine { * * [#6300] `query` is the author state (`z.input`), same as `find` above. */ - findOne(objectName: string, query?: EngineQueryOptions, options?: BaseEngineOptions): Promise | null>; + findOne(objectName: string, query?: EngineQueryOptions, options?: BaseEngineOptions): Promise | null>; insert(objectName: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise; - update(objectName: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise | number | null>; + update(objectName: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise | number | null>; delete(objectName: string, options?: EngineDeleteOptions): Promise; count(objectName: string, query?: EngineCountOptions, options?: BaseEngineOptions): Promise; aggregate(objectName: string, query: EngineAggregateOptions, options?: BaseEngineOptions): Promise; diff --git a/packages/spec/src/contracts/scoped-context.ts b/packages/spec/src/contracts/scoped-context.ts index a758e5493f..60d4c3b807 100644 --- a/packages/spec/src/contracts/scoped-context.ts +++ b/packages/spec/src/contracts/scoped-context.ts @@ -108,8 +108,9 @@ * here — the option VOCABULARY is `EngineQueryOptions`' to own and the engine's * to enforce (it rejects undeclared option keys since #4371); this interface's * job is the METHOD FACE. So the query bag is an object, the returns mirror - * `IDataEngine`'s (`Promise` / `Promise`), and nothing here claims - * to be the query schema. + * `IDataEngine`'s — `find` stays `Promise`, and since #16231 `findOne` + * and `update` carry the same declared answer shapes their `IDataEngine` + * counterparts do — and nothing here claims to be the query schema. */ import type { EngineTransactionInfo, EngineTransactionOptions } from './objectql-engine.js'; @@ -145,7 +146,7 @@ export interface IScopedObjectRepository { * nothing to do with what was asked, which no `if (!row)` can catch. When * any row genuinely will do, that is `find({ limit: 1 })`, which says so. */ - findOne(query?: Record): Promise | null>; + findOne(query?: Record): Promise | null>; /** Count the records the query selects. */ count(query?: Record): Promise; @@ -161,7 +162,7 @@ export interface IScopedObjectRepository { * key out of the payload. The bulk form is `update(data, { where, multi: true })`; * there is no `updateMany`. */ - update(data: any, options?: Record): Promise | number | null>; + update(data: any, options?: Record): Promise | number | null>; /** Update a single record by id — the id travels as the first argument. */ updateById(id: string | number, data: any): Promise; From 8fcfb351400ff59fa5fba0c08b326e26ced397a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 03:35:06 +0000 Subject: [PATCH 03/10] test(engine): pin the three declarations and repair the census consumers Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- .../src/batch-row-authoring-feedback.test.ts | 8 +- .../engine-autonumber-runtime-owned.test.ts | 47 +- .../objectql/src/engine-filter-alias.test.ts | 12 +- .../src/engine-verb-hook-result-shape.test.ts | 417 ++++++++++++++++++ packages/objectql/src/engine.test.ts | 3 +- packages/objectql/src/internal-fields.test.ts | 12 +- .../multi-update-hook-key-divergence.test.ts | 10 +- ...panion-read-projection-conformance.test.ts | 9 +- ...cord-lock-schedule-run.integration.test.ts | 4 +- .../rest/src/import-business-timezone.test.ts | 10 +- packages/runtime/src/seed-loader.test.ts | 8 +- ...field-expression-scale.integration.test.ts | 2 +- .../src/paused-run-visibility.test.ts | 2 +- .../src/runas-attribution-contract.test.ts | 14 +- .../runas-system-stamping.integration.test.ts | 40 +- .../spec/src/contracts/data-engine.test.ts | 17 +- 16 files changed, 553 insertions(+), 62 deletions(-) create mode 100644 packages/objectql/src/engine-verb-hook-result-shape.test.ts diff --git a/packages/objectql/src/batch-row-authoring-feedback.test.ts b/packages/objectql/src/batch-row-authoring-feedback.test.ts index 0810e34667..b88a79c237 100644 --- a/packages/objectql/src/batch-row-authoring-feedback.test.ts +++ b/packages/objectql/src/batch-row-authoring-feedback.test.ts @@ -140,7 +140,13 @@ describe('[#8502] a REAL validation refusal keeps its sentence on a batch row', expect(res.results[0].errors[0].message).toContain('Reason'); expect(res.results[0].errors[0].message).not.toContain('The reason is in the server log'); // The stored row is untouched: the refusal happened before the write. - expect((await engine.findOne('bf_leave_request', { where: { id: 'lr1' } })).reason).toBe('ok'); + // [#16231] `findOne` declares record-or-null now, so the read is + // narrowed before the field is asserted — the row's presence IS half of + // what this case measures ("the stored row is untouched"), and reading + // it through `?.` would have let a vanished row pass as `undefined`. + const untouched = await engine.findOne('bf_leave_request', { where: { id: 'lr1' } }); + expect(untouched).not.toBeNull(); + expect(untouched!.reason).toBe('ok'); }); it('the refusal carries no `status`, so a status-only rule WOULD have blanked it', async () => { diff --git a/packages/objectql/src/engine-autonumber-runtime-owned.test.ts b/packages/objectql/src/engine-autonumber-runtime-owned.test.ts index 9dce2cb645..b93e8b6633 100644 --- a/packages/objectql/src/engine-autonumber-runtime-owned.test.ts +++ b/packages/objectql/src/engine-autonumber-runtime-owned.test.ts @@ -35,6 +35,33 @@ import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protoco import { ObjectQL } from './engine.js'; import type { DroppedFieldsEvent } from '@objectstack/spec/data'; +/** + * [#16231] `findOne` and `update` declare what they answer now, so a pin that + * reads a field off either has to say WHICH limb it means. Said once here + * rather than scattered through the cases as `!` and casts: + * + * - every read below is a by-id `findOne`, so `null` means the row this case + * just wrote is gone — a failure of the case, not a shape to tolerate; + * - every write below is a by-id `update` (payload id plus a `where` naming + * the one row), so the affected-row COUNT limb is unreachable and a count + * arriving here would mean the dispatch ladder changed under the pin. + * + * Both refuse loudly, so the case fails where the wrong shape appeared instead + * of several lines later on an `undefined` field. + */ +function readRow(row: Record | null): Record { + if (row === null) throw new Error('expected findOne to answer the row this case wrote'); + return row; +} + +function writtenRow(result: Record | number | null): Record { + if (result === null) throw new Error('expected a by-id update to answer the written record, got null'); + if (typeof result === 'number') { + throw new Error(`expected a by-id update to answer the written record, got an affected count (${result})`); + } + return result; +} + const ACCOUNT = { name: 'an_account', label: 'Account', @@ -395,7 +422,7 @@ describe('#5503 x #5126 — strictReadonlyWrites covers runtime-owned fields', ( expect(err.fields).toEqual(['account_number']); expect([...err.drops].map((d: DroppedFieldsEvent) => d.reason)).toEqual(['readonly']); // Nothing was written — not even the legitimate rename. - const readback = await rig.engine.findOne('an_account', { where: { id } }); + const readback = readRow(await rig.engine.findOne('an_account', { where: { id } })); expect(readback.name).toBe('Acme'); expect(readback.account_number).toBe('ACC-0001'); }); @@ -413,7 +440,7 @@ describe('#5503 x #5126 — strictReadonlyWrites covers runtime-owned fields', ( ); expect(events.flatMap((e) => e.fields)).toContain('account_number'); - const readback = await rig.engine.findOne('an_account', { where: { id } }); + const readback = readRow(await rig.engine.findOne('an_account', { where: { id } })); expect(readback.name).toBe('renamed'); // the legitimate half landed expect(readback.account_number).toBe('ACC-0001'); // the forged half did not }); @@ -422,11 +449,11 @@ describe('#5503 x #5126 — strictReadonlyWrites covers runtime-owned fields', ( const rig = await makeEngine(); const created = await rig.protocol.createData({ object: 'an_account', data: { name: 'Acme' } }); const id = created.id as string; - const res = await rig.engine.update( + const res = writtenRow(await rig.engine.update( 'an_account', { id, account_number: 'LEGACY-0007' }, { where: { id }, strictReadonlyWrites: true, context: { preserveAudit: true } }, - ); + )); expect(res.account_number).toBe('LEGACY-0007'); }); @@ -467,7 +494,7 @@ describe('#5503 — autonumber is runtime-owned: UPDATE', () => { data: { account_number: 'ACC-888888' }, }); expect(res.record.account_number).toBe('ACC-0001'); - const readback = await rig.engine.findOne('an_account', { where: { id } }); + const readback = readRow(await rig.engine.findOne('an_account', { where: { id } })); expect(readback.account_number).toBe('ACC-0001'); }); @@ -512,20 +539,20 @@ describe('#5503 — autonumber is runtime-owned: UPDATE', () => { }); it('keeps an explicit value for a system write', async () => { - const res = await rig.engine.update( + const res = writtenRow(await rig.engine.update( 'an_account', { id, account_number: 'ACC-000042' }, { where: { id }, context: { isSystem: true } } as any, - ); + )); expect(res.account_number).toBe('ACC-000042'); }); it('keeps an explicit value for a `preserveAudit` historical import / undo (#3493)', async () => { - const res = await rig.engine.update( + const res = writtenRow(await rig.engine.update( 'an_account', { id, account_number: 'LEGACY-0007' }, { where: { id }, context: { preserveAudit: true } } as any, - ); + )); expect(res.account_number).toBe('LEGACY-0007'); }); @@ -533,7 +560,7 @@ describe('#5503 — autonumber is runtime-owned: UPDATE', () => { rig.engine.registerHook('beforeUpdate', async (ctx: any) => { ctx.input.data.account_number = 'HOOK-0002'; }, { object: 'an_account' }); - const res = await rig.engine.update('an_account', { id, name: 'x' }, { where: { id } } as any); + const res = writtenRow(await rig.engine.update('an_account', { id, name: 'x' }, { where: { id } } as any)); expect(res.account_number).toBe('HOOK-0002'); }); }); diff --git a/packages/objectql/src/engine-filter-alias.test.ts b/packages/objectql/src/engine-filter-alias.test.ts index d0dbcd448e..4d1846d9c4 100644 --- a/packages/objectql/src/engine-filter-alias.test.ts +++ b/packages/objectql/src/engine-filter-alias.test.ts @@ -119,8 +119,13 @@ describe('filter → where folds on every engine method (#4346)', () => { it('findOne({filter}) returns a MATCHING row, not the first row of the table', async () => { const row = await engine.findOne('task', { filter: { status: 'done' } } as any); - expect(row.status).toBe('done'); - expect(row.id).not.toBe(a.id); + // [#16231] The `null` is part of the declaration now, and it is part of + // what this case measures: an unfolded `filter` used to match ALL rows, + // so "a row came back at all" and "it is the MATCHING one" are two + // different assertions and both have to be made. + expect(row).not.toBeNull(); + expect(row!.status).toBe('done'); + expect(row!.id).not.toBe(a.id); }); it('count({filter}) counts the matching rows, not the whole table', async () => { @@ -201,7 +206,8 @@ describe('filter → where folds on every engine method (#4346)', () => { it('findOne({top}) stays single-row — the forced limit: 1 wins over the folded alias', async () => { const row = await engine.findOne('task', { top: 5, filter: { status: 'done' } } as any); - expect(row.status).toBe('done'); + expect(row).not.toBeNull(); + expect(row!.status).toBe('done'); }); // ── the documented hook call path (ScopedContext / ObjectRepository) ─ diff --git a/packages/objectql/src/engine-verb-hook-result-shape.test.ts b/packages/objectql/src/engine-verb-hook-result-shape.test.ts new file mode 100644 index 0000000000..a5c17dd0c1 --- /dev/null +++ b/packages/objectql/src/engine-verb-hook-result-shape.test.ts @@ -0,0 +1,417 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #16231 — `findOne`, `update` and `delete` DECLARE what they answer, and the +// `after*` seam that could break each declaration is closed. +// +// ## What this suite is for +// +// `engine.ts` has four `return hookContext.result` sites. #15823 closed the +// `find()` one and recorded why it could close only that one: `find` declared +// `Promise`, a concrete container to violate, while these three declared +// `Promise` and so carried nothing an `after*` handler could break. +// +// The maintainer ruled that gap shut (option A, 2026-09-07, director seat +// summon #17, decision batch #2; options B "declare only" and C "record `any` +// as intended" were refused). The declarations moved onto what each verb +// actually answers — read off the driver contract each exit delegates to, not +// invented — and each seam is guarded as `find()`'s is: +// +// findOne → `Record | null` (driver.findOne) +// update → `Record | number | null` (driver.update | driver.updateMany) +// delete → `boolean | number` (driver.delete | driver.updateMany's twin) +// +// ## The three things this suite has to hold apart +// +// (a) the declared limbs are ANSWERABLE — including the ones a lenient guard +// would eat: `null` from `findOne`, `null` and a count from `update`, +// and — the load-bearing pair — `false` and `0` from `delete`; +// (b) SHAPING STAYS LEGAL — a handler may mutate what it is handed, or assign +// a different value of a declared shape. This is the half that keeps the +// refusal from being a behaviour regression, and it is written against +// the SHAPE and never against identity; +// (c) a value outside the declaration is REFUSED, with the registered +// ADR-0112 envelope — asserted by `code` AND `status`, never a bare +// `toThrow()`: an unfixed engine throws nothing at all here, so a bare +// `toThrow()` would be satisfied by any unrelated failure. +// +// ⚠️ (a) is where this suite differs most from its `find()` elder. There, every +// non-array was refusable and `[]` was the only "nothing" answer. Here each +// verb has MORE than one legal answer and two of them are falsy, so a guard +// written as a truthiness check passes `find()`'s suite and destroys this one. + +import { describe, it, expect } from 'vitest'; +// [check:test-source-alias] Module-top, not `await import(...)` inside a case: +// objectql resolves this specifier through `dist/`, so a first load paid inside +// a test body transforms that whole module graph while `testTimeout` runs. +import { ErrorCode } from '@objectstack/spec/api'; +import { ObjectQL } from './engine.js'; +import { + FIND_ONE_HOOK_RESULT_NOT_RECORD_CODE, + FIND_ONE_HOOK_RESULT_NOT_RECORD_STATUS, + FindOneHookResultNotRecordError, + UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE, + UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_STATUS, + UpdateHookResultNotWriteShapeError, + DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE, + DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_STATUS, + DeleteHookResultNotWriteShapeError, +} from './verb-hook-result-shape.js'; + +function silentLogger() { + const logger: any = { + trace() {}, debug() {}, info() {}, warn() {}, error() {}, fatal() {}, + child() { return logger; }, + }; + return logger; +} + +const ROW = { id: 't1', name: 'first', done: false }; + +/** + * A driver that answers each exit the way `IDataDriver` declares it, so the + * engine's own limbs are exercised rather than simulated: + * `findOne` → record-or-null, `update` → record-or-null, `updateMany` → count, + * `delete` → boolean, `deleteMany` → count. + * + * `missing` flips the reads to their empty answer, which is how the `null` and + * `false` limbs below are reached through the REAL engine path rather than by + * a handler assigning them. + */ +function makeDriver(opts: { missing?: boolean } = {}) { + const driver: any = { + name: 'memory', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, + async find() { return opts.missing ? [] : [{ ...ROW }]; }, + async findOne() { return opts.missing ? null : { ...ROW }; }, + async create(_o: string, data: any) { return data; }, + async update(_o: string, id: string, data: any) { + return opts.missing ? null : { ...ROW, ...data, id }; + }, + async updateMany() { return 2; }, + async delete() { return !opts.missing; }, + async deleteMany() { return 2; }, + async count() { return opts.missing ? 0 : 1; }, + async bulkCreate(_o: string, rows: any[]) { return rows; }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + }; + return driver; +} + +async function makeEngine(opts: { missing?: boolean } = {}) { + const engine = new ObjectQL({ logger: silentLogger() }); + engine.registerDriver(makeDriver(opts), true); + await engine.init(); + engine.registry.registerObject({ + name: 'task', + fields: { + id: { name: 'id', type: 'text', primaryKey: true, readonly: true }, + name: { name: 'name', type: 'text' }, + done: { name: 'done', type: 'boolean' }, + }, + } as any, 'test'); + return engine; +} + +/** Run and return whatever came out — a value or the thrown error. */ +async function outcomeOf(run: () => Promise): Promise<{ value?: unknown; error?: any }> { + try { + return { value: await run() }; + } catch (error) { + return { error }; + } +} + +// --------------------------------------------------------------------------- +// findOne +// --------------------------------------------------------------------------- + +describe('#16231 findOne — the declared limbs are answerable', () => { + it('answers the record on the no-hook path', async () => { + const engine = await makeEngine(); + const row: any = await engine.findOne('task', { where: { id: 't1' } }); + expect(row).not.toBeNull(); + expect(row.id).toBe('t1'); + }); + + it('answers `null` when the query selects nothing — through the ENGINE, not a handler', async () => { + // The limb the declaration exists to write down, reached the way a caller + // reaches it. A guard that refused nullish would break this. + const engine = await makeEngine({ missing: true }); + expect(await engine.findOne('task', { where: { id: 'nope' } })).toBeNull(); + }); +}); + +describe('#16231 findOne — shaping stays legal', () => { + it('an afterFind that mutates the record IN PLACE still answers the record', async () => { + const engine = await makeEngine(); + engine.registerHook('afterFind', (ctx: any) => { delete ctx.result.name; }, { object: 'task' } as any); + + const row: any = await engine.findOne('task', { where: { id: 't1' } }); + expect(row.id).toBe('t1'); + expect('name' in row).toBe(false); + }); + + it('an afterFind that assigns a DIFFERENT record still answers the record', async () => { + // The identity half of the predicate: the container object is replaced and + // that is legal, because what replaced it is still a record. + const engine = await makeEngine(); + engine.registerHook('afterFind', (ctx: any) => { ctx.result = { id: ctx.result.id }; }, { object: 'task' } as any); + + expect(await engine.findOne('task', { where: { id: 't1' } })).toEqual({ id: 't1' }); + }); + + it('an afterFind that assigns `null` is LEGAL — it is a declared limb', async () => { + // ⚠️ The sharpest difference from `find()`'s guard, where nullish is + // refused because `[]` is the only way to answer "nothing". Here `null` IS + // the way to answer "no record", so refusing it would refuse the + // documented spelling. + const engine = await makeEngine(); + engine.registerHook('afterFind', (ctx: any) => { ctx.result = null; }, { object: 'task' } as any); + + expect(await engine.findOne('task', { where: { id: 't1' } })).toBeNull(); + }); +}); + +describe('#16231 findOne — a value outside the declaration is refused', () => { + it('an envelope is refused with FIND_ONE_HOOK_RESULT_NOT_RECORD', async () => { + const engine = await makeEngine(); + engine.registerHook('afterFind', (ctx: any) => { + ctx.result = [{ id: 'ENVELOPE' }]; + }, { object: 'task' } as any); + + const { value, error } = await outcomeOf(() => engine.findOne('task', { where: { id: 't1' } })); + + expect(value, 'findOne() answered instead of refusing').toBeUndefined(); + expect(error, 'findOne() did not refuse').toBeInstanceOf(FindOneHookResultNotRecordError); + // ADR-0112 envelope: the code AND the status, never a bare `toThrow()`. + expect(error.code).toBe(FIND_ONE_HOOK_RESULT_NOT_RECORD_CODE); + expect(error.code).toBe('FIND_ONE_HOOK_RESULT_NOT_RECORD'); + expect(error.status).toBe(FIND_ONE_HOOK_RESULT_NOT_RECORD_STATUS); + expect(error.status).toBe(500); + expect(error.event).toBe('afterFind'); + expect(error.object).toBe('task'); + expect(error.observed).toBe('array'); + // The remedy half is addressed to the handler's author and names both + // supported spellings — `null` for no record, a throw to refuse the read. + expect(error.developerMessage).toContain("assign 'null'"); + expect(error.developerMessage).toContain('throw from the handler'); + }); + + it('`undefined` is refused — decided here, and it is NOT the same as `null`', async () => { + const engine = await makeEngine(); + engine.registerHook('afterFind', (ctx: any) => { ctx.result = undefined; }, { object: 'task' } as any); + + const { error } = await outcomeOf(() => engine.findOne('task', { where: { id: 't1' } })); + expect(error?.code).toBe(FIND_ONE_HOOK_RESULT_NOT_RECORD_CODE); + expect(error.observed).toBe('undefined'); + }); + + it('a string is refused — the predicate is a shape test, not `typeof`', async () => { + const engine = await makeEngine(); + engine.registerHook('afterFind', (ctx: any) => { ctx.result = 'row'; }, { object: 'task' } as any); + + const { error } = await outcomeOf(() => engine.findOne('task', { where: { id: 't1' } })); + expect(error?.code).toBe(FIND_ONE_HOOK_RESULT_NOT_RECORD_CODE); + expect(error.observed).toBe('string'); + }); + + it('the refusal fires BEFORE maskSecretFields / stripSearchCompanionFromRead see it', async () => { + // Driven, not asserted about source, exactly as #15823 drives the same + // placement claim on `find()`: both consumers run on `hookContext.result` + // between the dispatch and the return, so a replaced value they walked + // first would be diagnosed from the wrong place. + const engine = await makeEngine(); + let poisonRead = 0; + engine.registerHook('afterFind', (ctx: any) => { + ctx.result = new Proxy([], { + get(target, prop, recv) { poisonRead += 1; return Reflect.get(target, prop, recv); }, + }); + }, { object: 'task' } as any); + + const { error } = await outcomeOf(() => engine.findOne('task', { where: { id: 't1' } })); + expect(error?.code).toBe(FIND_ONE_HOOK_RESULT_NOT_RECORD_CODE); + expect(poisonRead, 'a consumer walked the replaced value before the refusal').toBe(0); + }); +}); + +// --------------------------------------------------------------------------- +// update +// --------------------------------------------------------------------------- + +describe('#16231 update — the declared limbs are answerable', () => { + it('a BY-ID write answers the record', async () => { + const engine = await makeEngine(); + const out: any = await engine.update('task', { id: 't1', name: 'renamed' }); + expect(typeof out).toBe('object'); + expect(out.name).toBe('renamed'); + }); + + it('a PREDICATE write answers the affected-row COUNT, and the guard admits it', async () => { + // #4639: a predicate write names no row. This is the limb that made + // `update`'s declaration a UNION rather than a record type, and the + // measurement that a record-only guard would have refused. + const engine = await makeEngine(); + const out = await engine.update('task', { done: true }, { multi: true, where: { done: false } } as any); + expect(out).toBe(2); + }); +}); + +describe('#16231 update — shaping stays legal', () => { + it('an afterUpdate that mutates the record IN PLACE still answers the record', async () => { + const engine = await makeEngine(); + engine.registerHook('afterUpdate', (ctx: any) => { ctx.result.stamped = true; }, { object: 'task' } as any); + + const out: any = await engine.update('task', { id: 't1', name: 'renamed' }); + expect(out.stamped).toBe(true); + }); + + it('an afterUpdate may assign a different RECORD, `null`, or a count', async () => { + for (const [label, assigned, expected] of [ + ['a different record', { id: 't1' }, { id: 't1' }], + ['null', null, null], + ['a count', 7, 7], + ['a zero count', 0, 0], + ] as const) { + const engine = await makeEngine(); + engine.registerHook('afterUpdate', (ctx: any) => { ctx.result = assigned; }, { object: 'task' } as any); + expect(await engine.update('task', { id: 't1', name: 'x' }), label).toEqual(expected); + } + }); +}); + +describe('#16231 update — a value outside the declaration is refused', () => { + for (const [label, assigned, observed] of [ + ['a string', 'done', 'string'], + ['an array', [{ id: 't1' }], 'array'], + ['a boolean', true, 'boolean'], + ['undefined', undefined, 'undefined'], + ] as const) { + it(`${label} is refused with UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE`, async () => { + const engine = await makeEngine(); + engine.registerHook('afterUpdate', (ctx: any) => { ctx.result = assigned; }, { object: 'task' } as any); + + const { value, error } = await outcomeOf(() => engine.update('task', { id: 't1', name: 'x' })); + expect(value, 'update() answered instead of refusing').toBeUndefined(); + expect(error, 'update() did not refuse').toBeInstanceOf(UpdateHookResultNotWriteShapeError); + expect(error.code).toBe(UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE); + expect(error.code).toBe('UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE'); + expect(error.status).toBe(UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_STATUS); + expect(error.status).toBe(500); + expect(error.event).toBe('afterUpdate'); + expect(error.object).toBe('task'); + expect(error.observed).toBe(observed); + }); + } + + it("the message does NOT begin with a SQL verb — `sanitizeRowError` would blank it", async () => { + // ⛔ Load-bearing here in a way it was not on `find()`: this refusal is + // ABOUT `update`, so the obvious first word is the one `@objectstack/rest`'s + // importer replaces with generic text. The constraint is pinned rather than + // trusted to a comment. + const engine = await makeEngine(); + engine.registerHook('afterUpdate', (ctx: any) => { ctx.result = 'done'; }, { object: 'task' } as any); + + const { error } = await outcomeOf(() => engine.update('task', { id: 't1', name: 'x' })); + expect(error.message.startsWith('Refusing')).toBe(true); + for (const verb of ['insert', 'update', 'delete']) { + expect(error.message.toLowerCase().startsWith(verb), `message starts with '${verb}'`).toBe(false); + } + }); +}); + +// --------------------------------------------------------------------------- +// delete +// --------------------------------------------------------------------------- + +describe('#16231 delete — the declared limbs are answerable, INCLUDING the falsy ones', () => { + it('a BY-ID delete answers `true` when the row was there', async () => { + const engine = await makeEngine(); + expect(await engine.delete('task', { where: { id: 't1' } })).toBe(true); + }); + + it('a PREDICATE delete answers the affected-row COUNT', async () => { + const engine = await makeEngine(); + expect(await engine.delete('task', { multi: true, where: { done: false } } as any)).toBe(2); + }); + + it('an afterDelete may assign `false` or `0` — a truthiness guard would refuse both', async () => { + // ⭐ The case that separates this guard from a lenient one. `false` is what + // `@objectstack/metadata-protocol`'s `deleteData` turns into its 404, and + // `0` is "the predicate matched nothing". Both are ordinary answers. + for (const falsy of [false, 0] as const) { + const engine = await makeEngine(); + engine.registerHook('afterDelete', (ctx: any) => { ctx.result = falsy; }, { object: 'task' } as any); + expect(await engine.delete('task', { where: { id: 't1' } }), String(falsy)).toBe(falsy); + } + }); +}); + +describe('#16231 delete — a value outside the declaration is refused', () => { + for (const [label, assigned, observed] of [ + ['a record', { deleted: 1 }, 'object'], + ['an array', [], 'array'], + ['null', null, 'null'], + ['undefined', undefined, 'undefined'], + ['a string', 'ok', 'string'], + ] as const) { + it(`${label} is refused with DELETE_HOOK_RESULT_NOT_WRITE_SHAPE`, async () => { + const engine = await makeEngine(); + engine.registerHook('afterDelete', (ctx: any) => { ctx.result = assigned; }, { object: 'task' } as any); + + const { value, error } = await outcomeOf(() => engine.delete('task', { where: { id: 't1' } })); + expect(value, 'delete() answered instead of refusing').toBeUndefined(); + expect(error, 'delete() did not refuse').toBeInstanceOf(DeleteHookResultNotWriteShapeError); + expect(error.code).toBe(DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE); + expect(error.code).toBe('DELETE_HOOK_RESULT_NOT_WRITE_SHAPE'); + expect(error.status).toBe(DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_STATUS); + expect(error.status).toBe(500); + expect(error.event).toBe('afterDelete'); + expect(error.object).toBe('task'); + expect(error.observed).toBe(observed); + }); + } + + it('`{ deleted: 1 }` — the invented envelope two test doubles carried — is refused', async () => { + // Not hypothetical: `packages/spec/src/contracts/data-engine.test.ts` and + // `packages/runtime/src/seed-loader.test.ts` both modelled `delete` as + // answering this shape, which no driver and no engine has ever produced. + // `Promise` admitted it. Both were repaired with this card, and this + // case is what stops the shape coming back. + const engine = await makeEngine(); + engine.registerHook('afterDelete', (ctx: any) => { ctx.result = { deleted: 1 }; }, { object: 'task' } as any); + + const { error } = await outcomeOf(() => engine.delete('task', { where: { id: 't1' } })); + expect(error?.code).toBe(DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE); + }); +}); + +// --------------------------------------------------------------------------- +// The vocabulary +// --------------------------------------------------------------------------- + +describe('#16231 — all three refusals are registered ADR-0112 vocabulary', () => { + it('each code is a member of the generated ErrorCode union', () => { + for (const code of [ + FIND_ONE_HOOK_RESULT_NOT_RECORD_CODE, + UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE, + DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE, + ]) { + expect(ErrorCode.safeParse(code).success, code).toBe(true); + } + // Control: the union really does reject an unregistered spelling, so the + // assertions above are a reading rather than a schema that accepts + // anything. + expect(ErrorCode.safeParse('FIND_ONE_HOOK_RESULT_NOT_A_RECORD').success).toBe(false); + }); + + it('the three codes are distinct — one per declaration, not one shared refusal', () => { + const codes = new Set([ + FIND_ONE_HOOK_RESULT_NOT_RECORD_CODE, + UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE, + DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE, + ]); + expect(codes.size).toBe(3); + }); +}); diff --git a/packages/objectql/src/engine.test.ts b/packages/objectql/src/engine.test.ts index fba0c3f585..25173dcafd 100644 --- a/packages/objectql/src/engine.test.ts +++ b/packages/objectql/src/engine.test.ts @@ -2398,7 +2398,8 @@ describe('ObjectQL Engine', () => { expand: { assignee: { object: 'assignee' } }, }); - expect(result.assignee).toEqual({ id: 'u1', name: 'Alice' }); + expect(result).not.toBeNull(); + expect(result!.assignee).toEqual({ id: 'u1', name: 'Alice' }); }); it('should handle already-expanded objects (skip re-expansion)', async () => { diff --git a/packages/objectql/src/internal-fields.test.ts b/packages/objectql/src/internal-fields.test.ts index 26829bf035..5b11b83239 100644 --- a/packages/objectql/src/internal-fields.test.ts +++ b/packages/objectql/src/internal-fields.test.ts @@ -261,8 +261,16 @@ describe('#7728: the `internal` field flag omits a value from the generic data p const updated = await ctx.engine.update('itest_api_key', { id: created.id, revoked: true }, { context: { isSystem: true }, } as any); - expect(updated.key).toBe(HASH); - expect(updated.revoked).toBe(true); + // [#16231] `update` declares its dispatch union now. This is the BY-ID + // path — the payload carries `id` and no `where` — so the answer is the + // post-write record; the count limb belongs to the predicate path. Pinned + // here rather than cast away, because "the by-id result is a RECORD that + // keeps the flagged field" is exactly what this case is about. + expect(typeof updated).toBe('object'); + expect(updated).not.toBeNull(); + const updatedRow = updated as Record; + expect(updatedRow.key).toBe(HASH); + expect(updatedRow.revoked).toBe(true); }); }); diff --git a/packages/objectql/src/multi-update-hook-key-divergence.test.ts b/packages/objectql/src/multi-update-hook-key-divergence.test.ts index 9abe179c3f..a6e24a732e 100644 --- a/packages/objectql/src/multi-update-hook-key-divergence.test.ts +++ b/packages/objectql/src/multi-update-hook-key-divergence.test.ts @@ -183,7 +183,15 @@ describe('[#14099] the mixed batch the card measured is refused', () => { it('names the object, the diverging key and the prescription', async () => { const { engine } = await mixedBatch(); - const err = await run(engine).catch((e) => e as MultiUpdateHookKeyDivergenceError); + // [#16231] The same two-armed `then` the case above uses, and now for a + // second reason: `update()` declares its result union, so a bare `.catch` + // types `err` as "the refusal OR whatever the write resolved" and every + // assertion below reads through that union. The refusal arm is the only one + // this case is about — an update that RESOLVED here is the defect. + const err = await run(engine).then( + () => { throw new Error('expected the batch to be refused'); }, + (e: unknown) => e as MultiUpdateHookKeyDivergenceError, + ); expect(err.object).toBe('task'); expect(err.keys).toEqual(['completed_at']); expect(err.rows).toBe(2); diff --git a/packages/objectql/src/search-companion-read-projection-conformance.test.ts b/packages/objectql/src/search-companion-read-projection-conformance.test.ts index cd1d666ec1..a0e55b1c36 100644 --- a/packages/objectql/src/search-companion-read-projection-conformance.test.ts +++ b/packages/objectql/src/search-companion-read-projection-conformance.test.ts @@ -352,8 +352,13 @@ describe.each([ it('door: the update response body', async () => { bindCompanionStamp(engine); const updated = await engine.update(CONTACT, { id: contactId, name: '张伟明' }); - expect(updated?.name).toBe('张伟明'); - expectNoCompanion(updated); + // [#16231] A by-id update answers the RECORD; the count limb is the + // predicate path's. Narrowed here so "the update response body" — the door + // this case is named for — is asserted as the row it is. + expect(typeof updated).toBe('object'); + const updatedRow = updated as Record | null; + expect(updatedRow?.name).toBe('张伟明'); + expectNoCompanion(updatedRow); }); it('door: records nested by expand', async () => { diff --git a/packages/plugins/plugin-approvals/src/record-lock-schedule-run.integration.test.ts b/packages/plugins/plugin-approvals/src/record-lock-schedule-run.integration.test.ts index 957170b887..75dbf2446e 100644 --- a/packages/plugins/plugin-approvals/src/record-lock-schedule-run.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/record-lock-schedule-run.integration.test.ts @@ -125,7 +125,7 @@ describe('an owning run and the approvals record lock (#3703 / #3712 / #3760)', await expect(writeAs(dataCtx)).resolves.toBeDefined(); const row = await engine.findOne('opportunity', { where: { id: oppId } }); - expect(row.amount).toBe(200); + expect(row!.amount).toBe(200); }); it('still blocks a DIFFERENT run', async () => { @@ -172,6 +172,6 @@ describe('an owning run and the approvals record lock (#3703 / #3712 / #3760)', await expect(writeAs(dataCtx)).resolves.toBeDefined(); const row = await engine.findOne('opportunity', { where: { id: oppId } }); - expect(row.amount).toBe(200); + expect(row!.amount).toBe(200); }); }); diff --git a/packages/rest/src/import-business-timezone.test.ts b/packages/rest/src/import-business-timezone.test.ts index 0f4441aa94..7780a08ce3 100644 --- a/packages/rest/src/import-business-timezone.test.ts +++ b/packages/rest/src/import-business-timezone.test.ts @@ -447,12 +447,12 @@ describe('POST /data/:object/import — the round trip a customer actually runs' const original = await engine.findOne('shift', { where: { id: '1' } }); const reimported = await engine.findOne('shift', { where: { id: '2' } }); - expect(storedInstant(reimported.scanned_at)).toBe(storedInstant(original.scanned_at)); - expect(storedInstant(reimported.scanned_at)).toBe(CROSS_MONTH_UTC); + expect(storedInstant(reimported!.scanned_at)).toBe(storedInstant(original!.scanned_at)); + expect(storedInstant(reimported!.scanned_at)).toBe(CROSS_MONTH_UTC); // Stated as the report does: the row is still in the month it was exported // from. Under the process clock it stored 2026-08-01T13:00Z on this host. - expect(storedInstant(reimported.scanned_at)).not.toBe('2026-08-01T13:00:00.000Z'); - expect(String(reimported.due)).toContain('2026-08-01'); + expect(storedInstant(reimported!.scanned_at)).not.toBe('2026-08-01T13:00:00.000Z'); + expect(String(reimported!.due)).toContain('2026-08-01'); }); it('the same file imported by a UTC tenant is a different instant — the zone decides, not the host', async () => { @@ -468,6 +468,6 @@ describe('POST /data/:object/import — the round trip a customer actually runs' ); expect(jsonRes._json).toMatchObject({ total: 1, ok: 1, errors: 0, created: 1 }); const stored = await engine.findOne('shift', { where: { id: '2' } }); - expect(storedInstant(stored.scanned_at)).toBe('2026-08-01T06:00:00.000Z'); + expect(storedInstant(stored!.scanned_at)).toBe('2026-08-01T06:00:00.000Z'); }); }); diff --git a/packages/runtime/src/seed-loader.test.ts b/packages/runtime/src/seed-loader.test.ts index 51cda6f0e4..2d3c840669 100644 --- a/packages/runtime/src/seed-loader.test.ts +++ b/packages/runtime/src/seed-loader.test.ts @@ -70,7 +70,13 @@ function createMockEngine(data: Record = {}): IDataEngine { }), delete: vi.fn(async (_objectName: string, options?: any) => { assertEngineDeleteDispatch(options); - return { deleted: 1 }; + // [#16231] `IDataEngine.delete` declares its two dispatch answers now — + // whether the by-id row was there, or the predicate path's affected + // count. This double used to answer an invented envelope that no driver + // and no engine has ever produced; nothing in the loader reads it, which + // is precisely why `Promise` of `any` let the fiction stand. One row + // removed, spelled the way the predicate exit spells it. + return 1; }), count: vi.fn(async (objectName: string) => (store[objectName] || []).length), aggregate: vi.fn(async () => []), diff --git a/packages/services/service-automation/src/flow-field-expression-scale.integration.test.ts b/packages/services/service-automation/src/flow-field-expression-scale.integration.test.ts index 9e6b64b3fe..bb7b5616bc 100644 --- a/packages/services/service-automation/src/flow-field-expression-scale.integration.test.ts +++ b/packages/services/service-automation/src/flow-field-expression-scale.integration.test.ts @@ -127,7 +127,7 @@ describe('flow-computed money lands within its declared scale (#11060, oracle fo const row = await quoteByTitle('rounded'); expect(row, 'the quote row must persist').toBeTruthy(); // The PERSISTED value — not the expression result. - expect(row.total).toBe(126000); + expect(row!.total).toBe(126000); }); it('the assignment surface computes the same rounded value (config.assignments → interpolate)', async () => { diff --git a/packages/services/service-automation/src/paused-run-visibility.test.ts b/packages/services/service-automation/src/paused-run-visibility.test.ts index bde2672ed9..63d0b1252f 100644 --- a/packages/services/service-automation/src/paused-run-visibility.test.ts +++ b/packages/services/service-automation/src/paused-run-visibility.test.ts @@ -475,7 +475,7 @@ describe('#8050 cold boot over the same sqlite file (full stack)', () => { where: { id: runId }, context: { isSystem: true } as never, }); expect(row, 'the suspension row must survive the restart').toBeTruthy(); - expect(row.status).toBe('paused'); + expect(row!.status).toBe('paused'); // RED on main: `[]` for both listings, `null` for the detail. const listed = await second.automation.listRuns('showcase_budget_approval', { status: 'paused' }); diff --git a/packages/services/service-automation/src/runas-attribution-contract.test.ts b/packages/services/service-automation/src/runas-attribution-contract.test.ts index b03b18b595..f31b5ec148 100644 --- a/packages/services/service-automation/src/runas-attribution-contract.test.ts +++ b/packages/services/service-automation/src/runas-attribution-contract.test.ts @@ -211,12 +211,12 @@ describe("runAs:'system' attribution contract — elevation decides authorizatio const controlRow = await taskByTitle('control'); // (a) the elevated path: a `runAs:'system'` flow updates the row. - automation.registerFlow('elevated_touch', elevatedUpdateFlow('elevated_touch', String(elevatedRow.id)) as any); + automation.registerFlow('elevated_touch', elevatedUpdateFlow('elevated_touch', String(elevatedRow!.id)) as any); const res = await automation.execute('elevated_touch', { ...OPERATOR }); expect(res.success, `run failed: ${JSON.stringify(res)}`).toBe(true); // (b) the control: the same write, plain user context, no elevation. - await ql.update('crm_task', { id: controlRow.id, status: 'done' }, { context: { ...OPERATOR } }); + await ql.update('crm_task', { id: controlRow!.id, status: 'done' }, { context: { ...OPERATOR } }); const afterElevated = await taskByTitle('elevated'); const afterControl = await taskByTitle('control'); @@ -225,12 +225,12 @@ describe("runAs:'system' attribution contract — elevation decides authorizatio // its writes would land unattributed and lean on the actor label instead. // They do not: the operator is stamped, and byte-identically to the // unelevated write. - expect(afterElevated.updated_by, 'the elevated run must stamp the triggering operator').toBe('usr_operator'); - expect(afterElevated.updated_by, 'elevated attribution must equal the plain user path').toBe(afterControl.updated_by); + expect(afterElevated!.updated_by, 'the elevated run must stamp the triggering operator').toBe('usr_operator'); + expect(afterElevated!.updated_by, 'elevated attribution must equal the plain user path').toBe(afterControl!.updated_by); // The column MOVED off the creator — the assertion above is about this // update, not about the insert that seeded the row. - expect(afterElevated.created_by, 'the original creator is untouched').toBe('usr_creator'); - expect(afterElevated.status, 'the run must actually have written').toBe('done'); + expect(afterElevated!.created_by, 'the original creator is untouched').toBe('usr_creator'); + expect(afterElevated!.status, 'the run must actually have written').toBe('done'); // …and the envelope the audit writers read carries BOTH: elevation on // `isSystem` (authorization) and the operator on `userId` (attribution), @@ -254,7 +254,7 @@ describe("runAs:'system' attribution contract — elevation decides authorizatio // There is no operator to carry, so the user column stays NULL — ADR-0118 // D1 forbids a sentinel or pseudo-user standing in for one. - expect(row.created_by ?? null, 'a user-less run has no operator to stamp').toBeNull(); + expect(row!.created_by ?? null, 'a user-less run has no operator to stamp').toBeNull(); // …and the actor label is what keeps the write attributable anyway. This // is the half of the old prose that was TRUE — it was only ever true here. diff --git a/packages/services/service-automation/src/runas-system-stamping.integration.test.ts b/packages/services/service-automation/src/runas-system-stamping.integration.test.ts index 1671325617..0ec2975573 100644 --- a/packages/services/service-automation/src/runas-system-stamping.integration.test.ts +++ b/packages/services/service-automation/src/runas-system-stamping.integration.test.ts @@ -134,9 +134,9 @@ describe("runAs:'system' create_record stamps organization_id / owner_id / creat expect(row, 'the sweep must have created the row').toBeTruthy(); // The issue's step 2, inverted: the three platform columns are all // non-NULL, and each equals what the trigger context knew. - expect(row.created_by, 'created_by must be the triggering user').toBe('usr_admin'); - expect(row.owner_id, 'owner_id must be the acting user (the runAs:user default, restored)').toBe('usr_admin'); - expect(row.organization_id, "organization_id must be the trigger context's org").toBe('org_1'); + expect(row!.created_by, 'created_by must be the triggering user').toBe('usr_admin'); + expect(row!.owner_id, 'owner_id must be the acting user (the runAs:user default, restored)').toBe('usr_admin'); + expect(row!.organization_id, "organization_id must be the trigger context's org").toBe('org_1'); }); it("flow-authored ownership wins: an explicit `fields.owner_id` is never overwritten", async () => { @@ -153,10 +153,10 @@ describe("runAs:'system' create_record stamps organization_id / owner_id / creat // ADR-0073 D3 — flow logic sets ownership explicitly; the stamp is the // fill-only default underneath it, exactly like the security middleware's // own "empty means stamp" rule on the user path. - expect(row.owner_id).toBe('usr_assignee'); + expect(row!.owner_id).toBe('usr_assignee'); // Attribution is untouched by the ownership choice. - expect(row.created_by).toBe('usr_admin'); - expect(row.organization_id).toBe('org_1'); + expect(row!.created_by).toBe('usr_admin'); + expect(row!.organization_id).toBe('org_1'); }); it('a USER-LESS system run (schedule shape) stamps nothing — and that is the contract, not a gap', async () => { @@ -175,12 +175,12 @@ describe("runAs:'system' create_record stamps organization_id / owner_id / creat // banned alternative. ADR-0073's automation principal (a real identity for // these runs) is M2, gated on its first consumer. Provenance still names // the writer: the run's `svc:flow:*` actor label and flowRunId (#4366/#3712). - expect(row.created_by ?? null).toBeNull(); - expect(row.owner_id ?? null).toBeNull(); + expect(row!.created_by ?? null).toBeNull(); + expect(row!.owner_id ?? null).toBeNull(); // The schedule trigger supplies no org today, so there is nothing to // stamp; a schedule-run in an org-partitioned deployment needs the flow's // `fields` to place rows (or a future org-aware schedule binding). - expect(row.organization_id ?? null).toBeNull(); + expect(row!.organization_id ?? null).toBeNull(); }); it("REGRESSION: the runAs:'user' path is unchanged — audit + org stamps still land", async () => { @@ -192,8 +192,8 @@ describe("runAs:'system' create_record stamps organization_id / owner_id / creat expect(res.success, `run failed: ${JSON.stringify(res)}`).toBe(true); const row = await taskByTitle('renew D'); - expect(row.created_by).toBe('usr_admin'); - expect(row.organization_id).toBe('org_1'); + expect(row!.created_by).toBe('usr_admin'); + expect(row!.organization_id).toBe('org_1'); // (`owner_id` on the user path is the security middleware's stamp; the // full-security composition below covers it. This harness pins that the // audit + tenant machinery behave identically before and after the fix.) @@ -273,19 +273,19 @@ describe('the #5494 admission flip: row content, not caller, decides (real Secur expect(res.success, `run failed: ${JSON.stringify(res)}`).toBe(true); const row = await rowByTitle('flip A'); - expect(row.created_by).toBe('usr_member'); - expect(row.owner_id).toBe('usr_member'); - expect(row.organization_id).toBe('org_1'); + expect(row!.created_by).toBe('usr_member'); + expect(row!.owner_id).toBe('usr_member'); + expect(row!.organization_id).toBe('org_1'); // Step 3, inverted: the same member — no elevation, no transfer grant — // repairs and completes the record the sweep made for them. await expect( - ql.update('crm_task', { id: row.id, status: 'done' }, { context: { ...MEMBER_CTX } }), + ql.update('crm_task', { id: row!.id, status: 'done' }, { context: { ...MEMBER_CTX } }), ).resolves.toBeDefined(); - expect((await rowByTitle('flip A')).status).toBe('done'); + expect((await rowByTitle('flip A'))!.status).toBe('done'); await expect( - ql.delete('crm_task', { where: { id: row.id }, context: { ...MEMBER_CTX } }), + ql.delete('crm_task', { where: { id: row!.id }, context: { ...MEMBER_CTX } }), ).resolves.toBeDefined(); expect(await rowByTitle('flip A')).toBeFalsy(); }); @@ -298,7 +298,7 @@ describe('the #5494 admission flip: row content, not caller, decides (real Secur const res = await automation.execute('night_sweep', { event: 'schedule', params: {} } as any); expect(res.success).toBe(true); const row = await rowByTitle('flip B'); - expect(row.created_by ?? null).toBeNull(); + expect(row!.created_by ?? null).toBeNull(); // The SAME member context that succeeded above is denied here: the only // difference between the two attempts is the row's stamp columns — the @@ -323,10 +323,10 @@ describe('the #5494 admission flip: row content, not caller, decides (real Secur developerMessage: expect.stringContaining('(row-level security)'), }; await expect( - ql.update('crm_task', { id: row.id, status: 'done' }, { context: { ...MEMBER_CTX } }), + ql.update('crm_task', { id: row!.id, status: 'done' }, { context: { ...MEMBER_CTX } }), ).rejects.toMatchObject(rowLevelDenial); await expect( - ql.delete('crm_task', { where: { id: row.id }, context: { ...MEMBER_CTX } }), + ql.delete('crm_task', { where: { id: row!.id }, context: { ...MEMBER_CTX } }), ).rejects.toMatchObject(rowLevelDenial); }); }); diff --git a/packages/spec/src/contracts/data-engine.test.ts b/packages/spec/src/contracts/data-engine.test.ts index 07299694c5..4aafb887b4 100644 --- a/packages/spec/src/contracts/data-engine.test.ts +++ b/packages/spec/src/contracts/data-engine.test.ts @@ -28,7 +28,14 @@ describe('Data Engine Contract', () => { findOne: async (_objectName, _query?) => null, insert: async (_objectName, data, _options?) => data, update: async (_objectName, data, _options?) => data, - delete: async (_objectName, _options?) => { return { deleted: 1 }; }, + // [#16231] `delete` declares `Promise[boolean | number]` — whether the + // by-id row was there, or how many rows a predicate delete removed. + // This fake used to answer `{ deleted: 1 }`, a shape NO driver and NO + // engine has ever produced; `Promise[any]` admitted it, and a contract + // test modelling a shape the contract does not have is the drift the + // declaration exists to stop. Spelled with square brackets in this + // comment only where a generic would otherwise be typed out. + delete: async (_objectName, _options?) => 1, count: async (_objectName, _query?) => 0, aggregate: async (_objectName, _query) => [], }; @@ -53,7 +60,7 @@ describe('Data Engine Contract', () => { return data; }, update: async (_obj, data) => data, - delete: async () => ({ deleted: 1 }), + delete: async () => 1, count: async () => store.length, aggregate: async () => [], }; @@ -92,7 +99,7 @@ describe('Data Engine Contract', () => { }, insert: async (_obj, data) => data, update: async (_obj, data) => data, - delete: async () => ({}), + delete: async () => true, count: async (_obj, _query, options) => { seen.push({ method: 'count', isSystem: options?.context?.isSystem }); return 0; @@ -123,7 +130,7 @@ describe('Data Engine Contract', () => { findOne: async () => null, insert: async (_obj, data) => data, update: async (_obj, data) => data, - delete: async () => ({}), + delete: async () => true, count: async () => 0, aggregate: async () => [], vectorFind: async (_objectName, _vector, options?) => { @@ -156,7 +163,7 @@ describe('Data Engine Contract', () => { findOne: async () => null, insert: async (_obj, data) => data, update: async (_obj, data) => data, - delete: async () => ({}), + delete: async () => true, count: async () => 0, aggregate: async () => [], execute: async (command, options?) => { From e8faeb07361b7cf3839567e14b190ac9bdbf027a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 03:51:55 +0000 Subject: [PATCH 04/10] chore(engine): changeset for the verb result declarations Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- .changeset/engine-verb-result-declarations.md | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .changeset/engine-verb-result-declarations.md diff --git a/.changeset/engine-verb-result-declarations.md b/.changeset/engine-verb-result-declarations.md new file mode 100644 index 0000000000..4f3d30cbbe --- /dev/null +++ b/.changeset/engine-verb-result-declarations.md @@ -0,0 +1,27 @@ +--- +"@objectstack/spec": minor +"@objectstack/objectql": minor +"@objectstack/metadata": patch +"@objectstack/metadata-protocol": patch +"@objectstack/plugin-auth": patch +--- + +feat(engine)!: `findOne`, `update` and `delete` declare what they answer, and their hook seams are guarded (#16231) + + + +**BREAKING** on three published `.d.ts` surfaces. `ObjectQL.findOne`, `ObjectQL.update` and `ObjectQL.delete` — and the `IDataEngine` / `IScopedObjectRepository` contracts they implement — declared `Promise` and now declare the answers they have always given: + +- `findOne` → `Promise | null>` +- `update` → `Promise | number | null>` +- `delete` → `Promise` + +`any` is assignable to everything and admits every property read, so TypeScript consumers of these three methods can stop compiling — most often on the null check the declaration now demands. Shipped as `minor` under the repo's launch-window convention, in which `major` is refused by `check-changeset-no-major` and breaking-ness is carried by this banner plus the ADR-0087 disposition rather than by the level. The governing text is the **WHICH LEVEL** maintainer ruling of 2026-09-04 (decision batch #35, on #15294) recorded at `.github/workflows/pr-automation.yml`; `AGENTS.md`'s "a bug fix in a released package takes a patch changeset — never none" is the floor against `none` and was rejected as the ceiling here, because this PR also widens `@objectstack/objectql`'s index with new exported symbols, which that ruling puts at `minor` on its own. + +**Why.** `engine.ts` has four `return hookContext.result` sites, one per hook-bearing verb. #15823 closed the `find()` one — an `afterFind` handler that replaced the array made a method declared `Promise` resolve to an envelope, silently — and recorded that it could close only that one: the other three declared `Promise` and so carried no declaration a handler could break. A guard cannot exist before a declaration worth guarding does. The maintainer ruled the gap shut (option A, 2026-09-07, director seat summon #17, decision batch #2; option B "declare only, no enforcement" and option C "record `any` as intended" were refused). + +The shapes are read off the driver contract each engine exit delegates to, not invented: `driver.findOne` and the by-id `driver.update` declare `Record | null`, `driver.delete` declares `boolean`, and the predicate exits `driver.updateMany` / `driver.deleteMany` declare the affected-row `number` a bulk write resolves (#4639). Row FIELD values stay erased (`Record`), which is #15823's precedent extended exactly rather than softened: `find()` declares `Promise`, so the CONTAINER is the contract and the rows inside it are `any`. It is also the only spelling that can state "record or null" at all, since `any | null` collapses to `any`. + +**What is enforced now.** Each seam re-checks `hookContext.result` against its declaration immediately after the `after*` dispatch and ahead of the consumers that already assume the shape, and refuses a value outside it with a registered ADR-0112 envelope — `FIND_ONE_HOOK_RESULT_NOT_RECORD`, `UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE`, `DELETE_HOOK_RESULT_NOT_WRITE_SHAPE`, all `500`, all branchable on `error.code`. Shaping stays legal exactly as it does on `find()`: a handler may mutate what it is handed, drop keys, or assign a different value of a declared shape. The falsy answers are legal and deliberately so — `null` from `findOne`, `null` or a count from `update`, and `false` or `0` from `delete`, the two most ordinary answers that verb gives. + +**Who has to change something.** A TypeScript consumer that reads a field off `findOne`'s result without a null check, or off `update`'s result without separating the by-id record from the predicate count. In this repository that was measured before anything moved, at the maintainer's instruction: 18 files and 92 compile errors, all repaired here. A host that installs `after*` handlers assigning a value outside the declaration now receives a refusal where it previously received a silently wrong shape. From 4acb9ad6f0f3b97e6951ad5c8220c32c7bd878e8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 04:01:41 +0000 Subject: [PATCH 05/10] chore(spec): regenerate the error-code ledger docs for the three new codes Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- content/docs/references/api/contract.mdx | 5 ++++- content/docs/references/api/error-code-ledger.mdx | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/content/docs/references/api/contract.mdx b/content/docs/references/api/contract.mdx index d8ce065810..568a96e64f 100644 --- a/content/docs/references/api/contract.mdx +++ b/content/docs/references/api/contract.mdx @@ -27,7 +27,7 @@ const result = ApiErrorSchema.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +308 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) | +| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +311 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) | | **declaredCode** | `string` | optional | The producer-declared code, verbatim, when it is not a member of the closed `code` vocabulary — the open, author-authored channel (app-specific spellings; ADR-0112) | | **message** | `string` | ✅ | Readable error message | | **userMessage** | `string` | optional | Producer-marked user-facing refusal text, verbatim. Present exactly when the producer opted in at throw time; consumers render it to end users and keep their generic substitution for anything unmarked. Status-agnostic; never replaces `message`. | @@ -134,6 +134,7 @@ const result = ApiErrorSchema.parse(data); * `DATASET_INVALID` * `DATASOURCE_ADMIN_ERROR` * `DELEGABLE_SCOPE_FAILED` +* `DELETE_HOOK_RESULT_NOT_WRITE_SHAPE` * `DELIVERY_NEVER_SENT` * `DELIVERY_NOT_ELIGIBLE` * `DESTRUCTIVE_CHANGE` @@ -183,6 +184,7 @@ const result = ApiErrorSchema.parse(data); * `FILTER_TOKEN_UNKNOWN` * `FILTER_TOKEN_UNRESOLVED` * `FIND_HOOK_RESULT_NOT_ARRAY` +* `FIND_ONE_HOOK_RESULT_NOT_RECORD` * `FLOW_CONVERSION_CONFLICT` * `FLOW_DISABLED` * `FLOW_FAILED` @@ -343,6 +345,7 @@ const result = ApiErrorSchema.parse(data); * `UNSUPPORTED` * `UNSUPPORTED_QUERY_PARAM` * `UNSUPPORTED_TRANSFORM` +* `UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE` * `UPDATE_ID_MISMATCH` * `UPLOAD_SESSION_EXPIRED` * `UPLOAD_SESSION_NOT_FOUND` diff --git a/content/docs/references/api/error-code-ledger.mdx b/content/docs/references/api/error-code-ledger.mdx index 5a33e412ce..1d10e12e60 100644 --- a/content/docs/references/api/error-code-ledger.mdx +++ b/content/docs/references/api/error-code-ledger.mdx @@ -289,6 +289,7 @@ const result = ErrorCode.parse(data); * `DATASET_INVALID` * `DATASOURCE_ADMIN_ERROR` * `DELEGABLE_SCOPE_FAILED` +* `DELETE_HOOK_RESULT_NOT_WRITE_SHAPE` * `DELIVERY_NEVER_SENT` * `DELIVERY_NOT_ELIGIBLE` * `DESTRUCTIVE_CHANGE` @@ -338,6 +339,7 @@ const result = ErrorCode.parse(data); * `FILTER_TOKEN_UNKNOWN` * `FILTER_TOKEN_UNRESOLVED` * `FIND_HOOK_RESULT_NOT_ARRAY` +* `FIND_ONE_HOOK_RESULT_NOT_RECORD` * `FLOW_CONVERSION_CONFLICT` * `FLOW_DISABLED` * `FLOW_FAILED` @@ -498,6 +500,7 @@ const result = ErrorCode.parse(data); * `UNSUPPORTED` * `UNSUPPORTED_QUERY_PARAM` * `UNSUPPORTED_TRANSFORM` +* `UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE` * `UPDATE_ID_MISMATCH` * `UPLOAD_SESSION_EXPIRED` * `UPLOAD_SESSION_NOT_FOUND` From 6c60ea0458a7754935f059933337d6c852ce1b25 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 04:15:16 +0000 Subject: [PATCH 06/10] fix(engine): keep the tracker id out of the update refusal's runtime prose Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- packages/objectql/src/verb-hook-result-shape.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/objectql/src/verb-hook-result-shape.ts b/packages/objectql/src/verb-hook-result-shape.ts index eb59c5c9e8..6bab1ce86b 100644 --- a/packages/objectql/src/verb-hook-result-shape.ts +++ b/packages/objectql/src/verb-hook-result-shape.ts @@ -236,7 +236,10 @@ export class UpdateHookResultNotWriteShapeError extends Error { `'update()' declares 'Promise | number | null>' — the two answers its ` + `two dispatch paths give. A BY-ID write resolves the post-write record (or 'null' when the ` + `readback leaves the caller's row scope); a PREDICATE write resolves the affected-row COUNT ` + - `and names no row (#4639). A '${info.event}' handler may SHAPE what it is handed — mutate ` + + // The predicate write's affected-count contract is stated in this module's + // header; the tracker id stays OUT of the runtime string, which reaches + // authors, operators and generated surfaces that cannot resolve one. + `and names no row. A '${info.event}' handler may SHAPE what it is handed — mutate ` + `the record in place, drop keys, assign a different RECORD — but replacing it with a shape ` + `outside that union is refused, because the declaration is the contract. To REFUSE the ` + `write, throw from the handler. Branch on ` + From c5935b2445a301181271767e9b2718242eec1dd9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 04:28:00 +0000 Subject: [PATCH 07/10] fix(engine): the seam refusal names the seam, not a culprit; repair the off-contract driver doubles Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- .../objectql/src/engine-filter-tokens.test.ts | 6 +++- .../src/engine-verb-hook-result-shape.test.ts | 9 +++++ packages/objectql/src/engine.test.ts | 14 +++++--- .../objectql/src/plugin.integration.test.ts | 14 ++++++-- .../objectql/src/verb-hook-result-shape.ts | 34 ++++++++++++++++--- 5 files changed, 65 insertions(+), 12 deletions(-) diff --git a/packages/objectql/src/engine-filter-tokens.test.ts b/packages/objectql/src/engine-filter-tokens.test.ts index f6d5480766..98f33ec09b 100644 --- a/packages/objectql/src/engine-filter-tokens.test.ts +++ b/packages/objectql/src/engine-filter-tokens.test.ts @@ -61,7 +61,11 @@ function makeDriver() { update: vi.fn(async (_o: string, id: any, d: any) => { seen.updateId = id; return { id, ...d }; }), updateMany: vi.fn(async (_o: string, ast: any) => { seen.updateManyAst = ast; return { modified: 0 }; }), delete: vi.fn(async (_o: string, id: any) => { seen.deleteId = id; return true; }), - deleteMany: vi.fn(async (_o: string, ast: any) => { seen.deleteManyAst = ast; return { deleted: 0 }; }), + // [#16231] `IDataDriver.deleteMany` declares `Promise` — the + // affected-row count. This double answered an invented `{ deleted: n }` + // envelope that no driver produces; `Promise` on the engine door + // admitted it all the way out to the caller. + deleteMany: vi.fn(async (_o: string, ast: any) => { seen.deleteManyAst = ast; return 0; }), }; return { driver, seen }; } diff --git a/packages/objectql/src/engine-verb-hook-result-shape.test.ts b/packages/objectql/src/engine-verb-hook-result-shape.test.ts index a5c17dd0c1..b0a1d1baab 100644 --- a/packages/objectql/src/engine-verb-hook-result-shape.test.ts +++ b/packages/objectql/src/engine-verb-hook-result-shape.test.ts @@ -197,6 +197,15 @@ describe('#16231 findOne — a value outside the declaration is refused', () => // supported spellings — `null` for no record, a throw to refuse the read. expect(error.developerMessage).toContain("assign 'null'"); expect(error.developerMessage).toContain('throw from the handler'); + // ⚠️ …and it names BOTH sources, because the seam sees a driver's answer as + // well as a handler's. The user-facing sentence therefore states what is + // AT the seam and accuses nobody: a `driver.update` double resolving + // `undefined` is the measured case this wording exists for, and a sentence + // reading "your handler replaced it" would have sent four repairs in this + // repository to the wrong file. + expect(error.developerMessage).toContain('off its own contract'); + expect(error.message).toContain("after the 'afterFind' dispatch"); + expect(error.message).not.toContain('handler replaced'); }); it('`undefined` is refused — decided here, and it is NOT the same as `null`', async () => { diff --git a/packages/objectql/src/engine.test.ts b/packages/objectql/src/engine.test.ts index 25173dcafd..abe62bb642 100644 --- a/packages/objectql/src/engine.test.ts +++ b/packages/objectql/src/engine.test.ts @@ -83,8 +83,14 @@ describe('ObjectQL Engine', () => { find: vi.fn().mockResolvedValue([{ id: '1', name: 'Test Record' }]), findOne: vi.fn(), create: vi.fn().mockResolvedValue({ id: '1', success: true }), - update: vi.fn(), - delete: vi.fn(), + // [#16231] `IDataDriver.update` declares `Promise | null>` + // and `IDataDriver.delete` declares `Promise`. A bare `vi.fn()` + // resolves `undefined`, which is neither — and the engine used to hand that + // straight out under `Promise`. The engine's seam guard refuses it now, + // so the doubles answer the shapes their own contract declares. Individual + // cases still override these with `mockResolvedValue`. + update: vi.fn().mockResolvedValue({ id: '1' }), + delete: vi.fn().mockResolvedValue(true), count: vi.fn(), capabilities: {} as any // Simplified } as unknown as IDataDriver; @@ -96,8 +102,8 @@ describe('ObjectQL Engine', () => { find: vi.fn().mockResolvedValue([{ id: '2', name: 'Mongo Record' }]), findOne: vi.fn(), create: vi.fn().mockResolvedValue({ id: '2', success: true }), - update: vi.fn(), - delete: vi.fn(), + update: vi.fn().mockResolvedValue({ id: '2' }), + delete: vi.fn().mockResolvedValue(true), count: vi.fn(), capabilities: {} as any } as unknown as IDataDriver; diff --git a/packages/objectql/src/plugin.integration.test.ts b/packages/objectql/src/plugin.integration.test.ts index 37ced13f10..3c846fa7f7 100644 --- a/packages/objectql/src/plugin.integration.test.ts +++ b/packages/objectql/src/plugin.integration.test.ts @@ -1524,7 +1524,12 @@ describe('ObjectQLPlugin - Metadata Service Integration', () => { findOne: async () => null, create: async (_o: string, d: any) => ({ id: 'rec-1', ...d }), update: async (_o: string, _i: any, d: any) => ({ id: _i, ...d }), - updateMany: async (_o: string, _ast: any, d: any) => { bulkUpdates.push({ ...d }); return [{ ...d }]; }, + // [#16231] `IDataDriver.updateMany` declares `Promise` — the + // affected-row count a predicate write resolves. This double answered an + // ARRAY of rows, a shape no driver produces and one the engine used to + // hand straight out under `Promise`; the assertions here read the + // captured payload, never the return, so the drift was invisible. + updateMany: async (_o: string, _ast: any, d: any) => { bulkUpdates.push({ ...d }); return 1; }, delete: async () => true, syncSchema: async () => {}, }; await kernel.use({ @@ -1599,7 +1604,12 @@ describe('ObjectQLPlugin - Metadata Service Integration', () => { findOne: async () => null, create: async (_o: string, d: any) => ({ id: 'rec-1', ...d }), update: async (_o: string, _i: any, d: any) => ({ id: _i, ...d }), - updateMany: async (_o: string, _ast: any, d: any) => { bulkUpdates.push({ ...d }); return [{ ...d }]; }, + // [#16231] `IDataDriver.updateMany` declares `Promise` — the + // affected-row count a predicate write resolves. This double answered an + // ARRAY of rows, a shape no driver produces and one the engine used to + // hand straight out under `Promise`; the assertions here read the + // captured payload, never the return, so the drift was invisible. + updateMany: async (_o: string, _ast: any, d: any) => { bulkUpdates.push({ ...d }); return 1; }, delete: async () => true, syncSchema: async () => {}, }; await kernel.use({ diff --git a/packages/objectql/src/verb-hook-result-shape.ts b/packages/objectql/src/verb-hook-result-shape.ts index 6bab1ce86b..99617e572d 100644 --- a/packages/objectql/src/verb-hook-result-shape.ts +++ b/packages/objectql/src/verb-hook-result-shape.ts @@ -173,7 +173,10 @@ export class FindOneHookResultNotRecordError extends Error { this.developerMessage = `'findOne()' declares 'Promise | null>' — the ONE record the query ` + `selects, or 'null' — and its callers branch on 'if (!row)' rather than on a container ` + - `check. A '${info.event}' handler may SHAPE that record: mutate it in place, drop keys, ` + + `check. TWO things can put another shape here: a '${info.event}' handler that assigned ` + + `one, or a driver whose 'findOne' answered off its own contract ` + + `('Promise | null>'). A '${info.event}' handler may SHAPE that ` + + `record: mutate it in place, drop keys, ` + `or assign a different RECORD built from it. Replacing it with something that is neither ` + `a record nor 'null' is refused. To answer no record, assign 'null'. To REFUSE the read, ` + `throw from the handler — that is the supported way for a '${info.event}' guard to say no. ` + @@ -239,7 +242,10 @@ export class UpdateHookResultNotWriteShapeError extends Error { // The predicate write's affected-count contract is stated in this module's // header; the tracker id stays OUT of the runtime string, which reaches // authors, operators and generated surfaces that cannot resolve one. - `and names no row. A '${info.event}' handler may SHAPE what it is handed — mutate ` + + `and names no row. TWO things can put another shape here: a '${info.event}' handler that ` + + `assigned one, or a driver whose 'update' / 'updateMany' answered off its own contract ` + + `('Promise | null>' and 'Promise'). A '${info.event}' ` + + `handler may SHAPE what it is handed — mutate ` + `the record in place, drop keys, assign a different RECORD — but replacing it with a shape ` + `outside that union is refused, because the declaration is the contract. To REFUSE the ` + `write, throw from the handler. Branch on ` + @@ -297,8 +303,15 @@ export class DeleteHookResultNotWriteShapeError extends Error { this.developerMessage = `'delete()' declares 'Promise' — the two answers its two dispatch paths ` + `give. A BY-ID delete resolves whether the row was there ('false' is a real answer, and ` + - `'@objectstack/metadata-protocol' turns it into a 404); a PREDICATE delete resolves the ` + - `affected-row COUNT ('0' is a real answer). An '${info.event}' handler that wants to ` + + // ⛔ The metadata protocol is named without its quoted package specifier + // on purpose: `core-boundary.ratchet.test.ts` walks `core.ts`'s closure + // and flags a QUOTED forbidden package name anywhere in a file's text, + // import or not (ADR-0076 D2), and this module is inside that closure. + `the metadata protocol layer turns it into a 404); a PREDICATE delete resolves the ` + + `affected-row COUNT ('0' is a real answer). TWO things can put another shape here: an ` + + `'${info.event}' handler that assigned one, or a driver whose 'delete' / 'deleteMany' ` + + `answered off its own contract ('Promise' and 'Promise'). An ` + + `'${info.event}' handler that wants to ` + `REFUSE a delete throws from the handler; a delete has no post-state to reshape, so ` + `replacing 'ctx.result' with a record or an envelope is refused. Branch on ` + `\`code === '${DELETE_HOOK_RESULT_NOT_WRITE_SHAPE_CODE}'\` (ADR-0112) to detect this.`; @@ -312,6 +325,17 @@ export class DeleteHookResultNotWriteShapeError extends Error { /** * The user-facing sentence, one composer for all three refusals. * + * ⚠️ It names the SEAM and never a culprit, and that is a correction to the + * shape `find()`'s refusal could afford. On `find()` the value at the seam + * comes from `driver.find`, which every driver answers with an array, so + * "your handler replaced it" was true whenever the check fired. These three + * verbs have exits that can answer off-contract themselves — a `driver.update` + * double resolving `undefined` is the measured case, found by this very guard + * on four doubles in this repository — so a sentence blaming the handler would + * misattribute the fault on the most likely path. It states what is there + * after the dispatch; {@link FindOneHookResultNotRecordError.developerMessage} + * and its siblings name BOTH sources for the reader who has to go fix one. + * * ⛔ It must not begin with a SQL verb — `@objectstack/rest`'s importer runs row * errors through `sanitizeRowError`, whose SQL backstop replaces any message * STARTING with `insert`/`update`/`delete` with generic text. That constraint @@ -336,7 +360,7 @@ function refusalSentence( ? observed : `${'aeiou'.includes(observed[0]) ? 'an' : 'a'} ${observed}`; return ( - `Refusing the '${verb}' on '${object}': its '${event}' handler replaced 'ctx.result' with ` + + `Refusing the '${verb}' on '${object}': after the '${event}' dispatch 'ctx.result' is ` + `${what}, and '${verb}()' answers ${declared}. Shaping what it answers is supported; ` + `replacing it with another shape is not.` ); From 951369f510cd5231c93f6a18ede676fce016532f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 13:02:18 +0000 Subject: [PATCH 08/10] =?UTF-8?q?fix(changeset):=20grade=20the=20three=20r?= =?UTF-8?q?epaired=20consumers=20`minor`,=20as=20the=20clause-=E2=91=A1=20?= =?UTF-8?q?declaration=20requires?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Check Changeset`'s level axis is red on this PR: it declares `Clause-②: yes` and grades three packages whose `src/**` the diff moves at `patch`. A purely additive widening of a published package's public surface takes at least `minor` (maintainer ruling 2026-09-04, decision batch #35, on #15294). `@objectstack/metadata` and `@objectstack/metadata-protocol` are the two the gate can name. `@objectstack/plugin-auth` rises for the same reason and is NOT graded by the gate: `PUBLISHED_SOURCE_PATH` is anchored `^packages/([^/]+)/src/` and this package's changed source is `packages/plugins/plugin-auth/src/` — one directory level deeper, so it never enters the gate's "grown" set. That is the blind spot carded as #16713. The level floor comes from the act the PR declares, not from what the instrument happens to measure. No source, test or config byte moves; the level axis is the only thing this commit answers for. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- .changeset/engine-verb-result-declarations.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.changeset/engine-verb-result-declarations.md b/.changeset/engine-verb-result-declarations.md index 4f3d30cbbe..5ba7b7a49d 100644 --- a/.changeset/engine-verb-result-declarations.md +++ b/.changeset/engine-verb-result-declarations.md @@ -1,9 +1,9 @@ --- "@objectstack/spec": minor "@objectstack/objectql": minor -"@objectstack/metadata": patch -"@objectstack/metadata-protocol": patch -"@objectstack/plugin-auth": patch +"@objectstack/metadata": minor +"@objectstack/metadata-protocol": minor +"@objectstack/plugin-auth": minor --- feat(engine)!: `findOne`, `update` and `delete` declare what they answer, and their hook seams are guarded (#16231) From a996f102dacf3d1bcda7e9894f75be78127539c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 13:18:52 +0000 Subject: [PATCH 09/10] test(spec): pin the three narrowed verb declarations, not only their seam guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ruling A has two halves — the declarations and the seam guards — and only the guard half was pinned. Reverting `findOne` / `update` / `delete` to `Promise` while keeping the guards reddened nothing: every consumer repair the census produced compiles identically against `any`, so those repairs record that a narrowing once happened, not that it still holds. That is ADR-0049's enforce-or-remove target. Three `@ts-expect-error` cases under `check:test-typecheck` close it, on the mechanism the neighbouring #12248 block already relies on: each directive is resolved by tsc today, so a widening back to `Promise` leaves it UNUSED, which is itself an error in a file whose debt ledger is exact. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- .../spec/src/contracts/data-engine.test.ts | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/packages/spec/src/contracts/data-engine.test.ts b/packages/spec/src/contracts/data-engine.test.ts index 4aafb887b4..b44ad7fc5a 100644 --- a/packages/spec/src/contracts/data-engine.test.ts +++ b/packages/spec/src/contracts/data-engine.test.ts @@ -176,6 +176,76 @@ describe('Data Engine Contract', () => { expect(result.raw).toBe(true); }); }); + /** + * [#16231] The three narrowed verb declarations are PINNED, not only guarded. + * + * The maintainer ruling (option A, 2026-09-07, director seat summon #17, + * decision batch #2) has two halves: `findOne` / `update` / `delete` DECLARE + * what they answer, and each hook seam GUARDS `hookContext.result` against + * that declaration. The guard half is pinned by + * `packages/objectql/src/engine-verb-hook-result-shape.test.ts`. The + * DECLARATION half was pinned nowhere: reverting these three members to + * `Promise[any]` while leaving the guards in place reddened nothing in the + * repository. Every consumer repair the census produced (`if (!row)`, + * `typeof x === 'number' ? … : …`, `result!.assignee`) compiles identically + * against `any` — `any` admits every property read and is assignable in both + * directions — so those repairs are evidence that a narrowing once happened, + * never that it is still in force. A declared contract nothing can fail is + * ADR-0049's enforce-or-remove target; these three cases close that. + * + * ## The channel, and why it cannot be vitest + * + * These are COMPILE facts. vitest strips types through esbuild without ever + * resolving them, so all three cases run green over a reverted declaration. + * The check that actually reads them is `check:test-typecheck` + * (`tsconfig.test.json`, #5286) — the gate whose absence had made eighteen + * `@ts-expect-error` pins across this repo phantom checks. + * + * ## How a revert is caught + * + * The mechanism is the one the `#12248` block below already records: every + * directive here is RESOLVED by tsc today, so widening a member back to + * `Promise[any]` does not make a case fail an assertion — it makes the + * directive UNUSED, and an unused `@ts-expect-error` is itself an error + * (TS2578) in a file whose debt ledger is exact. + * + * ⛔ No new engine double. Each case reads the DECLARED member type through + * `IDataEngine[…]`, so nothing here can drift from the contract by being + * modelled beside it. + */ + describe('the narrowed verb declarations are pinned, not just guarded (#16231)', () => { + type FindOneAnswer = Awaited>; + type UpdateAnswer = Awaited>; + type DeleteAnswer = Awaited>; + + it('findOne: an un-null-checked property read does not compile', () => { + // The null limb is the entire point of the declaration — `findOne` reads + // the ONE record the query selects, or nothing — and TS18047 here is the + // dominant signature of this card's 92-error consumer census. + // @ts-expect-error - 'row' is possibly 'null' + const readsWithoutChecking = (row: FindOneAnswer): unknown => row.id; + expect(typeof readsWithoutChecking).toBe('function'); + }); + + it('update: the answer cannot enter a record slot unnarrowed', () => { + // TWO dispatch exits: the by-id post-write record (or `null`), and the + // affected-row COUNT a predicate write resolves (#4639). A consumer that + // wants the record has to separate the count limb first, which is what + // `any` let every call site skip. + // @ts-expect-error - 'number | null' is not assignable to a record slot + const intoRecordSlot = (written: UpdateAnswer): Record => written; + expect(typeof intoRecordSlot).toBe('function'); + }); + + it('delete: the answer is not a row and has no field to read', () => { + // `boolean | number` — whether the by-id row was there, or how many rows + // a predicate delete removed. The `{ deleted: n }` envelope this card + // found in four test doubles is the shape an unread declaration admits. + // @ts-expect-error - property does not exist on 'boolean | number' + const readsARow = (removed: DeleteAnswer): unknown => removed.id; + expect(typeof readsARow).toBe('function'); + }); + }); /** * [#5126] `WriteObservabilityOptions` is the IN-PROCESS write-options From 10d7a9f1d979bc4b1dbd5d9ae27e9f48121d8fa1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 13:21:58 +0000 Subject: [PATCH 10/10] docs(changeset): state the runtime FROM/TO per door, and name both sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The type FROM/TO was already per verb; the runtime half was one sentence for three doors and named only the handler. The refusals' own `developerMessage` names TWO sources — an `after*` handler that assigned an off-declaration value, and a driver whose exit answered off `IDataDriver` — and the second one is the source the seven test-double repairs in this PR actually came from, which is why the refusal sentence names the seam instead of accusing the handler. Three per-door lines now carry FROM (what the dispatch left, returned silently, and who read it first) to TO (the registered 500 code raised at that seam). Driver limbs cited are read off `packages/spec/src/contracts/data-driver.ts`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- .changeset/engine-verb-result-declarations.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.changeset/engine-verb-result-declarations.md b/.changeset/engine-verb-result-declarations.md index 5ba7b7a49d..300876eb9b 100644 --- a/.changeset/engine-verb-result-declarations.md +++ b/.changeset/engine-verb-result-declarations.md @@ -24,4 +24,12 @@ The shapes are read off the driver contract each engine exit delegates to, not i **What is enforced now.** Each seam re-checks `hookContext.result` against its declaration immediately after the `after*` dispatch and ahead of the consumers that already assume the shape, and refuses a value outside it with a registered ADR-0112 envelope — `FIND_ONE_HOOK_RESULT_NOT_RECORD`, `UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE`, `DELETE_HOOK_RESULT_NOT_WRITE_SHAPE`, all `500`, all branchable on `error.code`. Shaping stays legal exactly as it does on `find()`: a handler may mutate what it is handed, drop keys, or assign a different value of a declared shape. The falsy answers are legal and deliberately so — `null` from `findOne`, `null` or a count from `update`, and `false` or `0` from `delete`, the two most ordinary answers that verb gives. -**Who has to change something.** A TypeScript consumer that reads a field off `findOne`'s result without a null check, or off `update`'s result without separating the by-id record from the predicate count. In this repository that was measured before anything moved, at the maintainer's instruction: 18 files and 92 compile errors, all repaired here. A host that installs `after*` handlers assigning a value outside the declaration now receives a refusal where it previously received a silently wrong shape. +**Who has to change something, on the TYPE axis.** A TypeScript consumer that reads a field off `findOne`'s result without a null check, or off `update`'s result without separating the by-id record from the predicate count. In this repository that was measured before anything moved, at the maintainer's instruction: 18 files and 92 compile errors, all repaired here. + +**What changes at RUNTIME, per door.** TWO things can put an off-declaration value at a seam, and every refusal's `developerMessage` names both: an `after*` handler that assigned one, and a DRIVER whose own exit answered off `IDataDriver`. Each door goes from returning that value silently to refusing it — one door, one registered code, all `500`: + +- `findOne` — FROM: whatever the `afterFind` dispatch left in `ctx.result`, or whatever `driver.findOne` answered off its declared `Promise | null>`, returned to the caller as-is and walked first by `maskSecretFields` / `stripSearchCompanionFromRead`. TO: `500 FIND_ONE_HOOK_RESULT_NOT_RECORD`, raised at the seam when that value is neither a record nor `null`. +- `update` — FROM: whatever the `afterUpdate` dispatch left in the batch `ctx.result`, or whatever `driver.update` / `driver.updateMany` answered off their declared `Promise | null>` / `Promise`, returned as-is and read first by `stripSearchCompanion` and the realtime publish. TO: `500 UPDATE_HOOK_RESULT_NOT_WRITE_SHAPE`, raised when that value is outside record-or-count-or-`null`. +- `delete` — FROM: whatever the `afterDelete` dispatch left in `ctx.result`, or whatever `driver.delete` / `driver.deleteMany` answered off their declared `Promise` / `Promise`, returned as-is to a caller such as `metadata-protocol`'s `deleteData`, which turns `false` into a 404. TO: `500 DELETE_HOOK_RESULT_NOT_WRITE_SHAPE`, raised when that value is neither a boolean nor a number — never on `false` or `0`, which are declared answers. + +The driver half of each line is not hypothetical: the seven off-contract test doubles this PR repairs are exactly that source, and they are why the refusal sentence names the SEAM instead of accusing the handler.