Skip to content

Commit be7e242

Browse files
committed
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
1 parent 5ac93d4 commit be7e242

4 files changed

Lines changed: 229 additions & 0 deletions

File tree

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,8 +1534,62 @@ 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+
channels: ['inbox'],
1576+
severity: 'warning',
1577+
title: 'Urgent task: {record.title}',
1578+
message: 'Task "{record.title}" is now Urgent — it needs attention.',
1579+
actionUrl: '/showcase_task',
1580+
},
1581+
},
1582+
{ id: 'end', type: 'end', label: 'End' },
1583+
],
1584+
edges: [
1585+
{ id: 'e1', source: 'start', target: 'alert' },
1586+
{ id: 'e2', source: 'alert', target: 'end' },
1587+
],
1588+
});
1589+
15371590
export const allFlows = [
15381591
TaskCompletedFlow,
1592+
UrgentTaskAlertFlow,
15391593
ExpenseSignoffFlow,
15401594
CommitteeQuorumFlow,
15411595
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/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)