From 381ba9bca771c53393c54e00f0aba47a64d9dc41 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sat, 25 Jul 2026 10:43:00 +0800 Subject: [PATCH] fix(automation): array-form flow triggerType fails loudly, not silently never firing (#3481) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An array `triggerType` on a flow start node (e.g. ['record-after-create', 'record-after-delete']) — the shape an author or AI authoring pass naturally reaches for to fire on multiple events — was accepted everywhere and armed nowhere. Multi-event unions are deliberately unsupported (#3457), but nothing said so: defineFlow passed the array, the engine's `typeof === 'string'` check folded it to no trigger and misclassified the flow as manual (so it never entered the binding audit), and the flow-trigger-readiness lint used the same narrowing and produced no finding. The flow bound to nothing and never fired, with zero output at any layer — the last authoring shape still slipping past the #3427/#3472 silent-never-fire guards. Defensive fix (arrays stay unsupported; they now fail loudly): - lint (validate-flow-trigger-readiness): an array triggerType with any record-* element yields a flow-trigger-unknown-event warning at os validate time, steering to record-after-write or one flow per event. - engine (resolveTriggerBinding): route such an array to the record_change trigger — as an unmappable single token is — instead of folding to a manual flow, so it reaches the trigger's bind-time rejection and the binding audit. - trigger (record-change): the bind-time rejection detects the array shape and emits a targeted warning (names the flow, points at record-after-write and #3457) rather than the generic unknown-token line. Each new test proves red against the pre-fix code first. Co-Authored-By: Claude Fable 5 --- .../array-form-triggertype-not-silent.md | 37 +++++++++++ .../validate-flow-trigger-readiness.test.ts | 39 +++++++++++ .../src/validate-flow-trigger-readiness.ts | 28 ++++++++ .../services/service-automation/src/engine.ts | 28 ++++++++ .../trigger-dispatch-observability.test.ts | 65 +++++++++++++++++++ .../src/record-change-trigger.test.ts | 40 ++++++++++++ .../src/record-change-trigger.ts | 20 +++++- 7 files changed, 254 insertions(+), 3 deletions(-) create mode 100644 .changeset/array-form-triggertype-not-silent.md diff --git a/.changeset/array-form-triggertype-not-silent.md b/.changeset/array-form-triggertype-not-silent.md new file mode 100644 index 0000000000..16b19741e5 --- /dev/null +++ b/.changeset/array-form-triggertype-not-silent.md @@ -0,0 +1,37 @@ +--- +"@objectstack/service-automation": patch +"@objectstack/trigger-record-change": patch +"@objectstack/lint": patch +--- + +fix(automation): array-form flow `triggerType` fails loudly instead of silently never firing (#3481) + +An array `triggerType` on a flow start node — the shape an author (or an AI +authoring pass) naturally reaches for to fire on more than one event, e.g. + +```ts +config: { objectName: 'app_task', triggerType: ['record-after-create', 'record-after-delete'] } +``` + +was accepted everywhere and armed nowhere. Multi-event unions are deliberately +unsupported (only the single tokens plus the `record-after-write` create-OR-update +union exist — see #3457), but nothing said so: `defineFlow` passed the array +(start-node `config` is an open record), the engine's `typeof === 'string'` check +folded it to no trigger and misclassified the flow as **manual**, so it never +entered the trigger-binding audit, and the flow-trigger-readiness lint used the +same `typeof` narrowing and produced no finding. The flow bound to nothing and +never fired, with zero output at any layer — the same silent-never-fire class as +#3427 / #3472, and the last authoring shape still slipping past every guard. + +This is a **defensive** fix — arrays remain unsupported; they now fail loudly: + +- **lint** (`validate-flow-trigger-readiness`): an array `triggerType` containing + any `record-*` element now yields a `flow-trigger-unknown-event` warning at + `os validate` time, steering to `record-after-write` (for created-or-updated) or + one flow per event. +- **engine** (`resolveTriggerBinding`): such an array is routed to the + `record_change` trigger — exactly as an unmappable single token is — instead of + being folded to a manual flow, so it reaches the trigger's bind-time rejection. +- **trigger** (`record-change`): the bind-time rejection detects the array shape + and emits a targeted warning (naming the flow, pointing at `record-after-write` + and #3457) rather than the generic unknown-token line. diff --git a/packages/lint/src/validate-flow-trigger-readiness.test.ts b/packages/lint/src/validate-flow-trigger-readiness.test.ts index d2c1dffb1a..a9e5a9e829 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.test.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.test.ts @@ -248,6 +248,45 @@ describe('validateFlowTriggerReadiness', () => { } }); + it('flags an array-form triggerType — an unsupported multi-event shape that never fires (#3481)', () => { + // A non-string triggerType folds to "no trigger" at the runtime, so the flow + // is misclassified as manual and passes every gate silently. Catch it here. + const flow = recordFlow({ status: 'active' }); + (flow.nodes[0] as { config: Record }).config.triggerType = [ + 'record-after-create', + 'record-after-delete', + ]; + const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] }); + expect(findings.map((f) => f.rule)).toEqual([FLOW_TRIGGER_UNKNOWN_EVENT]); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].message).toMatch(/array/i); + expect(findings[0].message).toMatch(/never fires/i); + expect(findings[0].path).toBe('flows[0].nodes[0].config.triggerType'); + // The hint steers to the supported alternatives. + expect(findings[0].hint).toMatch(/record-after-write/); + expect(findings[0].hint).toMatch(/#3457/); + }); + + it('flags an array even when its elements are individually valid tokens', () => { + // ['record-after-create','record-after-update'] is exactly what record-after-write + // exists for — the array form is still unsupported, so still flagged. + const flow = recordFlow({ status: 'active' }); + (flow.nodes[0] as { config: Record }).config.triggerType = [ + 'record-after-create', + 'record-after-update', + ]; + const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] }); + expect(findings.map((f) => f.rule)).toEqual([FLOW_TRIGGER_UNKNOWN_EVENT]); + }); + + it('does not flag a non-record array (not the record-trigger silent-miss)', () => { + // An array with no record-* element is not the #3481 defect — leave it alone. + const flow = recordFlow({ status: 'active' }); + (flow.nodes[0] as { config: Record }).config.triggerType = ['schedule', 'manual']; + 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 2fcc5ad3f0..ea68340419 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.ts @@ -95,6 +95,14 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines const config = (start?.node.config ?? {}) as AnyRec; const triggerType = typeof config.triggerType === 'string' ? config.triggerType : undefined; const isRecordTriggered = !!triggerType && triggerType.startsWith('record-'); + // Array-form triggerType (e.g. ['record-after-create', 'record-after-delete']) + // is NOT supported — multi-event unions are deferred (#3457). It needs its own + // detection because a non-string triggerType folds to `undefined` above, so the + // runtime misclassifies the flow as manual and it never fires with zero output + // (#3481). Any record-* element is enough to recognize the (unsupported) intent. + const isArrayRecordTriggered = + Array.isArray(config.triggerType) && + (config.triggerType as unknown[]).some((t) => typeof t === 'string' && t.startsWith('record-')); const isTimeRelative = config.timeRelative != null && typeof config.timeRelative === 'object'; const isAutoTriggered = isRecordTriggered || triggerType === 'api' || config.schedule != null || @@ -163,6 +171,26 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines }); } + // 1d. Array-form triggerType — an unsupported multi-event shape (#3457). The + // runtime folds a non-string triggerType to "no trigger" and treats the + // flow as manual, so it binds to nothing and never fires, with zero output + // at any layer (#3481). Surface it at authoring time like the unmappable + // single tokens above (same rule id — both are "this token never fires"). + if (start && isArrayRecordTriggered) { + findings.push({ + severity: 'warning', + rule: FLOW_TRIGGER_UNKNOWN_EVENT, + where: `flow "${flowName}" › start node`, + path: `flows[${flowIndex}].nodes[${start.index}].config.triggerType`, + message: + `triggerType is an array (${JSON.stringify(config.triggerType)}), which is not supported — a start ` + + `node takes a single trigger event, so the flow binds to nothing and never fires (the runtime stays silent about it).`, + hint: + `Use one triggerType string. For "created or updated" use record-after-write (one flow, both events, #3427). ` + + `For any other combination, author one flow per event — multi-event arrays are deferred (#3457).`, + }); + } + // 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/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 00b50c8eba..6619e7f46f 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -913,6 +913,34 @@ export class AutomationEngine implements IAutomationService { }; } + // Array-form triggerType (e.g. ['record-after-create', 'record-after-delete']). + // Multi-event unions are deliberately unsupported (#3457). But a non-string + // triggerType would otherwise fall through every branch below and resolve to + // `undefined` — the flow silently becomes a manual/screen flow that never + // fires, invisible to the binding audit (#3481). Route it to the record-change + // trigger, exactly as an unmappable single token does, so the trigger rejects + // it LOUDLY at bind time (a warn naming the flow) instead of vanishing. The + // authoring-time lint gate (validate-flow-trigger-readiness) is the primary + // catch; this keeps runtime behavior consistent with the typo-token path. The + // raw array is preserved in `config` so the trigger can tailor its message; + // `event` is a joined string so the trigger's single-token mapper reports it + // verbatim and maps it to no hook. + if ( + Array.isArray(config.triggerType) && + config.triggerType.some((t) => typeof t === 'string' && (t as string).startsWith('record-')) + ) { + return { + triggerType: 'record_change', + binding: { + flowName, + object: typeof config.objectName === 'string' ? config.objectName : undefined, + event: config.triggerType.filter((t) => typeof t === 'string').join(','), + condition: (config.condition as FlowTriggerBinding['condition']) ?? undefined, + config, + }, + }; + } + // Declarative time-relative sweep (#1874): a start node carrying a // `timeRelative` descriptor is swept on a schedule and launched once per // record whose date field falls in the window. Checked BEFORE `schedule` diff --git a/packages/services/service-automation/src/trigger-dispatch-observability.test.ts b/packages/services/service-automation/src/trigger-dispatch-observability.test.ts index db357ff0b3..60a80e1b2a 100644 --- a/packages/services/service-automation/src/trigger-dispatch-observability.test.ts +++ b/packages/services/service-automation/src/trigger-dispatch-observability.test.ts @@ -127,6 +127,71 @@ describe('trigger-fired execution failures are loud (layer 1)', () => { }); }); +describe('array-form triggerType is routed, not silently folded to manual (#3481)', () => { + function arrayTriggeredFlow(name: string, object: string) { + return { + name, + label: name, + type: 'autolaunched', + status: 'active', + nodes: [ + { + id: 'start', + type: 'start', + label: 'Start', + config: { + objectName: object, + triggerType: ['record-after-create', 'record-after-delete'], + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end' }], + }; + } + + it('hands an array triggerType to the record_change trigger (so it can reject loudly), not folding it to a manual flow', () => { + const { logger } = spyLogger(); + const engine = new AutomationEngine(logger); + + let started: { event?: string; config?: Record } | undefined; + engine.registerTrigger({ + type: 'record_change', + start: (binding) => { started = binding as typeof started; }, + stop: () => {}, + }); + engine.registerFlow('array_flow', arrayTriggeredFlow('array_flow', 'wid') as never); + + // Without the fix, a non-string triggerType folds to "no trigger": the + // flow is treated as manual and the trigger is never handed the binding. + expect(started, 'array-form flow routed to the record_change trigger').toBeDefined(); + expect(engine.getActiveTriggerBindings().map((b) => b.triggerType)).toContain('record_change'); + // The raw array is forwarded (via config) so the trigger can produce a + // targeted rejection message rather than the generic one. + expect((started!.config as { triggerType?: unknown }).triggerType).toEqual([ + 'record-after-create', + 'record-after-delete', + ]); + }); + + it('ignores an array with no record-* element (not the record-trigger case)', () => { + const { logger } = spyLogger(); + const engine = new AutomationEngine(logger); + let started = false; + engine.registerTrigger({ + type: 'record_change', + start: () => { started = true; }, + stop: () => {}, + }); + const flow = arrayTriggeredFlow('sched_array', 'wid'); + (flow.nodes[0].config as Record).triggerType = ['schedule', 'manual']; + engine.registerFlow('sched_array', flow as never); + + // No record-* element ⇒ not routed to record_change (correctly manual). + expect(started).toBe(false); + }); +}); + describe('unbound triggered flows are audited at kernel:bootstrapped (layer 2)', () => { it('warns per enabled flow whose trigger type has no registered trigger', async () => { const warn = vi.fn(); diff --git a/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts b/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts index 74a036ff11..421ff4faa0 100644 --- a/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts +++ b/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts @@ -146,6 +146,46 @@ describe('RecordChangeTrigger', () => { expect(hooks).toHaveLength(0); }); + it('rejects an array-form trigger event with a targeted message, not bound (#3481)', () => { + // Multi-event arrays are unsupported (#3457). The engine forwards the raw + // array via config (with a joined `event` string that maps to no hook), so + // the trigger can steer the author to the supported alternatives instead of + // the generic "unsupported trigger event" line. + const { engine, hooks } = fakeEngine(); + const warn = vi.fn(); + const trigger = new RecordChangeTrigger(engine, { info: () => {}, warn, debug: () => {} }); + + trigger.start( + binding({ + event: 'record-after-create,record-after-delete', + config: { triggerType: ['record-after-create', 'record-after-delete'] }, + }), + async () => {}, + ); + + expect(hooks).toHaveLength(0); + expect(warn).toHaveBeenCalledTimes(1); + const msg = String(warn.mock.calls[0][0]); + expect(msg).toMatch(/task_assigned_notify/); + expect(msg).toMatch(/array/i); + expect(msg).toMatch(/record-after-write/); + expect(msg).toMatch(/#3457/); + }); + + it('keeps the generic unsupported-event warning for a non-array bad token', () => { + const { engine, hooks } = fakeEngine(); + const warn = vi.fn(); + const trigger = new RecordChangeTrigger(engine, { info: () => {}, warn, debug: () => {} }); + + trigger.start(binding({ event: 'record-after-updated' }), async () => {}); + + expect(hooks).toHaveLength(0); + expect(warn).toHaveBeenCalledTimes(1); + const msg = String(warn.mock.calls[0][0]); + expect(msg).toMatch(/unsupported trigger event/i); + expect(msg).toMatch(/record-after-updated/); + }); + it('binds BOTH afterInsert and afterUpdate for record-after-write (create OR update, #3427)', () => { const { engine, hooks } = fakeEngine(); const trigger = new RecordChangeTrigger(engine, silentLogger()); diff --git a/packages/triggers/trigger-record-change/src/record-change-trigger.ts b/packages/triggers/trigger-record-change/src/record-change-trigger.ts index 9e0d7ba229..581bf57dcc 100644 --- a/packages/triggers/trigger-record-change/src/record-change-trigger.ts +++ b/packages/triggers/trigger-record-change/src/record-change-trigger.ts @@ -175,9 +175,23 @@ export class RecordChangeTrigger implements FlowTrigger { // a double-dispatch. const hookEvents = triggerTypeToHookEvents(binding.event); if (hookEvents.length === 0) { - this.logger.warn( - `[record-change] flow '${binding.flowName}' has unsupported trigger event '${binding.event ?? '(none)'}' — not bound`, - ); + // Array-form triggerType (`['record-after-create', 'record-after-delete']`) + // reaches here via the engine, which forwards the raw array in `config` + // and a joined `event` string that maps to no hook. Multi-event arrays are + // unsupported (#3457); give a targeted message steering to the supported + // shapes rather than the generic unknown-token line (#3481). + const rawTriggerType = (binding.config as { triggerType?: unknown } | undefined)?.triggerType; + if (Array.isArray(rawTriggerType)) { + this.logger.warn( + `[record-change] flow '${binding.flowName}' has an ARRAY trigger event ${JSON.stringify(rawTriggerType)} — ` + + `multi-event arrays are not supported, so the flow is NOT bound and will never fire. ` + + `For "created or updated" use a single 'record-after-write'; for any other combination author one flow per event (#3457).`, + ); + } else { + this.logger.warn( + `[record-change] flow '${binding.flowName}' has unsupported trigger event '${binding.event ?? '(none)'}' — not bound`, + ); + } return; }