fix(objectql): gate by-id update/delete on record existence — 404 RECORD_NOT_FOUND instead of a 400 from further down the pipeline (#7867) (#7989)

* fix(objectql): gate by-id update/delete on record existence — 404 RECORD_NOT_FOUND instead of a 400 from further down the pipeline (#7867)

Nothing on the action-body write path ever asked whether the target row
existed. `ctx.api.object(name).update({ id, … })` reached `ObjectQL.update()`'s
by-id branch through `buildSandboxApi` → `ObjectRepository`, and that branch had
no existence gate at all: `engine.update()` on a ghost id was a silent no-op
that resolved `null`, so the write ran on into validation, the driver and the
hook chain and died on whichever complained first. Which one varied with the
object's declarations — a `HookConditionError` 400 on a hooked object, a
required-field `VALIDATION_FAILED` 400 on an unhooked one. The 400 class varied;
the missing 404 was the constant. `delete()` had the same shape and was worse:
it reported success for a row that was never there.

The gate goes at the engine, in the by-id branches of `update()` and `delete()`
— the one point all three action-body write faces funnel through
(`ctx.api.object()`, its context-less repo-facade fallback, and
`ctx.engine.update()`). Two sibling paths already gated correctly
(`protocol.updateData`/`deleteData`, `callData`'s ObjectQL fallback); all three
now throw the same `recordNotFoundError`, which moves to `@objectstack/core` so
`engine.ts` can reach it without importing `@objectstack/metadata-protocol`
(forbidden in the `/core` closure by ADR-0076 D2's boundary ratchet).
`@objectstack/metadata-protocol` re-exports it unchanged.

The `if (priorRecord) hookContext.previous = …` never-fabricate rule
(ADR-0058 Addendum II / #4649) is untouched — it was behaving correctly on a
path that should never have been entered, so the producer is removed rather
than the message it produced specialized.

Consequence worth knowing: the by-id prior-row read is now unconditional. The
#5284 / #5929 narrowings asked "does anything CONSUME the prior row?" and
skipped the read when nothing did; existence is a consumer that list never
enumerated and the one consumer every by-id write has, and no cheaper question
answers it. Their read-count pins are rewritten in place, each recording what
changed and why. The dispatch half of both cards — the per-object question,
`excludeObjects` subtraction, and the retired `sys_fetch_previous_*` builtins —
is untouched and still pinned.

`@objectstack/runtime`: the sandbox error passthrough also carries `status` now,
so an error that names its own HTTP status keeps it across the QuickJS
boundary. Without it the action surface served the right diagnosis at the wrong
status (`{ code: 'RECORD_NOT_FOUND', httpStatus: 400 }`); `domains/actions.ts`
already honoured `.status` first — the number never arrived.

Scope: by-id only. A `multi: true` write matching zero rows still resolves
"0 rows affected".

Tests: a new `engine-write-not-found-gate.test.ts` covering hooked AND unhooked
objects (the defect is not about hooks), the delete twin, the predicate-path
scope line, and the shared-envelope check; `status` cases in the runtime's
`error-passthrough.test.ts`; and an assertion on the dogfood fixture that has
been passing 23/23 while logging this exact error through a required 3-shard
gate.

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

* test(plugin-audit): the #5860 SKIP_OBJECTS pin measures the read, not the skip (#7867)

#5860's acceptance criterion — the per-object demand gate judges a
SKIP_OBJECTS object unhooked — is unchanged and still asserted directly by
the sibling cases. What changed is that the gate no longer decides whether
the engine LOOKS at the row: #7867's not-found gate needs the by-id prior
read unconditionally, so `sys_job_queue` pays one read like everything
else. One, not two — an audit handler forcing its own read would still be
caught, and the audit ledger is still empty for the skipped object.

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

* test(objectql): keep the #7867 pin out of the TEST_DEBT ledger — type its fixtures instead of casting (#7867)

`engine-write-not-found-gate.test.ts` added one raw tsc error to
`@objectstack/objectql`'s hidden test layer (TS2554 — `registerObject`
takes a required `packageId`), pushing TEST_DEBT from 355 to 356 and
failing `check:type-check-debt --re-measure`. That ledger is a
shrink-only ratchet (#5278), so the error is fixed rather than the
number raised.

Fixed by typing the two fixtures as `ServiceObject` and passing the
package id — not by widening the cast. Typing them also surfaced that
`primaryKey` is not a declared field property; the registry provisions
the primary key itself, so the key was a no-op the compiler could not
see while the fixture stayed untyped. Removed, with the reasoning in
place.

objectql's TEST_DEBT re-measures at 355 (its recorded number) and this
file now contributes zero errors. The ledger entry in
scripts/check-type-check-coverage.mjs is untouched, and --lower was not
run.

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

* test(plugin-auth): the #5941 guard case deletes a REAL row, not a ghost id (#7867)

`last-admin-guard.test.ts`'s 'deleting an unrelated row on another object
is not this guard's business' deleted the `sys_account` id 'nope' — never
seeded — and asserted it RESOLVED. It passed for a reason unrelated to the
guard: `ObjectQL.delete()` had no existence gate on its by-id path, so a
delete naming no row was a silent no-op reporting success. The case was
asserting the absence of a guard by way of that defect, so #7867's gate
turned it red.

Rewritten to delete a REAL `sys_account` row (the fixture already seeds one
via `accountProvider`), which states the same thing more strongly: the
guard does not merely fail to fire on a write that touched nothing, it lets
a write that really removes a row on this object through. The ghost-id half
is KEPT as its own assertion — refused by the ENGINE with RECORD_NOT_FOUND,
and explicitly not by the last-admin guard — so the two questions the case
conflated are now answered separately.

Reverse-verified: with the delete gate reverted the new assertion goes red
(`expected undefined to be 'RECORD_NOT_FOUND'`).

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

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: huangyiirene

.changeset/action-body-write-not-found-gate.md

@@ -0,0 +1,93 @@
+---
+"@objectstack/objectql": patch
+"@objectstack/core": patch
+"@objectstack/metadata-protocol": patch
+"@objectstack/runtime": patch
+"@objectstack/plugin-audit": patch
+"@objectstack/plugin-auth": patch
+---
+
+fix(objectql): a by-id `update()`/`delete()` against a nonexistent record answers 404 `RECORD_NOT_FOUND` instead of a 400 from further down the pipeline (#7867)
+
+Nothing on the action-body write path ever asked whether the target row existed.
+`ctx.api.object(name).update({ id, … })` reached `ObjectQL.update()`'s by-id
+branch through `buildSandboxApi` → `ObjectRepository`, and that branch had **no
+existence gate at all**: `engine.update()` on a ghost id was a silent no-op that
+resolved `null`, so the write ran on into validation, the driver and the hook
+chain and died on whichever complained first.
+
+**Which one it died on varied with the object's declarations**, which is why the
+defect read as several unrelated bugs:
+
+- a **hooked** object → `400` `HookConditionError`, from an `afterUpdate`
+  condition reading `previous` on a row nobody read;
+- an **unhooked** object → `400` `VALIDATION_FAILED` "X is required", because
+  with no prior row a PATCH is validated as if it were a whole record.
+
+The 400 class varied; the missing 404 was the constant. Measured on one showcase
+stack, same id, same object, same second: `POST /actions/showcase_task/
+showcase_mark_done/<ghost>` answered 400 while `PATCH /data/showcase_task/
+<ghost>` answered 404. Both answer **404 `RECORD_NOT_FOUND`** now.
+
+`delete()` had the same shape and was the worse of the two: with no gate it
+reported success for a row that was never there, so a typo'd id, an
+already-deleted row and a real deletion were indistinguishable.
+
+**This is not a `previous`-binding bug.** `if (priorRecord) hookContext.previous
+= …` is correct and is untouched — ADR-0058 Addendum II / #4649 require that an
+absent row leave `previous` UNBOUND rather than fabricated. It was behaving
+correctly on a path that should never have been entered, so the fix removes the
+producer rather than specializing what it produced.
+
+**Where the gate went, and why there.** At the engine, in the by-id branches of
+`update()` and `delete()` — the one point all three action-body write faces
+funnel through (`ctx.api.object()`, its context-less repo-facade fallback, and
+`ctx.engine.update()`). A repository-level gate would have closed one of the
+three and made `ql.update(o, { id })` and `ctx.api.object(o).update({ id })`
+answer one ghost id two different ways. Two sibling paths already gated
+correctly — `protocol.updateData`/`deleteData` (#4435) and `callData`'s ObjectQL
+fallback (#5138) — and all three now throw the **same** `recordNotFoundError`,
+which moved to `@objectstack/core` so the engine can reach it without importing
+`@objectstack/metadata-protocol` (forbidden in the `/core` closure by ADR-0076
+D2's boundary ratchet). `@objectstack/metadata-protocol` re-exports it unchanged.
+
+Existence is asked with a pre-write read, never off the write's own result:
+`IDataDriver.update` declares no not-found signal, and the engine's post-write
+readback is `null` for a second reason (a write that moves the row out of the
+caller's row scope), so reading either would answer 404 to a write that landed.
+
+**Behaviour change worth knowing about — the by-id prior-row read is now
+unconditional.** #5284 (update) and #5929 (delete) had narrowed it to "does
+anything CONSUME the prior row?", skipping the read for objects with no hook, no
+prior-reading validation rule and no roll-up. Existence is a consumer that
+demand list never enumerated and the one consumer every by-id write has, and no
+cheaper question answers it — so the skip and the gate are mutually exclusive.
+The measured cost is small: #5929's own record enumerates the global hook
+registrants (plugin-sharing, service-storage, plugin-auth, plugin-audit), so on
+any kernel that loads them the demand was already true for every object and the
+narrowing skipped nothing. The read is genuinely new only for a bare
+`@objectstack/objectql/core` embedder — which is buying a 404 it did not have.
+
+Three read-count pins measured the old skip and now measure the read, each
+recording what changed and why at its own site: #5284's and #5929's in
+`packages/objectql`, and #5860's `sys_job_queue` case in `@objectstack/plugin-audit`.
+The DISPATCH half all three are actually about — the per-object `hasHooksFor`
+question, the `excludeObjects` subtraction, and the retired
+`sys_fetch_previous_*` builtins — is untouched and still pinned.
+
+One further case encoded the old silent no-op as correct: `@objectstack/plugin-auth`'s
+#5941 last-admin-guard test deleted a `sys_account` id that was never seeded and
+asserted it RESOLVED, to show the guard does not write-guard that object. It now
+deletes a REAL row — which states the same thing more strongly — and separately
+pins that a ghost id there is refused by the ENGINE rather than by the guard.
+
+**Scope.** By-id only. A `multi: true` predicate write matching zero rows still
+resolves "0 rows affected" — the same line both sibling paths draw.
+
+`@objectstack/runtime`: the sandbox error passthrough now also carries `status`
+alongside `code` and `fields`, so an error that names its own HTTP status keeps
+it across the QuickJS boundary. Without it the action surface answered the right
+diagnosis at the wrong status (`{ code: 'RECORD_NOT_FOUND', httpStatus: 400 }`);
+`domains/actions.ts` already honoured `.status` first — the number simply never
+arrived. A permission refusal thrown inside a body likewise keeps its 403 now
+instead of flattening to 400.

packages/core/src/index.ts

@@ -44,6 +44,12 @@ export * from './utils/migration-journal.js';
 // Export the runtime filter-placeholder resolver (framework#3582)
 export * from './utils/filter-tokens.js';
 
+// Export the shared single-record 404 (#4435/#5138, moved down here in #7867) —
+// the one `RECORD_NOT_FOUND` envelope `protocol.updateData`/`deleteData`,
+// `callData`'s ObjectQL fallback and the engine's own by-id write gate answer
+// with. `@objectstack/metadata-protocol` re-exports it from its original home.
+export * from './utils/record-not-found.js';
+
 // Export in-memory fallbacks for core-criticality services
 export * from './fallbacks/index.js';
 

packages/core/src/utils/record-not-found.ts

@@ -0,0 +1,64 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#4435] The 404 a single-record operation answers when the id names no row.
+ *
+ * Extracted so the READ and the two WRITE paths cannot disagree about it. They
+ * did: `getData` answered `404 RECORD_NOT_FOUND` while `updateData` returned
+ * `200 { record: null }` and `deleteData` returned `200 { success: true }` for
+ * any string in the path — so a typo'd id, an already-deleted row and a real
+ * deletion were indistinguishable, and a client PATCHing a concurrently deleted
+ * record was told its write had landed.
+ *
+ * That is the same silent-no-op shape the v17 train removed everywhere else
+ * this window (#4240/#4303/#4315 refuse missing fields, #4169 refuses unknown
+ * params, #4190 stopped dropping filters) — a write that touched zero rows
+ * reporting 200 is that shape one level up, on the verb where it costs the
+ * most.
+ *
+ * [#5138] EXPORTED, for the same "cannot disagree about it" reason one layer
+ * out. `@objectstack/runtime`'s `callData` is protocol-first with an ObjectQL
+ * FALLBACK, and the fallback had reinvented this fact three incompatible ways
+ * (`get` → `null`, `update` → a bare `Error` with no status ⇒ 500, `delete` →
+ * no check at all ⇒ `200 { deleted: true }` for a row that never existed). It
+ * now calls THIS function, so the two paths behind one `callData` answer a
+ * missing id identically — which is the only reason a caller may stop caring
+ * which of them served it. Re-spelling the envelope there would have been a
+ * second not-found envelope; `RECORD_NOT_FOUND` (#5088) is the one this repo
+ * has.
+ *
+ * ── [#7867] Why it lives in `@objectstack/core` and not where it was written ──
+ *
+ * Because the THIRD path that needed it could not reach the second one. An
+ * action body's `ctx.api.object(name).update({ id, … })` traverses neither
+ * `protocol.updateData` nor `callData`: it reaches `ObjectQL.update()`'s by-id
+ * branch directly, which had no existence gate at all, so a ghost id was a
+ * silent no-op that then died on whatever the pipeline complained about first
+ * (a `HookConditionError` 400 on a hooked object, a required-field
+ * `VALIDATION_FAILED` 400 on an unhooked one — the 400 class varied with the
+ * object's declarations; the missing 404 was the constant).
+ *
+ * The gate for that path belongs in the engine, and `packages/objectql` cannot
+ * import `@objectstack/metadata-protocol` where this function was written:
+ * ADR-0076 D2's boundary ratchet (`core-boundary.ratchet.test.ts`) forbids the
+ * whole `@objectstack/objectql/core` closure — `engine.ts` included — from
+ * pulling that package in. So the choice was a FOURTH spelling of the envelope
+ * or one home both layers already depend on. #5138's own sentence rules the
+ * first out, so this is the second: the factory moved down to the lowest
+ * package the three producers share, and `@objectstack/metadata-protocol`
+ * re-exports it unchanged for every existing importer.
+ *
+ * This is the same move `engineCanRollBack` made for the same reason — a fact
+ * two layers must agree on lives in the layer beneath both, not in a copy each.
+ */
+export function recordNotFoundError(object: string, id: string | number): Error {
+    const err = new Error(`Record ${id} not found in ${object}`) as Error & {
+        code?: string;
+        status?: number;
+        object?: string;
+    };
+    err.code = 'RECORD_NOT_FOUND';
+    err.status = 404;
+    err.object = object;
+    return err;
+}

packages/metadata-protocol/src/protocol.ts

@@ -3,7 +3,7 @@
 import type {
     DataProtocol, MetadataProtocol, PackageProtocol,
 } from '@objectstack/spec/api';
-import { IDataEngine, engineCanRollBack } from '@objectstack/core';
+import { IDataEngine, engineCanRollBack, recordNotFoundError } from '@objectstack/core';
 import { readEnvWithDeprecation, resolveTenancyPosture } from '@objectstack/types';
 // [#6285] ADR-0105 D1's authority on "does this deployment wall organizations?".
 // `resolveMultiOrgEnabled()` is DEMOTED and its own doc comment says answering
@@ -640,43 +640,22 @@ export function zodIssuesToMetadataIssues(issues: unknown): MetadataIssueEntry[]
 }
 
 /**
- * [#4435] The 404 a single-record operation answers when the id names no row.
- *
- * Extracted so the READ and the two WRITE paths cannot disagree about it. They
- * did: `getData` answered `404 RECORD_NOT_FOUND` while `updateData` returned
- * `200 { record: null }` and `deleteData` returned `200 { success: true }` for
- * any string in the path — so a typo'd id, an already-deleted row and a real
- * deletion were indistinguishable, and a client PATCHing a concurrently deleted
- * record was told its write had landed.
- *
- * That is the same silent-no-op shape the v17 train removed everywhere else
- * this window (#4240/#4303/#4315 refuse missing fields, #4169 refuses unknown
- * params, #4190 stopped dropping filters) — a write that touched zero rows
- * reporting 200 is that shape one level up, on the verb where it costs the
- * most.
- *
- * [#5138] EXPORTED, for the same "cannot disagree about it" reason one layer
- * out. `@objectstack/runtime`'s `callData` is protocol-first with an ObjectQL
- * FALLBACK, and the fallback had reinvented this fact three incompatible ways
- * (`get` → `null`, `update` → a bare `Error` with no status ⇒ 500, `delete` →
- * no check at all ⇒ `200 { deleted: true }` for a row that never existed). It
- * now calls THIS function, so the two paths behind one `callData` answer a
- * missing id identically — which is the only reason a caller may stop caring
- * which of them served it. Re-spelling the envelope there would have been a
- * second not-found envelope; `RECORD_NOT_FOUND` (#5088) is the one this repo
- * has.
+ * [#4435/#5138] The 404 a single-record operation answers when the id names no
+ * row — the repo's ONE `RECORD_NOT_FOUND` envelope.
+ *
+ * [#7867] The body moved to `@objectstack/core`
+ * (`utils/record-not-found.ts` — full provenance lives there); this is a
+ * re-export, so every existing importer of
+ * `@objectstack/metadata-protocol`'s `recordNotFoundError` is unchanged and
+ * the three producers still share one function object.
+ *
+ * ⛔ Do not re-declare it here. It moved because a THIRD producer needed it and
+ * could not reach this package: `ObjectQL.update()`/`delete()`'s by-id gate
+ * lives in `packages/objectql`, whose `/core` entry closure is forbidden by
+ * ADR-0076 D2's boundary ratchet from importing `@objectstack/metadata-protocol`
+ * at all. A local copy here would be the second spelling #5138 ruled out.
  */
-export function recordNotFoundError(object: string, id: string | number): Error {
-    const err = new Error(`Record ${id} not found in ${object}`) as Error & {
-        code?: string;
-        status?: number;
-        object?: string;
-    };
-    err.code = 'RECORD_NOT_FOUND';
-    err.status = 404;
-    err.object = object;
-    return err;
-}
+export { recordNotFoundError };
 
 /**
  * A 400 for a `$filter` ARRAY that looks like a filter AST but is not one.

packages/objectql/src/engine-delete-dispatch.test.ts

@@ -35,7 +35,18 @@ function makeRecordingDriver() {
     supports: {},
     async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
     async find() { return []; },
-    async findOne() { return null; },
+    // [#7867] Answers the row the by-id branch's not-found gate asks for.
+    // This used to be `return null`, which — now that a by-id update/delete
+    // refuses a ghost id with `RECORD_NOT_FOUND` — would make every by-id case
+    // in this file die at the gate and never reach the driver: a DOUBLE looser
+    // than the producer, hiding the very behaviour the file exists to observe
+    // (#4434/#4550's shape). It echoes back whatever id it was asked for, so it
+    // stays agnostic about the dispatch and can never make a `reject` case look
+    // like a `by-id` one.
+    async findOne(_o: string, ast: any) {
+      const id = ast?.where?.id;
+      return id === undefined || id === null ? null : { id, title: 'stored' };
+    },
     async create(_o: string, data: Record<string, unknown>) { return { id: 'r1', ...data }; },
     async update(_o: string, id: string, data: Record<string, unknown>) { return { id, ...data }; },
     async delete(_o: string, id: string) { calls.push({ fn: 'delete', arg: id }); return true; },