refactor(trigger-record-change): 摘掉 input.doc 这条没有生产者的防御性 alias 读 (#5671)

.changeset/olive-pugs-repeat.md

@@ -0,0 +1,14 @@
+---
+'@objectstack/trigger-record-change': patch
+---
+
+record-change trigger: drop the unreachable `input.doc` alias read, seed the flow record from `input.data` only
+
+Behaviour is unchanged: no engine path has ever built `input.doc`, so the alias
+limb could not be reached. Every ObjectQL write context spells the payload
+`data` — measured and pinned by `hook-input-shape-contract.test.ts` in
+`@objectstack/objectql` ("insert carries `data` — never `doc`", #5273). The
+branch survived only because the old `HookContext.input` contract table
+documented insert as `{ doc, options }`; that table was corrected in #5668, so
+the fallback no longer had even a documented producer to defend against, and it
+is removed here rather than left as a second de-facto contract (PD #12).

packages/services/service-storage/src/attachment-lifecycle.test.ts

@@ -156,7 +156,10 @@ describe('installAttachmentLifecycleHooks — tombstoning', () => {
     await engine.trigger('afterInsert', {
       object: 'sys_attachment',
       event: 'afterInsert',
-      input: { doc: { file_id: 'f1' } },
+      // [#5671] An insert hook's payload arrives under `data` — `doc` was this
+      // fixture's spelling from the old (wrong) contract table, a key no engine
+      // path builds. Pinned in objectql's `hook-input-shape-contract.test.ts`.
+      input: { data: { file_id: 'f1' } },
       result: { id: 'a9', file_id: 'f1' },
     });
 

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

@@ -57,7 +57,10 @@ function hookCtx(overrides: Partial<HookContext> = {}): HookContext {
     return {
         object: 'showcase_task',
         event: 'afterUpdate',
-        input: { id: 't1', doc: { status: 'done' } },
+        // `data` is what every engine write path actually builds — see the
+        // objectql pin `hook-input-shape-contract.test.ts` (#5273). The old
+        // spelling here was `doc`, a key no producer ever wrote (#5671).
+        input: { id: 't1', data: { status: 'done' } },
         result: { _id: 't1', status: 'done', assignee: 'u2' },
         previous: { _id: 't1', status: 'open', assignee: 'u1' },
         session: { userId: 'u9' },
@@ -300,7 +303,15 @@ describe('RecordChangeTrigger', () => {
         expect(ctx.params).toEqual(ctx.record);
     });
 
-    it('falls back to input.doc when result is absent (e.g. before-hooks)', async () => {
+    // [#5671] `input.data` is the ONE key a write hook's payload arrives under.
+    // objectql's `hook-input-shape-contract.test.ts` measures that on the engine
+    // ("insert carries `data` — never `doc`"); these two pin that this consumer
+    // reads exactly that key and nothing else. The pair is deliberate: the
+    // positive case alone would stay green if the deleted `doc` alias limb were
+    // put back (`data` sits first in the read, so it wins either way), so the
+    // NEGATIVE case is the one carrying the weight — it goes red the moment any
+    // alias limb returns.
+    it('seeds the record from input.data when result is absent (e.g. before-hooks)', async () => {
         const { engine, hooks } = fakeEngine();
         const trigger = new RecordChangeTrigger(engine, silentLogger());
         let captured: AutomationContext | undefined;
@@ -314,6 +325,30 @@ describe('RecordChangeTrigger', () => {
         expect(captured?.record).toEqual({ status: 'done' });
     });
 
+    it('does NOT read a `doc` alias off input — no engine path produces that key', async () => {
+        const { engine, hooks } = fakeEngine();
+        const trigger = new RecordChangeTrigger(engine, silentLogger());
+        let captured: AutomationContext | undefined;
+
+        trigger.start(binding({ event: 'record-before-update' }), async (ctx) => {
+            captured = ctx;
+        });
+
+        // `previous` is dropped too, so the seed can only come from the payload:
+        // with `doc` unread there is nothing left and the record is empty.
+        await hooks[0].handler(
+            hookCtx({
+                event: 'beforeUpdate',
+                result: undefined,
+                previous: undefined,
+                input: { id: 't1', doc: { status: 'done' } } as HookContext['input'],
+            }),
+        );
+
+        expect(captured?.record).toEqual({});
+        expect((captured?.record as Record<string, unknown>).status).toBeUndefined();
+    });
+
     it('reads the __previous stash when ctx.previous is absent', async () => {
         const { engine, hooks } = fakeEngine();
         const trigger = new RecordChangeTrigger(engine, silentLogger());

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

@@ -277,36 +277,37 @@ 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 doc / previous row; the old record from `ctx.previous`
+     * mutation input payload / previous row; the old record from `ctx.previous`
      * (with the `__previous` stash audit also uses as a fallback).
      *
      * Async because the seeded `record` is hydrated with read-time computed
      * fields (see {@link hydrateComputedFields}) via a data-engine re-read.
      */
     private async buildContext(binding: FlowTriggerBinding, ctx: HookContext): Promise<AutomationContext> {
         // objectql lifecycle hooks carry the written row under `input.data` (insert /
-        // update payload); `id` is on update. (`doc` kept only as a defensive alias.)
-        const input = (ctx.input ?? {}) as { data?: Record<string, unknown>; doc?: Record<string, unknown>; id?: unknown };
+        // update payload); `id` is on update. `data` is the ONLY spelling any engine
+        // path produces — measured and pinned by objectql's
+        // `hook-input-shape-contract.test.ts` ("insert carries `data` — never `doc`",
+        // #5273). A `doc` alias limb used to sit below this read for a producer that
+        // never existed; removed in #5671 rather than left as a second de-facto
+        // contract (PD #12). before/afterDelete carry no payload at all and fall
+        // through to `previous` below.
+        const input = (ctx.input ?? {}) as { data?: Record<string, unknown>; id?: unknown };
         const after = ctx.result as Record<string, unknown> | undefined;
         const previous =
             (ctx.previous as Record<string, unknown> | undefined) ??
             ((ctx as unknown as { __previous?: Record<string, unknown> }).__previous ?? undefined);
 
-        const inputDoc =
-            input.data && typeof input.data === 'object'
-                ? input.data
-                : input.doc && typeof input.doc === 'object'
-                  ? input.doc
-                  : undefined;
+        const inputData = input.data && typeof input.data === 'object' ? input.data : undefined;
         const record: Record<string, unknown> =
             after && typeof after === 'object'
-                ? // #1872 — overlay the after-row on the input doc so fields the
+                ? // #1872 — overlay the after-row on the input payload so fields the
                   // driver did not echo back (notably `multiple: true` lookups,
                   // stored as an array column) stay visible to the flow's start
                   // condition and `{record.<field>}` interpolation. The after-row
                   // wins for every field it DOES return (id, DB-computed values).
-                  { ...(inputDoc ?? {}), ...after }
-                : inputDoc ?? (previous && typeof previous === 'object' ? previous : {});
+                  { ...(inputData ?? {}), ...after }
+                : inputData ?? (previous && typeof previous === 'object' ? previous : {});
 
         const session = (ctx.session ?? {}) as { userId?: string; organizationId?: string };