From 0554527ca4f97f2d0fbe3ca1669176b4c8ac047b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 16:14:26 +0000 Subject: [PATCH 1/3] test(objectql,runtime): pin #12601 envelope-precedence regression, pre-fix Written and run against unmodified origin/main before any fix lands, to serve as the ablation prediction: packages/objectql/src/hook-input-envelope-precedence.test.ts reproduces the get/getOwnPropertyDescriptor disagreement per reserved name (id/options/ast/data) and fails 5 of 7 cases (named set recorded in the dev's scratchpad prediction); the runtime-side consequence pin (packages/runtime/src/sandbox/hook-input-envelope-precedence.integration.test.ts) is already green pre-fix, "by accident" per its own header comment. Part of #12601 --- .../hook-input-envelope-precedence.test.ts | 225 ++++++++++++++++++ ...ut-envelope-precedence.integration.test.ts | 122 ++++++++++ 2 files changed, 347 insertions(+) create mode 100644 packages/objectql/src/hook-input-envelope-precedence.test.ts create mode 100644 packages/runtime/src/sandbox/hook-input-envelope-precedence.integration.test.ts diff --git a/packages/objectql/src/hook-input-envelope-precedence.test.ts b/packages/objectql/src/hook-input-envelope-precedence.test.ts new file mode 100644 index 0000000000..da8277033e --- /dev/null +++ b/packages/objectql/src/hook-input-envelope-precedence.test.ts @@ -0,0 +1,225 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#12601] The flat-input Proxy's `get` and `getOwnPropertyDescriptor` traps + * agree about `id` / `options` / `ast` / `data` when a record payload happens + * to declare a field sharing one of those names. + * + * ## The disagreement, measured on `origin/main` before this file's fix + * + * `installFlatInput` (`hook-wrappers.ts`) gives the four wrapper keys + * precedence in the `get` trap — a direct read always resolves against the + * WRAPPER (envelope), never the payload. The descriptor trap used to check + * `data` FIRST, so for an update-shaped envelope whose payload also carried a + * same-named field: + * + * ``` + * const raw = { data: { id: 'PAYLOAD-ID', subject: 'help' }, options: {}, id: 'WRAPPER-ID' }; + * input.id -> 'WRAPPER-ID' (get: envelope) + * Object.getOwnPropertyDescriptor(input, 'id').value -> 'PAYLOAD-ID' (descriptor: payload) <- DISAGREED + * Reflect.ownKeys(input).includes('id') -> true + * { ...input }.id -> 'WRAPPER-ID' (spread: envelope, via `get`) + * ``` + * + * `ownKeys` and spread ALREADY agreed with `get` before this fix — `ownKeys` + * lists the payload's own key set unconditionally (#12578, untouched here), + * and spread reads a key's VALUE through `get`, which always resolved the + * wrapper. Only a caller that reads a raw descriptor (rather than a value) + * saw the payload's own value — silently, and only for the four reserved + * names, and only when a payload happened to declare one of them. + * + * ## The maintainer ruling implemented here — Option A, "envelope wins + * consistently" + * + * `id` / `options` / `ast` / `data` are RESERVED NAMES on the hook flat-input + * face. A payload field carrying one of them stays a legal record field — + * it round-trips through storage exactly as declared — but it is not + * reachable through the flat face at all: `input.data.` is the only + * route to it. `getOwnPropertyDescriptor` now checks the reserved-name + * branch FIRST, exactly where `get` already does, so a descriptor read can + * never again report a different object's value than a plain read of the + * same key. `enumerable` still depends on whether `data` also owns the name + * (mirroring `ownKeys`, which is unchanged): that is what keeps `{...input}` + * and `Object.entries` carrying the envelope's value under the reserved name + * exactly as they did before this fix, rather than silently dropping the key. + * + * Declined: Option B (payload wins) — it would overturn the D4 + * `HookTargetRebindError` ruling and change `input.id`'s meaning on every + * update hook whose payload happens to carry a field called `id`. Option C + * (refuse the four names on the payload outright) is a recorded fallback, + * not this card — its prerequisite (#12397's maintainer floor) is unmet. + * + * `wrapDeclarativeHook` is driven directly rather than through `ObjectQL`, + * for the reason every sibling trap-set file in this directory 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 BEFORE-UPDATE hook over `raw` (the engine's + * own envelope, caller-owned so it can be inspected after the wrapper + * restores `ctx.input`). + */ +async function runHook(raw: Record, handler: (input: any) => void): Promise { + const meta: any = { name: 'envelope_precedence_probe', object: 'case', event: 'beforeUpdate' }; + const wrapped = wrapDeclarativeHook(meta, (async (ctx: any) => handler(ctx.input)) as any, { + logger: silentLogger, + }); + await wrapped({ object: 'case', event: 'beforeUpdate', input: raw } as any); +} + +describe('[#12601] the flat-input envelope wins consistently across get / descriptor / ownKeys / spread', () => { + it('REPRODUCTION — `id`: all four instruments answer the envelope, and `input.data.id` still answers the payload', async () => { + const raw: any = { data: { id: 'PAYLOAD-ID', subject: 'help' }, options: {}, id: 'WRAPPER-ID' }; + const seen: Record = {}; + await runHook(raw, (input) => { + seen.get = input.id; + seen.descriptorValue = Object.getOwnPropertyDescriptor(input, 'id')?.value; + seen.descriptorEnumerable = Object.getOwnPropertyDescriptor(input, 'id')?.enumerable; + seen.ownKeysIncludes = Reflect.ownKeys(input).includes('id'); + seen.spread = { ...input }.id; + seen.entries = Object.fromEntries(Object.entries(input)).id; + seen.dataId = input.data.id; + }); + + // The conjunction is the contract: every instrument that answers a VALUE + // for the reserved name answers the SAME value, and it is the envelope's. + expect(seen.get).toBe('WRAPPER-ID'); + expect(seen.descriptorValue).toBe('WRAPPER-ID'); + expect(seen.spread).toBe('WRAPPER-ID'); + expect(seen.entries).toBe('WRAPPER-ID'); + // The key stays listed (unchanged from #12578 — `ownKeys` reports the + // payload's own key set, and the payload really does own `id` here) and + // enumerable, which is what lets spread/`Object.entries` carry it at all. + expect(seen.ownKeysIncludes).toBe(true); + expect(seen.descriptorEnumerable).toBe(true); + // The payload's OWN value never vanished — it is exactly where the + // ruling says it stays reachable. + expect(seen.dataId).toBe('PAYLOAD-ID'); + // …and the persisted row is untouched by any of the above reads. + expect(raw.data).toEqual({ id: 'PAYLOAD-ID', subject: 'help' }); + }); + + it('`options`: a payload field named `options` is shadowed the same way', async () => { + const raw: any = { + data: { options: 'PAYLOAD-OPTIONS', subject: 'help' }, + options: { multi: true }, + }; + const seen: Record = {}; + await runHook(raw, (input) => { + seen.get = input.options; + seen.descriptorValue = Object.getOwnPropertyDescriptor(input, 'options')?.value; + seen.spread = { ...input }.options; + seen.dataOptions = input.data.options; + }); + + expect(seen.get).toEqual({ multi: true }); + expect(seen.descriptorValue).toEqual({ multi: true }); + expect(seen.spread).toEqual({ multi: true }); + expect(seen.dataOptions).toBe('PAYLOAD-OPTIONS'); + }); + + it('`data`: a payload field literally named `data` is shadowed by the whole payload bag', async () => { + // `data` collides trivially in one direction — `target.data` (the bag + // itself) always exists whenever a payload is present — but a payload + // whose OWN field is named `data` is the case this card is actually + // about: does a read of `input.data` mean "the bag" or "the bag's own + // `data` field"? The ruling says: always the bag. + const raw: any = { data: { data: 'PAYLOAD-NESTED-DATA', subject: 'help' }, options: {} }; + const seen: Record = {}; + await runHook(raw, (input) => { + seen.get = input.data; + seen.descriptorValue = Object.getOwnPropertyDescriptor(input, 'data')?.value; + seen.spread = { ...input }.data; + }); + + expect(seen.get).toEqual({ data: 'PAYLOAD-NESTED-DATA', subject: 'help' }); + expect(seen.descriptorValue).toEqual({ data: 'PAYLOAD-NESTED-DATA', subject: 'help' }); + expect(seen.spread).toEqual({ data: 'PAYLOAD-NESTED-DATA', subject: 'help' }); + }); + + it('`ast`: the fourth reserved name follows the identical rule', async () => { + const wrapperAst = { object: 'case', where: { id: 'ROW-1' } }; + const raw: any = { data: { ast: 'PAYLOAD-AST', subject: 'help' }, options: {}, ast: wrapperAst }; + const seen: Record = {}; + await runHook(raw, (input) => { + seen.get = input.ast; + seen.descriptorValue = Object.getOwnPropertyDescriptor(input, 'ast')?.value; + seen.spread = { ...input }.ast; + seen.dataAst = input.data.ast; + }); + + expect(seen.get).toBe(wrapperAst); + expect(seen.descriptorValue).toBe(wrapperAst); + expect(seen.spread).toBe(wrapperAst); + expect(seen.dataAst).toBe('PAYLOAD-AST'); + }); + + it('EDGE CASE — the envelope has no `id` (insert-shaped) but the payload does: the reserved name resolves to absent, not to the payload value', async () => { + // `get` never fell through to `data` for a reserved name, with or + // without this fix — `Reflect.get(target, 'id', receiver)` on a wrapper + // that never set `id` answers `undefined`, full stop. The descriptor + // trap now agrees: no wrapper-owned `id` means no descriptor for `id`, + // even though `ownKeys` (driven by `data` alone, unchanged) still lists + // it — a proxy is free to answer `undefined` for a key its `ownKeys` + // trap listed when the target (extensible, as this one always is) does + // not itself own that key; `Object.keys`/spread silently skip a listed + // key whose descriptor resolves to `undefined` rather than throwing. + const raw: any = { data: { id: 'PAYLOAD-ID', subject: 'help' }, options: {} }; + const seen: Record = {}; + await runHook(raw, (input) => { + seen.get = input.id; + seen.descriptor = Object.getOwnPropertyDescriptor(input, 'id'); + seen.ownKeysIncludes = Reflect.ownKeys(input).includes('id'); + seen.spreadHasId = 'id' in { ...input }; + seen.dataId = input.data.id; + }); + + expect(seen.get).toBeUndefined(); + expect(seen.descriptor).toBeUndefined(); + expect(seen.ownKeysIncludes).toBe(true); + expect(seen.spreadHasId).toBe(false); + expect(seen.dataId).toBe('PAYLOAD-ID'); + }); + + it('POSITIVE CONTROL — a non-reserved payload key is untouched by any of this', async () => { + const raw: any = { data: { id: 'PAYLOAD-ID', subject: 'help' }, options: {}, id: 'WRAPPER-ID' }; + const seen: Record = {}; + await runHook(raw, (input) => { + seen.get = input.subject; + seen.descriptorValue = Object.getOwnPropertyDescriptor(input, 'subject')?.value; + seen.ownKeysIncludes = Reflect.ownKeys(input).includes('subject'); + seen.spread = { ...input }.subject; + }); + + expect(seen.get).toBe('help'); + expect(seen.descriptorValue).toBe('help'); + expect(seen.ownKeysIncludes).toBe(true); + expect(seen.spread).toBe('help'); + }); + + it('DECLARED — the non-collision case is unchanged: wrapper keys stay out of enumeration when the payload does not share the name', async () => { + // Same case `hook-input-ownkeys-agreement.test.ts` pins as a declared + // exception; repeated here as a guardrail on THIS file's fix, since it + // is the shape this fix could plausibly have broken by over-applying the + // reserved-name branch. + const raw: any = { data: { subject: 'help' }, options: { multi: false }, id: 'WRAPPER-ID' }; + const seen: Record = {}; + await runHook(raw, (input) => { + seen.ownKeysIncludes = Reflect.ownKeys(input).includes('id'); + seen.objectKeys = Object.keys(input); + seen.readId = input.id; + seen.descriptorEnumerable = Object.getOwnPropertyDescriptor(input, 'id')?.enumerable; + }); + + expect(seen.ownKeysIncludes).toBe(false); + expect(seen.objectKeys).toEqual(['subject']); + expect(seen.readId).toBe('WRAPPER-ID'); + expect(seen.descriptorEnumerable).toBe(false); + }); +}); diff --git a/packages/runtime/src/sandbox/hook-input-envelope-precedence.integration.test.ts b/packages/runtime/src/sandbox/hook-input-envelope-precedence.integration.test.ts new file mode 100644 index 0000000000..a3903586c2 --- /dev/null +++ b/packages/runtime/src/sandbox/hook-input-envelope-precedence.integration.test.ts @@ -0,0 +1,122 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#12601] The load-bearing consequence: `unwrapProxyToPlain` +// (`body-runner.ts`) snapshots a hook body's `ctx.input` through +// `Object.entries` over the REAL flat-input Proxy from `@objectstack/objectql`. +// A sandboxed body reading `ctx.input.id` — for a record whose payload also +// declares a field literally named `id` — sees the ENVELOPE's value now BY +// CONTRACT (`getOwnPropertyDescriptor` and `get` agree, per the maintainer +// ruling on #12601), not by the accident of #12578's `ownKeys` widening +// happening to line up with `get`'s pre-existing wrapper-first read. This is +// the path the original #12601 finding measured, and the one that would +// silently regress if `getOwnPropertyDescriptor` ever went back to checking +// `data` first. +// +// ## ⚠️ REBUILD IS LOAD-BEARING FOR THIS FILE — determined, not assumed +// +// `packages/runtime/vitest.config.ts` does not alias `@objectstack/objectql` +// to source (registered in `KNOWN_UNALIASED_TEST_IMPORTS`, +// `scripts/check-test-source-alias.mjs`), so `wrapDeclarativeHook` below +// resolves through `exports` to `objectql/dist` — an edit to +// `packages/objectql/src/hook-wrappers.ts` that is not rebuilt is INVISIBLE +// here, and the dangerous direction is an ablation of the objectql half +// staying GREEN, certifying a pin that measured a stale artifact. +// `pnpm --filter @objectstack/objectql build` before running this file. +// +// ## This file is a CONSEQUENCE pin, not the ablation discriminator +// +// Measured: this test is ALREADY GREEN before the #12601 fix lands, because +// `unwrapProxyToPlain`'s `Object.entries` walk happened to line up anyway — +// `ownKeys` already listed the colliding name (#12578), the pre-fix +// descriptor trap reported the payload's own descriptor as `enumerable: +// true` (an ordinary assigned field), which passes `Object.entries`'s +// filter, and the VALUE then came from `get` — which always resolved the +// wrapper. That is exactly "by accident": correct today for a reason that +// has nothing to do with reserved-name precedence and would not survive, +// say, a payload that defined its colliding field non-enumerable. The +// instrument that actually disagreed — `getOwnPropertyDescriptor`'s VALUE — +// is pinned as the discriminator in +// `packages/objectql/src/hook-input-envelope-precedence.test.ts`; THIS file +// pins that the composition with the sandbox snapshot keeps holding once +// that trap is fixed, so a future regression of the trap has a second, +// consequence-level tripwire even though this file alone cannot distinguish +// "by accident" from "by contract". +// +// ## Why this composition and not a hand-rolled double +// +// `body-runner.test.ts`'s own flat-input double (`ownKeys`/`getOwnPropertyDescriptor` +// modelling `Object.getOwnPropertyNames`/`Reflect.getOwnPropertyDescriptor` +// directly on the backing object) does not implement wrapper-key precedence +// at all — it has no envelope/payload split to disagree about. The subject +// here is specifically the composition of objectql's REAL Proxy (its +// reserved-name precedence) with the sandbox's REAL `unwrapProxyToPlain` +// (its `Object.entries` walk) — a boundary no unit mock on either side alone +// exercises. + +import { describe, it, expect } from 'vitest'; +import { wrapDeclarativeHook } from '@objectstack/objectql'; +import { hookBodyRunnerFactory } from './body-runner.js'; +import { QuickJSScriptRunner } from './quickjs-runner.js'; + +const silentLogger = { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} }; + +describe('[#12601] a sandboxed hook body sees the envelope value for a reserved name colliding with a payload field', () => { + it('`ctx.input.id` inside the VM is the envelope id, and the payload write-back is unaffected', async () => { + const runner = new QuickJSScriptRunner(); + const factory = hookBodyRunnerFactory(runner, { ql: {}, appId: 'crm' }); + const fn = factory({ + name: 'envelope_precedence_probe', + object: 'case', + events: ['beforeUpdate'], + body: { + language: 'js', + // Written back through the flat proxy's `set` trap — a non-reserved + // key, so it lands in `data` and is observable from outside the VM. + source: 'return { observed_id: ctx.input.id };', + capabilities: [], + }, + } as any); + expect(typeof fn).toBe('function'); + + const wrapped = wrapDeclarativeHook( + { name: 'wrap_envelope_precedence_probe', object: 'case', event: 'beforeUpdate' } as any, + (async (ctx: any) => { await fn!(ctx); }) as any, + { logger: silentLogger }, + ); + + const raw: any = { data: { id: 'PAYLOAD-ID', subject: 'help' }, options: {}, id: 'WRAPPER-ID' }; + await wrapped({ object: 'case', event: 'beforeUpdate', input: raw } as any); + + // The sandboxed body observed the ENVELOPE's id, not the payload's. + expect(raw.data.observed_id).toBe('WRAPPER-ID'); + // The payload's own `id` field is untouched — it was never visible to the + // sandbox, and the merge-back never overwrote it. + expect(raw.data.id).toBe('PAYLOAD-ID'); + }); + + it('POSITIVE CONTROL — a non-colliding record field passes through the snapshot unaffected', async () => { + const runner = new QuickJSScriptRunner(); + const factory = hookBodyRunnerFactory(runner, { ql: {}, appId: 'crm' }); + const fn = factory({ + name: 'control_probe', + object: 'case', + events: ['beforeUpdate'], + body: { + language: 'js', + source: 'return { observed_subject: ctx.input.subject };', + capabilities: [], + }, + } as any); + + const wrapped = wrapDeclarativeHook( + { name: 'wrap_control_probe', object: 'case', event: 'beforeUpdate' } as any, + (async (ctx: any) => { await fn!(ctx); }) as any, + { logger: silentLogger }, + ); + + const raw: any = { data: { id: 'PAYLOAD-ID', subject: 'help' }, options: {}, id: 'WRAPPER-ID' }; + await wrapped({ object: 'case', event: 'beforeUpdate', input: raw } as any); + + expect(raw.data.observed_subject).toBe('help'); + }); +}); From a424bf4ec39091859b519d7dcd84a04e41ab82f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 16:30:28 +0000 Subject: [PATCH 2/3] fix(objectql): flat-input envelope wins consistently across get/descriptor/ownKeys (#12601) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit installFlatInput's getOwnPropertyDescriptor checked the record payload (`data`) before the reserved wrapper names (`id`/`options`/`ast`/`data`), while `get` always checked the wrapper first — so a payload field sharing one of those four names made a direct read (`input.id`) and a descriptor read (`Object.getOwnPropertyDescriptor(input, 'id').value`) disagree about the same key. Per the maintainer ruling (Option A, envelope wins consistently): the four names are reserved on the flat face. getOwnPropertyDescriptor now checks them first, matching get's order; enumerable still depends on whether `data` also owns the name (unchanged ownKeys, #12578), which is what keeps spread/ Object.entries carrying the envelope's value under the reserved name exactly as before. The payload's own value stays reachable at input.data.. Part of #12601 --- packages/objectql/src/hook-wrappers.ts | 83 ++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 6 deletions(-) diff --git a/packages/objectql/src/hook-wrappers.ts b/packages/objectql/src/hook-wrappers.ts index 12f49a0624..26a035dd7d 100644 --- a/packages/objectql/src/hook-wrappers.ts +++ b/packages/objectql/src/hook-wrappers.ts @@ -500,6 +500,17 @@ export function wrapDeclarativeHook( * read picks up mutations made by user code as `input.field = value`. * "Writes" means every mutation JS has, not assignment alone: `delete` and * `Object.defineProperty` route into `data` too (#12277). + * + * [#12601] ⚠️ `id` / `options` / `ast` / `data` are RESERVED on this flat + * face — EVERY instrument (`get`, `getOwnPropertyDescriptor`, `ownKeys`, + * `has`, spread, `Object.entries`) resolves one of these four names against + * the WRAPPER, never the payload, even when the payload itself declares a + * field sharing the name. That field does not vanish — it still round-trips + * through storage exactly as declared — it is simply not reachable through + * `ctx.input.`; reach it at `ctx.input.data.` instead. See the + * `get` and `getOwnPropertyDescriptor` traps below for the instrument-by- + * instrument account, and `content/docs/automation/hooks.mdx` (Hook Context) + * for the author-facing statement of the same rule. */ function installFlatInput(ctx: HookContext): () => void { const raw: any = ctx.input ?? {}; @@ -516,6 +527,14 @@ function installFlatInput(ctx: HookContext): () => void { }; const proxy = new Proxy(raw, { + // [#12601] Reserved-name precedence STARTS here: an unconditional, + // wrapper-only read for the four names, never falling through to `data` + // even when `data` owns a same-named field, and even when the WRAPPER + // itself does not (an insert-shaped envelope with no `id` reads + // `undefined`, not the payload's `id`). Every other trap that touches + // these four names (`getOwnPropertyDescriptor`, `has`) is written to + // agree with this one — this is the trap the others were made to match, + // not a peer that could equally have been changed instead. get(target, prop, receiver) { if (prop === 'id' || prop === 'options' || prop === 'ast' || prop === 'data') { return Reflect.get(target, prop, receiver); @@ -640,6 +659,17 @@ function installFlatInput(ctx: HookContext): () => void { // wrapper — NOT by subtracting those four names, which would hide a // genuine payload field that happens to be called `id`. // + // [#12601] That deliberate non-subtraction is why a payload field sharing + // one of the four names is still LISTED here when it exists — this trap + // is unchanged by #12601 and stays that way. What #12601 fixed sits one + // trap down: the descriptor trap used to answer such a listed key's VALUE + // from `data` (the payload), while `get` and spread already answered it + // from the wrapper (the envelope) — two instruments, one listed key, two + // objects. The descriptor trap now checks the reserved names first too, + // so a key this trap lists under a reserved name always resolves to the + // SAME value everywhere it is read. See the descriptor trap's own comment + // for the full account. + // // SYMBOL KEYS are deliberately still absent, and this is NOT a finding // that they do not belong on a payload: `Reflect.ownKeys(data)` here would // additionally publish them, and whether the record payload may carry a @@ -692,18 +722,59 @@ function installFlatInput(ctx: HookContext): () => void { // (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. + // + // [#12601] RESERVED-NAME PRECEDENCE mirrors `get`: checked FIRST, not last. + // `id`/`options`/`ast`/`data` are PLATFORM names on the flat face — a + // payload field sharing one of them is still a legal record field, but it + // is not reachable through the flat face at all, `input.data.` is + // the only route to it. Before this fix the order was reversed (`data` + // checked first, the reserved-name branch only reached when `data` did + // not own the key), so a payload that genuinely declared a field called + // `id` made this trap report the PAYLOAD's descriptor while `get` — which + // has always checked the reserved names unconditionally, first — reported + // the WRAPPER's value for the identical property access. Two instruments, + // one key, two different objects: + // + // const raw = { data: { id: 'PAYLOAD-ID', subject }, options: {}, id: 'WRAPPER-ID' }; + // input.id -> 'WRAPPER-ID' (get) + // Object.getOwnPropertyDescriptor(input, 'id').value -> 'PAYLOAD-ID' (descriptor, PRE-FIX) + // + // Reordering costs nothing `ownKeys` does not already pay for: `ownKeys` + // (#12578) is untouched and keeps listing the payload's own key set + // unconditionally, INCLUDING a reserved name the payload happens to + // share — so `enumerable` here still depends on whether `data` owns the + // name too. That is deliberate, not an oversight: it is what keeps + // `Object.keys`/spread/`Object.entries` carrying the ENVELOPE's value + // under the reserved name exactly as before this fix (get already won + // there, since spread reads a value through `get`), rather than silently + // dropping a key `ownKeys` just offered. When `data` does NOT own the + // name, the reserved-name branch still hides it from enumeration exactly + // as it always did (`enumerable: false`) — unaffected by this fix. + // + // A reserved name absent from the WRAPPER (e.g. `id` on an insert-shaped + // envelope) reports NO descriptor at all here, even when `data` owns a + // same-named field and `ownKeys` therefore still lists it — matching + // `get`'s unconditional wrapper-only read, which never falls through to + // `data` for these four names. A proxy may legally answer `undefined` for + // a key its OWN `ownKeys` trap listed, as long as the target (extensible, + // always, here) does not itself carry that key: `Object.keys`/spread + // silently skip such a key rather than throwing. Pinned, all of it, in + // `hook-input-envelope-precedence.test.ts` (and the sandbox-snapshot + // consequence in `packages/runtime/src/sandbox/hook-input-envelope-precedence.integration.test.ts`). getOwnPropertyDescriptor(target, prop) { + if (prop === 'id' || prop === 'options' || prop === 'ast' || prop === 'data') { + const desc = Object.getOwnPropertyDescriptor(target, prop); + if (!desc) return undefined; + const data = target.data; + const payloadOwnsName = + !!data && typeof data === 'object' && Object.prototype.hasOwnProperty.call(data, prop); + return { ...desc, configurable: true, enumerable: payloadOwnsName }; + } const data = target.data; 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(). - if (prop === 'id' || prop === 'options' || prop === 'ast' || prop === 'data') { - const desc = Object.getOwnPropertyDescriptor(target, prop); - return desc ? { ...desc, enumerable: false } : undefined; - } return Object.getOwnPropertyDescriptor(target, prop); }, }); From 6e1297bc42d05659eaf68fc65de3d38f17141b30 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 16:32:19 +0000 Subject: [PATCH 3/3] docs+changeset: document the four reserved names on ctx.input, add changeset (#12601) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit content/docs/automation/hooks.mdx now states loudly, at the Before Hook flat ctx.input section, that id/options/ast/data are reserved and always resolve to the envelope — a payload field sharing one of those names is reachable only at ctx.input.data.. Changeset (patch, argued in the file): same trap set, same shape, same scope as the two immediately preceding fixes here (#12397, #12578), both patch — no persisted data moves, and the paths that already worked (get, ownKeys, spread, Object.entries) are unaffected; only a direct descriptor-value read on a name that collides with a reserved name changes. Part of #12601 --- .changeset/flat-input-envelope-precedence.md | 62 ++++++++++++++++++++ content/docs/automation/hooks.mdx | 20 +++++++ 2 files changed, 82 insertions(+) create mode 100644 .changeset/flat-input-envelope-precedence.md diff --git a/.changeset/flat-input-envelope-precedence.md b/.changeset/flat-input-envelope-precedence.md new file mode 100644 index 0000000000..1185d702d3 --- /dev/null +++ b/.changeset/flat-input-envelope-precedence.md @@ -0,0 +1,62 @@ +--- +"@objectstack/objectql": patch +--- + +fix(objectql): the flat-input Proxy's descriptor trap agrees with `get` about the four reserved names (#12601) + +`installFlatInput` gives `id` / `options` / `ast` / `data` precedence in the +`get` trap — a direct read (`ctx.input.id`) always resolves against the +engine's `{ data, options, id? }` wrapper (the envelope), never against the +record payload, even when the payload itself declares a field sharing one of +those names. `getOwnPropertyDescriptor` checked the payload FIRST instead, so +for an update whose payload happened to carry a same-named field: + +```js +const raw = { data: { id: 'PAYLOAD-ID', subject: 'help' }, options: {}, id: 'WRAPPER-ID' }; +ctx.input.id // 'WRAPPER-ID' (get) +Object.getOwnPropertyDescriptor(ctx.input, 'id').value // 'PAYLOAD-ID' (descriptor, pre-fix) +``` + +Two instruments over the identical key, contradicting each other — and +anything that copies a value out of a raw descriptor rather than through +`get` inherited whichever one this trap picked. + +Per the maintainer ruling on #12601 (Option A — "the envelope wins +consistently"): the four names are reserved on the hook flat-input face. +`getOwnPropertyDescriptor` now checks them first, exactly where `get` already +does, so a descriptor read can never again disagree with a plain read of the +same key. A payload field sharing one of the four names stays a legal record +field — it round-trips through storage unchanged — it is simply not reachable +through the flat face; `ctx.input.data.` is the only route to it. + +## Why `patch`, argued rather than assumed + +This is the same trap set, the same shape of fix, and the same scope as the +two immediately preceding fixes here — #12397 (descriptor trap mirrors `data` +instead of synthesising) and #12578 (`ownKeys` reports the payload's own key +set) — both shipped `patch`, and both changed what a specific instrument +reports for specific inputs, exactly as this one does: + +- **No persisted data moves.** Unlike #12277 (`delete` actually deletes, + shipped `minor` because it changes what lands in storage), this fix changes + only what a READ instrument reports; the row the engine writes is byte-for- + byte identical before and after. +- **The paths that already worked stay byte-identical.** `ctx.input.id` + itself (`get`), `Object.keys`/spread/`Object.entries` + (`ownKeys` + the descriptor's `enumerable` flag + `get`'s value) all + already answered the envelope's value for a reserved name before this fix — + measured, not assumed (see the test file). The only caller this changes is + one that reads `Object.getOwnPropertyDescriptor(ctx.input, '').value` directly, on a payload that ALSO happens to declare a field + by that exact reserved name — a combination narrow enough that it is the + disagreement itself, previously unnoticed, that this card exists to close. +- **It restores an invariant the code already claimed to hold** (`get` and + the descriptor trap were always supposed to agree — that is the whole + premise of a "flat view"), rather than adding or removing a capability. A + hook body that worked correctly before this fix — i.e. one that never + happened to hit the exact disagreeing combination — is unaffected. + +Not declared breaking, so no ADR-0087 disposition marker applies (this +changeset removes/renames no authorable spec key, export, or config field — +`check-adr-0087-registration.mjs` only judges changesets that declare a +breaking/major change). diff --git a/content/docs/automation/hooks.mdx b/content/docs/automation/hooks.mdx index 2bffbdba39..6f9e3d3dc7 100644 --- a/content/docs/automation/hooks.mdx +++ b/content/docs/automation/hooks.mdx @@ -141,6 +141,26 @@ record's fields **directly on `ctx.input`** (a flat view over the internal `{ data, options }` wrapper — reads and writes of record fields route through `ctx.input.data`): + +These four names always resolve to the envelope, never to a record field — +even if your object declares a field with one of those names. A field named +`id` (or `options` / `ast` / `data`) still round-trips through storage +exactly as declared; it is simply unreachable as `ctx.input.`. Read it +at `ctx.input.data.` instead: + +```ts +handler: async (ctx) => { + ctx.input.id; // the row this write targets (or undefined on insert) — NEVER the payload's own `id` field + ctx.input.data.id; // the payload's own `id` field, if the object declares one +}; +``` + +This applies to every read of these names through `ctx.input` — direct +access, `Object.keys`/`for-in`, spread, and `Object.entries` all agree on the +envelope's value for a reserved name (`packages/objectql/src/hook-wrappers.ts`, +#12601). + + ```typescript import { Hook } from '@objectstack/spec/data';