Merge remote-tracking branch 'origin/main' into claude/issue-16849-agents-changelog-guardrail-row

Author: claude

.changeset/engine-verb-result-declarations.md

@@ -0,0 +1,35 @@
+---
+"@objectstack/spec": minor
+"@objectstack/objectql": minor
+"@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)
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable moves. No spec key, no authored metadata property, no config field, no accepted request shape and no stored artifact changes spelling or shape; `objectstack migrate meta` has nothing to rewrite, `spec-changes.json` has nothing to project and the upgrade guide has no row to gain. What moves is the declared RETURN TYPE of three TypeScript methods (`packages/spec/src/contracts/data-engine.ts`, its `scoped-context.ts` mirrors, and `ObjectQL` itself) plus three new registered ADR-0112 error codes. The rewrite this ships — add the null check the type now demands — is addressed to a TYPESCRIPT CONSUMER and is delivered by the compiler at their own call site, which is the audience the ADR-0087 ledger explicitly does not serve. `type-surface-only` is the category built for exactly this class and it is NOT claimed here, because its predicate 2 (`no-spec-diff`) is mechanically false for this PR: the surface the maintainer ruling names IS `packages/spec/src/contracts/**`. That gap is reported on the card rather than worked around, and the `**BREAKING**` banner below is carried rather than dropped. -->
+
+**BREAKING** on three published `.d.ts` surfaces. `ObjectQL.findOne`, `ObjectQL.update` and `ObjectQL.delete` — and the `IDataEngine` / `IScopedObjectRepository` contracts they implement — declared `Promise<any>` and now declare the answers they have always given:
+
+- `findOne` → `Promise<Record<string, any> | null>`
+- `update` → `Promise<Record<string, any> | number | null>`
+- `delete` → `Promise<boolean | number>`
+
+`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<any[]>` resolve to an envelope, silently — and recorded that it could close only that one: the other three declared `Promise<any>` 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<string, unknown> | 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<string, any>`), which is #15823's precedent extended exactly rather than softened: `find()` declares `Promise<any[]>`, 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, 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<Record<string, unknown> | 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<Record<string, unknown> | null>` / `Promise<number>`, 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<boolean>` / `Promise<number>`, 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.

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' \| … +322 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' \| … +325 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`
@@ -185,6 +186,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`
@@ -356,6 +358,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`

content/docs/references/api/error-code-ledger.mdx

@@ -293,6 +293,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`
@@ -344,6 +345,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`
@@ -515,6 +517,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`

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<any>` 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<string, any> | number | null): Record<string, any> {
+    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<string, any>;
+}
+
 /**
  * A 400 for a `$filter` ARRAY that looks like a filter AST but is not one.
  *
@@ -11096,7 +11132,7 @@ export class ObjectStackProtocolImplementation implements
         )
             ? { ...(request.data as Record<string, unknown>), 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
@@ -12132,7 +12168,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++;
@@ -12164,7 +12200,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 {
@@ -12547,7 +12583,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++;

packages/metadata/src/loaders/database-loader.ts

@@ -430,7 +430,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);
   }

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 () => {