Skip to content

May a hook-input record payload carry a SYMBOL key — and if so must enumeration publish it? (the half #12578 deliberately left open) #12603

Closed

Description

@os-warren

Filed by the domain:engine PM seat before PR #12602 merges, because that PR carries the closing keyword on #12578 — so without this card the question closes with it and evaporates. The measurement below is the #12578 dev's; the filing decision is mine.

The card's own premise was wrong, and measuring it is what produced this question

#12578 assumed — and my dispatch brief repeated — that publishing symbol keys "would newly expose" them, i.e. that a symbol on a record payload was unreachable today and enumeration would be the door.

Measured, symbols 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; the persisted row holds it. Only Object.getOwnPropertySymbols(input) returns [].

So this is the same three-instrument disagreement #12578 just closed for string keys, still open for symbols — not a new capability anyone is proposing to add. The question is not "may a hook put a symbol on the row" (it already can, and the engine already stores it). The question is what enumeration should publish, and behind it, whether that shape should exist at all.

Why #12578 did not settle it

That PR took Object.getOwnPropertyNames(data) rather than Reflect.ownKeys(data) — the status-quo-preserving half of its own fork — precisely so the symbol delta stayed a payload-contract question rather than being decided as a side effect of a string-key repair. The omission is declared in the trap's comment and pinned in the test in its open state, so it cannot later be mistaken for a decision nobody made. That is the right call, and it is why this needs a card rather than a follow-up commit.

The options, with the measured cost of each

  • A — as shipped: symbols stay unenumerated. The string half is closed; the symbol half stays open and pinned open. Costs nothing today, and leaves one instrument disagreeing with the other two on exactly one key kind.
  • B — Reflect.ownKeys(data): closes the symbol half the same way the string half just closed. Cost: enumerable symbol keys become newly copied by spread / Object.assign and reported by getOwnPropertySymbols. ⭐ The sandbox snapshot is unaffected either way — unwrapProxyToPlain is an Object.entries over this proxy, which applies the enumerable filter itself — so body-runner does not block it.
  • C — decide the contract first: establish whether a payload may carry a symbol key at all, and if not, refuse it at the set / defineProperty traps rather than hiding it from enumeration.

The dev's recommendation is C, with B falling out of it, and the reasoning is the one this family keeps arriving at: hiding a key the engine nonetheless persists is the failure shape #12277, #12397 and #12578 exist to abolish. A shipped only because it is the one option that does not decide the payload contract.

Either way it is one word of code once answered — the cost is entirely in the decision, not the implementation.

Why it is a maintainer floor

"What a record payload may hold" is a contract question about stored data, not about a proxy trap. This seat has applied that boundary three times today (#12397, #12578, #12166) and it applies here in its purest form: option C is only a contract decision.

Dedup

Searched 2026-08-26; the control fired on the flat-input / hook-payload vocabulary and returned the #12277 / #12397 / #12578 family. No existing card covers the symbol half — #12578 names it and explicitly defers it. The adjacent #12601 (a payload field named id/options/ast/data reading back the wrapper's value) is a different mechanism and separately filed.

Not claimed, not dispatched.

Activity

  1. os-zhuang commented on Aug 27, 2026

    @os-zhuang
    Contributor

    Maintainer ruling recorded — Option C, refusal arm: a record payload may NOT carry a symbol key

    Provenance: maintainer, 2026-08-27, PM chat decision-inbox batch 6 (adjudication session session_01DKWDdUJ2XNRESVVWUvcpnh), verbatim: 「其他同意」 — adopting the dev's recommendation C with the contract question answered in the refusal direction, ruled together with #12601 as one family.

    Ruled: a record payload is a declarable, string-keyed field set — no metadata schema can declare a symbol field, so a symbol key on a hook-input payload is a JS-runtime artifact leaking toward storage. The set and defineProperty traps REFUSE a symbol key loudly (a clear error naming the key kind and the surface) instead of hiding it from enumeration while persisting it — hiding-what-you-persist is the exact shape this family (#12277/#12397/#12578) exists to abolish. ⛔ B (enumerate symbols via Reflect.ownKeys) declined — it would publish an undeclarable key kind as contract; ⛔ A (leave the instrument disagreement pinned open) superseded by this ruling.

    Dispatch notes: one-word-plus-guard implementation per the card's own sizing; the #12578 pin that holds the omission open flips to a refusal pin (red-before-fix: a symbol write throws, nothing persists); serialize behind #12578's landed fix; may fold with #12601's alignment under the family gates (same trap set, both ruled).

    State: needs-user-decision → pm:queue.


    Generated by Claude Code

  2. self-assigned this
    on Aug 27, 2026
  3. os-zhuang commented on Aug 27, 2026

    @os-zhuang
    Contributor

    Claim: domain:engine PM seat (#6367), session session_01LZbWd2jNV1FErXTPSS4Dry, branch claude/issue-12603-hook-input-symbol-key-refusal.

    Serial constraints cleared: both prerequisites the ruling named are landed, verified by content on origin/main rather than by their closed state.

    packages/objectql/src/hook-wrappers.ts should therefore be free. If you find another open PR touching it, stop and report rather than racing — the repo enforces this with No other open PR may claim the same single-writer path, and a second claim turns that check red for both.

    Two things the ruling makes load-bearing

    This one throws. Unlike the rest of this family, the ruled behaviour is a refusal: set and defineProperty must reject a symbol key loudly, with an error naming the key kind and the surface. Code that today silently accepts a symbol write will now fail. That is the intent — the ruling is explicit that hiding-what-you-persist is the shape #12277 / #12397 / #12578 exist to abolish — but it means the error message is part of the deliverable, not decoration. It has to tell an author what they did and where.

    The pin is an inversion, not an addition. The ruling says the #12578 pin that currently holds the omission open "flips to a refusal pin." So: find that pin and turn it around; do not leave the old assertion standing next to a new one. A pin asserting the key is absent from enumeration and a pin asserting the write throws cannot both describe the same code, and leaving both is how a suite starts documenting two behaviours at once.

    ⛔ Out of scope: option B (Reflect.ownKeys) is declined by the ruling — it would publish an undeclarable key kind as contract. Option A (leave the disagreement pinned open) is superseded. Do not touch ownKeys itself; #12578 landed getOwnPropertyNames deliberately.


    Generated by Claude Code

  4. os-zhuang commented on Aug 27, 2026

    @os-zhuang
    Contributor

    os-dev-report

    {
      "issue": 12603,
      "status": "done",
      "branch": "claude/issue-12603-hook-input-symbol-key-refusal",
      "head": "651b9888ebc5cf75765cd6ae3744e6e408518db1",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/12801",
      "premise_still_valid": true,
      "clause2": "Yes -- this IS a contract change, and the ruling is the authority for it. installFlatInput's set/defineProperty traps are a surface every hook body touches (any before*/after* handler reading or writing ctx.input), and this narrows their accept set: a symbol-keyed write that used to succeed now throws. That is accept-set tightening, the same shape 8cc8401 (update/delete predicate-drop refusal) argued minor for. The maintainer ruling on #12603 (2026-08-27, Option C refusal arm) is the explicit authority: a record payload is a declarable, string-keyed field set, no metadata schema can declare a symbol field, so the narrowing is deliberate and ruled, not incidental. ownKeys itself is untouched -- the ruling's own boundary (do not re-open #12578) -- so the ENUMERATION contract is unchanged; only the WRITE-acceptance contract narrows.",
      "summary": "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, before the write reaches data, per the maintainer's Option-C refusal ruling on #12603. ownKeys is untouched. The #12578 pin that held the symbol instrument-disagreement OPEN in hook-input-ownkeys-agreement.test.ts is inverted in place into a REFUSAL pin (same it(), not a second assertion). Changeset is minor, argued in the file against the three prior patch-bumped siblings in this family. Docs updated with a Callout in hooks.mdx.",
      "tests": "pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 across the 5 hook-input trap-set suites: 41/41 passed both before and after the ablation window (identical count, confirming clean restore). pnpm --filter @objectstack/objectql run typecheck: clean (note: this script's tsconfig excludes **/*.test.ts, so it does NOT cover the edited test file -- coverage of that file comes from check:type-check-debt's re-measure pass instead, see gates.type_check_debt below). Full local gate sweep: 43 matched families, all green except two NOT MEASURED reads (both resolved/explained in gates below, neither caused by this diff).",
      "existing_pin": "packages/objectql/src/hook-input-ownkeys-agreement.test.ts, the single it() titled (pre-fix) 'OPEN QUESTION, pinned in its open state -- a symbol key carries the same disagreement' (used Symbol.for('objectstack.test.12578'), asserted ownness={enumeration:false,hasOwnProperty:true,descriptor:true} and that raw.data held the symbol post-write). Found by grepping 'symbol' (case-insensitive) across packages/objectql/src/hook-input-*.test.ts -- ran WITHOUT first proving the channel, so I re-ran it as its own positive control by observing it matched immediately (5 hits including the it() title and doc comment) rather than reporting a clean search; the immediate, unambiguous match against known content is what stands in for a separate control here, since the search tool itself (grep) has no failure mode that would silently report zero on a live corpus the way an API/search-index channel can. Inverted IN PLACE (same it(), same file, no second assertion stacked beside it) into '[#12603] REFUSAL, not agreement -- a symbol key is rejected before it can ever reach data', asserting both set and defineProperty now throw TypeError (message matches /symbol/i and /hook input/i) and Object.getOwnPropertySymbols(raw.data) stays empty. Also rewrote the file's header doc comment (the SYMBOL KEYS bullet) and the ownKeys trap's own inline comment in hook-wrappers.ts (previously describing the open question) to state the ruling closed it.",
      "error_message": "\"Cannot set SYMBOL_DESCRIPTION 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.\" (defineProperty gets the same text with verb 'Cannot define'; SYMBOL_DESCRIPTION is String(prop), e.g. Symbol(cache-entry).) Names the key kind explicitly ('a symbol key'), names the surface ('hook input'), states the underlying rule (record payload = declarable string-keyed field set), gives the fix (use a string key / keep it off the payload), and names the accidental scenario the dispatch brief called out (spreading a symbol-keyed cache entry). Thrown as a plain TypeError, not a new Error subclass: it 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 -- an author-code defect, not a declarative-layer diagnostic like HookConditionError -- so it correctly rides the ordinary handler error path (onError/retryPolicy apply to it) rather than being special-cased outside it.",
      "ablation": {
        "prediction_commit": "651b9888e (git commit --allow-empty, written BEFORE mutating)",
        "predicted": "removing both `typeof prop === 'symbol'` guards turns exactly 1 of 41 tests red (the inverted pin in hook-input-ownkeys-agreement.test.ts), 40 stay green including the named positive control (hook-input-mutation-traps.test.ts, 'POSITIVE CONTROL -- an assignment in the same call still lands')",
        "measured": "EXACT MATCH: Test Files 1 failed | 4 passed (5), Tests 1 failed | 40 passed (41). The single failure named is the inverted pin (AssertionError: expected undefined to be an instance of TypeError at line 226). The positive control was not printed by name (vitest's default reporter only names failures) but is proven green by elimination -- it is one of the 40 passes, since the failure section names exactly 1 test and it is not this one.",
        "mutation_confirmed_on_disk": "anchored grep -c on \"refuseSymbolPayloadKey(prop, 'set')\" and \"refuseSymbolPayloadKey(prop, 'defineProperty')\": 1 and 1 before, 0 and 0 after; marker text \"ABLATION-MARKER-REMOVED-GUARD\": 0 before, 2 after. All confirmed BEFORE any test was run.",
        "build_under_mutation": "ESM/CJS build succeeded; DTS build failed with TS6133 ('refuseSymbolPayloadKey' declared but never read) -- an EXPECTED, benign side effect of removing both call sites to a helper with no other caller. Does not affect the measurement: the vitest run resolves the same-package relative import './hook-wrappers.js' against src/*.ts directly (no vitest.config/alias found in the repo; standard TS-source resolution), not through the built dist/.d.ts, so a DTS-only build failure does not put the test run on a stale or wrong tree. Confirmed correct-tree reasoning by the fact that the mutation's effect (the pin going red) was in fact observed.",
        "restore_proof": "git checkout HEAD -- TARGET_FILE inside a trap ... EXIT INT TERM with an absolute REPO_ROOT/absolute target path. git hash-object on the restored file === git rev-parse HEAD:packages/objectql/src/hook-wrappers.ts (a6239076f2294c40e1dedff2630982cc8bc2ad09, both sides). git diff HEAD -- TARGET_FILE byte count: 0. Marker residue grep after restore: 0. Post-restore rebuild + re-run of the same 5 suites: 5 passed (5), 41 passed (41), confirming a genuinely clean tree, not just a hash match.",
        "positive_control_suite": "packages/objectql/src/hook-input-mutation-traps.test.ts, run in the SAME mutated window alongside the pin -- stayed green (see measured above)."
      },
      "gates": {
        "derivation": "node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, run twice: once before the changeset+docs edit (2 paths), once after the final commit (4 paths: hook-wrappers.ts, hook-input-ownkeys-agreement.test.ts, .changeset/hook-input-symbol-key-refusal.md, content/docs/automation/hooks.mdx). Second run confirmed 'nothing this answer derives from changed' despite origin/main having advanced meanwhile, and the matched path set was unchanged (43 local families).",
        "named_in_brief_all_green": ["check:engine-double-contract", "check:where-matcher", "check:objectql-double-limit", "check:query-options-erasure"],
        "full_local_sweep": "43 matched families run one by one (exit code captured via redirect-then-tail, never through a pipe) -- 41 exit=0 on first pass, 2 exceptions both diagnosed as NOT MEASURED and re-run to a real verdict, not the diff's fault:",
        "not_measured_resolved_1": "check:skill-examples (spec) -- exit=1 first pass: 'packages/client-react/dist holds no .d.ts declarations -- the package is not built' (a pre-existing worktree gap; no dist/ existed at all, unrelated to this diff which never touches client-react). Built the dependency closure (pnpm --filter '@objectstack/client-react^...' build) then the package itself, re-ran: exit=0, '260 prose examples type-check across 3 surface(s)'.",
        "not_measured_resolved_2": "node scripts/pm/check-half-states.mjs -- exit=3, PREREQUISITE NOT MET: the container's GITHUB_TOKEN is not a valid GitHub credential (401 on /rate_limit, no ghp_/gho_/ghs_/github_pat_ prefix -- the proxy's own placeholder). Explicitly documented by the gate itself as distinct from the unclassified-failure exit 2; read as NOT MEASURED per the gate's own text ('Nothing was swept ... not a clean board and it is not a dirty one'), not a red, and not something this diff can fix.",
        "type_check_debt": "pnpm check:type-check-debt required the full workspace closure built first (pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*', 70/70 successful, ~2m51s). Re-run: 'OK -- 65/78 workspace packages type-checked ... 1570 raw tsc error(s) total, none above its recorded number.' This is the gate that actually covers the edited **/*.test.ts file, since packages/objectql's own `typecheck` script excludes **/*.test.ts by its tsconfig (confirmed by reading packages/objectql/tsconfig.json directly) -- objectql carries a TEST_DEBT ledger entry (its hidden-test tsc re-measure reported 251 errors, down from the recorded 354 ceiling, i.e. strictly better, so the ratchet passes with margin). The several notes printed (plugin-approvals, objectql, runtime, plugin-auth) are pre-existing improvements unrelated to this diff -- none reference hook-input-ownkeys-agreement.test.ts or hook-wrappers.ts.",
        "package_level": "pnpm --filter @objectstack/objectql run typecheck: clean. pnpm --filter @objectstack/objectql build: clean (2/2 declared .d.ts present). pnpm --filter '@objectstack/objectql^...' build (dependency closure before any measurement): clean.",
        "nul_bytes": "node scripts/check-nul-bytes.mjs: OK, run twice (before and after the final commit), no raw ASCII control bytes."
      },
      "deviations": [
        "Self-corrected before pushing: my first commit message included a literal 'Fixes #12603' trailer, which the dispatch brief explicitly forbids for commit messages in this squashing repo (closing keyword belongs in the PR body only). Caught it before push, amended the commit to 'Part of #12603' instead, then pushed.",
        "Added a docs change (content/docs/automation/hooks.mdx, a Callout beside the existing reserved-names one) that the dispatch brief did not explicitly request. Justified because the surface throws now where it silently accepted before, on author-facing ctx.input, and the file already documents the sibling reserved-name behaviour from #12601 at the same call site -- an author hitting the new throw has nowhere else in the docs to learn why. This expanded the gate surface to ~15 doc-related families (all green) beyond what a code-only diff would have matched; noted here since it was not named in the brief.",
        "The dispatch brief's Ablation section asked to 'name a positive control that stays green ... ideally one of the sibling trap-set suites' -- I additionally named and confirmed the SPECIFIC test within hook-input-mutation-traps.test.ts ('POSITIVE CONTROL...') by elimination from the failure-count math, since vitest's default reporter does not print individual passing test names and a second mutated run just to get verbose per-test output felt like unjustified extra ablation exposure for a fact already provable from the counts."
      ],
      "open_questions": [],
      "out_of_scope_findings": [],
      "mcp_calls": 8
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions