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

.changeset/hook-input-delete-lands.md

@@ -0,0 +1,95 @@
+---
+'@objectstack/objectql': minor
+'@objectstack/runtime': minor
+---
+
+fix(objectql,runtime): `delete ctx.input.x` in a hook actually removes the field (#12277)
+
+A hook that stripped a field from its input with `delete` 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 in the platform
+reported it.
+
+Graded `minor` rather than `patch` deliberately: it moves data that reaches
+downstream consumers. Any shipped hook that already contains
+`delete ctx.input.<field>` has been a no-op until now and starts taking effect
+on upgrade — which is the point, and is also exactly why it must not arrive as
+a silent patch. No API is removed and no accept set narrows.
+
+### The two mechanisms, which were unrelated and produced one outcome
+
+**In-process (`installFlatInput`, `packages/objectql/src/hook-wrappers.ts`).**
+The flat-record `Proxy` a declarative hook receives over the engine's
+`{ data, options, id? }` wrapper trapped `get` / `set` / `has` / `ownKeys` /
+`getOwnPropertyDescriptor` — but not `deleteProperty`. The delete therefore fell
+through to `Reflect.deleteProperty` on the WRAPPER, one level above the record,
+removing a key that was never there and returning `true`. `set` was trapped and
+wrote into `data`, which is what the engine persists; hence assignment survived
+and deletion evaporated.
+
+**Sandboxed (`applyMutationsToInput`,
+`packages/runtime/src/sandbox/body-runner.ts`).** A QuickJS body's mutations
+were written home with `Object.assign(target, result.mutatedInput)`.
+`Object.assign` copies own enumerable properties and **has no way to represent a
+removal**: a key the VM deleted is simply not in the snapshot, and the host's
+key stayed. Deletions are now diffed against the entry snapshot and applied
+separately.
+
+Both are fixed in one change on purpose. Closing either alone would make the
+same authored `delete` behave differently depending on whether the hook body
+runs in-process or in the sandbox — a worse contract than the symmetric silence
+it replaced.
+
+### What an author could see, before and after
+
+The sandboxed path is the one with no tell at all. Measured on the pre-fix code,
+one hook call, host row alongside:
+
+```
+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' }
+```
+
+The in-process path was less deceptive than reported, and the correction is
+worth having in writing: only `delete`'s own return value lied there. `'k' in
+input`, `input.k` and `Object.keys(input)` all went on honestly reporting the key
+as present, so an author who checked with anything other than the return value
+would have seen the no-op.
+
+### `Object.defineProperty(ctx.input, …)` was the same gap, and nobody reported it
+
+Found while enumerating the trap set, fixed in the same stroke because it is the
+strictly worse shape: it defined on the wrapper, and the `get` trap's
+fall-through then read the value straight back — so `input.k` CONFIRMED a write
+that never reached `data`, while `Object.keys(input)` denied it and the record
+never received it. It now routes into `data` like `set` and `deleteProperty` do.
+One inherited JS invariant follows: 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 the one spread and
+`Object.assign` produce — is unaffected.
+
+### The direction the sandbox write-back deliberately does not overreach in
+
+Absence from the exit snapshot is the only evidence a deletion leaves, and on its
+own it is ambiguous: a key whose host value is `undefined` (or a function, or a
+symbol) never survived `JSON.stringify` INTO 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. Every failure mode of
+that probe is conservative — an unprobeable key is simply not deletable — because
+losing a delete is recoverable and destroying a field on evidence that was never
+there is not. One residual miss follows and is named here rather than discovered
+later: a `bigint`-valued key crosses into the VM as a string but is dropped by the
+probe, so deleting one is still lost.
+
+Measured consumer cost of the reported half: a guest-intake app stripped the
+fields an anonymous web-to-case / web-to-lead submitter must not write —
+internal staff notes, the resolution, the escalation flag, the owner — with
+fifteen `delete` statements, every one inert. A submission carrying
+`internal_notes` and `resolution` stored them verbatim, and the app's unit tests
+stayed green throughout, because they drive the handler with a plain object where
+`delete` genuinely works.

packages/objectql/src/hook-input-mutation-traps.test.ts

@@ -0,0 +1,167 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#12277] Every mutation JS offers on `ctx.input` lands in the row the engine
+ * persists — not just assignment.
+ *
+ * `installFlatInput` (`hook-wrappers.ts`) hands a declarative hook a flat-record
+ * Proxy over the engine's `{ data, options, id? }` wrapper. It trapped `set`
+ * but not `deleteProperty` or `defineProperty`, so those two fell through to
+ * `Reflect.*` on the WRAPPER — one level above `data` — and changed a key that
+ * was never there, on an object the engine does not read.
+ *
+ * ## What each assertion is worth, and why the two gaps are not the same shape
+ *
+ * The measurement that produced this file (pre-fix, one hook call):
+ *
+ * ```
+ * delete Object.defineProperty
+ * operation's own result → true (no throw)
+ * `k in input` → true —
+ * `input.k` → CALLER-VALUE DEFINED ← agrees!
+ * `Object.keys(input)` → includes k excludes k
+ * what the engine persisted → CALLER-VALUE absent
+ * ```
+ *
+ * `delete`'s lie was confined to its own return value: the three other
+ * read-backs stayed honest and reported the key still present. That is a
+ * silent no-op, and it is what the card reported.
+ *
+ * `Object.defineProperty` — which no one reported — is the strictly worse
+ * shape, and the reason this file pins BOTH: the `get` trap's fall-through to
+ * the wrapper read the value straight back, so `input.k` CONFIRMED a write
+ * that never reached `data`. A read-back that corroborates a write that did
+ * not happen leaves an author no instrument to catch it with.
+ *
+ * So every case below asserts the CONJUNCTION — what the hook observes AND
+ * what the engine is left holding — rather than either alone. Asserting only
+ * the stored row would pass on an engine whose read-backs lie in the other
+ * direction; asserting only the read-backs is what shipped the defect.
+ *
+ * The `assign-then-delete` case is the DISCRIMINATOR carried over from the
+ * report: a `{...callerData, ...hookInput}` merge upstream would produce the
+ * same symptoms as a missing trap, and it would restore the CALLER's value.
+ * Seeing the hook's own assigned value survive a delete rules the merge out —
+ * and post-fix, seeing the key vanish entirely rules out a merge just as
+ * firmly, from the other side.
+ *
+ * `wrapDeclarativeHook` is driven directly rather than through `ObjectQL`: the
+ * defect is in 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: 'trap_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('[#12277] `delete ctx.input.x` removes the field from the persisted row', () => {
+ it('the hook read-backs and the stored row agree that the key is gone', async () => {
+ const seen: Record<string, unknown> = {};
+ const persisted = await runHook(
+ { subject: 'help', owner_id: 'CALLER-VALUE' },
+ (input) => {
+ seen.deleteReturned = delete input.owner_id;
+ seen.inOperator = 'owner_id' in input;
+ seen.propertyRead = input.owner_id;
+ seen.objectKeys = Object.keys(input);
+ seen.spread = { ...input };
+ seen.descriptor = Object.getOwnPropertyDescriptor(input, 'owner_id');
+ },
+ );
+
+ // What the author observes. Pre-fix, only the first of these was `true`
+ // and every other line reported the key still present.
+ expect(seen.deleteReturned).toBe(true);
+ expect(seen.inOperator).toBe(false);
+ expect(seen.propertyRead).toBeUndefined();
+ expect(seen.objectKeys).toEqual(['subject']);
+ expect(seen.spread).toEqual({ subject: 'help' });
+ expect(seen.descriptor).toBeUndefined();
+
+ // …and what the engine is left holding. This is the half the author cannot
+ // reach from inside the hook, and the half the defect falsified.
+ expect(persisted).toEqual({ subject: 'help' });
+ });
+
+ it('POSITIVE CONTROL — an assignment in the same call still lands', async () => {
+ // Without this, every assertion above would also pass against a wrapper
+ // that had stopped writing anything through to `data` at all.
+ const persisted = await runHook({ subject: 'help', owner_id: 'CALLER-VALUE' }, (input) => {
+ input.subject = 'HELP';
+ delete input.owner_id;
+ });
+ expect(persisted).toEqual({ subject: 'HELP' });
+ });
+
+ it('DISCRIMINATOR — assign-then-delete leaves no key, not the caller value', async () => {
+ // A `{...callerData, ...hookInput}` merge would answer `CALLER-NOTE` here.
+ const seen: Record<string, unknown> = {};
+ const persisted = await runHook({ note: 'CALLER-NOTE' }, (input) => {
+ input.note = 'ASSIGNED-THEN-DELETED';
+ seen.afterAssign = input.note;
+ delete input.note;
+ seen.afterDelete = input.note;
+ });
+ expect(seen.afterAssign).toBe('ASSIGNED-THEN-DELETED');
+ expect(seen.afterDelete).toBeUndefined();
+ expect(persisted).toEqual({});
+ });
+
+ it('deleting a key that was never in the payload is a no-op that reports success', async () => {
+ const persisted = await runHook({ subject: 'help' }, (input) => {
+ expect(delete input.never_here).toBe(true);
+ });
+ expect(persisted).toEqual({ subject: 'help' });
+ });
+
+ it('the operation envelope is addressed separately from the record fields', async () => {
+ // `id`/`options`/`ast`/`data` are wrapper keys on every other trap, and
+ // `deleteProperty` routes them the same way — a hook deleting `options`
+ // must not punch a hole in a record field that happens to share the name.
+ const meta: any = { name: 'envelope', object: 'case', event: 'beforeUpdate' };
+ const wrapped = wrapDeclarativeHook(meta, (async (ctx: any) => {
+ delete ctx.input.options;
+ }) as any, { logger: silentLogger });
+ const raw: any = { id: 'r1', data: { options: 'A RECORD FIELD CALLED OPTIONS' }, options: { multi: true } };
+ await wrapped({ object: 'case', event: 'beforeUpdate', input: raw } as any);
+ expect('options' in raw).toBe(false);
+ expect(raw.data).toEqual({ options: 'A RECORD FIELD CALLED OPTIONS' });
+ });
+});
+
+describe('[#12277] `Object.defineProperty(ctx.input, …)` lands in the persisted row', () => {
+ it('the confirming read-back is now telling the truth', async () => {
+ // The pre-fix failure this case exists for: `input.defined_key` read back
+ // `DEFINED` while `data` never received it, so the instrument an author
+ // would reach for to check AGREED with a write that did not happen.
+ const seen: Record<string, unknown> = {};
+ const persisted = await runHook({ subject: 'help' }, (input) => {
+ Object.defineProperty(input, 'defined_key', {
+ value: 'DEFINED',
+ enumerable: true,
+ writable: true,
+ configurable: true,
+ });
+ seen.propertyRead = input.defined_key;
+ seen.inKeys = Object.keys(input).includes('defined_key');
+ });
+ expect(seen.propertyRead).toBe('DEFINED');
+ expect(seen.inKeys).toBe(true);
+ expect(persisted).toEqual({ subject: 'help', defined_key: 'DEFINED' });
+ });
+});

packages/objectql/src/hook-wrappers.ts

@@ -498,6 +498,8 @@ export function wrapDeclarativeHook(
  * of any other key fall through to `data`. Writes always go to `data`
  * (creating it if missing) so the engine's downstream `input.data`
  * 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).
  */
 function installFlatInput(ctx: HookContext): () => void {
  const raw: any = ctx.input ?? {};
@@ -532,6 +534,59 @@ function installFlatInput(ctx: HookContext): () => void {
  ensureData()[prop as string] = value;
  return true;
  },
+ // [#12277] The mutation traps are a SET, not a list: every operation JS
+ // offers for changing a property has to land in `data`, because `data` is
+ // the object the engine persists. `set` alone was trapped, so
+ // `delete input.x` and `Object.defineProperty(input, 'x', …)` fell through
+ // to `Reflect.*` on the WRAPPER — one level above the record — and did
+ // nothing to the row while reporting success.
+ //
+ // The two gaps had different shapes, and the worse-shaped one is the one
+ // nobody reported:
+ //
+ // - `delete input.x` returned `true` and changed nothing. The other
+ // read-backs stayed HONEST (`'x' in input`, `input.x`,
+ // `Object.keys(input)` all still showed the key), so the lie was
+ // confined to `delete`'s own return value.
+ // - `Object.defineProperty(input, 'x', …)` defined on the wrapper, and
+ // the `get` trap's fall-through to the wrapper then READ IT BACK — so
+ // `input.x` confirmed a write that never reached `data`. That is the
+ // shape with no instrument to catch it from inside a hook.
+ //
+ // Measured cost of the `delete` half before this landed: a guest-intake
+ // app stripped the fields an anonymous submitter must not write with 15
+ // `delete` statements, every one inert, and its unit tests stayed green
+ // because they drive the handler with a plain object.
+ //
+ // `deleteProperty` deliberately does NOT call `ensureData()`: with no
+ // `data` on the wrapper, `get` reads fall through to the wrapper itself,
+ // so that is where the key would live and where the delete belongs.
+ // Materialising an empty `data` just to delete out of it would be a write
+ // performed by a removal.
+ deleteProperty(target, prop) {
+ if (prop === 'id' || prop === 'options' || prop === 'ast' || prop === 'data') {
+ return Reflect.deleteProperty(target, prop);
+ }
+ const data = target.data;
+ if (data && typeof data === 'object') {
+ return Reflect.deleteProperty(data as object, prop);
+ }
+ return Reflect.deleteProperty(target, prop);
+ },
+ // Routed for the same reason `set` is. One inherited JS invariant is worth
+ // naming: a proxy may not report success for an explicitly
+ // `configurable: false` descriptor the TARGET does not carry, so
+ // `Object.defineProperty(input, 'x', { value: 1, configurable: false })`
+ // now throws a TypeError where it used to silently define on the wrapper.
+ // 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.
+ defineProperty(target, prop, desc) {
+ if (prop === 'id' || prop === 'options' || prop === 'ast' || prop === 'data') {
+ return Reflect.defineProperty(target, prop, desc);
+ }
+ return Reflect.defineProperty(ensureData(), prop, desc);
+ },
  has(target, prop) {
  if (prop === 'id' || prop === 'options' || prop === 'ast' || prop === 'data') {
  return prop in target;