fix(objectql): flat-input ownKeys reports the payload's own key set, not its enumerable subset

[os-warren]

Fixes#12578

Verified at 2bdb36a433 — every reading below (gate union, suites, lint, ablation) was taken on that tree, which is this branch's head.

The defect, reproduced before repairing

installFlatInput's ownKeys trap answered from Object.keys(data) — own enumerable string keys. That enumerable filtering was incidental to what the trap is for (hiding the wrapper keys id/options/ast/data), and it cost a key. Measured on the merged ref through the real proxy, with the row the engine keeps alongside:

// beforeInsert, caller payload { subject: 'help' }
Object.defineProperty(ctx.input, 'k', { value: 1, enumerable: false, configurable: true });
Object.getOwnPropertyDescriptor(input, 'k') -> { value: 1, enumerable: false, … } own
Object.prototype.hasOwnProperty.call(input, 'k') -> true own
Object.getOwnPropertyNames(input) -> ['subject'] not own?
Reflect.ownKeys(input) -> ['subject'] not own?
Object.getOwnPropertyNames(raw.data) -> ['subject', 'k'] the persisted row

Three instruments, one payload, two answers about own-ness — and the key the enumeration face denies is on the row the driver receives. Newly reachable rather than newly written: #12277 routed defineProperty into data, so a handler can put a non-default-attribute key on the payload for the first time, and #12397 made the descriptor trap mirror data instead of synthesising defaults, which is what gave the third instrument an opinion to disagree with.

The fork, and which way it went

Triage named two options and assigned the pick to this lane.

Taken — option 1, mirror the payload's own-key set, spelled Object.getOwnPropertyNames(data). [[OwnPropertyKeys]] is the wrong layer at which to apply an enumerability filter, because every consumer that wants one applies it itself, one layer up and through the descriptor trap. Filtering here as well made none of those answers cleaner — it only starved the two surfaces whose entire job is to report the whole set.

Rejected — option 2, declare ownKeys the enumerable face and make the other instruments agree with it. Making hasOwnProperty and the descriptor trap agree with an enumerable-only enumeration means reporting a key as not own — which reverts #12397's mirror for exactly the case it was built for, and buys a worse lie than the one it removes: the key would then be invisible to all three instruments and still persist. That is the #12277 silent-success shape, which body-runner.ts names as the one with no instrument to catch it.

Measured cost of option 1: exactly two lines move

The same probe, before and after, byte-compared:

3c3 A: a non-enumerable own string key
< "enumerationSurface": false,
---
> "enumerationSurface": true,
37c37,38 Object.getOwnPropertyNames(input)
< "subject"
---
> "subject",
> "k"

Everything else is identical, and deliberately so: Object.keys, spread, Object.entries and JSON.stringify still return ['subject'] / {subject:'help'}, because each applies the enumerable filter itself through the descriptor trap. Wrapper keys stay hidden, symbols stay unenumerated.

That is the answer to the sandbox contract this card had to satisfy. unwrapProxyToPlain (packages/runtime/src/sandbox/body-runner.ts) documents itself as materialising "only what installFlatInput's ownKeys enumerates", via Object.entries. Because Object.entries keeps own enumerable string keys, the marshalled set is unchanged — confirmed by running the sandbox suite against a rebuilt objectql (see Verification). The comment is tightened to say enumerable string subset, since after this card ownKeys is a strict superset of what that snapshot materialises and the loose wording would have read as equality.

⭐ The contract spelling is now settled and declared

The tree held two answers and declared neither: the implementation said Object.keys(data), and the sandbox test double at body-runner.test.ts:141 modelled Reflect.ownKeys. Closing that is this card's deliverable, so all three now say the same thing:

• declared in packages/spec/src/data/hook.zod.ts, the hook-context contract an app author reads — ownKeys reports the payload's own key set, not its enumerable subset; the three own-ness instruments agreeing is the contract;
• implemented in hook-wrappers.ts;
• modelled by the sandbox double, which now spells the settled trap.

Changed existing assertion — declared

packages/runtime/src/sandbox/body-runner.test.ts:141, ownKeys: (t) => Reflect.ownKeys(t) → Object.getOwnPropertyNames(t), with the reason stated on the line in the test file itself. This double was the tree's second answer; left as it was, this PR would have moved the inconsistency rather than removed it. No assertion in that test changes — its subject is write-back, and it still passes. No other existing assertion is touched.

What is NOT decided here — the maintainer floor

Symbol keys stay unenumerated, and this is reported rather than decided. Option 1's full spelling (Reflect.ownKeys(data)) would additionally publish them. The measurement sharpens what that would mean, and it is not what the card assumed:

input[sym] = 'symvalue';
Object.getOwnPropertyDescriptor(input, sym) -> own, enumerable: true
hasOwnProperty(input, sym) -> true
Object.getOwnPropertySymbols(input) -> [] <- same disagreement
Object.getOwnPropertySymbols(raw.data) -> [Symbol(…)] <- and it persists

Symbol keys already reach the payload and already persist. So publishing them is not about making them reachable — it is about what the enumeration face should say concerning what the payload is allowed to hold, which is a payload-contract question and the boundary #12397 drew. It stays open on #12578, it is one word away (Object.getOwnPropertyNames → Reflect.ownKeys) once answered, and it is pinned in its open state so that answering it changes a recorded fact rather than an unnoticed one. Both the code comment and the test say so explicitly, so the omission cannot later read as a decision.

The pin asserts the AGREEMENT, not one trap

packages/objectql/src/hook-input-ownkeys-agreement.test.ts (6 cases) reads all three instruments on one key through one object and compares the triple against itself, plus against the persisted row. Pinning a single trap's output in isolation is what let these halves diverge — on the same trap set, in the same file, in the same week as #12397. The two deliberate exceptions are pinned as exceptions so neither can be mistaken for residue of the defect: wrapper keys (hidden from enumeration by design) and the open symbol half.

Clause ②, judged against the diff

This is an observable behaviour change on a shipped surface, and it WIDENS what enumeration exposes — git diff --stat behind the claim:

 .changeset/flat-input-ownkeys-own-key-set.md | 48 +++++
.../src/hook-input-ownkeys-agreement.test.ts | 225 +++++++++++++++++++++
packages/objectql/src/hook-wrappers.ts | 64 +++++-
packages/runtime/src/sandbox/body-runner.test.ts | 11 +-
packages/runtime/src/sandbox/body-runner.ts | 19 +-
packages/spec/src/data/hook.zod.ts | 12 ++
6 files changed, 368 insertions(+), 11 deletions(-)

Of the 64 lines in hook-wrappers.ts, the executable change is the trap body alone; the rest is the comment stating the contract and its two exceptions. The widening is stated precisely rather than bare: previously-invisible keys become visible to Object.getOwnPropertyNames and Reflect.ownKeys only, it is bounded to keys the payload genuinely owns and the engine already persists, and it is measured as exactly the two lines shown above — no accept set is narrowed, no rejection path is added or removed, and the enumerable face that every documented idiom uses is byte-identical. No public type or spec shape changes; the spec edit is the contract declaration in prose.

Verification

Gate union derived by node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack from the real changeset (6 paths, three-dot vs merge base f93df4dbe3) — 31 path-matched families plus the convention-triggered set for adding test files. Per-command exits captured before any pipe; each verdict below is the gate's own line, not a $?.

Check	Verdict
@objectstack/objectql full suite	Test Files 239 passed (239) · Tests 4203 passed (4203)
@objectstack/runtimesrc/sandbox/ (after rebuild)	Test Files 15 passed (15) · Tests 168 passed (168)
typecheck objectql / runtime / spec	all Done; spec also check:test-typecheck: OK
pnpm lint (whole repo, eslint . --no-inline-config)	clean, 63s
check:nul-bytes	OK (scanned 6951 text file(s) … no raw ASCII control bytes)
check:authorable-surface	✅ Successfully generated 1596 schemas. — no artifact drift (git status clean after)
check:liveness / check:empty-state / check:strictness-ledger / check:variant-docs	all ✓
check:engine-double-contract	OK — 689 pinned, 134 in the DEBT ledger, 3 exempt
check:objectql-double-limit	ObjectQL double limit conformance holds: 280 double(s) graded
check:cross-package-test-inputs	OK: 20 package(s) read outside themselves, all declared
check:test-source-alias	OK — 72 packages with tests scanned
check:type-check-debt (--re-measure)	OK — 31 ledger entr(ies) re-measured in 236.1s, 1687 raw tsc error(s), none above its recorded number
check:type-check-coverage, check:where-matcher, check:query-options-erasure, check:durability-log-level, check:page-declaration-shape, check:slot-lookup, check:published-files, check:merge-driver, check:doc-authoring, check:spec-parsed-alias, check:type-source-resolution, check:objectui-changeset, check:changeset-gate-self-tests	exit 0
check-changeset-no-major, check-empty-changeset, check-adr-0087-registration, check-comment-mask-adoption, check-ci-filter-parity, check-engine-split-ratio, check-plugin-teardown-shape, release-rehearsal-clone --self-test, docs-audit/check-affected-docs, docs-audit/check-drift-comment	exit 0
check-dev-prereqs	exit 1 on an unbuilt worktree, ✓ 67 package build artifacts present after the full build — it measured the worktree, never the diff

Ablation

Direction and exact count predicted in writing first, then the mutation proved on disk with anchored grep -cF counts before any result was read, all under trap … EXIT INT TERM.

Mutation: the repaired trap line only, Object.getOwnPropertyNames(target.data) → Object.keys(target.data). Anchors after mutation: injected 1, removed 0 (MUTATION-CONFIRMED-ON-DISK; a zero-hit edit would have voided the run rather than passing silently).

Predicted: red, 2 failed / 32 passed of 34 across the four hook-input suites, both failures in the new file, and the enumerable-face assertions staying green under the mutation. Observed, exactly that:

 FAIL src/hook-input-ownkeys-agreement.test.ts > REPRODUCTION — a key defined non-enumerable is own to all three instruments…
- "enumeration": true + "enumeration": false
FAIL src/hook-input-ownkeys-agreement.test.ts > the ENUMERABLE face is unchanged — Object.keys, spread, entries and JSON still omit it
expected [ 'subject' ] to deeply equal [ 'subject', 'hidden' ] (line 152 — ownNames)
Test Files 1 failed | 3 passed (4)
Tests 2 failed | 32 passed (34)

The second failure landing on line 152 is the load-bearing detail: lines 148–151 (Object.keys, spread, entries, JSON) passed under the mutation, which is what proves that half of the contract genuinely untouched by either spelling.

No rebuild owed on this leg, justified by import form rather than assertion: the pin imports ./hook-wrappers.js, a relative specifier inside the same package that vitest resolves to the TypeScript source — no package exports, no dist on the path. Corroborated empirically, since the before/after measurements differed with no build between them. @objectstack/runtime's sandbox tests do reach objectql through exports → dist, so that suite was run only after pnpm --filter '@objectstack/runtime^...' build, with reach proved in the artifact itself: packages/objectql/dist/index.js:3721 carries Object.getOwnPropertyNames(target.data) and the pre-fix spelling is absent (grep -cF = 0).

Restore verified byte-for-byte (cmp → identical to the pre-ablation copy, anchors back to 1/0). Noted for the record: the restore was checked by byte comparison rather than by an empty git diff, because the ablation ran before the repair was committed — cmp is the stronger check of the two, but it is not the one the standard clause names.

Changeset

patch on @objectstack/objectql, the only package whose runtime behaviour changes. @objectstack/runtime and @objectstack/spec carry comment-only edits plus the test double, so they publish no behaviour change and are deliberately not listed. patch rather than minor is defended in the changeset body: no declared surface shape changes, the enumerable face every documented idiom uses is byte-identical, and the only newly-listed keys are ones the payload genuinely owns and the engine already persists.

Out of scope, filed not fixed

The measurement turned up a distinct defect in the same trap set, filed unassigned as #12601: a payload field literally named id reads back the wrapper's value through the get trap ('WRAPPER-ID') while the descriptor trap reports the payload's ('PAYLOAD-ID'), because the two traps order the wrapper and the payload differently. It is not repaired here — which side should win depends on whether a payload may declare a field named id at all, which is the same maintainer floor. Deliberately untouched by this PR in either direction; the measurement is identical before and after this change. That card is not addressed by this branch.

Generated by Claude Code

---

Generated by Claude Code

[os-warren]

PM review — accepted, flipped ready, auto-merge armed. The symbol half is now tracked as #12603, filed before merge.

That last part first, because it was time-critical: this PR carries the closing keyword on #12578, so merging closes the card — and the open symbol question would have gone with it. The report flagged exactly that ("if the symbol question should be tracked it needs its own card before or at merge"). #12603 is filed with the measurement, the three options and the recommendation. Nothing is lost on merge now.

⛔ My brief's premise about symbols was wrong, and measuring it is what produced #12603

I wrote that "symbol keys becoming reachable is the obvious way in" to the payload-contract question. Measured, they are already reachable and already persist: input[sym] = 'symvalue' routes through the set trap into data, the descriptor trap reports it own and enumerable, hasOwnProperty is true, and the persisted row holds it — only Object.getOwnPropertySymbols(input) returns [].

So it is the same three-instrument disagreement this PR just closed for strings, not a new capability anyone was proposing to add. The question is what enumeration should publish, not what a hook can already put on the row. That is a materially different card from the one my brief described, and it exists because the dev measured the premise instead of inheriting it. Fourth premise of mine corrected by measurement today; recorded in §7.

The fork, and why the narrower spelling is the right half

Option 1, spelled Object.getOwnPropertyNames(data) — deliberately not Reflect.ownKeys(data). Verified in the diff: the trap goes Object.keys(target.data) → Object.getOwnPropertyNames(target.data).

The argument against option 2 is the one that settles it: documenting ownKeys as "the enumerable face" would require the descriptor trap and hasOwnProperty to report a genuinely-own key as not own — reverting #12397's mirror for exactly the case it was built for, and replacing the current inconsistency with a worse one, where the key is invisible to all three instruments and still persists. That is the silent-success shape body-runner.ts itself names as having no instrument to catch it.

And the layering argument is right: ownKeys is the wrong place for an enumerability filter, because every consumer that wants one applies it itself one layer up through the descriptor trap. Filtering here made no answer cleaner and only starved the two surfaces whose job is to report the whole set.

The spelling is settled in three places that previously held two undeclared answers

	where
declared	packages/spec/src/data/hook.zod.ts — that ownKeys reports the payload's own key set, that the three own-ness instruments agreeing is the contract, and that symbols are the one split left open deliberately
implemented	hook-wrappers.ts
modelled	the sandbox double — Reflect.ownKeys(t) → Object.getOwnPropertyNames(t), carrying a [#12578] CHANGED note on the line

Verified all three. That third row is the card's actual deliverable: the double had been asserting a different trap than the implementation ran, and a fix that left it that way would have moved the inconsistency rather than removed it.

Clause ② — yes, and it widens, stated as such

Previously-invisible keys become visible to Object.getOwnPropertyNames and Reflect.ownKeysonly, bounded to keys the payload genuinely owns and the engine already persists. No accept set narrowed, no rejection path touched, no public type moved. And measured rather than argued: the same probe before and after differs by exactly two lines, with Object.keys, spread, Object.entries, JSON.stringify and the symbol and wrapper-key readings byte-identical across the pair. That is why the sandbox snapshot contract is untouched — unwrapProxyToPlain is an Object.entries, which applies the enumerable filter itself.

The ablation, and a declared deviation I want on the record as good practice

Predicted 2 failed / 32 passed of 34, naming which two — and, more usefully, predicting that the enumerable-face case would fail only at its final ownNames assertion. Observed exactly that: the second failure at line 152, so lines 148–151 (Object.keys, spread, entries, JSON) passed under the mutation. That is the leg proving the enumerable face genuinely untouched, and it could not have been faked by a coarser prediction.

The mutation script exits 70 "results void" if its anchor count is wrong, so a zero-hit edit voids the run rather than passing silently.

⭐ And the deviation: the restore was verified byte-for-byte by cmp rather than by an empty git diff, because the ablation ran before the repair was committed. The dev declared it, and noted cmp is the stronger check but not the one the standard clause names. Substituting a stronger check and saying so is exactly right; substituting one silently is how a discipline decays into a ritual.

Follow-on filed

#12601 — a payload field literally named id / options / ast / data reads back the wrapper's value through the get trap while the descriptor trap reports the payload's: input.id → 'WRAPPER-ID' versus descriptor value → 'PAYLOAD-ID', with the key listed by ownKeys, so a spread and the sandbox's Object.entries snapshot both carry the envelope id under a payload field's name. Distinct mechanism, correctly not ridden along — it fails the bounded in-place exemption on clause ②, since which side wins collides with the documented input.id spelling and D4's HookTargetRebindError ruling.

CI is the remaining gate.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/objectql, @objectstack/runtime, @objectstack/spec, touching 3 documentable anchor(s).

⛔ 1 release-owned page(s) name something this change touched. These are read-only:

• content/docs/releases/v17.mdx(via HookContextSchema (symbol))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

• the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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 — 132 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 f93df4dbe314d5133f4a8c395b255ba0e2aeeaff → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7ac130440edc94c808d0bba8c792ff607c58a2b6 — the merge of head 2bdb36a433573d719c94f78f795fd8ab23e1a2f3 into base f93df4dbe314d5133f4a8c395b255ba0e2aeeaff, 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 7ac130440edc94c808d0bba8c792ff607c58a2b6 && git checkout 7ac130440edc94c808d0bba8c792ff607c58a2b6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f93df4dbe314d5133f4a8c395b255ba0e2aeeaff 2bdb36a433573d719c94f78f795fd8ab23e1a2f3 && git checkout -B drift-repro f93df4dbe314d5133f4a8c395b255ba0e2aeeaff && git merge --no-ff 2bdb36a433573d719c94f78f795fd8ab23e1a2f3
node scripts/docs-audit/affected-docs.mjs --json f93df4dbe314d5133f4a8c395b255ba0e2aeeaff

⚠️ 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 f93df4dbe314d5133f4a8c395b255ba0e2aeeaff → pass the list as
args.docs, on the commit named under Which tree this was computed on.