fix(objectql): refuse a symbol key at the flat-input set/defineProperty traps (#12603)

.changeset/hook-input-symbol-key-refusal.md

@@ -0,0 +1,60 @@
+---
+'@objectstack/objectql': minor
+---
+
+fix(objectql): the flat-input proxy REFUSES a symbol key at `set`/`defineProperty` instead of silently persisting it (#12603)
+
+**Bump level, argued**: `minor`, not `patch`. Every sibling in this family (#12277,
+#12397, #12578, #12601) shipped `patch` because each closed an instrument
+DISAGREEMENT — the accepted set of writes never changed, only which read-back
+told the truth about them. This card is different in kind: `ctx.input[sym] =
+value` and `Object.defineProperty(ctx.input, sym, …)` **used to succeed**, and
+now **throw**. That is a narrowing of the accept set on `ctx.input` — a surface
+every hook body touches — which is the exact shape `8cc8401`
+(`@objectstack/objectql` 17.2.0, "BREAKING (accept-set tightening)") argued
+`minor` for under this repo's launch-window convention (pre-1.0 semantics: a
+breaking change does not burn a major version while the stack versions in
+lockstep — see `scripts/check-changeset-no-major.mjs`). `patch` here would
+under-declare a change that can turn a passing hook into a throwing one.
+
+**What changed.** `installFlatInput`'s `set` and `defineProperty` traps
+(`packages/objectql/src/hook-wrappers.ts`) now refuse a symbol-keyed write with
+a `TypeError` naming the key kind and the surface, instead of routing it into
+the record payload (`data`) the way every string-keyed write is routed.
+Measured on the pre-fix tree: a symbol-keyed `set` succeeded silently, the
+value reached `data` and persisted to the row the engine stores, and only
+`Object.getOwnPropertySymbols` / `Reflect.ownKeys` omitted it from enumeration
+— two instruments said "own", enumeration said "no", while the persisted row
+held it regardless.
+
+**Why a refusal, not a fourth instrument fix.** Maintainer ruling, 2026-08-27
+(Option C, refusal arm), on the payload-contract question #12578 measured and
+deliberately left open rather than decided: a record payload is a declarable,
+**string-keyed** field set — no metadata schema can declare a symbol field, so
+a symbol key on this surface is a JS-runtime artifact leaking toward storage,
+not a legal payload field. Option B (publish symbols too, via
+`Reflect.ownKeys`) was declined — it would have made an undeclarable key kind a
+published contract instead of closing the question. Hiding a key the engine
+nonetheless persists is precisely the shape #12277/#12397/#12578 exist to
+abolish; refusing the write at the boundary closes that gap from the other
+side, before persistence rather than after enumeration.
+
+`ownKeys` itself is **untouched** — still `Object.getOwnPropertyNames(data)`,
+exactly as #12578 landed it. With the write refused, `data` can never carry a
+symbol key for that trap (or `Reflect.ownKeys`) to disagree about, so there is
+nothing left for this card to change there.
+
+**Migration.** Code that wrote a symbol key onto `ctx.input` — almost always by
+accident, e.g. spreading an object that carried a symbol-keyed cache entry onto
+the payload — now throws instead of silently losing the write to enumeration.
+Use a string key, or keep the value off the payload entirely (a local
+variable, or a WeakMap keyed by the record) if it was never meant to be
+stored. No other hook-input read/write path changes: reads, `has`, `delete`,
+and every string-keyed write behave exactly as before.
+
+Inverts the pin `hook-input-ownkeys-agreement.test.ts` carried OPEN since
+#12578 (the disagreement, deliberately left standing) into a REFUSAL pin
+(the write throws, nothing persists) — the same case, turned around in place,
+not a second assertion stacked beside the first.
+
+<!-- adr-0087: not-required (no-migration-prescription) A JS-runtime Proxy trap now throws instead of silently accepting a write; no spec or Zod field, object definition, or stored representation is added, removed, or renamed. A symbol key was never a declarable metadata surface for objectstack migrate meta to know about, so there is nothing here for a migration to rewrite -- the Migration section above is authoring guidance for hook bodies, not a metadata-ledger prescription, and carries no arrow or FROM/TO pair. -->

content/docs/automation/hooks.mdx

@@ -161,6 +161,25 @@ envelope's value for a reserved name (`packages/objectql/src/hook-wrappers.ts`,
 #12601).
 </Callout>
 
+<Callout type="warn" title="A symbol key is refused, not stored">
+A record payload is a declarable, string-keyed field set — no metadata schema
+can declare a symbol field. Writing one onto `ctx.input`, whether by
+assignment or `Object.defineProperty`, throws a `TypeError` naming the key and
+the surface instead of silently accepting it:
+
+```ts
+const cacheKey = Symbol('cache-entry');
+handler: async (ctx) => {
+ ctx.input[cacheKey] = value; // throws: symbol keys are not a valid record-payload field
+};
+```
+
+This most often happens by accident — spreading an object that carries a
+symbol-keyed cache entry onto the payload. Use a string key, or keep the value
+off the payload entirely if it is not meant to be stored (`packages/objectql/src/hook-wrappers.ts`,
+#12603).
+</Callout>
+
 ```typescript
 import { Hook } from '@objectstack/spec/data';
 

packages/objectql/src/hook-input-ownkeys-agreement.test.ts

@@ -49,13 +49,17 @@
  * That disagreement is the trap's whole purpose (the payload-diff idiom must
  * see record fields only) and is pinned as DECLARED so it cannot be mistaken
  * for a residue of the defect above.
- * - SYMBOL KEYS carry the identical disagreement and are deliberately left
- * carrying it. Publishing them is a one-word change here
- * (`Object.getOwnPropertyNames` -> `Reflect.ownKeys`), but whether a record
- * payload may hold a symbol key at all is a question about the PAYLOAD
- * contract — the boundary #12397 drew and this card does not cross. It is
- * reported open on #12578 and pinned below in its open state, so answering
- * it changes a recorded fact instead of an unnoticed one.
+ * - SYMBOL KEYS used to carry the identical disagreement, pinned open below:
+ * the write succeeded silently, persisted into `data`, and only the
+ * enumeration face omitted it. [#12603] The maintainer ruling (2026-08-27,
+ * Option C refusal arm) answered the payload-contract question this file
+ * left open: a record payload is a declarable, string-keyed field set, and
+ * no metadata schema can declare a symbol field. `set` and `defineProperty`
+ * now REFUSE a symbol-keyed write, loudly, before it ever reaches `data` —
+ * Option B (publish via `Reflect.ownKeys`) was declined, because it would
+ * have made the undeclarable key kind a published contract instead of
+ * closing it. The case below is INVERTED accordingly, in place: it used to
+ * pin the disagreement open, and now pins the refusal.
  *
  * `wrapDeclarativeHook` is driven directly rather than through `ObjectQL`, for
  * the reason the sibling trap-set files give: the subject is the wrapper's
@@ -195,31 +199,52 @@ describe('[#12578] the flat-input `ownKeys` reports the payload own-key set, and
  expect(seen.readMulti).toBe(false);
  });
 
- it('OPEN QUESTION, pinned in its open state — a symbol key carries the same disagreement', async () => {
- // Reported on #12578 rather than decided here: publishing symbol keys
- // through `ownKeys` is `Reflect.ownKeys` in one line, but whether the
- // record payload may CARRY a symbol key is a payload-contract question and
- // a maintainer floor (#12397's boundary).
+ it('[#12603] REFUSAL, not agreement — a symbol key is rejected before it can ever reach data', async () => {
+ // INVERTS the OPEN QUESTION pin this case used to carry (verbatim, before
+ // this card): `input[sym] = value` succeeded silently, persisted into
+ // `data`, and only `Reflect.ownKeys`/`getOwnPropertySymbols` omitted it —
+ // two instruments said own, enumeration said no, and the payload the
+ // engine persisted held it regardless.
  //
- // What the measurement establishes, and what this case records: symbol keys
- // already reach `data` through the `set` trap and already persist. So the
- // open question is about what the enumeration face should PUBLISH, not
- // about what a hook can already put on the row.
+ // Maintainer ruling, 2026-08-27, Option C refusal arm: a record payload is
+ // a declarable, string-keyed field set — no metadata schema can declare a
+ // symbol field, so the write is refused at the boundary instead of hidden
+ // after it lands. There is no longer a persisted symbol key for the three
+ // instruments to disagree about, so this is a refusal pin, not an
+ // agreement pin — asserted for both traps the ruling names.
  const raw: any = { data: { subject: 'help' }, options: {} };
- const sym = Symbol.for('objectstack.test.12578');
- const seen: Record<string, unknown> = {};
+ const sym = Symbol.for('objectstack.test.12603');
+
+ let setThrew: unknown;
+ await runHook(raw, (input) => {
+ try {
+ input[sym] = 'symvalue';
+ } catch (e) {
+ setThrew = e;
+ }
+ });
+ expect(setThrew).toBeInstanceOf(TypeError);
+ const setMessage = (setThrew as TypeError).message;
+ expect(setMessage).toMatch(/symbol/i); // names the key kind
+ expect(setMessage).toMatch(/hook input/i); // names the surface
+ // Refused BEFORE `data` is touched — nothing persisted, sibling field intact.
+ expect(Object.getOwnPropertySymbols(raw.data)).toEqual([]);
+ expect(raw.data.subject).toBe('help');
+
+ let definePropertyThrew: unknown;
  await runHook(raw, (input) => {
- input[sym] = 'symvalue';
- seen.ownness = ownness(input, sym);
- seen.symbols = Object.getOwnPropertySymbols(input);
+ try {
+ Object.defineProperty(input, sym, { value: 'dp-value', enumerable: true, configurable: true });
+ } catch (e) {
+ definePropertyThrew = e;
+ }
  });
+ expect(definePropertyThrew).toBeInstanceOf(TypeError);
+ expect((definePropertyThrew as TypeError).message).toMatch(/symbol/i);
+ expect(Object.getOwnPropertySymbols(raw.data)).toEqual([]);
 
- // Today: two instruments say own, enumeration says no — the defect's shape,
- // deliberately left standing on this half.
- expect(seen.ownness).toEqual({ enumeration: false, hasOwnProperty: true, descriptor: true });
- expect(seen.symbols).toEqual([]);
- // …while the payload the engine persists holds it.
- expect(Object.getOwnPropertySymbols(raw.data)).toEqual([sym]);
- expect((raw.data as any)[sym]).toBe('symvalue');
+ // The three instruments now agree there is no such key at all — the
+ // refusal closes the disagreement this file otherwise exists to police.
+ expect(ownness(raw.data, sym)).toEqual(NOT_OWN);
  });
 });

packages/objectql/src/hook-wrappers.ts

@@ -511,7 +511,60 @@ export function wrapDeclarativeHook(
  * `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.
+ *
+ * [#12603] ⛔ SYMBOL keys are REFUSED, not routed. `set` and `defineProperty`
+ * throw a `TypeError` for a symbol-keyed write instead of letting it reach
+ * `data` — see those two traps below for the refusal and
+ * `refuseSymbolPayloadKey` for the error text. See "Symbol keys" below the
+ * reserved-name callout in `content/docs/automation/hooks.mdx`.
+ */
+
+/**
+ * [#12603] Maintainer ruling, 2026-08-27, Option C refusal arm: a record
+ * payload is a declarable, string-keyed field set — no metadata schema can
+ * declare a symbol field, so a symbol key on this flat `ctx.input` face is a
+ * JS-runtime artifact leaking toward storage, not a legal payload field.
+ *
+ * Thrown from the `set` and `defineProperty` traps, BEFORE either touches
+ * `data` — so a refused write never persists and never needs hiding from
+ * enumeration. That is the shape change from the pre-#12603 state: a
+ * symbol-keyed write used to succeed silently, reach `data`, and persist,
+ * while only `Object.getOwnPropertySymbols` / `Reflect.ownKeys` omitted it
+ * (pinned open in `hook-input-ownkeys-agreement.test.ts` under #12578). Hiding
+ * a key the engine nonetheless persisted is exactly the shape #12277, #12397
+ * and #12578 exist to abolish; refusing the write outright is the other way
+ * to close that gap, and the one this ruling chose (Option B — publish
+ * symbols via `Reflect.ownKeys` — was declined for minting an undeclarable
+ * key kind as contract).
+ *
+ * A plain `TypeError`, not a new subclass: this fires from INSIDE the hook
+ * BODY (an author wrote `input[sym] = …`), the same category as the native
+ * `TypeError` the `defineProperty` trap already lets through for a
+ * non-configurable descriptor the target does not carry (see that trap's own
+ * comment) — an author-code defect, not a declarative-layer diagnostic like
+ * `HookConditionError`. It is therefore subject to the ordinary handler error
+ * path: `onError: 'log'` can swallow it, `retryPolicy` can retry it, exactly
+ * as any other throw from the handler body (unlike `HookConditionError`,
+ * which is deliberately raised OUTSIDE that path — see the comment on that
+ * class for why the two are not the same shape).
+ *
+ * The message names the key kind (a symbol, not "a bad key"), the surface
+ * (hook input), and the fix (a string key, or keep the value off the payload)
+ * — written for the accidental case the ruling calls out: an author spreading
+ * an object that happens to carry a symbol-keyed cache entry onto `ctx.input`.
  */
+function refuseSymbolPayloadKey(prop: symbol, trap: 'set' | 'defineProperty'): never {
+ const verb = trap === 'set' ? 'Cannot set' : 'Cannot define';
+ throw new TypeError(
+ `${verb} ${String(prop)} on hook input: a symbol key is not a valid record-payload field. ` +
+ 'A record payload is a declarable, string-keyed field set — no metadata schema can declare ' +
+ 'a symbol field, so a symbol key here would be a JS-runtime artifact leaking toward storage. ' +
+ 'Use a string key, or keep the value off the payload entirely (e.g. a local variable) if it ' +
+ 'is not meant to be stored. This often happens by accident, such as spreading an object that ' +
+ 'carries a symbol-keyed cache entry onto ctx.input.'
+ );
+}
+
 function installFlatInput(ctx: HookContext): () => void {
  const raw: any = ctx.input ?? {};
  const looksWrapped =
@@ -545,7 +598,14 @@ function installFlatInput(ctx: HookContext): () => void {
  }
  return Reflect.get(target, prop, receiver);
  },
+ // [#12603] Symbol keys are REFUSED here, before anything else runs — a
+ // symbol can never equal one of the four reserved (string) names, so the
+ // check can sit first without disturbing that branch below. See
+ // `refuseSymbolPayloadKey` for why this throws instead of routing.
  set(target, prop, value) {
+ if (typeof prop === 'symbol') {
+ refuseSymbolPayloadKey(prop, 'set');
+ }
  if (prop === 'id' || prop === 'options' || prop === 'ast' || prop === 'data') {
  (target as any)[prop] = value;
  return true;
@@ -600,12 +660,23 @@ function installFlatInput(ctx: HookContext): () => void {
  // A throw is a diagnosis; the silence was not. Omitting `configurable`
  // entirely (the common spelling, and every spelling `Object.assign` and
  // spread produce) is unaffected.
+ //
+ // [#12603] Symbol keys are refused here too, identically to `set` and for
+ // the same reason — see `refuseSymbolPayloadKey`.
  defineProperty(target, prop, desc) {
+ if (typeof prop === 'symbol') {
+ refuseSymbolPayloadKey(prop, 'defineProperty');
+ }
  if (prop === 'id' || prop === 'options' || prop === 'ast' || prop === 'data') {
  return Reflect.defineProperty(target, prop, desc);
  }
  return Reflect.defineProperty(ensureData(), prop, desc);
  },
+ // [#12603] `deleteProperty` is deliberately NOT guarded: since `set` and
+ // `defineProperty` now refuse every symbol-keyed write before it reaches
+ // `data`, a symbol key can never be there to delete. `delete input[sym]`
+ // falls through exactly as it always has for any key `data` does not
+ // own — a harmless no-op reporting success, not a persistence lie.
  has(target, prop) {
  if (prop === 'id' || prop === 'options' || prop === 'ast' || prop === 'data') {
  return prop in target;
@@ -670,15 +741,30 @@ function installFlatInput(ctx: HookContext): () => void {
  // 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
- // symbol key at all is a question about the PAYLOAD contract (they already
- // reach `data` through the `set` trap and already persist — measured), not
- // about this trap. It is open, reported on #12578, and the day it is
- // answered "yes" this line becomes `Reflect.ownKeys`. Until then the
- // symbol half of the disagreement is pinned AS open in the sibling test,
- // so an answer changes a recorded fact rather than an unnoticed one.
+ // [#12603] SYMBOL KEYS are absent here for a settled reason now, not an
+ // open one: the maintainer ruling (2026-08-27, Option C refusal arm)
+ // answered the payload-contract question this trap's comment used to
+ // leave open ("may a record payload carry a symbol key at all?") with
+ // NO — a record payload is a declarable, string-keyed field set, and no
+ // metadata schema can declare a symbol field. `set` and `defineProperty`
+ // now REFUSE a symbol-keyed write before it ever reaches `data` (see
+ // `refuseSymbolPayloadKey`), so a symbol can no longer BE an own key of
+ // `data` for this trap to omit or report.
+ //
+ // This trap itself is deliberately UNCHANGED by that ruling —
+ // `Object.getOwnPropertyNames(target.data)` stays exactly what #12578
+ // landed. `Reflect.ownKeys(data)` (Option B) was the alternative the
+ // ruling declined: publishing symbols through enumeration would mint an
+ // undeclarable key kind as contract, which is the opposite of what was
+ // ruled. With the write refused at the boundary, the two spellings would
+ // agree anyway — `data` can never carry a symbol key for them to differ
+ // on — so there is no remaining reason to touch this line, and #12578's
+ // own ruling (this card must not re-litigate `ownKeys`) forbids it.
+ //
+ // What used to be pinned OPEN in `hook-input-ownkeys-agreement.test.ts`
+ // (the instrument disagreement, deliberately left standing) is now
+ // pinned as a REFUSAL in the same file: the write throws, so there is no
+ // persisted symbol key left for the three instruments to disagree about.
  ownKeys(target) {
  return target.data && typeof target.data === 'object'
  ? Object.getOwnPropertyNames(target.data)