feat(objectql): find() guarantees its array — an afterFind that replaces the container is refused - #16232
Conversation
…ainer
`ObjectQL.find` declares `Promise<any[]>` but ended its hook path with
`return hookContext.result`, with nothing between the `afterFind` dispatch
and that return re-checking the value. A handler assigning
`ctx.result = { records: [ ... ] }` therefore made a `find()` declared to
resolve to an array resolve to an envelope, silently.
Ruled 2026-09-06 (direction 1): `find()` guarantees the array, and a hook
that replaces the container is refused loudly. The check sits immediately
after `triggerHooks('afterFind', ...)` and BEFORE `maskSecretFields` /
`stripSearchCompanionFromRead`, both of which already assume the array.
Shaping stays legal: mutating rows in place, dropping keys, filtering rows
out and assigning a different ARRAY are all untouched. `Array.isArray` is
the whole predicate.
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
Co-authored-by: Claude <noreply@anthropic.com>
…e envelope #15823 closes the seam this control drove: an `afterFind` assigning a non-array no longer produces an envelope at the fourteen real reads, it produces `FIND_HOOK_RESULT_NOT_ARRAY`. The control keeps both of its jobs — the mechanism is driven on every block and the refusal asserted, and `expectBareArray`'s discrimination is now checked directly, since no engine can hand it an envelope any more. #15597's own conclusion is untouched: the fourteen limbs were removed on the argument that they were right to remove even given an open seam. Also fixes the refusal message's article (`a object` -> `an object`) and pins it. Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ Co-authored-by: Claude <noreply@anthropic.com>
…odule top
Two gate repairs the guard's diff owes:
- `check:system-context-census` reads DOC anchors keyed by LINE NUMBER, and
the 21 lines the guard adds (plus the import) shifted fourteen of them in
`content/docs/permissions/system-context.mdx`. Repaired with the script's
own `--fix`; line numbers only, nothing semantic. Measured green at the
merge base first, so the rot is this branch's.
- `check:test-source-alias` refuses a first module load paid inside a clocked
test body: the new pin's `await import('@objectstack/spec/api')` moves to a
module-top import, so the transform is paid during collection.
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
Co-authored-by: Claude <noreply@anthropic.com>
…nd-afterfind-array-guard
`main` moved onto three of this branch's files (engine.ts, objectql/index.ts and the system-context census page). Landed through `scripts/pm/os-regen-merge.sh` so the os-regen-driven artifact is regenerated from the MERGED tree rather than text-merged: the driver merges those paths at exit 0 while silently keeping one side, and only a regeneration exposes it. The one artifact the pre-commit hook held the merge for — `content/docs/permissions/system-context.mdx` — is regenerated here with `pnpm gen:system-context-census` (15 anchors re-derived against the merged engine.ts; line numbers only). Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 131 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a0c6ffad64fe306a29e4c3b2149cdd81e599a9de && git checkout a0c6ffad64fe306a29e4c3b2149cdd81e599a9de
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0c3eaf578ec9e2786318f9cf868be06e14c4b6a0 5c8f04199e90162fe3a44539a259bf3e872dd881 && git checkout -B drift-repro 0c3eaf578ec9e2786318f9cf868be06e14c4b6a0 && git merge --no-ff 5c8f04199e90162fe3a44539a259bf3e872dd881
node scripts/docs-audit/affected-docs.mjs --json 0c3eaf578ec9e2786318f9cf868be06e14c4b6a0
|
Contract review (clause ②) — PASS on content · landable on green CI — PR #16232 at head
|
|
Director seat, 07:50Z — Generated by Claude Code |
Fixes #15823
Implements the director ruling of 2026-09-06 (comment 5557017787), direction 1:
find()guarantees the array, and anafterFindthat replaces the container is refused loudly.(Angle brackets do not survive this surface's sanitizer, so declared types are spelled with square brackets below:
Promise[any[]]is the realPromiseofany[].)The defect, reproduced before anything was changed
packages/objectql/src/engine.tsdeclaresasync find(object, query?, options?): Promise[any[]]and, on the hook path, ended withreturn hookContext.result— nothing between theafterFinddispatch and that return re-checked the value. A handler assigningctx.result = { records: [ ... ] }therefore made a read declared to resolve to an array resolve to an envelope: no throw, no diagnostic, no log.Reproduced live on unmodified
origin/main(bdc02182b) by runningpackages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts -t 'the control': 1 passed, its assertion beingexpect(Array.isArray(value)).toBe(false)across fourteen real read call sites driven through their real production entry points, over a realObjectQLon a realSqlDriver.What changed
1. The guard, at the seam the ruling names. In
find(), immediately afterawait this.triggerHooks('afterFind', hookContext)and beforemaskSecretFields/stripSearchCompanionFromRead— both of which already assume the array and both of which readhookContext.result. Not at thereturn.2. A named ADR-0112 error.
FindHookResultNotArrayError(packages/objectql/src/find-hook-result-shape.ts), codeFIND_HOOK_RESULT_NOT_ARRAY, status500. One code, one wording; the message names the hook event and the object, anddeveloperMessagecarries the remedy. Registered inERROR_CODE_LEDGERunder@objectstack/objectql.3. Shaping stays legal.
Array.isArrayis the whole predicate, deliberately: mutating rows in place, deleting keys, filtering rows out and assigning a different array are all untouched. Anything keyed on identity (comparing againstopCtx.result, freezing, cloning) would refuse legitimate reshaping, which is what ADR-0077 line 71 means by "shape reads".Two judgement calls, declared
undefined/nullare refused too. The ruling names only the envelope case. A handler that assigns neither is not replacing the container with an envelope, but it breaksPromise[any[]]exactly as much, and admitting it leaves a second hole in the same slot that no caller can distinguish from the first. A read that should answer nothing assigns[]; a handler that wants to refuse a read throws — the supported spelling every other hook guard already uses. Pinned either way, and argued in the module docblock.500, not400or403. The request was well-formed and authorized; a hook this deployment installed broke a server-side contract, and there is nothing the caller can change or retry. The cost is stated rather than discovered later: a declared 5xx has its prose withheld at the HTTP doors (declaresServerFault), so an HTTP caller readsInternal server errorplus this code. That is the right split — the registered code is the machine-readable half and crosses intact, while the message is addressed to the hook's author, who meets it in-process and in the server log.Which published surfaces widened — measured, not assumed
The dispatch brief asked whether the code's home is a second published surface in
packages/types/src/response-envelope.ts. It is not. That file importsErrorCodeas a type from@objectstack/spec/apiand declares no union of its own;packages/typesis untouched by this diff (git diff bdc02182b..HEAD -- packages/typesis empty) and 0 of its published declarations carry the new member (live control:ErrorCodeappears 9 times intypes/dist/index.d.ts, so the grep is real).The two surfaces that do widen, read off built declarations under each package's
files[]:@objectstack/objectqldist/index.d.ts,dist/index.d.mts@objectstack/specdist/api/index.d.ts@objectstack/specdist/export.zod-w7_kGqGb.d.ts(shared chunk)Worth recording because it is a live instance of a known false-negative shape:
packages/spec/dist/index.d.ts— the root barrel — carries 0 occurrences (live control on the same file: 23exportlines). Reading the root barrel alone would have answered "nothing widened" with confidence; the widening lives on the./apisubpath and in a shared chunk neither barrel names by symbol.Both are additive — no export removed, no code renamed.
content/docs/references/api/{contract,error-code-ledger}.mdxregenerated bygen:docs; the union's own count line moves+295 moreto+296 more, which is the widening stated by the generator.Pins — the ruling's three cases, plus the one it left open
packages/objectql/src/engine-find-hook-result-shape.test.ts, 10 cases:afterFindassigningctx.result = { records: [...] }is refused — asserting the ADR-0112 envelope (codeandstatus, never a baretoThrow()), the class, the event, the object and the observed shape;length/0getter pair and asserting the getters were read 0 times, rather than asserting about source order;[]all still answer an array ([]being exactly what a truthiness-shaped guard would have refused by accident);undefined,nulland a string all refused with the same code, each naming its observed shape;ErrorCodeunion, with a control proving the union rejects an unregistered spelling.The one consequence in the tree, handled without skipping anything
Exactly one suite in
packages/**registers anafterFindthat assigns a non-array: the#15597discrimination control inpackages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts. Measured with the guard in: 1 failed, 24 passed — itsexpect(Array.isArray(value)).toBe(false)is precisely what the ruling retires.Nothing was skipped, disabled or quarantined. That case is rewritten to assert what is now true, keeping both of its jobs: it drives the same handler across all fourteen real reads and asserts every one refuses with
FIND_HOOK_RESULT_NOT_ARRAY(a stronger statement of the same fact), and a second case exercisesexpectBareArray's discrimination directly, since no engine can hand it an envelope any more. That suite is now 26 passed.#15597's own conclusion is untouched here and is not reopened — its argument was that the fourteen limbs were right to remove even given an open seam.The two other real
afterFindregistrations both shape rows in place and keep the array (packages/rest/src/export-integration.test.ts, the FLS-delete and partial-masking hooks);plugin-audit'sread-audit.tsnever touchesctx.result. None of them moves. The remainingctx.result =sites in the tree belong to hand-built fake engines that construct their own context and never reach this seam.Ablation — the pins were seen to fail
Guard deleted with a whole-line anchor (
ANCHOR_HITS=1), mutation proved on disk before measuring (git hash-object630e3c67to4af01f1b; removed-text count 0, injected-marker count 1), then@objectstack/objectqlrebuilt andablation-dist-preflight --absentconfirming the marker gone from all 8 built files — which matters becauseplugin-authresolves@objectstack/objectqlthroughdist/, unaliased.engine-find-hook-result-shape.test.ts: 5 failed / 5 passed — the five refusal cases go red; the shaping, no-hook and ledger cases correctly do not depend on the guard.find-envelope-limb-removal.test.ts: 1 failed / 25 passed — the rewritten control goes red.(Ablation figures are from the pre-merge tree; the merge changed no line of the guard, and both suites were re-run green on the merge head.) Restore leg proved in the same shell: whole-tree
git status --porcelain0 lines, blob back to the HEAD blob630e3c67,git diff HEADempty; then rebuilt andablation-dist-preflight(present form) confirming the marker back in 4 built files and the tree clean. The mutating script carried anEXIT INT TERMtrap restoring through an absoluteREPO_ROOTpath.Verification
Derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackfrom the real change set (no hand-written path list): 106 families, re-derived three times — twice before the merge and once after — and byte-identical every time.mainmoved onto three of this branch's files while this work was in flight (engine.ts,objectql/src/index.tsand the census page), soorigin/mainwas merged in throughscripts/pm/os-regen-merge.sh— the repo's own sequence, which commits the merge before regenerating so an os-regen-driven artifact is re-derived from the merged tree rather than text-merged (that driver merges at exit 0 while silently keeping one side). The pre-commit hook held the merge for exactly one stale artifact,content/docs/permissions/system-context.mdx, regenerated withpnpm gen:system-context-censusand discharged in the same commit. GitHub now reportsmergeable: true.Everything below was re-run on the merge head
5c8f04199, not inherited from the pre-merge run:5c8f04199, working tree clean, exit codes captured by redirect-then-read (never through a pipe).pnpm lint(repo-wideeslint . --no-inline-config): exit 0, before and after the merge. Not narrowed.turbo run build --filter=./packages/* --filter=./packages/*/*: 71 successful, 71 total, re-run after the merge.pnpm --filter @objectstack/spec check:generatedon the merged tree: 15 / 15 artifacts current; and this branch's own ledger row, engine guard, module, index export, changeset and both generated doc entries verified present by quoted-exactgit grepafter the merge — the step-4 assertion the merge script prints.:9566, refusal:9581,maskSecretFields:9591.pnpm --filter @objectstack/objectql typecheck: green, and both new files proven inside thetsconfig.test.jsonprogram by--listFiles(1 hit each; control on a non-existent path 0).check:system-context-census(the guard's 21 lines shifted fourteen line-number doc anchors — measured green at the merge base first, so the rot is mine, then the script's own--fix; line numbers only) andcheck:test-source-alias(the new pin'sawait importmoved to a module-top import so the first load is paid during collection, not inside a clocked test body).NOT MEASUREDwas chased to a real reading rather than banked:check:type-check-debtfirst exited 3 (V8 OOM under a 4 GB wrapper heap — the gate's own text says exit 3 is not a finding). Re-run at 10 GB: exit 0, 12 ledger entries re-measured, none above its recorded number.Clause-② and release
Clause-②: yes, ruled — the accepted behaviour of the published hook contract narrows, and
@objectstack/spec'sErrorCodeaccept-set widens.needs:contract-reviewis hung by me, the PR creator, on both carriers — issue #15823 and this PR — additively, each with a comparative read-back provingunion(read, target)exactly and nothing stripped.Recorded because it is a live instance of a known incident: after the merge push the label was gone from both carriers — the auto-labeler's whole-set write had landed
documentation/size/l/tests/toolingover it. It was re-hung additively (the labeler's four kept), re-read after a delay, and both carriers hold it now. A contract-review clear is bound to a head, so the re-hang was owed by the push regardless.node scripts/pm/check-clause2-carriers.mjs --pair 16232returns exit 0 on the current head — the checker proven current againstorigin/mainby blob hash first, and the exit code captured before any pipe.⛔ 免复核不放行. This PR is left draft and is not released, queued, armed or approved by this seat.
Changeset:
@objectstack/objectqlminor and@objectstack/specminor, stating the refusal and its code.Scope fence
findOne(:9692),update(:10858) anddelete(:13075) also end onreturn hookContext.result, and all three declarePromise[any]— no enforceable declaration to violate. The ruling files that as its ownfindingfor the engine seat and it is not ridden in here — filed instead as #16231, unassigned, labelledfinding+domain:engine. The#15094normalizer-limb sweep is likewise not touched — the ruling's item 3 is a consequence recorded for later.Re-derived readings from the dispatch brief, published as measured on
bdc02182b:return hookContext.resultstill has four sites, not three —find:9425(Promise[any[]]),findOne:9671(Promise[any]),update:10837(Promise[any]),delete:13054(Promise[any]) — sofindremains the only one with a concrete declared shape. No fifth site has appeared. The brief's anchors held exactly:triggerHooks('afterFind', ...)at:9552,maskSecretFieldsat:9557,stripSearchCompanionFromReadat:9563.Authored by Claude Code in session
session_01ARYe3yQTQCUFm5qPYNgKaJ. (Attribution is kept in prose here: a body edit does not preserve the session-URL footer, and the platform appends its own block.)Generated by Claude Code