feat(lint,showcase,automation): record-after-write follow-ups — validation lint, showcase flow, create-leg previous fix (#3427)

.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-<op>`) 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-<noun>` shapes (e.g. `record-change`) are out of scope.

.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.<field>`
+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).

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

@@ -1534,8 +1534,67 @@ 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',
+        // 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}',
+        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,

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<string, unknown> = {}) {
@@ -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<string, unknown> }).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<string, unknown> }).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<string, unknown> }).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<string, unknown> }).config.triggerType = tt;
+      const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] });
+      expect(findings, `${tt} should not be flagged`).toEqual([]);
+    }
+  });
+
+  it('does not flag bare record-<noun> 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<string, unknown> }).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<string, unknown> }).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({

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<string, unknown>;
 
+/**
+ * Recognized record-change lifecycle ops. A start node's `triggerType` fires only
+ * when it matches `record-(before|after)-<op>` 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-<noun>` 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')) {

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)

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

@@ -133,13 +133,50 @@ 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,
   fields: {
     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<AutomationEngine>('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);
 });