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

[claude]

Fixes #17219

Clause-②: no — no new error code is registered in packages/spec's error-code-ledger.zod.ts, and no key is added to any published payload. The hard stop in the dispatch order was not reached; the route below deliberately avoids it, and the reasoning is recorded under Deliberate limits.

Authored by Claude Code, session session_01XTBcV7zZHmokdyQgXjbyEU.

Landing site — reported before widening, as the dispatch ordered

The dispatch named the expected surface as the sandbox error path (packages/runtime/src/sandbox/** and whatever composes the SandboxError wrapper). Measured, the landing site is not there. It is packages/objectql, which is also the lane triage derived (domain:engine).

The measurement that decides it: the sandbox face is a plain JSON snapshot (body-runner.ts takes unwrapProxyToPlain(engineCtx.input), then it is marshalled into QuickJS), so a withheld key is simply absent there. Absent-because-the-platform-took-it-away is indistinguishable, downstream of the engine, from absent-because-nobody-sent-it — and stating that difference is exactly what ruling 4 requires of the message. Only the hide pass knows it. So the explanation is composed where the hide pass is, and packages/runtime/src/sandbox/** gains no production code at all — one test file there is tightened, nothing more.

Files:

file	what
packages/objectql/src/hook-withheld-readonly-fault.ts	new — the composer and the shared dispatch wrapper
packages/objectql/src/engine.ts	+19/-4 — one import, three one-line wraps at the beforeUpdate dispatch sites inside the hide window
packages/objectql/src/hook-withheld-readonly-fault.test.ts	new — the composer's accept case and its three declines
packages/objectql/src/engine-readonly-hook-input.test.ts	end-to-end cases appended to the #16344 suite, reusing its existing driver double
packages/runtime/src/sandbox/hook-input-writeback-readonly-provenance.integration.test.ts	the one real-QuickJS case, tightened from toThrow to the envelope
.changeset/hook-withheld-readonly-key-diagnostic.md	patch, @objectstack/objectql

Shared-file fence — re-derived by symbol on my own head

#15719 is claimed in the same round and lands in the CREATE path. Re-derived at origin/main 501959b72a (the card's line numbers were taken at 66c580b0c and have moved):

staticReadonlyInsertSubject   packages/objectql/src/engine.ts:10568   (inside insert())
                              packages/objectql/src/validation/rule-validator.js  (import, :211)
readonlyHiddenFromHooks       packages/objectql/src/engine.ts:11439, 11452, 11454, 11886, 11894
control - a fabricated symbol  0 hits

Disjoint. My region is the UPDATE path, ~870 lines below #15719's create-path site, and I do not touch packages/objectql/src/validation/rule-validator.ts at all. My import line was placed beside the #14088 recorder import rather than on the rule-validator import line, so even the import block does not overlap. No serial conflict; nothing to edit around.

What was measured, before anything was written

Re-fetched origin/main first (ruling 5): the premise commit d2c1d19807 is present; measurement base 501959b72a.

Both doors, on main, through the real harness — QuickJS, the flat-input proxy, the #14088 recorder and the strip:

direct   SandboxError: hook 'guard_task_body' threw:
           TypeError: cannot set property 'who' of undefined
         innerMessage = "TypeError: cannot set property 'who' of undefined"
         code = undefined   status = undefined

REST     500 {"error":"Internal server error","code":"INTERNAL_ERROR"}

The card's quoted text is still exact — but the REST reading is the worse of the two outcomes triage allowed for, and it is the door an author actually authors against. error-response.ts's isScriptFaultMessage correctly classifies a leading TypeError: as a crash (#7543) and sanitises it, so the author was told nothing at all: not the key, not the reason, not the remedy. A control fired in the same measurement — a body throwing an authored Error still answers 400 with its own words verbatim — so the blank envelope is a real reading, not a broken probe.

Candidate error shapes were then measured through mapDataError before choosing one:

shape	answer
plain Error	500 INTERNAL_ERROR
Error + status = 400	400 + message verbatim, no code
Error + ledgered code only	500 INTERNAL_ERROR
control — the raw fault, untouched	500 INTERNAL_ERROR

status is what makes the remedy reachable; a code does not. That is the whole reason the route needs no ledger entry.

The fix

The engine already records which caller-supplied read-only keys it withheld (readonlyHiddenFromHooks). All three beforeUpdate dispatch sites inside that window — by-id, unscoped-multi, per-row — now share one wrapper, so a hook that faults there is answered with an error that:

1. names the withheld key (all of them, when the pass hid more than one);
2. says the platform withheld it because the field is readonly: true — "withheld by the platform, not missing by accident", in those words;
3. names ctx.previous.KEY, the remedy PR fix(objectql)!: beforeUpdate receives the persist image; the caller submission moves to ctx.submitted (#16344) #17195's changeset documents under Who is affected;
4. carries the original fault text through, so an author debugging the body still gets the line that threw;
5. declares HTTP 400, which is what carries all of the above past the script-fault sanitiser.

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

Mechanically this is still a crash, so the fault channel was never the wrong pipe. What was wrong is the causal attribution — one of the platform's own contract enforcements was reported to the author as a bug in their code. That is the line rest-hook-refusal-classification.test.ts and its siblings already draw, and this puts the case back on the side of it that it belongs on.

What this deliberately does NOT do

• ⛔ The refusal is not touched (ruling 1). Every end-to-end case re-asserts that the row is untouched — neither the forged read-only value nor the writable status reaches it — so a future repair of the diagnostic cannot quietly restore the old write.
• ⛔ Nothing is marshalled onto the sandbox face (ruling 2). No ctx.submitted, no new ctx member, no new key on any authoring face. packages/runtime/src/sandbox/** gains no production code.
• ⛔ No error code is registered (ruling 3). The 400 rides the existing "message verbatim, no code" channel a body's own authored refusal already uses. ERR_READONLY_FIELD_REJECTED was considered and refused: that code names the strictReadonlyWrites refusal and carries drops as part of its contract, so borrowing it would make the error lie about which refusal happened.
• ⛔ No cause is claimed. Nothing at this seam can prove the crash was the dereference of a withheld key rather than an unrelated bug, so the sentence is anchored on what is known — faulted while these keys were withheld from it. Asserting a cause that cannot be established would relocate this card's defect rather than fix it.

Three declines keep the wrapper from repeating the card's defect in the opposite direction, each with its own test: nothing withheld (an ordinary crash keeps its raw words), an authored refusal (never rewritten — mapDataError serves that text to the caller verbatim, so overwriting it would destroy the author's own words), and a non-object throw. All three rethrow the original error object identically.

Verification

Measured at 94193545fd. Every exit code captured by redirect-then-$?, never across a pipe.

Reverse verification — the real direction, from the committed state. The ablation reverts packages/objectql/src/engine.ts to the pre-fix commit (git restore --source=BASE, tree only, never the index), which removes the import and all three wraps. Both legs prove the mutation reached the built artifact the runtime suite consumes, because that suite resolves @objectstack/objectql through exports to dist/, not src/:

mutate   marker occurrences 4 -> 0 ; blob efa1a3ba88 -> e54df038b6   (on disk, proven)
         pnpm --filter @objectstack/objectql build --force            exit 0
         ablation-dist-preflight --absent   ✓ absent from all 14 built files
         objectql  src/hook-withheld-readonly-fault.test.ts + src/engine-readonly-hook-input.test.ts
                   exit 1   ->  3 failed | 25 passed (28)
         runtime   sandbox/hook-input-writeback-readonly-provenance.integration.test.ts
                   exit 1   ->  1 failed |  8 passed (9)

restore  git checkout HEAD -- PATH ; whole-tree `git status --porcelain` EMPTY
         disk blob == HEAD blob (efa1a3ba88) ; marker count back to 4
         build --force  exit 0 ; preflight ✓ present in 4 built files ; ✓ tree clean
         objectql  exit 0  ->  28 passed (28)
         runtime   exit 0  ->   9 passed (9)

The direction was predicted before running and held: the three failures are exactly the three dispatch-site cases. The three control cases stayed green under ablation — they assert pass-through, which is the pre-fix behaviour — and so did the composer's own unit suite, which exercises the module in isolation rather than its wiring. That split is the point: the unit suite proves the composer, the end-to-end cases prove it is reached.

Suites (after the type fix, against a forced rebuild):

command	result
pnpm --filter @objectstack/objectql typecheck	exit 0 (tsc, tsconfig.scripts.json, check:test-typecheck all clean)
pnpm --filter @objectstack/runtime typecheck	exit 0
objectql — the two suites above	exit 0, 28 passed
runtime — the sandbox integration suite	exit 0, 9 passed

Gate families, derived from the real change set rather than from a list — node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, then reconciled with --ran carrying every exit code so the NOT-MEASURED count is derived, not claimed:

64 derived · 62 run green · 2 NOT-MEASURED · 0 UNRUN

⛔ The two are NOT passes and NOT findings — both exited 3, PREREQUISITE NOT MET, and neither names a file in this diff:

• check:dual-build-cjs-loads — "this gate reads built output, and some package has no dist/" (38 packages unbuilt in this worktree).
• check:type-check-debt — "--re-measure cannot run: 2 workspace dependencies have no built type entry point on disk".

Both need a full workspace build (pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*'), which is the repo-wide sweep the required Lint & Repo Gates context performs on a fresh checkout after its own build step. Declared to CI rather than skipped.

Lint — the full population, not a narrowing. pnpm lint (eslint . --no-inline-config) finished inside the foreground budget, so no narrowing had to be justified: 6483 files linted — the population read from --format json output length, i.e. from eslint's own config rather than from any guess of mine — 0 errors, 0 warnings, exit 0, at 94193545fd. .cache/ is absent from this worktree, so the objectui-checkout false positives AGENTS.md warns about cannot be in that count.

Docs drift advisory — 22 pages, and what was done about them. 3 are release-owned (content/docs/releases/v17/17-0, 17-1, 17-3) and were not touched: read-only by directive. The other 19 hand-written pages were read and are unaffected, and the reason is structural rather than a skim: this change alters no behaviour any of them documents — the #16344 hide, the five strip rules, ctx.submitted, ctx.previous and onError: abort are all exactly as written. Two spots were checked closely because they were the only ones that could have gone stale:

• content/docs/api/error-catalog.mdx states the crash-vs-rejection rule and already carries the governing clause verbatim — "An error carrying its own status is served with it." That is precisely the channel this fix uses, so the page is correct as written, not merely unmentioned.
• content/docs/kernel/contracts/data-engine.mdx documents ReadonlyFieldRejectedError / ERR_READONLY_FIELD_REJECTED. That code was deliberately not reused here, so its description stays true.

Process note, stated rather than quietly fixed

The first push (e94f8ae054) was red: @objectstack/objectql#typecheck exited 2 on ErrorOptions / a two-argument super(), because this repo compiles against lib: ES2020. I opened the PR before running the package typecheck; that was the wrong order and it cost a red head. The fix (94193545fd) follows the precedent already written down in the same package — duplicate-record-error.ts declares readonly cause: unknown on the class for exactly this reason — rather than reaching for a cast or a suppression, and both suites plus the reverse verification were re-run against a forced rebuild afterwards.

Acceptance notes

• The old assertion at the real sandbox door was rejects.toThrow(/locked_meta|cannot set property 'who' of undefined/) — which stayed green through the entire defect, since the raw TypeError matches the second arm. It is now an envelope assertion. Noted as the reason a toThrow is not a refusal test.
• dispatchUnscopedMultiWriteHooks is reachable only for a code hook carrying dispatchUnscopedMultiWrite, which no metadata binding can set today. It is wrapped for parity with the other two sites and covered by its own case, rather than left to drift.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 10 documentable anchor(s).

19 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json cca1dc0bfb1e380442670843606d6a0c7762f6ba.

⛔ 3 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see

• 4 name(s) were too generic to anchor anything (single lowercase words)
• the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json cca1dc0bfb1e380442670843606d6a0c7762f6ba → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 579766ba6067e1dc2d6218d594bc6203b2549253 — the merge of head 94193545fd0d378ccca1ceddc9038f76c12cadbb into base cca1dc0bfb1e380442670843606d6a0c7762f6ba, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 579766ba6067e1dc2d6218d594bc6203b2549253 && git checkout 579766ba6067e1dc2d6218d594bc6203b2549253
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cca1dc0bfb1e380442670843606d6a0c7762f6ba 94193545fd0d378ccca1ceddc9038f76c12cadbb && git checkout -B drift-repro cca1dc0bfb1e380442670843606d6a0c7762f6ba && git merge --no-ff 94193545fd0d378ccca1ceddc9038f76c12cadbb

node scripts/docs-audit/affected-docs.mjs --json cca1dc0bfb1e380442670843606d6a0c7762f6ba

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs cca1dc0bfb1e380442670843606d6a0c7762f6ba → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[claude]

Contract review — in-seat clause-② review at default tier · VERDICT: PASS

⚠️ Reviewed by the seat that dispatched it. Every claim carries a citation. Reviewing head 94193545fd.

---

1. ⛔ This seat's dispatch order was WRONG about the lane, and the round caught it

My order named packages/runtime/src/sandbox/** as the expected landing surface. Triage had already ruled otherwise — it called that path the domain:cli lane, needing a cross-domain exception. The round flagged the conflict rather than silently picking, and resolved it the conservative way: every line of production code landed in packages/objectql (this lane), and the only packages/runtime touch is one test file.

⭐ That is the correct resolution and it is the opposite of the failure mode: an implementer that had simply followed my order would have put production code in another seat's lane on my say-so. A dispatch order is not a licence to cross a lane boundary triage already drew.

2. The landing site was measured, not assumed — and the measurement moved it

The order required the site be reported before widening. The reported reason it is not the sandbox: that face is a plain JSON snapshot (unwrapProxyToPlain → JSON marshal), so a withheld key is simply absent there and is indistinguishable downstream from "nobody sent it" — which is precisely what ruling 4 requires the message to say. ⇒ The explanation can only be composed at the hide pass, where the withheld set still exists.

Verified in the diff: engine.ts:221 imports dispatchHooksExplainingWithheldReadonly, and all three beforeUpdate dispatch sites inside the hide window are wrapped (:11709, :11786, :11819) — the single-row path, the unscoped multi-write path and the per-row path. ⭐ Wrapping one and calling it done would have left two doors emitting the raw TypeError.

3. ⭐ The card UNDERSTATED the defect, and the round measured the worse case

The card quotes the direct door's SandboxError: … TypeError: cannot set property 'who' of undefined. At the REST door the answer was 500 {"error":"Internal server error","code":"INTERNAL_ERROR"} — the author is told nothing at all.

⛔ Not taken on report: packages/rest/src/error-response.ts documents that classification in its own words at :143 ("or a plain handler bug (TypeError: x is not a function). Both are server…"), :194, :221 and :253. The mechanism the round names is the one the shipped code describes. And the round's own control fired — an authored throw still answers 400 with its own words — so the blank envelope is a reading, not an inference.

4. Published surface — unchanged, and ⚠️ the export grep alone would have said the opposite

git diff origin/main...pr | grep '^+\s*export' ⇒ 3 added export lines. ⛔ Reported alone that would read as a surface enlargement. Reachability, measured on the PR head:

reading	result
src/index.ts re-exports the new module	NO
control — export … from './…' lines in index.ts	38 (the grep fires)
export * from barrels in index.ts that could sweep it in	0
precedent — hook-write-provenance (#14088, same shape) in index.ts	0, same treatment
packages/spec diff	0 lines ⇒ no ledger code registered

⇒ The three exports are package-private. Clause-②: no holds, and the patch is not borrowing an export rung. check-clause2-carriers --pair 17327 exit 0, and it reports no widening tell in the diff.

⭐ The module says so itself rather than leaving it to a reviewer: its header states ⛔ it registers no error code, because a ledger entry "is a published member" that would flip clause ② to yes, and re-declaring that is not the implementer's. It also records why ERR_READONLY_FIELD_REJECTED was refused for reuse — that code names the strictReadonlyWrites refusal and carries drops in its contract, so borrowing it would make the error lie about which refusal happened.

5. Grade patch — correct

An envelope on an existing refusal: the write was already refused before this change; what moves is the message and the status. No published export moves, no ledger member is added, packages/spec is untouched. ⇒ ⛔ Not minor.

⚠️ The one thing worth naming: readonly status = 400 replaces a 500 at the REST door. That is a status change on a published API — but it neither widens the accept set nor enlarges the published surface, and the module argues 400 is the truthful status because the trigger is a value the caller sent. Clause ② is not touched.

6. Rulings 1 and 2 held

⛔ The refusal is untouched and re-pinned in every case; ⛔ nothing was marshalled onto the sandbox face. ⭐ And the sandbox face was refused on the measurement, not merely obeyed: composing there is impossible, which is a stronger reason than the order's.

7. The evidence shape — the ablation is the strongest this lane has seen tonight

• Run from the committed state, in the real direction, with the direction predicted before running.
• On-disk proof before the run: marker occurrences 4 → 0, blob efa1a3ba88 → e54df038b6 — a non-empty HEAD blob hash compared, ⛔ not an exit code.
• ⭐ DIST proof, which the order did not ask for: the runtime suite resolves @objectstack/objectql through exports to dist/, not src/, so a source-only ablation would have proved nothing about it. The round rebuilt and confirmed absence from all 14 built files. An ablation that does not reach the artefact under test is a no-op wearing an ablation's clothes.
• The 3 failures are exactly the 3 dispatch-site cases; the 3 controls stayed green (they assert pass-through, i.e. pre-fix behaviour), and so did the composer's unit suite in isolation.
• Restore proven by whole-tree git status --porcelain EMPTY and blob equality and marker count back to 4 and a rebuild.

8. ⭐ The round declared its own process failure plainly, and that is worth more than a clean-looking report

It opened the PR before running the package typecheck; objectql#typecheck exited 2 on ErrorOptions / a two-argument super() under lib:ES2020. ⛔ The fix was not a cast and not a suppression — it followed the precedent already written in the same package (duplicate-record-error.ts declares readonly cause: unknown for exactly this reason), pushed as a new commit on top (94193545fd, one parent, ⛔ no rebase/amend/force-push).

⭐ A round that pushes red once and says so is worth more than one that hides it. Recorded as a process reading, ⛔ not as a finding against the delivery.

9. Gates, NOT MEASURED, and docs

• dispatch-gates --ran: 64 derived, 62 green, 2 NOT-MEASURED, 0 UNRUN — and the two are exit 3 = PREREQUISITE NOT MET (check:dual-build-cjs-loads, check:type-check-debt), both needing a full workspace build, neither naming a file in this diff. ⭐ Declared to CI, ⛔ not skipped, and ⛔ not counted as a pass.
• Lint ran the FULL population — eslint . --no-inline-config exit 0, 6,483 files, counted from eslint's own JSON output length rather than guessed ⇒ ⛔ no narrowing argument owed.
• docs-drift: 22 pages named. 3 release-owned — ⛔ NOT touched (read-only by directive). The 19 hand-written pages were READ and are UNAFFECTED, with the two plausible staleness candidates checked closely: api/error-catalog.mdx already carries the governing clause verbatim ("An error carrying its own status is served with it") — which is the exact channel this fix rides — and kernel/contracts/data-engine.mdx documents ERR_READONLY_FIELD_REJECTED, deliberately not reused, so it stays true. ⭐ 「Advisory only」 was not accepted as an answer, and it was not given as one.

⚠️ A VOID reading of my own during this review, recorded rather than quietly re-run

Checking whether the new module is reachable from the package entry, my first grep returned zero references anywhere — which contradicted the report. Cause: I grepped the local checkout (packages/objectql/src), which is on origin/main and has no such module, instead of the PR head. ⭐ The pattern was right and the tree was wrong. A new variant of tonight's recurring failure, and the only reason it was caught is that the answer contradicted a claim I could check.

Gates and CI

check-clause2-carriers --pair 17327 ⇒ 0. Governed predicate over the FULL 6-file list ⇒ 0 hits, firing control (adding AGENTS.md) ⇒ exit 3, GOVERNED. CI on 94193545fd: 33 distinct, 0 red, 0 pending; legacy combined status read separately: success.

open_questions: [] — ⛔ no PASS over an unruled question.

---

VERDICT: PASS. The lane conflict was flagged and resolved conservatively against this seat's own wrong order; the landing site was moved by measurement; the surface is package-private with reachability verified on the PR head; patch is an envelope on an existing refusal; the ablation reaches the built artefact the test actually loads. Landing next.

---

Generated by Claude Code