Make delete ctx.input.x in a hook actually remove the field — the flat-input Proxy trap and the sandbox write-back

[os-warren]

Fixes#12277

delete ctx.input.FIELD in a hook did nothing, on both execution paths, while an assignment made two lines above it on the same object in the same call landed normally. Nothing raised and nothing reported it. Both halves are closed here, because closing either one alone would make the same authored delete behave differently depending on whether the body runs in-process or in QuickJS.

Placeholders below are spelled FIELD / IDENT.MEMBER rather than with angle brackets on purpose: the body sanitizer eats short angle-bracket fragments, and it silently turned this PR's first draft of the positive control into delete ., which reads as nonsense. Same class as #12133, one surface over.

What was measured, and one correction to the card

Reproduced before touching anything, at wrapDeclarativeHook / hookBodyRunnerFactory directly.

In-process (installFlatInput, packages/objectql/src/hook-wrappers.ts) — the card's mechanism is exactly right, its severity gloss is not. The card says "every read-back confirms the delete succeeded". Measured, that is false here, and the card's own table already showed it: only delete's return value lied.

probe	pre-fix	post-fix
delete input.owner_id	true	true
'owner_id' in input	true	false
input.owner_id	"CALLER-VALUE"	undefined
Object.keys(input)	['title','owner_id','note']	['title','note']
{...input}	carries owner_id	does not
Object.getOwnPropertyDescriptor	full descriptor	undefined
what the engine persisted	owner_id: "CALLER-VALUE"	absent

So an author who checked with anything other than the return value would have seen the no-op. The defect is real and the mechanism is as reported; the "no instrument can catch it" framing belongs to the other two findings below.

Sandboxed (applyMutationsToInput, packages/runtime/src/sandbox/body-runner.ts) — this is where the confirming read-back actually lives. Note the file has moved since the card's addendum was written (packages/runtime/src/sandbox/, not packages/objectql/src/); relocated by symbol. Inside QuickJS the delete is real — the body holds a JSON snapshot. What was lost is the trip home: Object.assign copies own enumerable properties and cannot represent a removal.

delete ctx.input.internal_notes -> true
'internal_notes' in ctx.input -> false # the VM agrees
Object.keys(ctx.input) -> ['subject'] # ...and so does this
host ctx.input after write-back -> { subject: 'HELP',
internal_notes: 'STAFF-ONLY' }

Every instrument reachable from inside the body confirms the removal, the assignment in the same call lands, and the field is stored anyway.

The discriminator held on both paths. Assign a key and then delete it, and the assigned value survived pre-fix ("ASSIGNED-THEN-DELETED" where the caller sent "CALLER-VALUE"), which rules out a {...callerData, ...hookInput} merge. Post-fix the key is simply absent, which rules it out from the other side.

The trap set, enumerated — including what is not fixed here

installFlatInput's Proxy implemented get / set / has / ownKeys / getOwnPropertyDescriptor. Every mutation JS offers has to land in data — data is the object the engine persists — and only set did.

• deleteProperty — missing. Fixed. The card.
• defineProperty — missing, same class, worse shape. Fixed in the same stroke (bounded, and named here with its evidence rather than slipped in). It defined on the wrapper, and the get trap's fall-through to the wrapper then read the value straight back: Object.defineProperty(input, 'defined_key', ...) followed by input.defined_key returned 'DEFINED' while Object.keys(input) denied it and data never received it. That is the read-back-confirms-a-write-that-never-happened shape, unreported, in the same six lines as the reported one. Its correct form is pinned by set's existing precedent, no other claim is on the file, and it adds no gate family.
  • One inherited JS invariant follows and is documented at the trap: a proxy may not report success for an explicitly configurable: false descriptor its target does not carry, so Object.defineProperty(input, 'x', { value: 1, configurable: false }) now throws a TypeError where it used to define, silently and uselessly, on the wrapper. Omitting configurable — the common spelling, and what spread and Object.assign produce — is unaffected.
• getOwnPropertyDescriptor — present, and NOT changed. Reported, not fixed (filed as [finding] The flat-input Proxy's getOwnPropertyDescriptor synthesises a descriptor instead of mirroring data's — now reachable, since #12277 routed defineProperty into data #12397). For a key on data it synthesises { configurable: true, enumerable: true, writable: true, value } rather than mirroring data's real descriptor. configurable: true is forced by a proxy invariant (the target does not carry the key), so it cannot simply mirror; and for every key created by assignment the synthesised descriptor is already the true one. The residual is narrow — a key defined on data with non-default attributes is described inaccurately — and widening the trap to chase it is a separate change with its own accessor-descriptor questions.
• set — checked for the same class, and it is clean. It returns true unconditionally, but the write it reports on is a strict-mode assignment into data that throws rather than failing quietly, so there is no silent-success limb.
• Sibling flat-record Proxies — swept, and there are none.installFlatInput is the only new Proxy in packages/objectql/src. ctx.previous is not proxied at all; it reaches a handler as the plain payload pickPreviousPayload builds. So there is no second surface for this fix to cover.

Clause ②: who is relying on the no-op today

Nobody in this repo.delete ctx.input.FIELD / delete input.FIELD returns zero hits across every file type (--exclude-dir=node_modules --exclude-dir=.git --exclude-dir=dist), as does a sweep of examples/** hook modules and of authored hook bodies. Positive control for that search: the same walk for delete IDENT.MEMBER matches 39 files (e.g. delete process.env.NODE_PATH in packages/types/src/node.test.ts), so the zero is a measurement rather than a broken grep.

The only measured consumers are external: the guest-intake app behind the card stripped staff-only fields with fifteen delete statements, every one inert, and its unit tests stayed green because they drive the handler with a plain object where delete genuinely works. That app-side repair is already done and does not depend on this.

The direction the sandbox write-back deliberately does not overreach in

Absence from the exit snapshot is the only evidence a deletion leaves, and alone it is ambiguous: a key whose host value is undefined (or a function, or a symbol) never survived JSON.stringifyinto the VM either, so it is missing from the dump without anyone having deleted it. The diff is filtered through the same JSON lens the boundary uses, so such a key is left alone, and every failure mode of that probe is conservative — an unprobeable key is simply not deletable. Losing a delete is recoverable; destroying a field on evidence that was never there is not. One residual miss is named in the code rather than left to be discovered: a bigint-valued key crosses into the VM as a string but is dropped by the probe, so deleting one is still lost.

Verification

All of the below at 6c568d6bb9, working tree clean, foreground, exit codes captured before any pipe.

Suites — pnpm --filter @objectstack/objectql exec vitest run: Test Files 236 passed (236) / Tests 4180 passed (4180). pnpm --filter @objectstack/runtime exec vitest run: Test Files 194 passed (194) / Tests 2856 passed (2856). typecheck green for both (tsc --noEmit).

Gate union derived, not recalled — node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, re-derived after the changeset existed: 19 matched families + the test-file and changeset conventions. All green, each quoting its own verdict line:

• check-adr-0087-registration: this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen).
• check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured in 242.1s, 1843 raw tsc error(s) total, none above its recorded number. / surplus: none — every entry sits exactly at its measurement, so any new error is red. (run on the built closure, so the two new test files really were measured)
• check-nul-bytes: OK (scanned 6855 text file(s) ...)
• check-engine-double-contract: 397 (file, verb) row(s) held by the RETAINED ledger
• where-matcher conformance holds: 302 matcher(s) discovered, 302 answer the combinator battery correctly or refuse it
• query-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new
• check-test-source-alias OK — 72 packages with tests scanned
• plus cross-package-test-inputs, ci-filter-parity, durability-log-level, page-declaration-shape, published-files, slot-lookup, type-source-resolution, engine-split-ratio, plugin-teardown-shape, affected-docs, drift-comment, objectui-changeset, changeset-gate-self-tests, changeset-no-major, empty-changeset, release-rehearsal-clone.

Repo-wide lint, not narrowed — pnpm lint (eslint . --no-inline-config) over the whole tree, 62s, exit 0 recorded before any pipe. No narrowing is claimed and none is owed.

Ablation — three legs, direction predicted in writing before the run, implementation committed first. All three mutate a file its own package's pin imports relatively (./hook-wrappers.js, ./body-runner.js), i.e. through vitest's TS source resolution and not through exports -> dist, so no rebuild is owed between mutation and measurement and the reds are evidence about the mutation rather than about a stale artifact. Each leg proved the mutation on disk with single-line grep -cF anchor counts (1 -> 0) plus git diff --numstat, and each restore leg proved absence the same way (0 -> 1, git diff empty) and re-ran green. The ablation script carried trap ... EXIT INT TERM so a foreground timeout could not leave the tree mutated.

leg	mutation	predicted	observed
A	drop deleteProperty	3 of 6 red	3 failed / 3 passed — expected true to be false on 'owner_id' in input, plus the stored row and the discriminator
B	drop defineProperty	1 of 6 red, read-back assertion staying green	1 failed / 5 passed, failing at line 164 (inKeys) — line 163, expect(seen.propertyRead).toBe('DEFINED'), passed
C	drop the deletion diff	2 of 5 red, in-VM assertions staying green	2 failed / 3 passed, failing at line 88 ('internal_notes' in engineCtx.input) — the four in-VM read-backs above it passed

Legs B and C are the ones that matter for the grading: in both, the pin fires only because it asserts the read-back and the stored row together. A pin asserting either half alone would have stayed green on the confirming-read-back defect. (A first attempt at leg C was voided and re-run rather than quietly retried: its cut was structural rather than surgical and the file failed to transform, so vitest exited 1 on no tests — a red for the wrong reason. Its grep anchor also read 0 before the mutation because a BRE turned [key] into a character class; the re-run uses grep -cF throughout.)

Changeset

.changeset/hook-input-delete-lands.md, graded minor for both @objectstack/objectql and @objectstack/runtime rather than patch, and the grade is the argument: this moves data that reaches downstream consumers. Any shipped hook already containing delete ctx.input.FIELD has been a no-op and starts taking effect on upgrade. No API is removed and no accept set narrows, so it is not declared breaking — but it must not arrive as a silent patch either.

Serial constraints

Surface is packages/objectql/** and packages/runtime/** only. It touches none of packages/drivers/driver-sql/**, packages/platform-objects/**, or packages/metadata/**.

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/objectql, @objectstack/runtime, touching 5 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/protocol/objectql/security.mdx(via internal_notes (literal))
• content/docs/protocol/objectui/concept.mdx(via internal_notes (literal))
• content/docs/ui/forms.mdx(via internal_notes (literal))

What this run could not see

• the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 31 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 078bdcdcb66adf73e4d6dec9c0613234cf08e6a4 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from ef069dba18c434eac0f68c12fa12a2fe8c76ed9d — the merge of head 6c568d6bb98badadc39fdea621b909ccc7ba83e3 into base 078bdcdcb66adf73e4d6dec9c0613234cf08e6a4, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin ef069dba18c434eac0f68c12fa12a2fe8c76ed9d && git checkout ef069dba18c434eac0f68c12fa12a2fe8c76ed9d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 078bdcdcb66adf73e4d6dec9c0613234cf08e6a4 6c568d6bb98badadc39fdea621b909ccc7ba83e3 && git checkout -B drift-repro 078bdcdcb66adf73e4d6dec9c0613234cf08e6a4 && git merge --no-ff 6c568d6bb98badadc39fdea621b909ccc7ba83e3
node scripts/docs-audit/affected-docs.mjs --json 078bdcdcb66adf73e4d6dec9c0613234cf08e6a4

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 078bdcdcb66adf73e4d6dec9c0613234cf08e6a4 → pass the list as
args.docs, on the commit named under Which tree this was computed on.