Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions .changeset/flat-input-descriptor-mirror.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
---
"@objectstack/objectql": patch
---

fix(objectql): the flat-input Proxy mirrors `data`'s own descriptor instead of synthesising one (#12397)

`installFlatInput` hands a declarative hook a flat-record Proxy over the
engine's `{ data, options, id? }` wrapper. Its `getOwnPropertyDescriptor` trap
answered every key `data` carries with one fixed literal —
`{ configurable: true, enumerable: true, writable: true, value: data[prop] }` —
and never read `data`'s real descriptor. For a key created by ordinary
assignment that synthesis is the truth, which is why it cost nothing for as
long as assignment was the only way a key could arrive.

#12277 routed `defineProperty` into `data`, so a hook can now put a key on the
record payload with non-default attributes for the first time, and the
synthesis reported the defaults back regardless:

```js
Object.defineProperty(ctx.input, 'k', { value: 1, enumerable: false, configurable: true });
Object.getOwnPropertyDescriptor(ctx.input, 'k'); // reported enumerable: true — it is not
Object.keys(ctx.input); // …while this correctly omitted 'k'
```

Two instruments over one payload, contradicting each other. The trap now
mirrors `data`'s own descriptor.

`configurable` is the one attribute that cannot be mirrored: the proxy target
is the wrapper, which does not carry the record key, and a proxy may not report
a property its target lacks as non-configurable — a verbatim mirror throws
`TypeError` on any key `data` holds as `configurable: false`, and takes
`Object.keys` and spread down with it, since both reach every listed key
through this trap. It is forced `true`; `enumerable` / `writable` are mirrored.

Two further observable consequences, both pinned:

- Reading a descriptor no longer runs author code. The synthesis evaluated
`data[prop]` to fill `value`, so asking a payload that holds an accessor for
its descriptor invoked the getter; a mirror copies `get`/`set` across
untouched.
- `prop in data` is true for the whole prototype chain, so the synthesis
answered for inherited keys too — `Object.getOwnPropertyDescriptor(input,
'toString')` returned an own, enumerable, writable data property no payload
has ever held, and `Object.hasOwn(input, 'toString')` was `true`. Only an own
key has a descriptor to mirror; inherited keys now report `undefined`, while
`'toString' in input` and the read itself are unchanged.

Enumeration is untouched: `ownKeys` still lists exactly `data`'s own enumerable
keys and the mirror reports those as enumerable, so `Object.keys`, spread,
`Object.entries` and the sandbox's `unwrapProxyToPlain` see byte-identical
results. What a record payload may hold, how `defineProperty` routes into
`data`, and how the engine persists it are all untouched.
244 changes: 244 additions & 0 deletions packages/objectql/src/hook-input-descriptor-mirror.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#12397] `Object.getOwnPropertyDescriptor(ctx.input, k)` reports what `data`
* actually holds — it does not synthesise an answer.
*
* `installFlatInput` (`hook-wrappers.ts`) answered the descriptor trap for any
* key `data` carries with a fixed literal:
*
* ```
* { configurable: true, enumerable: true, writable: true, value: data[prop] }
* ```
*
* For a key created by ordinary assignment that synthesis is the truth, which
* is why it cost nothing for as long as assignment was the only way a key could
* arrive. #12277 routed `defineProperty` into `data`, so a hook can now put a
* key on the record payload with NON-DEFAULT attributes — and the synthesis
* reports the defaults back regardless.
*
* ## The constraint that shapes the fix
*
* The proxy target is the `{ data, options, id? }` WRAPPER, which does not
* carry the record key at all. A proxy may not report a property the target
* does not have as non-configurable, so a naive mirror throws `TypeError` on
* any key `data` holds as `configurable: false` — and, because `Object.keys`
* walks `ownKeys` through this trap, it throws on plain enumeration too. The
* mirror therefore FORCES `configurable: true` and mirrors the rest. Both legs
* are pinned below: the forced one (`a data key held non-configurable`) and the
* mirrored ones (`enumerable`, `writable`).
*
* ## What these cases deliberately do NOT pin
*
* Whether a record payload may carry an ACCESSOR at all — and what the engine
* should do persisting one, since it persists a payload by evaluating it — is a
* contract question about the payload, not about this trap. Nothing here widens
* or narrows it: the accessor case below asserts only the two facts that hold
* under every answer to it (the trap does not throw, and reading a descriptor
* does not RUN the getter — the synthesis did, via `data[prop]`). Routing
* (`defineProperty` → `data`) and persistence are untouched by this card.
*
* `wrapDeclarativeHook` is driven directly rather than through `ObjectQL`, for
* the reason the sibling trap-set file (`hook-input-mutation-traps.test.ts`)
* gives: the subject is the wrapper's Proxy, and a full engine dispatch would
* put a driver's own copy semantics between the hook and the assertion.
*/

import { describe, it, expect } from 'vitest';
import { wrapDeclarativeHook } from './hook-wrappers.js';

const silentLogger = { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} };

/** Run `handler` as a declarative hook over a caller payload; return the row the engine keeps. */
async function runHook(
data: Record<string, unknown>,
handler: (input: any) => void,
): Promise<Record<string, unknown>> {
const meta: any = { name: 'descriptor_probe', object: 'case', event: 'beforeInsert' };
const wrapped = wrapDeclarativeHook(meta, (async (ctx: any) => handler(ctx.input)) as any, {
logger: silentLogger,
});
const raw: any = { data, options: {} };
await wrapped({ object: 'case', event: 'beforeInsert', input: raw } as any);
return raw.data as Record<string, unknown>;
}

describe('[#12397] the flat-input descriptor trap mirrors `data` instead of synthesising', () => {
it('REPRODUCTION — a key defined non-enumerable reports non-enumerable', async () => {
// The card's repro, verbatim. Pre-fix this reported `enumerable: true`
// for a key that is not enumerable — and `Object.keys` agreed with the
// truth, not with the descriptor, so the two instruments an author has
// contradicted each other.
const seen: Record<string, unknown> = {};
const persisted = await runHook({ subject: 'help' }, (input) => {
Object.defineProperty(input, 'k', { value: 1, enumerable: false, configurable: true });
seen.descriptor = Object.getOwnPropertyDescriptor(input, 'k');
seen.objectKeys = Object.keys(input);
seen.read = input.k;
});

expect(seen.descriptor).toEqual({
value: 1,
writable: false,
enumerable: false,
configurable: true,
});
// The conjunction: the descriptor the author reads agrees with the row the
// engine is left holding. Asserting either alone would pass on a proxy
// whose two halves disagree, which is the defect itself.
expect(Object.getOwnPropertyDescriptor(persisted, 'k')).toEqual({
value: 1,
writable: false,
enumerable: false,
configurable: true,
});
expect(seen.objectKeys).toEqual(['subject']);
expect(seen.read).toBe(1);
});

it('`writable: false` is reported as such — the descriptor agrees with what assignment does', async () => {
// The instrument and the operation used to disagree in the loudest possible
// way: the descriptor advertised `writable: true` while the very next
// assignment threw, because the `set` trap writes into `data` from strict
// module code.
const seen: Record<string, unknown> = {};
await runHook({ subject: 'help' }, (input) => {
Object.defineProperty(input, 'frozen_key', {
value: 'V',
enumerable: true,
writable: false,
configurable: true,
});
seen.descriptor = Object.getOwnPropertyDescriptor(input, 'frozen_key');
try {
input.frozen_key = 'REASSIGNED';
seen.assignmentThrew = false;
} catch (err) {
seen.assignmentThrew = true;
seen.assignmentError = (err as Error).constructor.name;
}
seen.afterAssign = input.frozen_key;
});

expect(seen.descriptor).toEqual({
value: 'V',
writable: false,
enumerable: true,
configurable: true,
});
expect(seen.assignmentThrew).toBe(true);
expect(seen.assignmentError).toBe('TypeError');
expect(seen.afterAssign).toBe('V');
});

it('INVARIANT — a data key held non-configurable is reported configurable, and does not throw', async () => {
// The reason the mirror cannot be naive. The proxy target is the wrapper,
// which does not carry `locked`; reporting a target-absent property as
// non-configurable is a proxy invariant violation, so mirroring
// `configurable` verbatim throws `TypeError` here — and takes
// `Object.keys`/spread down with it, since those reach every listed key
// through this same trap.
const data: Record<string, unknown> = { subject: 'help' };
Object.defineProperty(data, 'locked', {
value: 'L',
enumerable: true,
writable: false,
configurable: false,
});

const seen: Record<string, unknown> = {};
await runHook(data, (input) => {
seen.descriptor = Object.getOwnPropertyDescriptor(input, 'locked');
seen.objectKeys = Object.keys(input);
seen.spread = { ...input };
// `Object.prototype.hasOwnProperty.call`, not `Object.hasOwn`: this
// package's programs run against `lib: ES2020` (see the workspace
// tsconfig), where the ES2022 spelling is a type error — and this file is
// read by the TEST_DEBT re-measure program, whose count is a ratchet.
seen.hasOwn = Object.prototype.hasOwnProperty.call(input, 'locked');
});

// `configurable` is FORCED, `enumerable`/`writable` are MIRRORED. That is
// the whole contract of this trap in one assertion.
expect(seen.descriptor).toEqual({
value: 'L',
writable: false,
enumerable: true,
configurable: true,
});
expect(seen.objectKeys).toEqual(['subject', 'locked']);
expect(seen.spread).toEqual({ subject: 'help', locked: 'L' });
expect(seen.hasOwn).toBe(true);
});

it('POSITIVE CONTROL — an ordinary assigned key still reads back as a plain data descriptor', async () => {
// Every existing consumer sees this shape and must keep seeing it: the
// mirror is only visible on keys that were not created by assignment.
const seen: Record<string, unknown> = {};
await runHook({ subject: 'help' }, (input) => {
input.owner_id = 'U1';
seen.assigned = Object.getOwnPropertyDescriptor(input, 'owner_id');
seen.caller = Object.getOwnPropertyDescriptor(input, 'subject');
});
expect(seen.assigned).toEqual({
value: 'U1',
writable: true,
enumerable: true,
configurable: true,
});
expect(seen.caller).toEqual({
value: 'help',
writable: true,
enumerable: true,
configurable: true,
});
});

it('an INHERITED key has no own descriptor to mirror, and no longer gets a fabricated one', async () => {
// `prop in data` is true for the whole prototype chain, so the synthesis
// answered `Object.getOwnPropertyDescriptor(input, 'toString')` with an own,
// enumerable, writable data property that no payload has ever held. `in`
// stays true (inherited keys ARE `in` the object) and the read still
// resolves up the chain — only the own-ness claim changes.
const seen: Record<string, unknown> = {};
await runHook({ subject: 'help' }, (input) => {
seen.descriptor = Object.getOwnPropertyDescriptor(input, 'toString');
seen.hasOwn = Object.prototype.hasOwnProperty.call(input, 'toString');
seen.inOperator = 'toString' in input;
seen.readable = typeof input.toString;
});
expect(seen.descriptor).toBeUndefined();
expect(seen.hasOwn).toBe(false);
expect(seen.inOperator).toBe(true);
expect(seen.readable).toBe('function');
});

it('an accessor on the payload: the trap neither throws nor RUNS the getter', async () => {
// Deliberately narrow — see the file header. Whether a record payload may
// carry an accessor at all is a contract question this card does not
// answer, so this pins only what is true under either answer. The second
// half is a property the synthesis did NOT have: it read `data[prop]` to
// fill `value`, so merely asking for a descriptor invoked author code.
let getterCalls = 0;
const data: Record<string, unknown> = { subject: 'help' };
Object.defineProperty(data, 'derived', {
get() {
getterCalls += 1;
return 'COMPUTED';
},
enumerable: true,
configurable: true,
});

const seen: Record<string, unknown> = {};
await runHook(data, (input) => {
seen.descriptorRead = () => Object.getOwnPropertyDescriptor(input, 'derived');
seen.callsAfterDescriptor = ((): number => {
Object.getOwnPropertyDescriptor(input, 'derived');
return getterCalls;
})();
});

expect(seen.callsAfterDescriptor).toBe(0);
});
});
43 changes: 41 additions & 2 deletions packages/objectql/src/hook-wrappers.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -606,10 +606,49 @@ function installFlatInput(ctx: HookContext): () => void {
: [];
return Array.from(new Set(dataKeys));
},
// [#12397] MIRRORS `data`'s own descriptor; it does not synthesise one.
// The literal that stood here — `{ configurable: true, enumerable: true,
// writable: true, value: data[prop] }` — happens to be the truth for every
// key created by ordinary assignment, which is why it cost nothing while
// assignment was the only way a key could arrive. #12277 routed
// `defineProperty` into `data`, so a hook can now put a key on the record
// payload with NON-DEFAULT attributes, and the synthesis kept reporting the
// defaults: `Object.defineProperty(input, 'k', { enumerable: false, … })`
// read back `enumerable: true` while `Object.keys(input)` — which reaches
// the same key through `ownKeys`/`data` — correctly omitted it. Two
// instruments, one payload, contradicting answers.
//
// `configurable` is the one attribute that CANNOT be mirrored. The proxy
// target is the `{ data, options, id? }` wrapper, which does not carry the
// record key at all, and a proxy may not report a property its target lacks
// as non-configurable — so mirroring it verbatim throws `TypeError` on any
// key `data` holds as `configurable: false`, and takes `Object.keys` and
// spread down with it, since both reach every listed key through this trap.
// It is therefore FORCED true and the rest mirrored. That forcing is the
// proxy's own constraint, not a claim about the payload.
//
// Two consequences worth naming, both pinned in
// `hook-input-descriptor-mirror.test.ts`:
//
// - Reading a descriptor no longer RUNS author code. The synthesis
// evaluated `data[prop]` to fill `value`, so asking a payload holding
// an accessor for its descriptor invoked the getter; a mirror copies
// `get`/`set` across untouched.
// - `prop in data` is true for the whole prototype chain, so the
// synthesis answered for INHERITED keys too — `toString` reported as an
// own, enumerable, writable data property no payload has ever held.
// Only an own key has a descriptor to mirror; the rest fall through.
//
// What this trap deliberately does NOT decide: whether a record payload may
// carry an accessor at all, and what the engine should do persisting one
// (it persists a payload by evaluating it). That is a contract question
// about the payload, and neither routing nor persistence is touched here —
// the trap reports what is there, under every answer to it.
getOwnPropertyDescriptor(target, prop) {
const data = target.data;
if (data && typeof data === 'object' && prop in data) {
return { configurable: true, enumerable: true, writable: true, value: (data as any)[prop] };
if (data && typeof data === 'object') {
const own = Object.getOwnPropertyDescriptor(data, prop);
if (own) return { ...own, configurable: true };
}
// Wrapper keys: still descriptors so `prop in input` works, but
// marked non-enumerable so they don't appear in Object.keys().
Expand Down
Loading