From fb2e7ba59cf30f5b9d190d051695cc7b8d73a1d7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 11:22:12 +0000 Subject: [PATCH] =?UTF-8?q?refactor(trigger-record-change):=20=E6=91=98?= =?UTF-8?q?=E6=8E=89=20`input.doc`=20=E8=BF=99=E6=9D=A1=E6=B2=A1=E6=9C=89?= =?UTF-8?q?=E7=94=9F=E4=BA=A7=E8=80=85=E7=9A=84=E9=98=B2=E5=BE=A1=E6=80=A7?= =?UTF-8?q?=20alias=20=E8=AF=BB=20(#5671)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `buildContext()` 里的 `inputDoc` 三元把 `input.data` 之后又兜了一条 `input.doc`。该分支在任何引擎路径上都不可达:ObjectQL 的写事件上下文一律把 载荷拼在 `data` 上(insert / update),before/afterDelete 两者都不带、直接落 到 `previous`。真值由 objectql 的 `hook-input-shape-contract.test.ts` 钉住 (「insert carries `data` — never `doc`」,#5273 / PR #5668)。 该 alias 当初留下的唯一依据是旧的 `HookContext.input` 契约表把 insert 写成 `{ doc, options }` —— 那张表本身是错的,已由 PR #5668 改成 `{ data, options }`。 依据消失后按 PD #12 删掉,而不是继续养成第二套事实契约。 行为零变化(该分支不可达)。同时: - `record-change-trigger.test.ts` 的 `hookCtx` fixture 原本拼的是 `doc`,正是 它让这条死分支在测试里保持「活着」;按引擎真值 re-spell 成 `data`。 - 原「falls back to input.doc」用例整条替换为一对承重 pin:正向钉 `input.data` 播种 record;反向钉「只拼 `doc` 的上下文不播种」—— 后者才是 承重的那条(`data` 在读取链首位,alias 限支复活时正向用例仍绿)。 - `attachment-lifecycle.test.ts` 的 insert fixture 同样按引擎真值 re-spell。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015a5qkLzpGXhLL2F5gvJ7dD --- .changeset/olive-pugs-repeat.md | 14 +++++++ .../src/attachment-lifecycle.test.ts | 5 ++- .../src/record-change-trigger.test.ts | 39 ++++++++++++++++++- .../src/record-change-trigger.ts | 25 ++++++------ 4 files changed, 68 insertions(+), 15 deletions(-) create mode 100644 .changeset/olive-pugs-repeat.md diff --git a/.changeset/olive-pugs-repeat.md b/.changeset/olive-pugs-repeat.md new file mode 100644 index 0000000000..4f99b88ebd --- /dev/null +++ b/.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). diff --git a/packages/services/service-storage/src/attachment-lifecycle.test.ts b/packages/services/service-storage/src/attachment-lifecycle.test.ts index 6f0c0da328..bd1a2d2b6c 100644 --- a/packages/services/service-storage/src/attachment-lifecycle.test.ts +++ b/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' }, }); 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 421ff4faa0..a4a4ea066d 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 @@ -57,7 +57,10 @@ function hookCtx(overrides: Partial = {}): 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).status).toBeUndefined(); + }); + it('reads the __previous stash when ctx.previous is absent', async () => { const { engine, hooks } = fakeEngine(); const trigger = new RecordChangeTrigger(engine, silentLogger()); 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 581bf57dcc..8ada12af40 100644 --- a/packages/triggers/trigger-record-change/src/record-change-trigger.ts +++ b/packages/triggers/trigger-record-change/src/record-change-trigger.ts @@ -277,7 +277,7 @@ 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 @@ -285,28 +285,29 @@ export class RecordChangeTrigger implements FlowTrigger { */ private async buildContext(binding: FlowTriggerBinding, ctx: HookContext): Promise { // 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; doc?: Record; 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; id?: unknown }; const after = ctx.result as Record | undefined; const previous = (ctx.previous as Record | undefined) ?? ((ctx as unknown as { __previous?: Record }).__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 = 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.}` 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 };