diff --git a/.changeset/hook-ctx-previous-stash-limb-removed.md b/.changeset/hook-ctx-previous-stash-limb-removed.md new file mode 100644 index 0000000000..072751bb08 --- /dev/null +++ b/.changeset/hook-ctx-previous-stash-limb-removed.md @@ -0,0 +1,32 @@ +--- +'@objectstack/trigger-record-change': patch +--- + +record-change trigger: drop the `ctx.__previous` stash fallback — read the engine's declared `ctx.previous` only + +Behaviour is unchanged: the limb guarded against a producer that no longer +exists. `plugin-audit`'s `captureBefore` was the **only** writer of +`ctx.__previous` in the repo, and #6656 retired it, so `buildContext`'s + +```ts +ctx.previous ?? (ctx as { __previous? }).__previous +``` + +had a second operand nothing could ever bind. The engine is the single producer +of the pre-image and it binds the declared key ahead of every dispatch — by-id +update (`engine.ts:7010`, immediately before the `beforeUpdate` dispatch at +`:7012`), by-id delete (`bindPreImage`, `engine.ts:7869`, called at `:7897` +before the `beforeDelete` dispatch at `:7899`), and each per-row context of a +predicate write (`engine.ts:1746` after-phase, `:1825` before-phase) — +#5272 / #5574 / #5846. + +Removed rather than kept "for safety", under ADR-0049 enforce-or-remove and +PD #12: a fallback with zero producers is a second de-facto contract waiting to +be rediscovered. The consequence is deliberate and stated here rather than left +to be found — **a future producer of `ctx.__previous` is now silently ignored**; +the declared way to hand this consumer a pre-image is `ctx.previous`. + +The test that fed the limb synthesised `__previous` in its own body, which is +what kept the dead limb looking live (#4984). It is replaced by the inverted pin +the deletion actually needs — the same treatment the `doc` alias got in #5671 — +so restoring the limb goes red instead of unnoticed. diff --git a/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts b/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts index a4a4ea066d..a12c49662e 100644 --- a/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts +++ b/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts @@ -349,7 +349,20 @@ describe('RecordChangeTrigger', () => { expect((captured?.record as Record).status).toBeUndefined(); }); - it('reads the __previous stash when ctx.previous is absent', async () => { + // [#6978] The `ctx.__previous` stash limb, deleted — same family as the `doc` + // alias above, same negative-pin treatment. `plugin-audit`'s `captureBefore` + // was the stash's ONLY producer and #6656 retired it, leaving the fallback + // with zero producers; ADR-0049 enforce-or-remove says it goes. + // + // This case REPLACES the one that used to live here ("reads the __previous + // stash when ctx.previous is absent"), which synthesised the key in its own + // body — the #4984 shape: a fixture spelling a key no producer emits, keeping + // a dead limb looking live. A positive case can never carry the weight here + // either, because the canonical key sits FIRST in the read and wins whether or + // not the limb exists. So the pin is inverted: restore the limb and this case + // goes red on both assertions (`previous` becomes `{ status: 'old' }`, and the + // record seeds from it instead of staying empty). + it('does NOT read the `__previous` stash — no producer emits that key (#6978)', async () => { const { engine, hooks } = fakeEngine(); const trigger = new RecordChangeTrigger(engine, silentLogger()); let captured: AutomationContext | undefined; @@ -358,11 +371,14 @@ describe('RecordChangeTrigger', () => { captured = ctx; }); - const ctx = hookCtx({ previous: undefined }); + // Every declared source of a pre-image is dropped, so the stash is the only + // thing left that could answer — and it must not. + const ctx = hookCtx({ result: undefined, previous: undefined, input: { id: 't1' } }); (ctx as unknown as { __previous: Record }).__previous = { status: 'old' }; await hooks[0].handler(ctx); - expect(captured?.previous).toEqual({ status: 'old' }); + expect(captured?.previous).toBeUndefined(); + expect(captured?.record).toEqual({}); }); it('isolates flow errors so the CRUD write is never broken', async () => { diff --git a/packages/triggers/trigger-record-change/src/record-change-trigger.ts b/packages/triggers/trigger-record-change/src/record-change-trigger.ts index 8ada12af40..96d5bd783f 100644 --- a/packages/triggers/trigger-record-change/src/record-change-trigger.ts +++ b/packages/triggers/trigger-record-change/src/record-change-trigger.ts @@ -277,8 +277,8 @@ export class RecordChangeTrigger implements FlowTrigger { /** * Build the flow execution context from an ObjectQL hook context. The new * record comes from `ctx.result` (after-hooks) or falls back to the - * mutation input payload / previous row; the old record from `ctx.previous` - * (with the `__previous` stash audit also uses as a fallback). + * mutation input payload / previous row; the old record from `ctx.previous`, + * which the engine binds ahead of every dispatch. * * Async because the seeded `record` is hydrated with read-time computed * fields (see {@link hydrateComputedFields}) via a data-engine re-read. @@ -294,9 +294,23 @@ export class RecordChangeTrigger implements FlowTrigger { // through to `previous` below. const input = (ctx.input ?? {}) as { data?: Record; id?: unknown }; const after = ctx.result as Record | undefined; - const previous = - (ctx.previous as Record | undefined) ?? - ((ctx as unknown as { __previous?: Record }).__previous ?? undefined); + // `ctx.previous` is the ONE key the pre-image arrives under, and the ENGINE + // is its single producer: it binds `previous` before dispatching the hook on + // every write shape — by-id update (`engine.ts:7010`, immediately ahead of + // the `beforeUpdate` dispatch at `:7012`), by-id delete (`bindPreImage`, + // `engine.ts:7869`, called at `:7897` ahead of the `beforeDelete` dispatch at + // `:7899`), and each per-row context of a predicate write (`engine.ts:1746` + // after-phase / `:1825` before-phase) — #5272 / #5574 / #5846. + // + // A `ctx.__previous` stash limb used to sit below this read, for the + // side-channel `plugin-audit`'s `captureBefore` wrote. #6656 retired + // `captureBefore`, which left the stash with ZERO producers, so the limb was + // removed here (#6978) instead of being kept "for safety" — ADR-0049 + // enforce-or-remove, and PD #12: an undeclared side-channel key is exactly + // the second de-facto contract a consumer-side `??` fossilizes. A future + // producer of `__previous` is therefore ignored by design; the way to hand + // this consumer a pre-image is to bind the declared `ctx.previous`. + const previous = ctx.previous as Record | undefined; const inputData = input.data && typeof input.data === 'object' ? input.data : undefined; const record: Record =