Skip to content

Commit 9ee0fc3

Browse files
refactor(trigger-record-change): drop the ctx.__previous stash fallback, dead since #6656 (#6978) (#7081)
`plugin-audit`'s `captureBefore` was the only writer of `ctx.__previous` in the repo; #6656 retired it, leaving `buildContext`'s `ctx.previous ?? ctx.__previous` with a second operand nothing can bind. The engine is the single producer of the pre-image and binds the declared key ahead of every dispatch (engine.ts:7010 for by-id update, bindPreImage at :7869 for by-id delete, :1746/:1825 for the per-row contexts of a predicate write — #5272/#5574/#5846). Removed under ADR-0049 enforce-or-remove rather than kept "for safety": a future producer of `__previous` is now ignored by design, which is the intended post-state (declared = enforced, PD #12). The test that fed the limb synthesised the key in its own body — the #4984 shape that kept it looking live. Replaced with the inverted negative pin the deletion needs, the same treatment #5671 gave the `doc` alias in this file: restoring the limb turns it red on both assertions. Claude-Session: https://claude.ai/code/session_01USNUyHEr7uaU6MoEWXitei Co-authored-by: Claude <noreply@anthropic.com>
1 parent f5a9bc2 commit 9ee0fc3

3 files changed

Lines changed: 70 additions & 8 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
'@objectstack/trigger-record-change': patch
3+
---
4+
5+
record-change trigger: drop the `ctx.__previous` stash fallback — read the engine's declared `ctx.previous` only
6+
7+
Behaviour is unchanged: the limb guarded against a producer that no longer
8+
exists. `plugin-audit`'s `captureBefore` was the **only** writer of
9+
`ctx.__previous` in the repo, and #6656 retired it, so `buildContext`'s
10+
11+
```ts
12+
ctx.previous ?? (ctx as { __previous? }).__previous
13+
```
14+
15+
had a second operand nothing could ever bind. The engine is the single producer
16+
of the pre-image and it binds the declared key ahead of every dispatch — by-id
17+
update (`engine.ts:7010`, immediately before the `beforeUpdate` dispatch at
18+
`:7012`), by-id delete (`bindPreImage`, `engine.ts:7869`, called at `:7897`
19+
before the `beforeDelete` dispatch at `:7899`), and each per-row context of a
20+
predicate write (`engine.ts:1746` after-phase, `:1825` before-phase) —
21+
#5272 / #5574 / #5846.
22+
23+
Removed rather than kept "for safety", under ADR-0049 enforce-or-remove and
24+
PD #12: a fallback with zero producers is a second de-facto contract waiting to
25+
be rediscovered. The consequence is deliberate and stated here rather than left
26+
to be found — **a future producer of `ctx.__previous` is now silently ignored**;
27+
the declared way to hand this consumer a pre-image is `ctx.previous`.
28+
29+
The test that fed the limb synthesised `__previous` in its own body, which is
30+
what kept the dead limb looking live (#4984). It is replaced by the inverted pin
31+
the deletion actually needs — the same treatment the `doc` alias got in #5671
32+
so restoring the limb goes red instead of unnoticed.

packages/triggers/trigger-record-change/src/record-change-trigger.test.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,20 @@ describe('RecordChangeTrigger', () => {
349349
expect((captured?.record as Record<string, unknown>).status).toBeUndefined();
350350
});
351351

352-
it('reads the __previous stash when ctx.previous is absent', async () => {
352+
// [#6978] The `ctx.__previous` stash limb, deleted — same family as the `doc`
353+
// alias above, same negative-pin treatment. `plugin-audit`'s `captureBefore`
354+
// was the stash's ONLY producer and #6656 retired it, leaving the fallback
355+
// with zero producers; ADR-0049 enforce-or-remove says it goes.
356+
//
357+
// This case REPLACES the one that used to live here ("reads the __previous
358+
// stash when ctx.previous is absent"), which synthesised the key in its own
359+
// body — the #4984 shape: a fixture spelling a key no producer emits, keeping
360+
// a dead limb looking live. A positive case can never carry the weight here
361+
// either, because the canonical key sits FIRST in the read and wins whether or
362+
// not the limb exists. So the pin is inverted: restore the limb and this case
363+
// goes red on both assertions (`previous` becomes `{ status: 'old' }`, and the
364+
// record seeds from it instead of staying empty).
365+
it('does NOT read the `__previous` stash — no producer emits that key (#6978)', async () => {
353366
const { engine, hooks } = fakeEngine();
354367
const trigger = new RecordChangeTrigger(engine, silentLogger());
355368
let captured: AutomationContext | undefined;
@@ -358,11 +371,14 @@ describe('RecordChangeTrigger', () => {
358371
captured = ctx;
359372
});
360373

361-
const ctx = hookCtx({ previous: undefined });
374+
// Every declared source of a pre-image is dropped, so the stash is the only
375+
// thing left that could answer — and it must not.
376+
const ctx = hookCtx({ result: undefined, previous: undefined, input: { id: 't1' } });
362377
(ctx as unknown as { __previous: Record<string, unknown> }).__previous = { status: 'old' };
363378
await hooks[0].handler(ctx);
364379

365-
expect(captured?.previous).toEqual({ status: 'old' });
380+
expect(captured?.previous).toBeUndefined();
381+
expect(captured?.record).toEqual({});
366382
});
367383

368384
it('isolates flow errors so the CRUD write is never broken', async () => {

packages/triggers/trigger-record-change/src/record-change-trigger.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -277,8 +277,8 @@ export class RecordChangeTrigger implements FlowTrigger {
277277
/**
278278
* Build the flow execution context from an ObjectQL hook context. The new
279279
* record comes from `ctx.result` (after-hooks) or falls back to the
280-
* mutation input payload / previous row; the old record from `ctx.previous`
281-
* (with the `__previous` stash audit also uses as a fallback).
280+
* mutation input payload / previous row; the old record from `ctx.previous`,
281+
* which the engine binds ahead of every dispatch.
282282
*
283283
* Async because the seeded `record` is hydrated with read-time computed
284284
* fields (see {@link hydrateComputedFields}) via a data-engine re-read.
@@ -294,9 +294,23 @@ export class RecordChangeTrigger implements FlowTrigger {
294294
// through to `previous` below.
295295
const input = (ctx.input ?? {}) as { data?: Record<string, unknown>; id?: unknown };
296296
const after = ctx.result as Record<string, unknown> | undefined;
297-
const previous =
298-
(ctx.previous as Record<string, unknown> | undefined) ??
299-
((ctx as unknown as { __previous?: Record<string, unknown> }).__previous ?? undefined);
297+
// `ctx.previous` is the ONE key the pre-image arrives under, and the ENGINE
298+
// is its single producer: it binds `previous` before dispatching the hook on
299+
// every write shape — by-id update (`engine.ts:7010`, immediately ahead of
300+
// the `beforeUpdate` dispatch at `:7012`), by-id delete (`bindPreImage`,
301+
// `engine.ts:7869`, called at `:7897` ahead of the `beforeDelete` dispatch at
302+
// `:7899`), and each per-row context of a predicate write (`engine.ts:1746`
303+
// after-phase / `:1825` before-phase) — #5272 / #5574 / #5846.
304+
//
305+
// A `ctx.__previous` stash limb used to sit below this read, for the
306+
// side-channel `plugin-audit`'s `captureBefore` wrote. #6656 retired
307+
// `captureBefore`, which left the stash with ZERO producers, so the limb was
308+
// removed here (#6978) instead of being kept "for safety" — ADR-0049
309+
// enforce-or-remove, and PD #12: an undeclared side-channel key is exactly
310+
// the second de-facto contract a consumer-side `??` fossilizes. A future
311+
// producer of `__previous` is therefore ignored by design; the way to hand
312+
// this consumer a pre-image is to bind the declared `ctx.previous`.
313+
const previous = ctx.previous as Record<string, unknown> | undefined;
300314

301315
const inputData = input.data && typeof input.data === 'object' ? input.data : undefined;
302316
const record: Record<string, unknown> =

0 commit comments

Comments
 (0)