From be7e2423e33e26a67c9036245c02c8f835c90c0f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Jul 2026 15:38:08 +0000 Subject: [PATCH 1/3] feat(lint,showcase): flag never-firing record trigger tokens; add record-after-write showcase flow (#3427) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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-`) 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-` 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 Claude-Session: https://claude.ai/code/session_018939yJ413zG3irLzcTtqaa --- .../src/automation/flows/index.ts | 54 ++++++++++++++ .../validate-flow-trigger-readiness.test.ts | 65 +++++++++++++++++ .../src/validate-flow-trigger-readiness.ts | 37 ++++++++++ .../src/record-change-integration.test.ts | 73 +++++++++++++++++++ 4 files changed, 229 insertions(+) diff --git a/examples/app-showcase/src/automation/flows/index.ts b/examples/app-showcase/src/automation/flows/index.ts index 3da954c266..d5a6dfafc0 100644 --- a/examples/app-showcase/src/automation/flows/index.ts +++ b/examples/app-showcase/src/automation/flows/index.ts @@ -1534,8 +1534,62 @@ export const TaskDueReminderFlow = defineFlow({ ], }); +/** + * Urgent Task Alert — a single `record-after-write` flow that fires on BOTH + * create and update (#3427), so "a task was created urgent OR just escalated to + * urgent" is one flow, not two near-identical copies. + * + * `record-after-write` binds afterInsert + afterUpdate; exactly one fires per + * mutation. The start condition uses the create/update discrimination the write + * trigger enables: `previous == null` is the create leg (no prior row), so the + * `||` also matches a brand-new urgent task; on the update leg it fires only when + * priority actually crosses INTO 'urgent' (not on every later save while urgent). + */ +export const UrgentTaskAlertFlow = defineFlow({ + name: 'showcase_urgent_task_alert', + label: 'Alert on Urgent Task (created or escalated)', + description: + 'One record-after-write flow: notifies when a task is created as Urgent or its priority is raised to Urgent.', + type: 'record_change', + status: 'active', + nodes: [ + { + id: 'start', + type: 'start', + label: 'On Task Created or Updated', + config: { + objectName: 'showcase_task', + // create OR update in one flow (#3427) + triggerType: 'record-after-write', + // Fire on the transition into 'urgent': a freshly-created urgent task + // (previous == null) OR an escalation (previous.priority != 'urgent'). + condition: "priority == 'urgent' && (previous == null || previous.priority != 'urgent')", + }, + }, + { + id: 'alert', + type: 'notify', + label: 'Notify Assignee', + config: { + topic: 'task.urgent', + channels: ['inbox'], + severity: 'warning', + title: 'Urgent task: {record.title}', + message: 'Task "{record.title}" is now Urgent — it needs attention.', + actionUrl: '/showcase_task', + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'alert' }, + { id: 'e2', source: 'alert', target: 'end' }, + ], +}); + export const allFlows = [ TaskCompletedFlow, + UrgentTaskAlertFlow, ExpenseSignoffFlow, CommitteeQuorumFlow, TaskDueReminderFlow, diff --git a/packages/lint/src/validate-flow-trigger-readiness.test.ts b/packages/lint/src/validate-flow-trigger-readiness.test.ts index 84c1e9267f..802aa89017 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.test.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.test.ts @@ -5,6 +5,7 @@ import { validateFlowTriggerReadiness, FLOW_TRIGGER_UNKNOWN_OBJECT, FLOW_DRAFT_STATUS_AMBIGUOUS, + FLOW_TRIGGER_UNKNOWN_EVENT, } from './validate-flow-trigger-readiness.js'; function recordFlow(overrides: Record = {}) { @@ -175,6 +176,70 @@ describe('validateFlowTriggerReadiness', () => { expect(findings[0].path).toBe('flows[0].nodes[0].config.timeRelative.object'); }); + it('passes the record-after-write (create-OR-update) token (#3427)', () => { + const flow = recordFlow({ status: 'active' }); + (flow.nodes[0] as { config: Record }).config.triggerType = 'record-after-write'; + const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] }); + expect(findings).toEqual([]); + }); + + it('flags a record-lifecycle-shaped token with a typo op that never fires', () => { + const flow = recordFlow({ status: 'active' }); + (flow.nodes[0] as { config: Record }).config.triggerType = 'record-after-updated'; + const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] }); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(FLOW_TRIGGER_UNKNOWN_EVENT); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].message).toContain("'updated'"); + expect(findings[0].message).toMatch(/never fires/i); + expect(findings[0].path).toBe('flows[0].nodes[0].config.triggerType'); + }); + + it('flags any invalid op on either phase (before/after)', () => { + const mk = (tt: string) => { + const flow = recordFlow({ status: 'active' }); + (flow.nodes[0] as { config: Record }).config.triggerType = tt; + return validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] }); + }; + expect(mk('record-before-frobnicate').map((f) => f.rule)).toEqual([FLOW_TRIGGER_UNKNOWN_EVENT]); + expect(mk('record-after-writes').map((f) => f.rule)).toEqual([FLOW_TRIGGER_UNKNOWN_EVENT]); + }); + + it('does not flag the canonical firing tokens (incl. insert synonym)', () => { + for (const tt of [ + 'record-after-create', + 'record-after-insert', + 'record-after-update', + 'record-before-update', + 'record-after-delete', + 'record-after-write', + 'record-before-write', + ]) { + const flow = recordFlow({ status: 'active' }); + (flow.nodes[0] as { config: Record }).config.triggerType = tt; + const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] }); + expect(findings, `${tt} should not be flagged`).toEqual([]); + } + }); + + it('does not flag bare record- shapes (e.g. record-change) with this rule', () => { + // `record-change` lacks a before/after phase, so it is out of this rule's + // scope (a separate concern); the UNKNOWN_EVENT rule must stay silent on it. + const flow = recordFlow({ status: 'active' }); + (flow.nodes[0] as { config: Record }).config.triggerType = 'record-change'; + const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] }); + expect(findings.some((f) => f.rule === FLOW_TRIGGER_UNKNOWN_EVENT)).toBe(false); + }); + + it('does not flag non-record triggerTypes (schedule/api/manual)', () => { + for (const tt of ['schedule', 'api', 'manual']) { + const flow = recordFlow({ status: 'active' }); + (flow.nodes[0] as { config: Record }).config.triggerType = tt; + const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] }); + expect(findings.some((f) => f.rule === FLOW_TRIGGER_UNKNOWN_EVENT)).toBe(false); + } + }); + it('handles map-keyed flows/objects and stacks with no flows', () => { expect(validateFlowTriggerReadiness({})).toEqual([]); const findings = validateFlowTriggerReadiness({ diff --git a/packages/lint/src/validate-flow-trigger-readiness.ts b/packages/lint/src/validate-flow-trigger-readiness.ts index c6482bc5f0..eadecf07c4 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.ts @@ -40,9 +40,21 @@ export interface FlowTriggerReadinessFinding { // Rule ids (registry entries). export const FLOW_TRIGGER_UNKNOWN_OBJECT = 'flow-trigger-unknown-object'; export const FLOW_DRAFT_STATUS_AMBIGUOUS = 'flow-draft-status-ambiguous'; +export const FLOW_TRIGGER_UNKNOWN_EVENT = 'flow-trigger-unknown-event'; type AnyRec = Record; +/** + * Recognized record-change lifecycle ops. A start node's `triggerType` fires only + * when it matches `record-(before|after)-` with `op` in this set — the exact + * grammar the record-change trigger's `triggerTypeToHookEvents` maps to ObjectQL + * hooks. `insert` is a synonym for `create`; `write` is the create-OR-update union + * (#3427). Kept in sync with that trigger (one small, stable contract). + */ +const RECORD_TRIGGER_OPS = new Set(['create', 'insert', 'update', 'delete', 'write']); +/** A record-lifecycle-SHAPED token: `record-before-…` / `record-after-…`. */ +const RECORD_EVENT_SHAPE = /^record-(?:before|after)-(.+)$/; + /** Coerce an array-or-name-keyed-map collection to an array (name injected). */ function asArray(v: unknown): AnyRec[] { if (Array.isArray(v)) return v as AnyRec[]; @@ -130,6 +142,31 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines } } + // 1c. Record-lifecycle-SHAPED triggerType (`record-before|after-…`) whose op + // is not one the trigger can map — a typo like `record-after-updated`. The + // engine still routes any `record-` token to the record-change trigger, + // which then maps it to NO hook and never fires (only a runtime warn). This + // is a definite never-fire defect, so surface it at authoring time. (Bare + // `record-` shapes without a before/after phase — e.g. `record-change` + // — are a separate concern and not flagged here.) + if (start && triggerType) { + const shape = RECORD_EVENT_SHAPE.exec(triggerType); + if (shape && !RECORD_TRIGGER_OPS.has(shape[1])) { + findings.push({ + severity: 'warning', + rule: FLOW_TRIGGER_UNKNOWN_EVENT, + where: `flow "${flowName}" › start node`, + path: `flows[${flowIndex}].nodes[${start.index}].config.triggerType`, + message: + `triggerType '${triggerType}' names an unrecognized lifecycle event '${shape[1]}' — the flow binds to ` + + `the record-change trigger but never fires (the runtime stays silent about it).`, + hint: + `Use record-{before,after}-{create,update,delete,write}. 'write' fires on create OR update in one ` + + `flow (#3427); create/insert are synonyms.`, + }); + } + } + // 2. Auto-triggered flow whose status is 'draft' — authored or defaulted // (defineFlow parses at definition time, so the two are the same here). if (isAutoTriggered && (flow.status == null || flow.status === 'draft')) { diff --git a/packages/triggers/trigger-record-change/src/record-change-integration.test.ts b/packages/triggers/trigger-record-change/src/record-change-integration.test.ts index 2f23a26688..1703b187ad 100644 --- a/packages/triggers/trigger-record-change/src/record-change-integration.test.ts +++ b/packages/triggers/trigger-record-change/src/record-change-integration.test.ts @@ -133,6 +133,41 @@ function mirrorWriteFlow(name: string, object: string) { }; } +/** + * A `record-after-write` flow whose START CONDITION uses the create/update + * discrimination the write trigger enables (mirrors the showcase + * `UrgentTaskAlertFlow`): fire when a record is created urgent (`previous == null`) + * OR escalated to urgent (`previous.priority != 'urgent'`) — but NOT on a later + * save while already urgent. Validates that `previous == null` is truthy on the + * afterInsert leg (previous is absent on create) and that the engine's start-node + * condition gate short-circuits before touching `previous.priority` there. + */ +function urgentAlertFlow(name: string, object: string) { + return { + name, + label: name, + type: 'record_change', + nodes: [ + { + id: 'start', + type: 'start', + label: 'Start', + config: { + objectName: object, + triggerType: 'record-after-write', + condition: "priority == 'urgent' && (previous == null || previous.priority != 'urgent')", + }, + }, + { id: 'alert', type: 'update_record', label: 'Alert', config: { objectName: object, filter: { id: '{record.id}' }, fields: { alerted: 'yes' } } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'alert' }, + { id: 'e2', source: 'alert', target: 'end' }, + ], + }; +} + const objectDef = (name: string) => ({ name, label: name, @@ -140,6 +175,8 @@ const objectDef = (name: string) => ({ status: { name: 'status', label: 'S', type: 'text' }, stamp: { name: 'stamp', label: 'St', type: 'text' }, mirror: { name: 'mirror', label: 'M', type: 'text' }, + priority: { name: 'priority', label: 'P', type: 'text' }, + alerted: { name: 'alerted', label: 'A', type: 'text' }, }, }); @@ -250,4 +287,40 @@ describe('record-change trigger — end-to-end (#1491)', () => { await sleep(200); expect((await data.findOne('wid3', { where: { id } }))?.mirror).toBe('b'); }, 15000); + + it('record-after-write start condition uses `previous == null` to discriminate create vs update (#3427)', async () => { + const kernel = new ObjectKernel({ logLevel: 'silent' }); + await kernel.use(new ObjectQLPlugin()); + await kernel.use(new AutomationServicePlugin()); + await kernel.use(new RecordChangeTriggerPlugin()); + await kernel.bootstrap(); + + const objectql = kernel.getService('objectql') as any; + const data = kernel.getService('data') as any; + const automation = kernel.getService('automation'); + + objectql.registerDriver(makeMemoryDriver(), true); + objectql.registry.registerObject(objectDef('wid5'), 'test', 'test'); + automation.registerFlow('urgent_alert', urgentAlertFlow('urgent_alert', 'wid5') as any); + + // Create leg — a brand-new URGENT record: `previous == null` makes the + // condition true, so the flow fires on afterInsert (the create-discrimination + // pattern the docs/showcase advertise). + const urgent = await data.insert('wid5', { priority: 'urgent' }); + const urgentId = Array.isArray(urgent) ? urgent[0]?.id : urgent?.id ?? urgent; + await sleep(200); + expect((await data.findOne('wid5', { where: { id: urgentId } }))?.alerted).toBe('yes'); + + // Create leg — a NON-urgent record: the condition is false, no fire. + const low = await data.insert('wid5', { priority: 'low' }); + const lowId = Array.isArray(low) ? low[0]?.id : low?.id ?? low; + await sleep(200); + expect((await data.findOne('wid5', { where: { id: lowId } }))?.alerted).toBeFalsy(); + + // Update leg — escalate that low record to urgent: `previous.priority` was + // 'low', so the transition guard fires the flow on afterUpdate. + await data.update('wid5', { id: lowId, priority: 'urgent' }); + await sleep(200); + expect((await data.findOne('wid5', { where: { id: lowId } }))?.alerted).toBe('yes'); + }, 15000); }); From 6dcf2bec5681f1fa7052299ccc85f50f44a7333c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Jul 2026 15:53:55 +0000 Subject: [PATCH 2/3] fix(service-automation): bind `previous` as null on the create leg; fix showcase notify recipient (#3427) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_018939yJ413zG3irLzcTtqaa --- .changeset/previous-null-on-create-leg.md | 18 ++++++++++++++++++ .../app-showcase/src/automation/flows/index.ts | 5 +++++ .../services/service-automation/src/engine.ts | 10 +++++++--- 3 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 .changeset/previous-null-on-create-leg.md diff --git a/.changeset/previous-null-on-create-leg.md b/.changeset/previous-null-on-create-leg.md new file mode 100644 index 0000000000..0ed50415a5 --- /dev/null +++ b/.changeset/previous-null-on-create-leg.md @@ -0,0 +1,18 @@ +--- +"@objectstack/service-automation": patch +--- + +fix(service-automation): bind `previous` (as null) on the create leg so start conditions can discriminate create vs update (#3427) + +The engine bound `previous` into the flow condition scope only when it was +truthy, so on a record insert (`record-after-create`, and the create leg of +`record-after-write`) `previous` was an **unknown** CEL variable. Any reference to +it — including the documented `previous == null` create-discrimination — threw +`condition failed to evaluate as CEL: Unknown variable: previous`, failing the +whole start condition and dropping the run. + +`previous` is now always bound, to `null` when there is no prior row. So +`previous == null` is the create leg and `previous != null` / `previous.` +the update leg — the pattern the `record-after-write` docs and the Studio flow +designer advertise. Update-triggered flows are unaffected (`previous` was, and +stays, the prior row there). diff --git a/examples/app-showcase/src/automation/flows/index.ts b/examples/app-showcase/src/automation/flows/index.ts index d5a6dfafc0..6888f90faf 100644 --- a/examples/app-showcase/src/automation/flows/index.ts +++ b/examples/app-showcase/src/automation/flows/index.ts @@ -1572,6 +1572,11 @@ export const UrgentTaskAlertFlow = defineFlow({ label: 'Notify Assignee', config: { topic: 'task.urgent', + // Notify the assignee; fall back to whoever raised the priority + // (`{$User.Id}` = the triggering user) so an as-yet-unassigned urgent task + // still pings someone. Empty recipients are dropped, so the fallback only + // applies when `assignee` is unset. + recipients: ['{record.assignee}', '{$User.Id}'], channels: ['inbox'], severity: 'warning', title: 'Urgent task: {record.title}', diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 93aa989faa..00b50c8eba 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -1623,9 +1623,13 @@ export class AutomationEngine implements IAutomationService { if (!variables.has(k)) variables.set(k, v); } } - if (context?.previous) { - variables.set('previous', context.previous); - } + // Always bind `previous` — to `null` on the create/insert leg (there is no + // prior row) — so a start condition can DISCRIMINATE create vs update on a + // `record-after-write` flow: `previous == null` is the create leg (#3427). + // Binding only-when-truthy left `previous` an unknown CEL variable on + // insert, so ANY reference to it (even `previous == null`) threw + // "Unknown variable: previous" and failed the whole condition. + variables.set('previous', context?.previous ?? null); const runId = this.nextRunId(); // Expose the run id to executors (ADR-0019): a pausing node (e.g. Approval) From b575f6992c4e106638c7d949a4b7f7154d41b78c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Jul 2026 15:56:31 +0000 Subject: [PATCH 3/3] chore: add changeset for the flow-trigger-unknown-event lint rule Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_018939yJ413zG3irLzcTtqaa --- .changeset/flow-trigger-unknown-event-lint.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .changeset/flow-trigger-unknown-event-lint.md diff --git a/.changeset/flow-trigger-unknown-event-lint.md b/.changeset/flow-trigger-unknown-event-lint.md new file mode 100644 index 0000000000..bd0a6e6092 --- /dev/null +++ b/.changeset/flow-trigger-unknown-event-lint.md @@ -0,0 +1,13 @@ +--- +"@objectstack/lint": minor +--- + +feat(lint): flag never-firing record trigger tokens at authoring time (#3427) + +New `flow-trigger-unknown-event` rule in `validateFlowTriggerReadiness`: a flow +start node whose `triggerType` is record-lifecycle-shaped +(`record-before|after-`) but names an op the record-change trigger cannot map +— e.g. a typo like `record-after-updated` — binds to the record-change trigger +yet maps to no ObjectQL hook and never fires, with only a runtime warning. The +rule surfaces that never-fire defect at `os validate` time. Warning severity; +bare `record-` shapes (e.g. `record-change`) are out of scope.