fix(objectql): name the withheld read-only key when a hook faults reaching through it (#17327)

* fix(objectql): name the withheld read-only key when a hook faults reaching through it

Since #16344 the update path hides a caller-supplied static `readonly` value
from `before*` hooks. A hook reaching THROUGH such a key dereferences
`undefined` and throws, and a `body` hook's default `onError: abort` refuses
the caller's whole write.

The refusal is correct and is not touched here. The diagnostic was the defect.
Measured at both doors before this change:

    direct  SandboxError: hook 'guard_task_body' threw:
              TypeError: cannot set property 'who' of undefined
    REST    500 {"error":"Internal server error","code":"INTERNAL_ERROR"}

The REST reading is the worse of the two and it is the door an author authors
against: a leading `TypeError:` is correctly classified as a script fault and
sanitised (#7543), so nothing reached the author -- not the key, not the
reason, not the remedy.

The engine is the only actor that can tell "the platform took this away" from
"nobody sent it": the sandbox face is a plain JSON snapshot, so a withheld key
is simply absent there. So the explanation is composed at the hide pass, and
all three `beforeUpdate` dispatch sites inside the hide window share one
wrapper. Nothing is marshalled onto the sandbox face.

The composed error names the withheld key, says the platform withheld it
because the field is `readonly: true`, points at `ctx.previous`, carries the
original fault text, and declares HTTP 400 -- which is what carries it past
`mapDataError`'s sanitiser instead of into a blank 500. No error code is
registered and no published payload gains a key.

Three declines keep it from repeating the card's own defect in the opposite
direction: nothing withheld, an authored refusal, and a non-object throw all
rethrow the original error untouched.

Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
Co-authored-by: Claude <noreply@anthropic.com>

* fix(objectql): declare `cause` on the fault error instead of passing it through `super()`

`pnpm --filter @objectstack/objectql typecheck` exited 2 on the previous
commit:

    src/hook-withheld-readonly-fault.ts(140,20): error TS2554: Expected 0-1 arguments, but got 2.
    src/hook-withheld-readonly-fault.ts(140,31): error TS2304: Cannot find name 'ErrorOptions'.

This repo compiles against `lib: ES2020`, where `Error` has neither a `cause`
member nor the `ErrorOptions` constructor overload that carries one. The
precedent is in this same package: `duplicate-record-error.ts` declares
`readonly cause: unknown` on the class and assigns it by hand, and its comment
already records why -- an undeclared assignment would be invisible to every
TypeScript consumer of the field. Followed verbatim rather than reinvented.

No behaviour change: the same original error is attached under the same name,
and both suites plus the reverse verification were re-run against a forced
rebuild after the edit.

Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
Co-authored-by: Claude <noreply@anthropic.com>

---------

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

Author: claude[bot]

.changeset/hook-withheld-readonly-key-diagnostic.md

@@ -0,0 +1,37 @@
+---
+"@objectstack/objectql": patch
+---
+
+fix(objectql): a hook that faults reaching through a withheld read-only key now names the key, says the platform withheld it, and points at `ctx.previous` (#17219)
+
+Since #16344 the update path hides a caller-supplied static `readonly` value from `before*` hooks. A hook body that reaches **through** such a key — `ctx.input.locked_meta.who = 'hook'`, where `locked_meta` is a caller-supplied read-only `json` column — therefore dereferences `undefined` and throws, and a `body` hook's default `onError: abort` refuses the caller's whole write.
+
+**The refusal is correct and is unchanged.** What it replaced is a write that succeeded while persisting a value derived from the caller's forgery, and #16344 exists to close exactly that route. What this fixes is the diagnostic. Measured before this change, at both doors:
+
+```
+direct   SandboxError: hook 'guard_task_body' threw:
+           TypeError: cannot set property 'who' of undefined
+REST     500 {"error":"Internal server error","code":"INTERNAL_ERROR"}
+```
+
+The REST reading is the one that matters, and it is the worse of the two: a leading `TypeError:` is correctly classified as a script fault and sanitised (#7543), so an author was told nothing at all — not which key, not that the platform had taken it away, not what to read instead.
+
+### Who is affected
+
+Anyone whose `beforeUpdate` hook reads a read-only field that the caller may also send. The write was already being refused; only the message changes. A hook that needs the stored value reads it from **`ctx.previous.<field>`** — the same remedy PR #17195's changeset documents.
+
+### What the message says now
+
+```
+A `beforeUpdate` hook faulted while `locked_meta` was withheld from it. That field is
+`readonly: true`, and the engine withholds a caller-supplied value for a read-only field
+from `beforeUpdate` hooks, so `ctx.input.locked_meta` reads `undefined` — withheld by the
+platform, not missing by accident. Read the stored value from `ctx.previous.locked_meta`
+instead. Original fault: TypeError: cannot set property 'who' of undefined
+```
+
+The error declares **HTTP 400**, which is what carries it past the script-fault sanitiser onto the same "message verbatim" channel a body's own authored refusal already rides; REST callers who previously saw `500 INTERNAL_ERROR` for this case now see 400 with the text above. The original fault is carried inside the message rather than replaced.
+
+### Deliberate limits
+
+No new error code is registered and no key is added to any published payload — a dedicated `ERROR_CODE_LEDGER` entry for this refusal is a separate decision. The explanation claims only what is knowable at the seam: *faulted while these keys were withheld*, never a proven cause. An **authored** refusal (`throw new Error('…')`) is never rewritten, and a crash on an operation where nothing was withheld passes through untouched.

packages/objectql/src/engine-readonly-hook-input.test.ts

@@ -407,3 +407,180 @@ describe('#16344 — caller-forged readonly values are hidden from beforeUpdate'
     expect(afterSubmitted).toEqual([{ id: 'kpi_1', actual_value: 380, target_value: 1 }]);
   });
 });
+
+/**
+ * [#17219] The OTHER half of the same hide pass: what an author is told when a
+ * hook reaches THROUGH a key #16344 withheld.
+ *
+ * ⛔ The refusal itself is not under test here and is not moved: a body's
+ * default `onError: abort` refuses the caller's whole write, and what that
+ * replaced is a write that succeeded while persisting a value derived from the
+ * caller's forgery. Every case below re-asserts that the row is untouched, so a
+ * future repair of the DIAGNOSTIC cannot quietly restore the old write.
+ *
+ * Measured on `origin/main` `501959b72a` before this fix, both doors:
+ *
+ *   direct   SandboxError: hook '…' threw: TypeError: cannot set property 'who' of undefined
+ *   REST     500 {"error":"Internal server error","code":"INTERNAL_ERROR"}
+ *
+ * The REST reading is the one that decides the shape of the fix: a leading
+ * `TypeError:` is correctly classified as a crash (#7543) and sanitised, so at
+ * the door an author actually authors against, the old behaviour said nothing
+ * at all — not the key, not the reason, not the remedy.
+ *
+ * A code hook is the subject rather than a sandboxed body deliberately: the
+ * dispatch sites and the hide pass are what this card wraps, and they are
+ * common to both. The sandbox spelling is measured end to end, through a real
+ * QuickJS, in `runtime`'s `hook-input-writeback-readonly-provenance` suite.
+ */
+describe('#17219 — a hook that faults reaching THROUGH a withheld readonly key names it', () => {
+  let engine: ObjectQL;
+  let storeFor: ReturnType<typeof makeDriver>['storeFor'];
+
+  const OBJECT = 'guard_task';
+
+  /** Reaches through the read-only `locked_meta`, which the hide pass removed. */
+  const reachThrough = async (ctx: any) => {
+    ctx.input.data.locked_meta.who = 'hook';
+  };
+
+  async function boot(
+    hook: (ctx: any) => Promise<void>,
+    opts?: { unscopedMulti?: boolean },
+  ) {
+    engine = new ObjectQL({ logger: {
+      warn() {}, debug() {}, info() {}, error() {}, trace() {}, fatal() {},
+      child() { return this as any; },
+    } as any });
+    const d = makeDriver();
+    storeFor = d.storeFor;
+    engine.registerDriver(d.driver, true);
+    await engine.init();
+    engine.registry.registerObject({
+      name: OBJECT,
+      fields: {
+        status: { type: 'text' },
+        bucket: { type: 'text' },
+        locked_meta: { type: 'json', readonly: true },
+      },
+    } as any);
+    storeFor(OBJECT).set('t1', {
+      id: 't1', status: 'open', bucket: 'b1', locked_meta: { seeded: true },
+    });
+    engine.registerHook('beforeUpdate', hook, {
+      object: OBJECT,
+      priority: 50,
+      ...(opts?.unscopedMulti ? { dispatchUnscopedMultiWrite: true } : {}),
+    } as any);
+  }
+
+  const row = () => storeFor(OBJECT).get('t1');
+
+  /** Every requirement the card places on the message, asserted as one set. */
+  const expectActionable = (err: any) => {
+    // ① the withheld KEY is named — the old message named nothing.
+    expect(err.message).toContain('`locked_meta`');
+    // ② WITHHELD BY THE PLATFORM, not absent by accident.
+    expect(err.message).toContain('withheld by the platform, not missing by accident');
+    // ③ the documented remedy, reachable from the message itself.
+    expect(err.message).toContain('`ctx.previous.locked_meta`');
+    // ④ and it reaches the author at the REST door: `declaredHttpStatus` reads
+    //    this, and without it the body is the sanitised 500.
+    expect(err.status).toBe(400);
+    // The original fault is carried through, never swallowed.
+    expect(err.message).toMatch(/cannot set propert|Cannot set propert/);
+  };
+
+  it('BY-ID: the refusal stands and now names the key, the reason and the remedy', async () => {
+    await boot(reachThrough);
+
+    const err = await engine.update(OBJECT, {
+      id: 't1', status: 'done', locked_meta: { who: 'caller' },
+    } as any).then(() => null, (e) => e);
+
+    expect(err).toBeTruthy();
+    expectActionable(err);
+    // ⛔ RULING 1, re-pinned: the write is still refused WHOLE. Neither the
+    // forged read-only value nor the writable `status` reached the row.
+    expect(row().locked_meta).toEqual({ seeded: true });
+    expect(row().status).toBe('open');
+  });
+
+  it('PREDICATE: the per-row dispatch site answers on the same terms', async () => {
+    await boot(reachThrough);
+
+    const err = await engine.update(
+      OBJECT,
+      { status: 'done', locked_meta: { who: 'caller' } } as any,
+      { multi: true, where: { bucket: 'b1' } } as any,
+    ).then(() => null, (e) => e);
+
+    expect(err).toBeTruthy();
+    expectActionable(err);
+    expect(row().locked_meta).toEqual({ seeded: true });
+    expect(row().status).toBe('open');
+  });
+
+  it('UNSCOPED-MULTI: the third dispatch site inside the hide window answers too', async () => {
+    await boot(reachThrough, { unscopedMulti: true });
+
+    const err = await engine.update(
+      OBJECT,
+      { status: 'done', locked_meta: { who: 'caller' } } as any,
+      { multi: true } as any,
+    ).then(() => null, (e) => e);
+
+    expect(err).toBeTruthy();
+    expectActionable(err);
+    expect(row().locked_meta).toEqual({ seeded: true });
+  });
+
+  it('CONTROL — nothing withheld: an ordinary crash keeps its own raw words', async () => {
+    // The caller sends NO read-only key, so the hide pass never runs and
+    // `readonlyHiddenFromHooks` stays unset. The hook still faults (the column
+    // is simply absent from this payload), and that fault must pass through
+    // untouched: the diagnostic is tied to the WITHHOLDING, not to any crash
+    // that happens to occur on an object with a read-only field. Without this
+    // leg the case above would pass just as well for a wrapper that rewrote
+    // every hook error it saw.
+    await boot(reachThrough);
+
+    const err = await engine.update(OBJECT, { id: 't1', status: 'done' } as any)
+      .then(() => null, (e) => e);
+
+    expect(err).toBeTruthy();
+    expect(err.message).not.toContain('withheld by the platform');
+    expect(err.status).toBeUndefined();
+    expect(row().status).toBe('open');
+  });
+
+  it('CONTROL — an AUTHORED refusal is never rewritten, even while a key is withheld', async () => {
+    // The regression this guards is the card's own defect aimed the other way:
+    // `mapDataError` serves an authored message to the caller verbatim, so
+    // overwriting it would destroy the author's words to explain a key they
+    // never asked about.
+    await boot(async () => { throw new Error('仍有未结清的发票'); });
+
+    const err = await engine.update(OBJECT, {
+      id: 't1', status: 'done', locked_meta: { who: 'caller' },
+    } as any).then(() => null, (e) => e);
+
+    expect(err.message).toBe('仍有未结清的发票');
+    expect(err.status).toBeUndefined();
+    expect(row().status).toBe('open');
+  });
+
+  it('CONTROL — a hook that does NOT fault still runs, and the strip still refuses the forgery', async () => {
+    // The over-narrowing guard: if the wrapper had broken the dispatch, every
+    // case above would pass for the wrong reason. Here the same withheld key is
+    // in play, the hook completes, and the write lands MINUS the forgery.
+    await boot(async (ctx: any) => { ctx.input.data.status = 'hooked'; });
+
+    await engine.update(OBJECT, {
+      id: 't1', status: 'done', locked_meta: { who: 'caller' },
+    } as any);
+
+    expect(row().status).toBe('hooked');
+    expect(row().locked_meta).toEqual({ seeded: true });
+  });
+});

packages/objectql/src/engine.ts

@@ -214,6 +214,11 @@ import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, str
 // SAME value. Armed and sealed in `update()`; the module owns the argument for
 // why neither end may move.
 import { recordHookPayloadWrites } from './hook-write-provenance.js';
+// [#17219] The hide pass's other half: when a hook faults reaching THROUGH a
+// key that pass withheld, this names the key, says the platform withheld it,
+// and points at `ctx.previous` — the module owns the measurement and the
+// reason the explanation cannot be composed any further downstream.
+import { dispatchHooksExplainingWithheldReadonly } from './hook-withheld-readonly-fault.js';
 import {
   divergingHookPayloadKeys,
   MultiUpdateHookKeyDivergenceError,
@@ -11701,7 +11706,13 @@ export class ObjectQL implements IObjectQLEngine {
            // permanently true here: it states the invariant, and the invariant
            // outlives this call site.
            if (priorRecord) hookContext.previous = coerceBooleanFields(updateSchema as any, priorRecord as any) as any;
-           await this.triggerHooks('beforeUpdate', hookContext);
+           // [#17219] All three `beforeUpdate` dispatch sites inside the hide
+           // window share one wrapper, so a hook that faults reaching THROUGH a
+           // key this pass withheld names that key instead of surfacing the
+           // platform's own contract enforcement as the author's crash. It
+           // rethrows the original error untouched on every other path.
+           await dispatchHooksExplainingWithheldReadonly(readonlyHiddenFromHooks, 'beforeUpdate',
+             () => this.triggerHooks('beforeUpdate', hookContext));
            // The retired lever, refused. Everything above — `previous`, and
            // below it the `readonlyWhen` strip and every validation rule — was
            // computed against the row the ladder chose.
@@ -11772,7 +11783,8 @@ export class ObjectQL implements IObjectQLEngine {
            // predicate is unscoped.
            const rawWhere = (hookContext.input.options as { where?: unknown } | undefined)?.where;
            if (rawWhere === undefined || rawWhere === null) {
-               await this.dispatchUnscopedMultiWriteHooks('beforeUpdate', object, hookContext);
+               await dispatchHooksExplainingWithheldReadonly(readonlyHiddenFromHooks, 'beforeUpdate',
+                 () => this.dispatchUnscopedMultiWriteHooks('beforeUpdate', object, hookContext));
            }
            const preOpts = this.buildDriverOptions(object, opCtx.context, hookContext.input.options as any);
            readPriorRows = async () => {
@@ -11804,7 +11816,8 @@ export class ObjectQL implements IObjectQLEngine {
                // [D1] Zero matched rows is zero dispatches — a batch that
                // changed nothing is not a record change.
                if (perRowBeforeHooks && rows.length > 0) {
-                   await this.dispatchPerRowBeforeHooks(object, 'beforeUpdate', rows, hookContext);
+                   await dispatchHooksExplainingWithheldReadonly(readonlyHiddenFromHooks, 'beforeUpdate',
+                     () => this.dispatchPerRowBeforeHooks(object, 'beforeUpdate', rows, hookContext));
                }
            }
        }

packages/objectql/src/hook-withheld-readonly-fault.test.ts

@@ -0,0 +1,122 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#17219] The composer's contract, at the seam rather than through a driver.
+ *
+ * The three DECLINE conditions carry as much weight as the accept case, and for
+ * the reason the card is about: a diagnostic that fires on the wrong error is
+ * the same defect this fixes, aimed the other way. So each decline is asserted
+ * with the SAME error the accept case uses wherever the condition permits it,
+ * which is what makes the assertions about the condition rather than about the
+ * error.
+ */
+
+import { describe, it, expect } from 'vitest';
+import {
+  withheldReadonlyHookFault,
+  dispatchHooksExplainingWithheldReadonly,
+  HookWithheldReadonlyFaultError,
+} from './hook-withheld-readonly-fault.js';
+
+/** The real shape `quickjs-runner.ts` throws — name `SandboxError`, native prefix kept on `innerMessage`. */
+class SandboxErrorLike extends Error {
+  innerMessage?: string;
+  constructor(message: string, innerMessage?: string) {
+    super(message);
+    this.name = 'SandboxError';
+    this.innerMessage = innerMessage;
+  }
+}
+
+/** Measured verbatim on `origin/main` `501959b72a` — see the module header. */
+const REAL_CRASH = () =>
+  new SandboxErrorLike(
+    "hook 'guard_task_body' threw: TypeError: cannot set property 'who' of undefined",
+    "TypeError: cannot set property 'who' of undefined",
+  );
+
+const HIDDEN = { locked_meta: { who: 'caller' } };
+
+describe('#17219 withheldReadonlyHookFault', () => {
+  it('names the key, says the platform withheld it, and points at ctx.previous', () => {
+    const out = withheldReadonlyHookFault(REAL_CRASH(), HIDDEN, 'beforeUpdate');
+    expect(out).toBeInstanceOf(HookWithheldReadonlyFaultError);
+    const msg = out!.message;
+    // ① the withheld KEY is named …
+    expect(msg).toContain('`locked_meta`');
+    // ② … as WITHHELD BY THE PLATFORM, not absent by accident …
+    expect(msg).toContain('withheld by the platform, not missing by accident');
+    expect(msg).toContain('`readonly: true`');
+    // ③ … and the documented remedy is reachable from the message itself.
+    expect(msg).toContain('`ctx.previous.locked_meta`');
+    // The original fault is carried, never replaced: an author debugging the
+    // body still gets the line that actually threw.
+    expect(msg).toContain("TypeError: cannot set property 'who' of undefined");
+    expect(out!.withheldKeys).toEqual(['locked_meta']);
+    expect(out!.cause).toBeDefined();
+  });
+
+  it('declares 400 — the whole envelope change, and what makes the message reachable', () => {
+    // Measured: without a declared status `mapDataError` answers
+    // `UNCLASSIFIED_FAULT` (500, sanitised body) and the message above never
+    // reaches the author at the REST door. `packages/rest`'s
+    // `declaredHttpStatus` reads exactly this property.
+    expect(withheldReadonlyHookFault(REAL_CRASH(), HIDDEN, 'beforeUpdate')!.status).toBe(400);
+  });
+
+  it('names every withheld key when the pass hid more than one', () => {
+    const out = withheldReadonlyHookFault(
+      REAL_CRASH(), { locked_meta: {}, locked_note: 'CALLER' }, 'beforeUpdate',
+    );
+    expect(out!.message).toContain('`locked_meta`, `locked_note`');
+    expect(out!.message).toContain('`ctx.previous.locked_meta`, `ctx.previous.locked_note`');
+    expect(out!.withheldKeys).toEqual(['locked_meta', 'locked_note']);
+  });
+
+  it('DECLINES when nothing was withheld — an unrelated crash keeps its own words', () => {
+    expect(withheldReadonlyHookFault(REAL_CRASH(), undefined, 'beforeUpdate')).toBeUndefined();
+    expect(withheldReadonlyHookFault(REAL_CRASH(), {}, 'beforeUpdate')).toBeUndefined();
+  });
+
+  it('DECLINES on an AUTHORED refusal, so a business message is never rewritten', () => {
+    // The one that would be a real regression: `mapDataError` serves this text
+    // to the caller verbatim at 400, and overwriting it would destroy the
+    // author's own words while a readonly key happened to be hidden.
+    const authored = new SandboxErrorLike("hook 'guard' threw: 仍有未结清的发票", '仍有未结清的发票');
+    expect(withheldReadonlyHookFault(authored, HIDDEN, 'beforeUpdate')).toBeUndefined();
+    // And the non-sandboxed spelling of the same thing.
+    expect(withheldReadonlyHookFault(new Error('仍有未结清的发票'), HIDDEN, 'beforeUpdate')).toBeUndefined();
+  });
+
+  it('ACCEPTS a CODE hook crash, which carries the native name in `name` instead', () => {
+    const out = withheldReadonlyHookFault(
+      new TypeError("Cannot set properties of undefined (setting 'who')"), HIDDEN, 'beforeUpdate',
+    );
+    expect(out).toBeInstanceOf(HookWithheldReadonlyFaultError);
+    expect(out!.message).toContain('`locked_meta`');
+  });
+
+  it('declines on a non-object throw rather than fabricating a shape', () => {
+    expect(withheldReadonlyHookFault('boom', HIDDEN, 'beforeUpdate')).toBeUndefined();
+  });
+});
+
+describe('#17219 dispatchHooksExplainingWithheldReadonly', () => {
+  it('is transparent on success', async () => {
+    await expect(dispatchHooksExplainingWithheldReadonly(HIDDEN, 'beforeUpdate', async () => 'ok'))
+      .resolves.toBe('ok');
+  });
+
+  it('rethrows the ORIGINAL error object when the composer declines', async () => {
+    const authored = new Error('仍有未结清的发票');
+    await expect(
+      dispatchHooksExplainingWithheldReadonly(HIDDEN, 'beforeUpdate', async () => { throw authored; }),
+    ).rejects.toBe(authored);
+  });
+
+  it('replaces an anonymous crash with the named refusal', async () => {
+    await expect(
+      dispatchHooksExplainingWithheldReadonly(HIDDEN, 'beforeUpdate', async () => { throw REAL_CRASH(); }),
+    ).rejects.toBeInstanceOf(HookWithheldReadonlyFaultError);
+  });
+});