Skip to content

Commit 5a22dd7

Browse files
os-warrenclaude
andauthored
fix(objectql): flat-input ownKeys reports the payload own-key set, not its enumerable subset (#12578) (#12602)
`installFlatInput`'s `ownKeys` trap answered from `Object.keys(data)` — own enumerable string keys. The `enumerable` filtering was incidental to what the trap is for (hiding the wrapper keys), and it cost a key: an own non-enumerable key on the record payload was absent from `Object.getOwnPropertyNames`/`Reflect.ownKeys` while `hasOwnProperty` and the descriptor trap both reported it, and while the engine persisted the row holding it. Three instruments, one payload, two answers about own-ness. The trap now reports `Object.getOwnPropertyNames(data)`. The enumerable face is unchanged — `Object.keys`, spread, `Object.entries`, `for…in` and `JSON.stringify` apply the `enumerable` filter themselves, through the descriptor trap — so the sandbox body snapshot (`unwrapProxyToPlain`, an `Object.entries` over this proxy) marshals exactly what it marshalled before. Settles the spelling the tree held two undeclared answers to: the implementation said `Object.keys(data)`, the sandbox test double modelled `Reflect.ownKeys`. Declared in `packages/spec`'s hook-context contract, pinned in objectql as the AGREEMENT of the three own-ness instruments, and the double now models the settled spelling. Symbol keys stay unenumerated — an open payload-contract question, reported rather than decided. Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o Co-authored-by: Claude <noreply@anthropic.com>
1 parent dfebfc8 commit 5a22dd7

6 files changed

Lines changed: 368 additions & 11 deletions

File tree

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
fix(objectql): the flat-input proxy's `ownKeys` reports the payload's own key set, not its enumerable subset (#12578)
6+
7+
`installFlatInput` answered the `ownKeys` trap from `Object.keys(data)` — own **enumerable
8+
string** keys. That filtering was incidental to what the trap is for (hiding the wrapper keys
9+
`id`/`options`/`ast`/`data` from `Object.keys`/`for…in`), and it cost a key: an own
10+
**non-enumerable** key on the record payload was absent from `Object.getOwnPropertyNames(input)`
11+
and `Reflect.ownKeys(input)` while `hasOwnProperty` and the descriptor trap both reported it —
12+
and while the engine persisted the row holding it. Measured on the merged ref, for a payload
13+
`{ subject }` a handler had added `k` to with
14+
`Object.defineProperty(ctx.input, 'k', { value: 1, enumerable: false, configurable: true })`:
15+
16+
```
17+
Object.getOwnPropertyDescriptor(input, 'k') -> own, enumerable:false
18+
Object.prototype.hasOwnProperty.call(input, 'k') -> true
19+
Object.getOwnPropertyNames(input) -> ['subject'] <- not own?
20+
Object.getOwnPropertyNames(persisted row) -> ['subject', 'k']
21+
```
22+
23+
Three instruments, one payload, two answers about own-ness. Newly reachable rather than newly
24+
written: #12277 routed `defineProperty` into the payload, so a handler can put a
25+
non-default-attribute key there for the first time, and #12397 made the descriptor trap mirror
26+
the payload instead of synthesising defaults — which is what gave the third instrument an
27+
opinion to disagree with.
28+
29+
The trap now reports `Object.getOwnPropertyNames(data)`. **The enumerable face is unchanged**:
30+
`Object.keys`, spread, `Object.entries`, `for…in` and `JSON.stringify` still omit a
31+
non-enumerable key, because each applies the `enumerable` filter itself, one layer up, through
32+
the descriptor trap. Applying it inside `[[OwnPropertyKeys]]` as well did not make those answers
33+
cleaner — it only starved the two surfaces whose entire job is to report the whole set. The
34+
sandbox body face is byte-identical for the same reason: `unwrapProxyToPlain`
35+
(`@objectstack/runtime`) snapshots `ctx.input` as `Object.entries` over this proxy.
36+
37+
Wrapper keys stay excluded, which is the trap's purpose — achieved by reading `data` and never
38+
the wrapper, not by subtracting those four names, which would hide a genuine payload field named
39+
`id`. **Symbol keys remain unenumerated**: they already reach the payload and already persist, so
40+
publishing them through `ownKeys` is a question about what a record payload may hold rather than
41+
about this trap, and it is left open on #12578 rather than decided here.
42+
43+
Pinned in `hook-input-ownkeys-agreement.test.ts` as the AGREEMENT of the three own-ness
44+
instruments — not as one trap's output, which is the pin shape that let the halves diverge — with
45+
the wrapper-key and symbol exceptions pinned as deliberate exceptions. Reverse-verified by
46+
ablation: restoring `Object.keys(target.data)` fails exactly 2 of the 6 new cases (32 of 34 green
47+
across the four hook-input suites), and the enumerable-face assertions stay green under the
48+
mutation, which is what proves that half untouched.
Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#12578] The flat-input Proxy's own-key ENUMERATION agrees with its other two
5+
* own-ness instruments, and with the row the engine persists.
6+
*
7+
* `installFlatInput` (`hook-wrappers.ts`) answered `ownKeys` from
8+
* `Object.keys(data)` — own **enumerable string** keys. The `enumerable`
9+
* filtering was incidental to what the trap is for (hiding the wrapper keys),
10+
* and it cost a key: an own NON-ENUMERABLE key on the payload was absent from
11+
* `Object.getOwnPropertyNames` / `Reflect.ownKeys` while `hasOwnProperty` and
12+
* the descriptor trap both reported it. Measured on the merged ref before the
13+
* repair, for `Object.defineProperty(ctx.input, 'k', { value: 1,
14+
* enumerable: false, configurable: true })` over a payload `{ subject }` the
15+
* engine then persisted holding BOTH keys:
16+
*
17+
* ```
18+
* Object.getOwnPropertyDescriptor(input, 'k') -> own, enumerable:false
19+
* Object.prototype.hasOwnProperty.call(input, 'k') -> true
20+
* Object.getOwnPropertyNames(input) -> ['subject'] <- not own?
21+
* Object.getOwnPropertyNames(raw.data) -> ['subject','k']
22+
* ```
23+
*
24+
* Newly reachable, not newly written: #12277 routed `defineProperty` into
25+
* `data`, so a hook can put a non-default-attribute key on the payload for the
26+
* first time, and #12397 made the descriptor trap mirror `data` rather than
27+
* synthesise defaults — which is what gave the third instrument an opinion.
28+
*
29+
* ## What is pinned here, and why it is the AGREEMENT rather than one trap
30+
*
31+
* The contract these cases assert is not "`ownKeys` returns X". It is that the
32+
* three instruments an author can reach — the enumeration surfaces,
33+
* `hasOwnProperty`, and the descriptor trap — give the SAME answer about
34+
* own-ness for a given key, and that the answer is the one the persisted row
35+
* gives. A pin asserting a single trap's output in isolation is what let the
36+
* two halves diverge in the first place: #12397 pinned the descriptor trap and
37+
* this file's subject drifted out from under it, on the same trap set, in the
38+
* same file, within the same week.
39+
*
40+
* The settled spelling, stated once so the tree stops holding two answers:
41+
* **`ownKeys` reports the record payload's own key set, not its enumerable
42+
* subset.** Enumerability is applied by the CONSUMERS, one layer up and through
43+
* the descriptor trap, which is why the enumerable face below is unchanged.
44+
*
45+
* ## The two deliberate exceptions, pinned as exceptions
46+
*
47+
* - WRAPPER KEYS (`id`/`options`/`ast`/`data`) stay out of the enumeration
48+
* face while `hasOwnProperty` and the descriptor trap still report them.
49+
* That disagreement is the trap's whole purpose (the payload-diff idiom must
50+
* see record fields only) and is pinned as DECLARED so it cannot be mistaken
51+
* for a residue of the defect above.
52+
* - SYMBOL KEYS carry the identical disagreement and are deliberately left
53+
* carrying it. Publishing them is a one-word change here
54+
* (`Object.getOwnPropertyNames` -> `Reflect.ownKeys`), but whether a record
55+
* payload may hold a symbol key at all is a question about the PAYLOAD
56+
* contract — the boundary #12397 drew and this card does not cross. It is
57+
* reported open on #12578 and pinned below in its open state, so answering
58+
* it changes a recorded fact instead of an unnoticed one.
59+
*
60+
* `wrapDeclarativeHook` is driven directly rather than through `ObjectQL`, for
61+
* the reason the sibling trap-set files give: the subject is the wrapper's
62+
* Proxy, and a full engine dispatch would put a driver's own copy semantics
63+
* between the hook and the assertion.
64+
*/
65+
66+
import { describe, it, expect } from 'vitest';
67+
import { wrapDeclarativeHook } from './hook-wrappers.js';
68+
69+
const silentLogger = { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} };
70+
71+
/**
72+
* Run `handler` as a declarative hook over `raw` (the engine's own envelope);
73+
* the caller keeps `raw` and reads `raw.data` — the row the engine is left
74+
* holding — after the wrapper has restored `ctx.input`.
75+
*/
76+
async function runHook(raw: Record<string, unknown>, handler: (input: any) => void): Promise<void> {
77+
const meta: any = { name: 'ownkeys_probe', object: 'case', event: 'beforeInsert' };
78+
const wrapped = wrapDeclarativeHook(meta, (async (ctx: any) => handler(ctx.input)) as any, {
79+
logger: silentLogger,
80+
});
81+
await wrapped({ object: 'case', event: 'beforeInsert', input: raw } as any);
82+
}
83+
84+
/**
85+
* The three instruments, read on ONE key through ONE object. The assertions
86+
* below compare this triple against itself — that is the contract — rather than
87+
* asserting any member on its own.
88+
*/
89+
function ownness(obj: any, key: string | symbol) {
90+
return {
91+
enumeration: Reflect.ownKeys(obj).includes(key),
92+
hasOwnProperty: Object.prototype.hasOwnProperty.call(obj, key),
93+
descriptor: Object.getOwnPropertyDescriptor(obj, key) !== undefined,
94+
};
95+
}
96+
97+
/** All three instruments say "own". */
98+
const OWN = { enumeration: true, hasOwnProperty: true, descriptor: true };
99+
/** All three instruments say "not own". */
100+
const NOT_OWN = { enumeration: false, hasOwnProperty: false, descriptor: false };
101+
102+
describe('[#12578] the flat-input `ownKeys` reports the payload own-key set, and the instruments agree', () => {
103+
it('REPRODUCTION — a key defined non-enumerable is own to all three instruments, and to the persisted row', async () => {
104+
// The card's repro, verbatim. Pre-fix the `enumeration` member of this
105+
// triple was `false` while the other two were `true`.
106+
const raw: any = { data: { subject: 'help' }, options: {} };
107+
let seen: ReturnType<typeof ownness> | undefined;
108+
let names: string[] | undefined;
109+
await runHook(raw, (input) => {
110+
Object.defineProperty(input, 'k', { value: 1, enumerable: false, configurable: true });
111+
seen = ownness(input, 'k');
112+
names = Object.getOwnPropertyNames(input);
113+
});
114+
115+
expect(seen).toEqual(OWN);
116+
// The conjunction that makes it a contract and not three coincidences: the
117+
// proxy's own-key set IS the persisted payload's own-key set. Asserting
118+
// either side alone passes on a proxy whose halves disagree.
119+
expect(names).toEqual(Object.getOwnPropertyNames(raw.data));
120+
expect(names).toEqual(['subject', 'k']);
121+
// …and the payload really does carry it — the key is not an artefact of the
122+
// proxy face, it is on the row the driver receives.
123+
expect(ownness(raw.data, 'k')).toEqual(OWN);
124+
});
125+
126+
it('the ENUMERABLE face is unchanged — Object.keys, spread, entries and JSON still omit it', async () => {
127+
// The other half of the repair, and the reason reporting the full own-key
128+
// set costs nothing downstream: every consumer that wants enumerability
129+
// filters for it ITSELF, through the descriptor trap, which mirrors `data`.
130+
// `unwrapProxyToPlain` (`packages/runtime/src/sandbox/body-runner.ts`) is
131+
// the consumer this protects — it snapshots the hook body's `ctx.input` as
132+
// `Object.entries` over this proxy, so the marshalled set is exactly what
133+
// it was before this card.
134+
const raw: any = { data: { subject: 'help' }, options: {} };
135+
const seen: Record<string, unknown> = {};
136+
await runHook(raw, (input) => {
137+
Object.defineProperty(input, 'hidden', { value: 1, enumerable: false, configurable: true });
138+
seen.objectKeys = Object.keys(input);
139+
seen.spread = Object.keys({ ...input });
140+
seen.entries = Object.entries(input).map(([k]) => k);
141+
seen.json = JSON.parse(JSON.stringify(input));
142+
// The full set, alongside, in the same breath: this is the ONE surface
143+
// pair whose answers legitimately differ, and they differ by exactly the
144+
// non-enumerable key.
145+
seen.ownNames = Object.getOwnPropertyNames(input);
146+
});
147+
148+
expect(seen.objectKeys).toEqual(['subject']);
149+
expect(seen.spread).toEqual(['subject']);
150+
expect(seen.entries).toEqual(['subject']);
151+
expect(seen.json).toEqual({ subject: 'help' });
152+
expect(seen.ownNames).toEqual(['subject', 'hidden']);
153+
});
154+
155+
it('agrees the other way: a key the payload does not hold is own to none of them', async () => {
156+
const raw: any = { data: { subject: 'help' }, options: {} };
157+
let seen: ReturnType<typeof ownness> | undefined;
158+
await runHook(raw, (input) => {
159+
seen = ownness(input, 'absent');
160+
});
161+
expect(seen).toEqual(NOT_OWN);
162+
});
163+
164+
it('agrees on an ordinarily assigned key — the positive control', async () => {
165+
const raw: any = { data: {}, options: {} };
166+
let seen: ReturnType<typeof ownness> | undefined;
167+
await runHook(raw, (input) => {
168+
input.subject = 'help';
169+
seen = ownness(input, 'subject');
170+
});
171+
expect(seen).toEqual(OWN);
172+
expect(Object.getOwnPropertyNames(raw.data)).toEqual(['subject']);
173+
});
174+
175+
it('DECLARED EXCEPTION — wrapper keys stay out of enumeration while the other two report them', async () => {
176+
// Not a residue of the defect: hiding `id`/`options`/`ast`/`data` from
177+
// `Object.keys`/`for-in` is what this trap exists for. Pinned so the
178+
// exception stays deliberate and visible.
179+
const raw: any = { data: { subject: 'help' }, options: { multi: false }, id: 'WRAPPER-ID' };
180+
const seen: Record<string, unknown> = {};
181+
await runHook(raw, (input) => {
182+
seen.options = ownness(input, 'options');
183+
seen.id = ownness(input, 'id');
184+
seen.ownNames = Object.getOwnPropertyNames(input);
185+
// Still reachable by the spellings the contract names — hidden from
186+
// enumeration is not hidden from the author.
187+
seen.readId = input.id;
188+
seen.readMulti = (input.options as any).multi;
189+
});
190+
191+
expect(seen.options).toEqual({ enumeration: false, hasOwnProperty: true, descriptor: true });
192+
expect(seen.id).toEqual({ enumeration: false, hasOwnProperty: true, descriptor: true });
193+
expect(seen.ownNames).toEqual(['subject']);
194+
expect(seen.readId).toBe('WRAPPER-ID');
195+
expect(seen.readMulti).toBe(false);
196+
});
197+
198+
it('OPEN QUESTION, pinned in its open state — a symbol key carries the same disagreement', async () => {
199+
// Reported on #12578 rather than decided here: publishing symbol keys
200+
// through `ownKeys` is `Reflect.ownKeys` in one line, but whether the
201+
// record payload may CARRY a symbol key is a payload-contract question and
202+
// a maintainer floor (#12397's boundary).
203+
//
204+
// What the measurement establishes, and what this case records: symbol keys
205+
// already reach `data` through the `set` trap and already persist. So the
206+
// open question is about what the enumeration face should PUBLISH, not
207+
// about what a hook can already put on the row.
208+
const raw: any = { data: { subject: 'help' }, options: {} };
209+
const sym = Symbol.for('objectstack.test.12578');
210+
const seen: Record<string, unknown> = {};
211+
await runHook(raw, (input) => {
212+
input[sym] = 'symvalue';
213+
seen.ownness = ownness(input, sym);
214+
seen.symbols = Object.getOwnPropertySymbols(input);
215+
});
216+
217+
// Today: two instruments say own, enumeration says no — the defect's shape,
218+
// deliberately left standing on this half.
219+
expect(seen.ownness).toEqual({ enumeration: false, hasOwnProperty: true, descriptor: true });
220+
expect(seen.symbols).toEqual([]);
221+
// …while the payload the engine persists holds it.
222+
expect(Object.getOwnPropertySymbols(raw.data)).toEqual([sym]);
223+
expect((raw.data as any)[sym]).toBe('symvalue');
224+
});
225+
});

packages/objectql/src/hook-wrappers.ts

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -595,16 +595,64 @@ function installFlatInput(ctx: HookContext): () => void {
595595
if (data && typeof data === 'object' && prop in data) return true;
596596
return prop in target;
597597
},
598+
// [#12578] Reports the record payload's OWN key set — not its ENUMERABLE
599+
// subset. The trap answered from `Object.keys(data)`, which filters by
600+
// `enumerable`, and that filtering was incidental to what the trap is for:
601+
// hiding the WRAPPER keys. The two are different exclusions, and reading
602+
// one through the other cost a key.
603+
//
604+
// `[[OwnPropertyKeys]]` is the wrong place to apply an enumerability
605+
// filter, because every consumer that wants one applies it itself, one
606+
// layer up and through the descriptor trap: `Object.keys`, spread,
607+
// `Object.entries`, `for…in` and `JSON.stringify` all walk this list and
608+
// then drop what is not `enumerable`. Filtering here too does not make
609+
// those answers cleaner — it only starves the surfaces that ask for the
610+
// whole set, `Object.getOwnPropertyNames` and `Reflect.ownKeys`, which is
611+
// exactly what those two are for.
612+
//
613+
// #12277 routed `defineProperty` into `data`, so a hook can now put a
614+
// non-default-attribute key on the payload, and #12397 made the descriptor
615+
// trap mirror `data` instead of synthesising defaults. Measured on the
616+
// merged ref, for a key defined `{ enumerable: false }` on a payload the
617+
// engine then persisted with that key on it:
618+
//
619+
// Object.getOwnPropertyDescriptor(input, 'k') -> own, enumerable:false
620+
// Object.prototype.hasOwnProperty.call(input,'k') -> true
621+
// Object.getOwnPropertyNames(input) -> ['subject'] <- not own?
622+
//
623+
// Three instruments, one payload, two answers about own-ness — the same
624+
// shape #12397 closed one trap over, and legal for a proxy (extensible
625+
// target, no non-configurable own key) but untrue. The enumerable face is
626+
// deliberately NOT changed by reporting the full set: `Object.keys`,
627+
// spread, `Object.entries` and `JSON.stringify` still omit a
628+
// non-enumerable key, because they filter through the descriptor trap,
629+
// which mirrors `data`. That is what keeps the sandbox snapshot contract
630+
// (`unwrapProxyToPlain`, `packages/runtime/src/sandbox/body-runner.ts` —
631+
// `Object.entries` over this proxy) materialising exactly the fields it
632+
// materialised before. Both halves are pinned in
633+
// `hook-input-ownkeys-agreement.test.ts`.
634+
//
635+
// WRAPPER KEYS remain excluded, which is what this trap exists for:
636+
// `id`/`options`/`ast`/`data` stay reachable by dot/bracket notation but
637+
// out of `Object.keys`/`for-in`, so the payload-diff idiom
638+
// `Object.keys(input).filter(k => input[k] !== previous[k])` sees record
639+
// fields only. The exclusion is achieved by reading `data` and never the
640+
// wrapper — NOT by subtracting those four names, which would hide a
641+
// genuine payload field that happens to be called `id`.
642+
//
643+
// SYMBOL KEYS are deliberately still absent, and this is NOT a finding
644+
// that they do not belong on a payload: `Reflect.ownKeys(data)` here would
645+
// additionally publish them, and whether the record payload may carry a
646+
// symbol key at all is a question about the PAYLOAD contract (they already
647+
// reach `data` through the `set` trap and already persist — measured), not
648+
// about this trap. It is open, reported on #12578, and the day it is
649+
// answered "yes" this line becomes `Reflect.ownKeys`. Until then the
650+
// symbol half of the disagreement is pinned AS open in the sibling test,
651+
// so an answer changes a recorded fact rather than an unnoticed one.
598652
ownKeys(target) {
599-
// Only enumerate the flat record fields. Wrapper keys
600-
// (id/options/ast/data) remain accessible via dot/bracket notation
601-
// but are hidden from Object.keys/for-in so user code that does
602-
// `Object.keys(input).filter(k => input[k] !== previous[k])` only
603-
// sees actual record fields.
604-
const dataKeys = target.data && typeof target.data === 'object'
605-
? Object.keys(target.data)
653+
return target.data && typeof target.data === 'object'
654+
? Object.getOwnPropertyNames(target.data)
606655
: [];
607-
return Array.from(new Set(dataKeys));
608656
},
609657
// [#12397] MIRRORS `data`'s own descriptor; it does not synthesise one.
610658
// The literal that stood here — `{ configurable: true, enumerable: true,

0 commit comments

Comments
 (0)