|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#12277] Every mutation JS offers on `ctx.input` lands in the row the engine |
| 5 | + * persists — not just assignment. |
| 6 | + * |
| 7 | + * `installFlatInput` (`hook-wrappers.ts`) hands a declarative hook a flat-record |
| 8 | + * Proxy over the engine's `{ data, options, id? }` wrapper. It trapped `set` |
| 9 | + * but not `deleteProperty` or `defineProperty`, so those two fell through to |
| 10 | + * `Reflect.*` on the WRAPPER — one level above `data` — and changed a key that |
| 11 | + * was never there, on an object the engine does not read. |
| 12 | + * |
| 13 | + * ## What each assertion is worth, and why the two gaps are not the same shape |
| 14 | + * |
| 15 | + * The measurement that produced this file (pre-fix, one hook call): |
| 16 | + * |
| 17 | + * ``` |
| 18 | + * delete Object.defineProperty |
| 19 | + * operation's own result → true (no throw) |
| 20 | + * `k in input` → true — |
| 21 | + * `input.k` → CALLER-VALUE DEFINED ← agrees! |
| 22 | + * `Object.keys(input)` → includes k excludes k |
| 23 | + * what the engine persisted → CALLER-VALUE absent |
| 24 | + * ``` |
| 25 | + * |
| 26 | + * `delete`'s lie was confined to its own return value: the three other |
| 27 | + * read-backs stayed honest and reported the key still present. That is a |
| 28 | + * silent no-op, and it is what the card reported. |
| 29 | + * |
| 30 | + * `Object.defineProperty` — which no one reported — is the strictly worse |
| 31 | + * shape, and the reason this file pins BOTH: the `get` trap's fall-through to |
| 32 | + * the wrapper read the value straight back, so `input.k` CONFIRMED a write |
| 33 | + * that never reached `data`. A read-back that corroborates a write that did |
| 34 | + * not happen leaves an author no instrument to catch it with. |
| 35 | + * |
| 36 | + * So every case below asserts the CONJUNCTION — what the hook observes AND |
| 37 | + * what the engine is left holding — rather than either alone. Asserting only |
| 38 | + * the stored row would pass on an engine whose read-backs lie in the other |
| 39 | + * direction; asserting only the read-backs is what shipped the defect. |
| 40 | + * |
| 41 | + * The `assign-then-delete` case is the DISCRIMINATOR carried over from the |
| 42 | + * report: a `{...callerData, ...hookInput}` merge upstream would produce the |
| 43 | + * same symptoms as a missing trap, and it would restore the CALLER's value. |
| 44 | + * Seeing the hook's own assigned value survive a delete rules the merge out — |
| 45 | + * and post-fix, seeing the key vanish entirely rules out a merge just as |
| 46 | + * firmly, from the other side. |
| 47 | + * |
| 48 | + * `wrapDeclarativeHook` is driven directly rather than through `ObjectQL`: the |
| 49 | + * defect is in the wrapper's Proxy, and a full engine dispatch would put a |
| 50 | + * driver's own copy semantics between the hook and the assertion. |
| 51 | + */ |
| 52 | + |
| 53 | +import { describe, it, expect } from 'vitest'; |
| 54 | +import { wrapDeclarativeHook } from './hook-wrappers.js'; |
| 55 | + |
| 56 | +const silentLogger = { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} }; |
| 57 | + |
| 58 | +/** Run `handler` as a declarative hook over a caller payload; return the row the engine keeps. */ |
| 59 | +async function runHook( |
| 60 | + data: Record<string, unknown>, |
| 61 | + handler: (input: any) => void, |
| 62 | +): Promise<Record<string, unknown>> { |
| 63 | + const meta: any = { name: 'trap_probe', object: 'case', event: 'beforeInsert' }; |
| 64 | + const wrapped = wrapDeclarativeHook(meta, (async (ctx: any) => handler(ctx.input)) as any, { |
| 65 | + logger: silentLogger, |
| 66 | + }); |
| 67 | + const raw: any = { data, options: {} }; |
| 68 | + await wrapped({ object: 'case', event: 'beforeInsert', input: raw } as any); |
| 69 | + return raw.data as Record<string, unknown>; |
| 70 | +} |
| 71 | + |
| 72 | +describe('[#12277] `delete ctx.input.x` removes the field from the persisted row', () => { |
| 73 | + it('the hook read-backs and the stored row agree that the key is gone', async () => { |
| 74 | + const seen: Record<string, unknown> = {}; |
| 75 | + const persisted = await runHook( |
| 76 | + { subject: 'help', owner_id: 'CALLER-VALUE' }, |
| 77 | + (input) => { |
| 78 | + seen.deleteReturned = delete input.owner_id; |
| 79 | + seen.inOperator = 'owner_id' in input; |
| 80 | + seen.propertyRead = input.owner_id; |
| 81 | + seen.objectKeys = Object.keys(input); |
| 82 | + seen.spread = { ...input }; |
| 83 | + seen.descriptor = Object.getOwnPropertyDescriptor(input, 'owner_id'); |
| 84 | + }, |
| 85 | + ); |
| 86 | + |
| 87 | + // What the author observes. Pre-fix, only the first of these was `true` |
| 88 | + // and every other line reported the key still present. |
| 89 | + expect(seen.deleteReturned).toBe(true); |
| 90 | + expect(seen.inOperator).toBe(false); |
| 91 | + expect(seen.propertyRead).toBeUndefined(); |
| 92 | + expect(seen.objectKeys).toEqual(['subject']); |
| 93 | + expect(seen.spread).toEqual({ subject: 'help' }); |
| 94 | + expect(seen.descriptor).toBeUndefined(); |
| 95 | + |
| 96 | + // …and what the engine is left holding. This is the half the author cannot |
| 97 | + // reach from inside the hook, and the half the defect falsified. |
| 98 | + expect(persisted).toEqual({ subject: 'help' }); |
| 99 | + }); |
| 100 | + |
| 101 | + it('POSITIVE CONTROL — an assignment in the same call still lands', async () => { |
| 102 | + // Without this, every assertion above would also pass against a wrapper |
| 103 | + // that had stopped writing anything through to `data` at all. |
| 104 | + const persisted = await runHook({ subject: 'help', owner_id: 'CALLER-VALUE' }, (input) => { |
| 105 | + input.subject = 'HELP'; |
| 106 | + delete input.owner_id; |
| 107 | + }); |
| 108 | + expect(persisted).toEqual({ subject: 'HELP' }); |
| 109 | + }); |
| 110 | + |
| 111 | + it('DISCRIMINATOR — assign-then-delete leaves no key, not the caller value', async () => { |
| 112 | + // A `{...callerData, ...hookInput}` merge would answer `CALLER-NOTE` here. |
| 113 | + const seen: Record<string, unknown> = {}; |
| 114 | + const persisted = await runHook({ note: 'CALLER-NOTE' }, (input) => { |
| 115 | + input.note = 'ASSIGNED-THEN-DELETED'; |
| 116 | + seen.afterAssign = input.note; |
| 117 | + delete input.note; |
| 118 | + seen.afterDelete = input.note; |
| 119 | + }); |
| 120 | + expect(seen.afterAssign).toBe('ASSIGNED-THEN-DELETED'); |
| 121 | + expect(seen.afterDelete).toBeUndefined(); |
| 122 | + expect(persisted).toEqual({}); |
| 123 | + }); |
| 124 | + |
| 125 | + it('deleting a key that was never in the payload is a no-op that reports success', async () => { |
| 126 | + const persisted = await runHook({ subject: 'help' }, (input) => { |
| 127 | + expect(delete input.never_here).toBe(true); |
| 128 | + }); |
| 129 | + expect(persisted).toEqual({ subject: 'help' }); |
| 130 | + }); |
| 131 | + |
| 132 | + it('the operation envelope is addressed separately from the record fields', async () => { |
| 133 | + // `id`/`options`/`ast`/`data` are wrapper keys on every other trap, and |
| 134 | + // `deleteProperty` routes them the same way — a hook deleting `options` |
| 135 | + // must not punch a hole in a record field that happens to share the name. |
| 136 | + const meta: any = { name: 'envelope', object: 'case', event: 'beforeUpdate' }; |
| 137 | + const wrapped = wrapDeclarativeHook(meta, (async (ctx: any) => { |
| 138 | + delete ctx.input.options; |
| 139 | + }) as any, { logger: silentLogger }); |
| 140 | + const raw: any = { id: 'r1', data: { options: 'A RECORD FIELD CALLED OPTIONS' }, options: { multi: true } }; |
| 141 | + await wrapped({ object: 'case', event: 'beforeUpdate', input: raw } as any); |
| 142 | + expect('options' in raw).toBe(false); |
| 143 | + expect(raw.data).toEqual({ options: 'A RECORD FIELD CALLED OPTIONS' }); |
| 144 | + }); |
| 145 | +}); |
| 146 | + |
| 147 | +describe('[#12277] `Object.defineProperty(ctx.input, …)` lands in the persisted row', () => { |
| 148 | + it('the confirming read-back is now telling the truth', async () => { |
| 149 | + // The pre-fix failure this case exists for: `input.defined_key` read back |
| 150 | + // `DEFINED` while `data` never received it, so the instrument an author |
| 151 | + // would reach for to check AGREED with a write that did not happen. |
| 152 | + const seen: Record<string, unknown> = {}; |
| 153 | + const persisted = await runHook({ subject: 'help' }, (input) => { |
| 154 | + Object.defineProperty(input, 'defined_key', { |
| 155 | + value: 'DEFINED', |
| 156 | + enumerable: true, |
| 157 | + writable: true, |
| 158 | + configurable: true, |
| 159 | + }); |
| 160 | + seen.propertyRead = input.defined_key; |
| 161 | + seen.inKeys = Object.keys(input).includes('defined_key'); |
| 162 | + }); |
| 163 | + expect(seen.propertyRead).toBe('DEFINED'); |
| 164 | + expect(seen.inKeys).toBe(true); |
| 165 | + expect(persisted).toEqual({ subject: 'help', defined_key: 'DEFINED' }); |
| 166 | + }); |
| 167 | +}); |
0 commit comments