Skip to content

Commit 04f36b6

Browse files
feat(service-automation): a record-triggered flow's notification links to the record that started the run when it names no link (#30252)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgM2GxWhdgy4dyjBtN2tYY9u)_ domain:server#4 · os-bill · session_01Utp6CZMkHvuDeVYgTD73qF ## 维护者速读 - **改了什么:** 由记录触发的流程里,通知节点没写 `sourceObject`/`sourceId`/`actionUrl` 时,通知默认链到触发这次运行的那条记录;点开通知直接进记录页。 - **为什么改:** 技术员收到「工单分配给你」的通知,点开却落在通知列表;作者照最自然的写法(收件人、标题、正文)写,`os validate` 也通过,链接却是空的。卡 #30121「第一组:会卡住人或误导人」。 - **风险与回滚:** 只在三项都没写、且运行确有触发记录时生效;显式的一对或 `actionUrl` 照旧优先,半对照旧丢弃,定时/API/无记录的手动运行不变,删除事件不链(记录已不在)。回滚即撤本 PR。 - **先看:** `packages/services/service-automation/src/builtin/notify-node.ts` 的 `defaultClickThroughOf` 与调用处,再看 `notify-node.test.ts` 末尾新的 describe。 - **两分钟验证:** `pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 src/builtin/notify-node.test.ts`,预期 23 条全绿。 - **需要你做的:** 无,席位落地。 Fixes #30121 Clause-②: no — no accepted key, value or exported type changes; `sourceObject` / `sourceId` / `actionUrl` keep their meaning when written, and no option or type member was added to reach the trigger record (the node already reads it through `triggerRecordOf`). Metadata-protocol: semantic Skills-docs: follow-up owed — skills/objectstack-automation/SKILL.md (the notify example's `click-through: a PAIR, else dropped` line), content/docs/automation/flows.mdx (the notify `sourceId` paragraph), packages/spec/src/automation/io-node-config.zod.ts (the `sourceObject` / `sourceId` describes, which feed content/docs/references/automation/io-node-config.mdx). The skill line is a governed (Tier H) edit and the spec describe is out of this order's scope, so neither rides here; the seat files the card. ## What changes **`@objectstack/service-automation` — the `notify` node** (`src/builtin/notify-node.ts`): - New `defaultClickThroughOf(cfg, context, trigger)`. When the node writes none of `sourceObject`, `sourceId` and `actionUrl`, the click-through record is the run's trigger record, read through the existing `triggerRecordOf(context)`: the same reading `recipients: { emailField }` and the organization fallback already use. The messaging service then stores it as `sys_notification.source_object` / `source_id` and synthesizes `/OBJECT/ID`, exactly as for an authored pair. - What counts as a trigger record, measured on `origin/main` at the run-starting doors: - record change (`trigger-record-change`): `object` + the row, `record.id` present. **Defaults.** - time-relative sweep (`trigger-schedule` `time-relative-trigger.ts`): `object: desc.object`, `record: row`. That object's row is the record the run is about, so it **defaults**. - action on a record (`runtime` `action-execution.ts`): `object` + `record` with the requested `recordId` stamped. **Defaults.** - schedule tick, inbound API post (`trigger-api`: `record: payload` with no `object`), manual REST start (`object` but no `record`): no trigger record. **Unchanged.** - Delete events (`beforeDelete`, `afterDelete`) are not defaulted: the record is gone once the write lands, and this node already drops a half-written pair so the inbox never renders a dead link. An authored pair on a delete flow is still honoured. - A half-written pair keeps today's behaviour: it is dropped and nothing is defaulted in its place, because the author did write a link. - An `actionUrl` written alone keeps today's behaviour too: the URL is the link and no `source` is defaulted beside it, per the card's "an explicit `actionUrl` or pair still wins". - The organization fallback (#23284) asked `[source, trigger]`; a defaulted `source` is the trigger record itself, so the second candidate is skipped when it is the same object (no second `findOne`). The organization it resolves is unchanged. - The Studio form help for `sourceObject`, `sourceId` and `actionUrl` (the node descriptor's `configSchema`) says the default. ## Tests `src/builtin/notify-node.test.ts`, one new top-level `describe` in the existing harness, six pins: 1. record-triggered notify, no link written ⇒ `source` is `{ object: 'repair_ticket', id: 't_7' }`, no `url`; 2. explicit pair wins over the trigger record; 3. explicit `actionUrl` alone: `url` as written, no `source` defaulted; 4. half-written pair (`sourceObject` only): still dropped; 5. run with no trigger record (`object`, no `record`): no `source`; 6. `afterDelete`: no `source`. <details><summary>Local runs (head after the final commit is in the report)</summary> - `pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 src/builtin/notify-node.test.ts`: Test Files 1 passed, Tests 23 passed (17 before + 6). - `pnpm --filter @objectstack/service-automation test` (under `os-verify-lock.sh`): Test Files 263 passed (263), Tests 3407 passed (3407), lock VERDICT command-exit 0. - `pnpm --filter @objectstack/service-automation typecheck`: exit 0 (test layer included, `check:test-typecheck` OK, 0 debt). </details> <details><summary>Ablation (one, on the behaviour the pins hold)</summary> `node scripts/ablation-replace.mjs` on `notify-node.ts`, anchor `return trigger;` (the default's answer) replaced by `return trigger && undefined; // ABLATED_30121`: anchor x1 to x0, replacement x0 to x1, blob 94746f6 to 381704ef410f. Run: Tests 1 failed, 22 passed; the one red is pin 1, "links a record-triggered notify that names no source to the trigger record". Restored: blob equals HEAD (94746f6), `git diff HEAD` empty. The test imports the executor from `src`, so no build sat between the mutation and the run. A first attempt used a bare identifier as the marker, which threw a ReferenceError and turned eight unrelated tests red; that run was discarded as a malformed mutation, not counted. </details> ## Acceptance notes - The spec's own describes for `sourceObject` / `sourceId` (`packages/spec/src/automation/io-node-config.zod.ts`) still say "Only takes effect together with …", which stays true for an authored pair but does not mention the default. Out of this order's scope (`packages/spec/src/**`); listed under `Skills-docs:` above. - A flow started by an action whose caller could not read the row (`recordLoadDenied: true`) still carries the requested id, so its notification links to that id, as the authored `sourceId: record.id` would. The record page applies the recipient's own access when opened. ## Verification Gate list derived with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`; each command's exit code was recorded before any pipe and reconciled with `--ran`. Readings are in the os-dev-report comment on #30121. `origin/main` was merged once before opening (no incoming commit touched `service-automation` or `packages/spec`). --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CZ1tD2uZm21w4Bj1xs)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent eb6199b commit 04f36b6

3 files changed

Lines changed: 139 additions & 7 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/service-automation': minor
3+
---
4+
5+
feat(service-automation): a record-triggered flow's notification opens the record that started the run, without the author writing `sourceObject` and `sourceId`
6+
7+
Clause-②: no — no accepted key, value or exported type changes; the node's existing `sourceObject` / `sourceId` / `actionUrl` keys keep their meaning when written.
8+
Metadata-protocol: semantic
9+
10+
- **Before.** A `notify` node built its click-through link only from an explicit `sourceObject` + `sourceId` pair (or an `actionUrl`). The obvious record-triggered flow — `recipients`, `title`, `message` — passed `os validate` and sent a notification whose link was empty, so a click opened the notification list instead of the record.
11+
- **Now.** A `notify` node that writes none of `sourceObject`, `sourceId` and `actionUrl` links to the run's trigger record when the run has one: a record change (create or update), a time-relative sweep's row, or an action started on a record. The notification stores that record as its `source_object` / `source_id`, and the inbox opens `/OBJECT/ID`.
12+
- **Unchanged.** An explicit pair or an explicit `actionUrl` wins, as before. A half-written pair is still dropped rather than defaulted. A run with no trigger record (a schedule, an inbound API post, a manual start with no record) links nowhere, as before. A delete event is not linked: its record is gone once the write lands.
13+
- **To link elsewhere** from a record-triggered flow, write the `actionUrl` or the `sourceObject` + `sourceId` pair the notification should open.

‎packages/services/service-automation/src/builtin/notify-node.test.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,3 +393,86 @@ describe('notify (baseline node)', () => {
393393
});
394394
});
395395
});
396+
397+
// A notify node that writes no link (no `sourceObject`, `sourceId` or
398+
// `actionUrl`) links to the run's trigger record, when the run has one. The
399+
// author's own link — a pair, an `actionUrl`, or a half-written pair — is kept
400+
// as written, and a run with no trigger record links nowhere, as before.
401+
describe('notify click-through defaults to the trigger record', () => {
402+
let engine: AutomationEngine;
403+
let messaging: ReturnType<typeof fakeMessaging>;
404+
405+
/** A record-change run's context: the row that triggered it, and its object. */
406+
const recordRun = (event = 'afterInsert') =>
407+
({ record: { id: 't_7', name: 'Pump' }, object: 'repair_ticket', event }) as any;
408+
409+
beforeEach(() => {
410+
messaging = fakeMessaging();
411+
engine = new AutomationEngine(createTestLogger());
412+
registerNotifyNode(engine, createCtx(messaging.service));
413+
});
414+
415+
it('links a record-triggered notify that names no source to the trigger record', async () => {
416+
engine.registerFlow('notify_flow', notifyFlow({ recipients: ['user_1'], title: 'Assigned to you' }));
417+
418+
const result = await engine.execute('notify_flow', recordRun());
419+
420+
expect(result.success).toBe(true);
421+
expect(messaging.emitted[0].source).toEqual({ object: 'repair_ticket', id: 't_7' });
422+
expect(messaging.emitted[0].payload.url).toBeUndefined();
423+
});
424+
425+
it('keeps an explicit pair over the trigger record', async () => {
426+
engine.registerFlow('notify_flow', notifyFlow({
427+
recipients: ['user_1'],
428+
title: 'Quote ready',
429+
sourceObject: 'mtc_quotation',
430+
sourceId: 'q_42',
431+
}));
432+
433+
await engine.execute('notify_flow', recordRun());
434+
435+
expect(messaging.emitted[0].source).toEqual({ object: 'mtc_quotation', id: 'q_42' });
436+
});
437+
438+
it('keeps an explicit actionUrl alone: no source is defaulted beside it', async () => {
439+
engine.registerFlow('notify_flow', notifyFlow({
440+
recipients: ['user_1'],
441+
title: 'See the board',
442+
actionUrl: '/boards/main',
443+
}));
444+
445+
await engine.execute('notify_flow', recordRun());
446+
447+
expect(messaging.emitted[0].source).toBeUndefined();
448+
expect(messaging.emitted[0].payload.url).toBe('/boards/main');
449+
});
450+
451+
it('still drops a half-written pair rather than defaulting it', async () => {
452+
engine.registerFlow('notify_flow', notifyFlow({
453+
recipients: ['user_1'],
454+
title: 'Heads up',
455+
sourceObject: 'opportunity',
456+
}));
457+
458+
await engine.execute('notify_flow', recordRun());
459+
460+
expect(messaging.emitted[0].source).toBeUndefined();
461+
});
462+
463+
it('links nowhere on a run with no trigger record', async () => {
464+
engine.registerFlow('notify_flow', notifyFlow({ recipients: ['user_1'], title: 'Daily digest' }));
465+
466+
await engine.execute('notify_flow', { object: 'repair_ticket', event: 'manual' } as any);
467+
468+
expect(messaging.emitted[0].source).toBeUndefined();
469+
});
470+
471+
it('does not link a delete event to the record it removed', async () => {
472+
engine.registerFlow('notify_flow', notifyFlow({ recipients: ['user_1'], title: 'Ticket removed' }));
473+
474+
await engine.execute('notify_flow', recordRun('afterDelete'));
475+
476+
expect(messaging.emitted[0].source).toBeUndefined();
477+
});
478+
});

‎packages/services/service-automation/src/builtin/notify-node.ts‎

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,10 @@ const SUBJECT_ORGANIZATION_READ = { context: { isSystem: true } } as const;
106106
*
107107
* The candidates are tried in order and the first one that answers wins:
108108
*
109-
* 1. the click-through record the node declares (`sourceObject` + `sourceId`)
110-
* — what `sys_notification.source_object` / `source_id` store, and the
109+
* 1. the click-through record — the one the node declares (`sourceObject` +
110+
* `sourceId`), or the trigger record it defaults to when the node writes
111+
* no link (`defaultClickThroughOf`) — what
112+
* `sys_notification.source_object` / `source_id` store, and the
111113
* anchor ADR-0131's ownership inventory attributes a stored notification
112114
* by, so the producer and the inventory answer the same question the same
113115
* way;
@@ -156,6 +158,35 @@ function triggerRecordOf(context: AutomationContext): RecordRef | undefined {
156158
return object && id ? { object, id } : undefined;
157159
}
158160

161+
/** The record-change events whose trigger record no longer exists, or soon will not. */
162+
const DELETE_EVENTS: ReadonlySet<string> = new Set(['beforeDelete', 'afterDelete']);
163+
164+
/**
165+
* The click-through record of a notify node that names none: the run's trigger
166+
* record, so a record-triggered flow's notification opens the record it is
167+
* about without the author writing `sourceObject` + `sourceId: record.id`.
168+
*
169+
* Only when the node leaves the whole link to the runtime — none of
170+
* `sourceObject`, `sourceId` and `actionUrl` written. An explicit pair, an
171+
* explicit `actionUrl`, and a half-written pair (still dropped, as before) are
172+
* the author's word and are kept as written.
173+
*
174+
* Only when the run has a trigger record (`triggerRecordOf`): a record-change
175+
* run, a time-relative sweep's row, an action started on a record. A schedule
176+
* tick, an inbound API post and a manual start with no record have none and
177+
* link nowhere, as before. A delete event's record is gone once the write
178+
* lands, so it is not linked: the inbox never renders a dead link.
179+
*/
180+
function defaultClickThroughOf(
181+
cfg: Pick<NotifyConfigParsed, 'sourceObject' | 'sourceId' | 'actionUrl'>,
182+
context: AutomationContext,
183+
trigger: RecordRef | undefined,
184+
): RecordRef | undefined {
185+
if (cfg.sourceObject != null || cfg.sourceId != null || cfg.actionUrl != null) return undefined;
186+
if (typeof context.event === 'string' && DELETE_EVENTS.has(context.event)) return undefined;
187+
return trigger;
188+
}
189+
159190
/**
160191
* The code a thrown failure carries (`err.code`, the ADR-0112 envelope's
161192
* machine-readable half), so a step that fails on the engine's refusal names
@@ -851,11 +882,11 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext)
851882
// literals, read as written.
852883
sourceObject: {
853884
type: 'string',
854-
description: 'Object name of the record the notification links to (writes sys_notification.source_object) — read as written: a single-brace {token} is not resolved here and is refused, as is a CEL value envelope; write the value. Only takes effect together with sourceId — a half-specified click-through target is dropped at execute time, so the inbox never renders a dead link.',
885+
description: 'Object name of the record the notification links to (writes sys_notification.source_object) — read as written: a single-brace {token} is not resolved here and is refused, as is a CEL value envelope; write the value. Only takes effect together with sourceId — a half-specified click-through target is dropped at execute time, so the inbox never renders a dead link. When sourceObject, sourceId and actionUrl are all left out, a run started by a record (a record change other than a delete, a time-relative sweep, an action on a record) links to that record.',
855886
},
856887
sourceId: {
857888
type: 'string',
858-
description: 'Record id the notification links to (writes sys_notification.source_id) — a string, or a CEL value envelope evaluated per run to one, such as { dialect: \'cel\', source: \'record.id\' }; a single-brace {token} is refused. Only takes effect together with sourceObject — a half-specified click-through target is dropped at execute time, so the inbox never renders a dead link. The inbox synthesizes a `/{object}/{id}` deep-link from these.',
889+
description: 'Record id the notification links to (writes sys_notification.source_id) — a string, or a CEL value envelope evaluated per run to one, such as { dialect: \'cel\', source: \'record.id\' }; a single-brace {token} is refused. Only takes effect together with sourceObject — a half-specified click-through target is dropped at execute time, so the inbox never renders a dead link. The inbox synthesizes a `/{object}/{id}` deep-link from these. When sourceObject, sourceId and actionUrl are all left out, a run started by a record (a record change other than a delete, a time-relative sweep, an action on a record) links to that record.',
859890
},
860891
actorId: {
861892
type: 'string',
@@ -868,7 +899,7 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext)
868899
// [#19939] A TEXT slot, rendered with {{ }} holes like `title`.
869900
actionUrl: {
870901
type: 'string',
871-
description: 'Explicit click-through URL, rendered per run with {{ }} placeholders like title (e.g. /crm_case/{{ record.id }}); a single-brace {token} is refused. Overrides the link synthesized from sourceObject/sourceId.',
902+
description: 'Explicit click-through URL, rendered per run with {{ }} placeholders like title (e.g. /crm_case/{{ record.id }}); a single-brace {token} is refused. Overrides the link synthesized from sourceObject/sourceId, and written alone keeps the notification from linking to the record that started the run.',
872903
},
873904
payload: {
874905
type: 'object',
@@ -985,9 +1016,12 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext)
9851016
const sourceObject = toStr(cfg.sourceObject);
9861017
const sourceId = resolveStringSlot(engine, 'sourceId', cfg.sourceId, variables, context);
9871018
if (!sourceId.ok) return { success: false, error: sourceId.error };
1019+
// A node that writes no link at all links to the run's trigger
1020+
// record, when it has one (`defaultClickThroughOf`).
1021+
const trigger = triggerRecordOf(context);
9881022
const source = sourceObject && sourceId.value
9891023
? { object: sourceObject, id: sourceId.value }
990-
: undefined;
1024+
: defaultClickThroughOf(cfg, context, trigger);
9911025
const actorId = toStr(cfg.actorId);
9921026

9931027
// With a `template` reference the content lives in the template
@@ -1089,9 +1123,11 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext)
10891123
if (!organizationId) {
10901124
const data = getData();
10911125
if (data) {
1126+
// A defaulted click-through IS the trigger record:
1127+
// asked once, not twice.
10921128
organizationId = await organizationOfRecordAbout(data, wallResolverFor(data), [
10931129
source,
1094-
triggerRecordOf(context),
1130+
source === trigger ? undefined : trigger,
10951131
]);
10961132
}
10971133
}

0 commit comments

Comments
 (0)