Skip to content

Commit 9bf4588

Browse files
os-zhuangclaude
andauthored
feat(lint,showcase,automation): record-after-write follow-ups — validation lint, showcase flow, create-leg previous fix (#3427) (#3467)
* feat(lint,showcase): flag never-firing record trigger tokens; add record-after-write showcase flow (#3427) Follow-ups to the record-after-write feature (#3446). Lint (item 3): new `flow-trigger-unknown-event` rule in validateFlowTriggerReadiness flags a start node whose `triggerType` is record-lifecycle-shaped (`record-before|after-<op>`) but names an op the record-change trigger cannot map — a typo like `record-after-updated`. The engine still routes any `record-` token to the record trigger, which then binds to NO hook and never fires (only a runtime warn), so this surfaces the never-fire defect at authoring/`os validate` time. Warning severity, consistent with the file's existing rules. Bare `record-<noun>` shapes (e.g. `record-change`) are out of scope for this rule. Showcase (item 2): add `UrgentTaskAlertFlow`, a single `record-after-write` flow that fires when a task is created Urgent OR escalated to Urgent — using the `previous == null` create/update discrimination the write trigger enables. Plus a durable integration test booting a real kernel that verifies the create leg (`previous == null` → fires on afterInsert), the non-urgent create (no fire), and the escalation leg (fires on afterUpdate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018939yJ413zG3irLzcTtqaa * fix(service-automation): bind `previous` as null on the create leg; fix showcase notify recipient (#3427) Dogfooding the record-after-write showcase flow in a live app surfaced two bugs: 1. The engine bound `previous` into the flow condition scope only when truthy, so on a record insert `previous` was an unknown CEL variable — the documented `previous == null` create-discrimination threw "Unknown variable: previous" and failed the whole start condition, dropping the run. `previous` is now always bound (null when there is no prior row), making the create/update discrimination the record-after-write docs + Studio designer advertise actually work. Verified end-to-end (integration test + a live showcase boot: the flow fires on create-urgent and on escalation, and correctly skips a non-urgent create). 2. The showcase `UrgentTaskAlertFlow` notify node had no recipient, so every run failed "at least one recipient is required". Now notifies the assignee, falling back to the triggering user (`{$User.Id}`) so an unassigned urgent task still pings someone. Live-verified: a `task.urgent` notification is delivered. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018939yJ413zG3irLzcTtqaa * chore: add changeset for the flow-trigger-unknown-event lint rule Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018939yJ413zG3irLzcTtqaa --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4340f13 commit 9bf4588

7 files changed

Lines changed: 272 additions & 3 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
feat(lint): flag never-firing record trigger tokens at authoring time (#3427)
6+
7+
New `flow-trigger-unknown-event` rule in `validateFlowTriggerReadiness`: a flow
8+
start node whose `triggerType` is record-lifecycle-shaped
9+
(`record-before|after-<op>`) but names an op the record-change trigger cannot map
10+
— e.g. a typo like `record-after-updated` — binds to the record-change trigger
11+
yet maps to no ObjectQL hook and never fires, with only a runtime warning. The
12+
rule surfaces that never-fire defect at `os validate` time. Warning severity;
13+
bare `record-<noun>` shapes (e.g. `record-change`) are out of scope.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
"@objectstack/service-automation": patch
3+
---
4+
5+
fix(service-automation): bind `previous` (as null) on the create leg so start conditions can discriminate create vs update (#3427)
6+
7+
The engine bound `previous` into the flow condition scope only when it was
8+
truthy, so on a record insert (`record-after-create`, and the create leg of
9+
`record-after-write`) `previous` was an **unknown** CEL variable. Any reference to
10+
it — including the documented `previous == null` create-discrimination — threw
11+
`condition failed to evaluate as CEL: Unknown variable: previous`, failing the
12+
whole start condition and dropping the run.
13+
14+
`previous` is now always bound, to `null` when there is no prior row. So
15+
`previous == null` is the create leg and `previous != null` / `previous.<field>`
16+
the update leg — the pattern the `record-after-write` docs and the Studio flow
17+
designer advertise. Update-triggered flows are unaffected (`previous` was, and
18+
stays, the prior row there).

examples/app-showcase/src/automation/flows/index.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,8 +1534,67 @@ export const TaskDueReminderFlow = defineFlow({
15341534
],
15351535
});
15361536

1537+
/**
1538+
* Urgent Task Alert — a single `record-after-write` flow that fires on BOTH
1539+
* create and update (#3427), so "a task was created urgent OR just escalated to
1540+
* urgent" is one flow, not two near-identical copies.
1541+
*
1542+
* `record-after-write` binds afterInsert + afterUpdate; exactly one fires per
1543+
* mutation. The start condition uses the create/update discrimination the write
1544+
* trigger enables: `previous == null` is the create leg (no prior row), so the
1545+
* `||` also matches a brand-new urgent task; on the update leg it fires only when
1546+
* priority actually crosses INTO 'urgent' (not on every later save while urgent).
1547+
*/
1548+
export const UrgentTaskAlertFlow = defineFlow({
1549+
name: 'showcase_urgent_task_alert',
1550+
label: 'Alert on Urgent Task (created or escalated)',
1551+
description:
1552+
'One record-after-write flow: notifies when a task is created as Urgent or its priority is raised to Urgent.',
1553+
type: 'record_change',
1554+
status: 'active',
1555+
nodes: [
1556+
{
1557+
id: 'start',
1558+
type: 'start',
1559+
label: 'On Task Created or Updated',
1560+
config: {
1561+
objectName: 'showcase_task',
1562+
// create OR update in one flow (#3427)
1563+
triggerType: 'record-after-write',
1564+
// Fire on the transition into 'urgent': a freshly-created urgent task
1565+
// (previous == null) OR an escalation (previous.priority != 'urgent').
1566+
condition: "priority == 'urgent' && (previous == null || previous.priority != 'urgent')",
1567+
},
1568+
},
1569+
{
1570+
id: 'alert',
1571+
type: 'notify',
1572+
label: 'Notify Assignee',
1573+
config: {
1574+
topic: 'task.urgent',
1575+
// Notify the assignee; fall back to whoever raised the priority
1576+
// (`{$User.Id}` = the triggering user) so an as-yet-unassigned urgent task
1577+
// still pings someone. Empty recipients are dropped, so the fallback only
1578+
// applies when `assignee` is unset.
1579+
recipients: ['{record.assignee}', '{$User.Id}'],
1580+
channels: ['inbox'],
1581+
severity: 'warning',
1582+
title: 'Urgent task: {record.title}',
1583+
message: 'Task "{record.title}" is now Urgent — it needs attention.',
1584+
actionUrl: '/showcase_task',
1585+
},
1586+
},
1587+
{ id: 'end', type: 'end', label: 'End' },
1588+
],
1589+
edges: [
1590+
{ id: 'e1', source: 'start', target: 'alert' },
1591+
{ id: 'e2', source: 'alert', target: 'end' },
1592+
],
1593+
});
1594+
15371595
export const allFlows = [
15381596
TaskCompletedFlow,
1597+
UrgentTaskAlertFlow,
15391598
ExpenseSignoffFlow,
15401599
CommitteeQuorumFlow,
15411600
TaskDueReminderFlow,

packages/lint/src/validate-flow-trigger-readiness.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
validateFlowTriggerReadiness,
66
FLOW_TRIGGER_UNKNOWN_OBJECT,
77
FLOW_DRAFT_STATUS_AMBIGUOUS,
8+
FLOW_TRIGGER_UNKNOWN_EVENT,
89
} from './validate-flow-trigger-readiness.js';
910

1011
function recordFlow(overrides: Record<string, unknown> = {}) {
@@ -175,6 +176,70 @@ describe('validateFlowTriggerReadiness', () => {
175176
expect(findings[0].path).toBe('flows[0].nodes[0].config.timeRelative.object');
176177
});
177178

179+
it('passes the record-after-write (create-OR-update) token (#3427)', () => {
180+
const flow = recordFlow({ status: 'active' });
181+
(flow.nodes[0] as { config: Record<string, unknown> }).config.triggerType = 'record-after-write';
182+
const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] });
183+
expect(findings).toEqual([]);
184+
});
185+
186+
it('flags a record-lifecycle-shaped token with a typo op that never fires', () => {
187+
const flow = recordFlow({ status: 'active' });
188+
(flow.nodes[0] as { config: Record<string, unknown> }).config.triggerType = 'record-after-updated';
189+
const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] });
190+
expect(findings).toHaveLength(1);
191+
expect(findings[0].rule).toBe(FLOW_TRIGGER_UNKNOWN_EVENT);
192+
expect(findings[0].severity).toBe('warning');
193+
expect(findings[0].message).toContain("'updated'");
194+
expect(findings[0].message).toMatch(/never fires/i);
195+
expect(findings[0].path).toBe('flows[0].nodes[0].config.triggerType');
196+
});
197+
198+
it('flags any invalid op on either phase (before/after)', () => {
199+
const mk = (tt: string) => {
200+
const flow = recordFlow({ status: 'active' });
201+
(flow.nodes[0] as { config: Record<string, unknown> }).config.triggerType = tt;
202+
return validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] });
203+
};
204+
expect(mk('record-before-frobnicate').map((f) => f.rule)).toEqual([FLOW_TRIGGER_UNKNOWN_EVENT]);
205+
expect(mk('record-after-writes').map((f) => f.rule)).toEqual([FLOW_TRIGGER_UNKNOWN_EVENT]);
206+
});
207+
208+
it('does not flag the canonical firing tokens (incl. insert synonym)', () => {
209+
for (const tt of [
210+
'record-after-create',
211+
'record-after-insert',
212+
'record-after-update',
213+
'record-before-update',
214+
'record-after-delete',
215+
'record-after-write',
216+
'record-before-write',
217+
]) {
218+
const flow = recordFlow({ status: 'active' });
219+
(flow.nodes[0] as { config: Record<string, unknown> }).config.triggerType = tt;
220+
const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] });
221+
expect(findings, `${tt} should not be flagged`).toEqual([]);
222+
}
223+
});
224+
225+
it('does not flag bare record-<noun> shapes (e.g. record-change) with this rule', () => {
226+
// `record-change` lacks a before/after phase, so it is out of this rule's
227+
// scope (a separate concern); the UNKNOWN_EVENT rule must stay silent on it.
228+
const flow = recordFlow({ status: 'active' });
229+
(flow.nodes[0] as { config: Record<string, unknown> }).config.triggerType = 'record-change';
230+
const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] });
231+
expect(findings.some((f) => f.rule === FLOW_TRIGGER_UNKNOWN_EVENT)).toBe(false);
232+
});
233+
234+
it('does not flag non-record triggerTypes (schedule/api/manual)', () => {
235+
for (const tt of ['schedule', 'api', 'manual']) {
236+
const flow = recordFlow({ status: 'active' });
237+
(flow.nodes[0] as { config: Record<string, unknown> }).config.triggerType = tt;
238+
const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] });
239+
expect(findings.some((f) => f.rule === FLOW_TRIGGER_UNKNOWN_EVENT)).toBe(false);
240+
}
241+
});
242+
178243
it('handles map-keyed flows/objects and stacks with no flows', () => {
179244
expect(validateFlowTriggerReadiness({})).toEqual([]);
180245
const findings = validateFlowTriggerReadiness({

packages/lint/src/validate-flow-trigger-readiness.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,21 @@ export interface FlowTriggerReadinessFinding {
4040
// Rule ids (registry entries).
4141
export const FLOW_TRIGGER_UNKNOWN_OBJECT = 'flow-trigger-unknown-object';
4242
export const FLOW_DRAFT_STATUS_AMBIGUOUS = 'flow-draft-status-ambiguous';
43+
export const FLOW_TRIGGER_UNKNOWN_EVENT = 'flow-trigger-unknown-event';
4344

4445
type AnyRec = Record<string, unknown>;
4546

47+
/**
48+
* Recognized record-change lifecycle ops. A start node's `triggerType` fires only
49+
* when it matches `record-(before|after)-<op>` with `op` in this set — the exact
50+
* grammar the record-change trigger's `triggerTypeToHookEvents` maps to ObjectQL
51+
* hooks. `insert` is a synonym for `create`; `write` is the create-OR-update union
52+
* (#3427). Kept in sync with that trigger (one small, stable contract).
53+
*/
54+
const RECORD_TRIGGER_OPS = new Set(['create', 'insert', 'update', 'delete', 'write']);
55+
/** A record-lifecycle-SHAPED token: `record-before-…` / `record-after-…`. */
56+
const RECORD_EVENT_SHAPE = /^record-(?:before|after)-(.+)$/;
57+
4658
/** Coerce an array-or-name-keyed-map collection to an array (name injected). */
4759
function asArray(v: unknown): AnyRec[] {
4860
if (Array.isArray(v)) return v as AnyRec[];
@@ -130,6 +142,31 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines
130142
}
131143
}
132144

145+
// 1c. Record-lifecycle-SHAPED triggerType (`record-before|after-…`) whose op
146+
// is not one the trigger can map — a typo like `record-after-updated`. The
147+
// engine still routes any `record-` token to the record-change trigger,
148+
// which then maps it to NO hook and never fires (only a runtime warn). This
149+
// is a definite never-fire defect, so surface it at authoring time. (Bare
150+
// `record-<noun>` shapes without a before/after phase — e.g. `record-change`
151+
// — are a separate concern and not flagged here.)
152+
if (start && triggerType) {
153+
const shape = RECORD_EVENT_SHAPE.exec(triggerType);
154+
if (shape && !RECORD_TRIGGER_OPS.has(shape[1])) {
155+
findings.push({
156+
severity: 'warning',
157+
rule: FLOW_TRIGGER_UNKNOWN_EVENT,
158+
where: `flow "${flowName}" › start node`,
159+
path: `flows[${flowIndex}].nodes[${start.index}].config.triggerType`,
160+
message:
161+
`triggerType '${triggerType}' names an unrecognized lifecycle event '${shape[1]}' — the flow binds to ` +
162+
`the record-change trigger but never fires (the runtime stays silent about it).`,
163+
hint:
164+
`Use record-{before,after}-{create,update,delete,write}. 'write' fires on create OR update in one ` +
165+
`flow (#3427); create/insert are synonyms.`,
166+
});
167+
}
168+
}
169+
133170
// 2. Auto-triggered flow whose status is 'draft' — authored or defaulted
134171
// (defineFlow parses at definition time, so the two are the same here).
135172
if (isAutoTriggered && (flow.status == null || flow.status === 'draft')) {

packages/services/service-automation/src/engine.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,9 +1623,13 @@ export class AutomationEngine implements IAutomationService {
16231623
if (!variables.has(k)) variables.set(k, v);
16241624
}
16251625
}
1626-
if (context?.previous) {
1627-
variables.set('previous', context.previous);
1628-
}
1626+
// Always bind `previous` — to `null` on the create/insert leg (there is no
1627+
// prior row) — so a start condition can DISCRIMINATE create vs update on a
1628+
// `record-after-write` flow: `previous == null` is the create leg (#3427).
1629+
// Binding only-when-truthy left `previous` an unknown CEL variable on
1630+
// insert, so ANY reference to it (even `previous == null`) threw
1631+
// "Unknown variable: previous" and failed the whole condition.
1632+
variables.set('previous', context?.previous ?? null);
16291633

16301634
const runId = this.nextRunId();
16311635
// Expose the run id to executors (ADR-0019): a pausing node (e.g. Approval)

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,13 +133,50 @@ function mirrorWriteFlow(name: string, object: string) {
133133
};
134134
}
135135

136+
/**
137+
* A `record-after-write` flow whose START CONDITION uses the create/update
138+
* discrimination the write trigger enables (mirrors the showcase
139+
* `UrgentTaskAlertFlow`): fire when a record is created urgent (`previous == null`)
140+
* OR escalated to urgent (`previous.priority != 'urgent'`) — but NOT on a later
141+
* save while already urgent. Validates that `previous == null` is truthy on the
142+
* afterInsert leg (previous is absent on create) and that the engine's start-node
143+
* condition gate short-circuits before touching `previous.priority` there.
144+
*/
145+
function urgentAlertFlow(name: string, object: string) {
146+
return {
147+
name,
148+
label: name,
149+
type: 'record_change',
150+
nodes: [
151+
{
152+
id: 'start',
153+
type: 'start',
154+
label: 'Start',
155+
config: {
156+
objectName: object,
157+
triggerType: 'record-after-write',
158+
condition: "priority == 'urgent' && (previous == null || previous.priority != 'urgent')",
159+
},
160+
},
161+
{ id: 'alert', type: 'update_record', label: 'Alert', config: { objectName: object, filter: { id: '{record.id}' }, fields: { alerted: 'yes' } } },
162+
{ id: 'end', type: 'end', label: 'End' },
163+
],
164+
edges: [
165+
{ id: 'e1', source: 'start', target: 'alert' },
166+
{ id: 'e2', source: 'alert', target: 'end' },
167+
],
168+
};
169+
}
170+
136171
const objectDef = (name: string) => ({
137172
name,
138173
label: name,
139174
fields: {
140175
status: { name: 'status', label: 'S', type: 'text' },
141176
stamp: { name: 'stamp', label: 'St', type: 'text' },
142177
mirror: { name: 'mirror', label: 'M', type: 'text' },
178+
priority: { name: 'priority', label: 'P', type: 'text' },
179+
alerted: { name: 'alerted', label: 'A', type: 'text' },
143180
},
144181
});
145182

@@ -250,4 +287,40 @@ describe('record-change trigger — end-to-end (#1491)', () => {
250287
await sleep(200);
251288
expect((await data.findOne('wid3', { where: { id } }))?.mirror).toBe('b');
252289
}, 15000);
290+
291+
it('record-after-write start condition uses `previous == null` to discriminate create vs update (#3427)', async () => {
292+
const kernel = new ObjectKernel({ logLevel: 'silent' });
293+
await kernel.use(new ObjectQLPlugin());
294+
await kernel.use(new AutomationServicePlugin());
295+
await kernel.use(new RecordChangeTriggerPlugin());
296+
await kernel.bootstrap();
297+
298+
const objectql = kernel.getService('objectql') as any;
299+
const data = kernel.getService('data') as any;
300+
const automation = kernel.getService<AutomationEngine>('automation');
301+
302+
objectql.registerDriver(makeMemoryDriver(), true);
303+
objectql.registry.registerObject(objectDef('wid5'), 'test', 'test');
304+
automation.registerFlow('urgent_alert', urgentAlertFlow('urgent_alert', 'wid5') as any);
305+
306+
// Create leg — a brand-new URGENT record: `previous == null` makes the
307+
// condition true, so the flow fires on afterInsert (the create-discrimination
308+
// pattern the docs/showcase advertise).
309+
const urgent = await data.insert('wid5', { priority: 'urgent' });
310+
const urgentId = Array.isArray(urgent) ? urgent[0]?.id : urgent?.id ?? urgent;
311+
await sleep(200);
312+
expect((await data.findOne('wid5', { where: { id: urgentId } }))?.alerted).toBe('yes');
313+
314+
// Create leg — a NON-urgent record: the condition is false, no fire.
315+
const low = await data.insert('wid5', { priority: 'low' });
316+
const lowId = Array.isArray(low) ? low[0]?.id : low?.id ?? low;
317+
await sleep(200);
318+
expect((await data.findOne('wid5', { where: { id: lowId } }))?.alerted).toBeFalsy();
319+
320+
// Update leg — escalate that low record to urgent: `previous.priority` was
321+
// 'low', so the transition guard fires the flow on afterUpdate.
322+
await data.update('wid5', { id: lowId, priority: 'urgent' });
323+
await sleep(200);
324+
expect((await data.findOne('wid5', { where: { id: lowId } }))?.alerted).toBe('yes');
325+
}, 15000);
253326
});

0 commit comments

Comments
 (0)