Skip to content

Commit 9dcc0ae

Browse files
os-zhuangclaude
andauthored
fix(automation): array-form flow triggerType fails loudly, not silently never firing (#3481) (#3484)
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: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 85e1e4e commit 9dcc0ae

7 files changed

Lines changed: 254 additions & 3 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@objectstack/service-automation": patch
3+
"@objectstack/trigger-record-change": patch
4+
"@objectstack/lint": patch
5+
---
6+
7+
fix(automation): array-form flow `triggerType` fails loudly instead of silently never firing (#3481)
8+
9+
An array `triggerType` on a flow start node — the shape an author (or an AI
10+
authoring pass) naturally reaches for to fire on more than one event, e.g.
11+
12+
```ts
13+
config: { objectName: 'app_task', triggerType: ['record-after-create', 'record-after-delete'] }
14+
```
15+
16+
was accepted everywhere and armed nowhere. Multi-event unions are deliberately
17+
unsupported (only the single tokens plus the `record-after-write` create-OR-update
18+
union exist — see #3457), but nothing said so: `defineFlow` passed the array
19+
(start-node `config` is an open record), the engine's `typeof === 'string'` check
20+
folded it to no trigger and misclassified the flow as **manual**, so it never
21+
entered the trigger-binding audit, and the flow-trigger-readiness lint used the
22+
same `typeof` narrowing and produced no finding. The flow bound to nothing and
23+
never fired, with zero output at any layer — the same silent-never-fire class as
24+
#3427 / #3472, and the last authoring shape still slipping past every guard.
25+
26+
This is a **defensive** fix — arrays remain unsupported; they now fail loudly:
27+
28+
- **lint** (`validate-flow-trigger-readiness`): an array `triggerType` containing
29+
any `record-*` element now yields a `flow-trigger-unknown-event` warning at
30+
`os validate` time, steering to `record-after-write` (for created-or-updated) or
31+
one flow per event.
32+
- **engine** (`resolveTriggerBinding`): such an array is routed to the
33+
`record_change` trigger — exactly as an unmappable single token is — instead of
34+
being folded to a manual flow, so it reaches the trigger's bind-time rejection.
35+
- **trigger** (`record-change`): the bind-time rejection detects the array shape
36+
and emits a targeted warning (naming the flow, pointing at `record-after-write`
37+
and #3457) rather than the generic unknown-token line.

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,45 @@ describe('validateFlowTriggerReadiness', () => {
248248
}
249249
});
250250

251+
it('flags an array-form triggerType — an unsupported multi-event shape that never fires (#3481)', () => {
252+
// A non-string triggerType folds to "no trigger" at the runtime, so the flow
253+
// is misclassified as manual and passes every gate silently. Catch it here.
254+
const flow = recordFlow({ status: 'active' });
255+
(flow.nodes[0] as { config: Record<string, unknown> }).config.triggerType = [
256+
'record-after-create',
257+
'record-after-delete',
258+
];
259+
const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] });
260+
expect(findings.map((f) => f.rule)).toEqual([FLOW_TRIGGER_UNKNOWN_EVENT]);
261+
expect(findings[0].severity).toBe('warning');
262+
expect(findings[0].message).toMatch(/array/i);
263+
expect(findings[0].message).toMatch(/never fires/i);
264+
expect(findings[0].path).toBe('flows[0].nodes[0].config.triggerType');
265+
// The hint steers to the supported alternatives.
266+
expect(findings[0].hint).toMatch(/record-after-write/);
267+
expect(findings[0].hint).toMatch(/#3457/);
268+
});
269+
270+
it('flags an array even when its elements are individually valid tokens', () => {
271+
// ['record-after-create','record-after-update'] is exactly what record-after-write
272+
// exists for — the array form is still unsupported, so still flagged.
273+
const flow = recordFlow({ status: 'active' });
274+
(flow.nodes[0] as { config: Record<string, unknown> }).config.triggerType = [
275+
'record-after-create',
276+
'record-after-update',
277+
];
278+
const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] });
279+
expect(findings.map((f) => f.rule)).toEqual([FLOW_TRIGGER_UNKNOWN_EVENT]);
280+
});
281+
282+
it('does not flag a non-record array (not the record-trigger silent-miss)', () => {
283+
// An array with no record-* element is not the #3481 defect — leave it alone.
284+
const flow = recordFlow({ status: 'active' });
285+
(flow.nodes[0] as { config: Record<string, unknown> }).config.triggerType = ['schedule', 'manual'];
286+
const findings = validateFlowTriggerReadiness({ objects: [candidateObject], flows: [flow] });
287+
expect(findings.some((f) => f.rule === FLOW_TRIGGER_UNKNOWN_EVENT)).toBe(false);
288+
});
289+
251290
it('handles map-keyed flows/objects and stacks with no flows', () => {
252291
expect(validateFlowTriggerReadiness({})).toEqual([]);
253292
const findings = validateFlowTriggerReadiness({

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,14 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines
9595
const config = (start?.node.config ?? {}) as AnyRec;
9696
const triggerType = typeof config.triggerType === 'string' ? config.triggerType : undefined;
9797
const isRecordTriggered = !!triggerType && triggerType.startsWith('record-');
98+
// Array-form triggerType (e.g. ['record-after-create', 'record-after-delete'])
99+
// is NOT supported — multi-event unions are deferred (#3457). It needs its own
100+
// detection because a non-string triggerType folds to `undefined` above, so the
101+
// runtime misclassifies the flow as manual and it never fires with zero output
102+
// (#3481). Any record-* element is enough to recognize the (unsupported) intent.
103+
const isArrayRecordTriggered =
104+
Array.isArray(config.triggerType) &&
105+
(config.triggerType as unknown[]).some((t) => typeof t === 'string' && t.startsWith('record-'));
98106
const isTimeRelative = config.timeRelative != null && typeof config.timeRelative === 'object';
99107
const isAutoTriggered =
100108
isRecordTriggered || triggerType === 'api' || config.schedule != null ||
@@ -163,6 +171,26 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines
163171
});
164172
}
165173

174+
// 1d. Array-form triggerType — an unsupported multi-event shape (#3457). The
175+
// runtime folds a non-string triggerType to "no trigger" and treats the
176+
// flow as manual, so it binds to nothing and never fires, with zero output
177+
// at any layer (#3481). Surface it at authoring time like the unmappable
178+
// single tokens above (same rule id — both are "this token never fires").
179+
if (start && isArrayRecordTriggered) {
180+
findings.push({
181+
severity: 'warning',
182+
rule: FLOW_TRIGGER_UNKNOWN_EVENT,
183+
where: `flow "${flowName}" › start node`,
184+
path: `flows[${flowIndex}].nodes[${start.index}].config.triggerType`,
185+
message:
186+
`triggerType is an array (${JSON.stringify(config.triggerType)}), which is not supported — a start ` +
187+
`node takes a single trigger event, so the flow binds to nothing and never fires (the runtime stays silent about it).`,
188+
hint:
189+
`Use one triggerType string. For "created or updated" use record-after-write (one flow, both events, #3427). ` +
190+
`For any other combination, author one flow per event — multi-event arrays are deferred (#3457).`,
191+
});
192+
}
193+
166194
// 2. Auto-triggered flow whose status is 'draft' — authored or defaulted
167195
// (defineFlow parses at definition time, so the two are the same here).
168196
if (isAutoTriggered && (flow.status == null || flow.status === 'draft')) {

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,34 @@ export class AutomationEngine implements IAutomationService {
934934
};
935935
}
936936

937+
// Array-form triggerType (e.g. ['record-after-create', 'record-after-delete']).
938+
// Multi-event unions are deliberately unsupported (#3457). But a non-string
939+
// triggerType would otherwise fall through every branch below and resolve to
940+
// `undefined` — the flow silently becomes a manual/screen flow that never
941+
// fires, invisible to the binding audit (#3481). Route it to the record-change
942+
// trigger, exactly as an unmappable single token does, so the trigger rejects
943+
// it LOUDLY at bind time (a warn naming the flow) instead of vanishing. The
944+
// authoring-time lint gate (validate-flow-trigger-readiness) is the primary
945+
// catch; this keeps runtime behavior consistent with the typo-token path. The
946+
// raw array is preserved in `config` so the trigger can tailor its message;
947+
// `event` is a joined string so the trigger's single-token mapper reports it
948+
// verbatim and maps it to no hook.
949+
if (
950+
Array.isArray(config.triggerType) &&
951+
config.triggerType.some((t) => typeof t === 'string' && (t as string).startsWith('record-'))
952+
) {
953+
return {
954+
triggerType: 'record_change',
955+
binding: {
956+
flowName,
957+
object: typeof config.objectName === 'string' ? config.objectName : undefined,
958+
event: config.triggerType.filter((t) => typeof t === 'string').join(','),
959+
condition: (config.condition as FlowTriggerBinding['condition']) ?? undefined,
960+
config,
961+
},
962+
};
963+
}
964+
937965
// Declarative time-relative sweep (#1874): a start node carrying a
938966
// `timeRelative` descriptor is swept on a schedule and launched once per
939967
// record whose date field falls in the window. Checked BEFORE `schedule`

packages/services/service-automation/src/trigger-dispatch-observability.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,71 @@ describe('trigger-fired execution failures are loud (layer 1)', () => {
127127
});
128128
});
129129

130+
describe('array-form triggerType is routed, not silently folded to manual (#3481)', () => {
131+
function arrayTriggeredFlow(name: string, object: string) {
132+
return {
133+
name,
134+
label: name,
135+
type: 'autolaunched',
136+
status: 'active',
137+
nodes: [
138+
{
139+
id: 'start',
140+
type: 'start',
141+
label: 'Start',
142+
config: {
143+
objectName: object,
144+
triggerType: ['record-after-create', 'record-after-delete'],
145+
},
146+
},
147+
{ id: 'end', type: 'end', label: 'End' },
148+
],
149+
edges: [{ id: 'e1', source: 'start', target: 'end' }],
150+
};
151+
}
152+
153+
it('hands an array triggerType to the record_change trigger (so it can reject loudly), not folding it to a manual flow', () => {
154+
const { logger } = spyLogger();
155+
const engine = new AutomationEngine(logger);
156+
157+
let started: { event?: string; config?: Record<string, unknown> } | undefined;
158+
engine.registerTrigger({
159+
type: 'record_change',
160+
start: (binding) => { started = binding as typeof started; },
161+
stop: () => {},
162+
});
163+
engine.registerFlow('array_flow', arrayTriggeredFlow('array_flow', 'wid') as never);
164+
165+
// Without the fix, a non-string triggerType folds to "no trigger": the
166+
// flow is treated as manual and the trigger is never handed the binding.
167+
expect(started, 'array-form flow routed to the record_change trigger').toBeDefined();
168+
expect(engine.getActiveTriggerBindings().map((b) => b.triggerType)).toContain('record_change');
169+
// The raw array is forwarded (via config) so the trigger can produce a
170+
// targeted rejection message rather than the generic one.
171+
expect((started!.config as { triggerType?: unknown }).triggerType).toEqual([
172+
'record-after-create',
173+
'record-after-delete',
174+
]);
175+
});
176+
177+
it('ignores an array with no record-* element (not the record-trigger case)', () => {
178+
const { logger } = spyLogger();
179+
const engine = new AutomationEngine(logger);
180+
let started = false;
181+
engine.registerTrigger({
182+
type: 'record_change',
183+
start: () => { started = true; },
184+
stop: () => {},
185+
});
186+
const flow = arrayTriggeredFlow('sched_array', 'wid');
187+
(flow.nodes[0].config as Record<string, unknown>).triggerType = ['schedule', 'manual'];
188+
engine.registerFlow('sched_array', flow as never);
189+
190+
// No record-* element ⇒ not routed to record_change (correctly manual).
191+
expect(started).toBe(false);
192+
});
193+
});
194+
130195
describe('unbound triggered flows are audited at kernel:bootstrapped (layer 2)', () => {
131196
it('warns per enabled flow whose trigger type has no registered trigger', async () => {
132197
const warn = vi.fn();

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,46 @@ describe('RecordChangeTrigger', () => {
146146
expect(hooks).toHaveLength(0);
147147
});
148148

149+
it('rejects an array-form trigger event with a targeted message, not bound (#3481)', () => {
150+
// Multi-event arrays are unsupported (#3457). The engine forwards the raw
151+
// array via config (with a joined `event` string that maps to no hook), so
152+
// the trigger can steer the author to the supported alternatives instead of
153+
// the generic "unsupported trigger event" line.
154+
const { engine, hooks } = fakeEngine();
155+
const warn = vi.fn();
156+
const trigger = new RecordChangeTrigger(engine, { info: () => {}, warn, debug: () => {} });
157+
158+
trigger.start(
159+
binding({
160+
event: 'record-after-create,record-after-delete',
161+
config: { triggerType: ['record-after-create', 'record-after-delete'] },
162+
}),
163+
async () => {},
164+
);
165+
166+
expect(hooks).toHaveLength(0);
167+
expect(warn).toHaveBeenCalledTimes(1);
168+
const msg = String(warn.mock.calls[0][0]);
169+
expect(msg).toMatch(/task_assigned_notify/);
170+
expect(msg).toMatch(/array/i);
171+
expect(msg).toMatch(/record-after-write/);
172+
expect(msg).toMatch(/#3457/);
173+
});
174+
175+
it('keeps the generic unsupported-event warning for a non-array bad token', () => {
176+
const { engine, hooks } = fakeEngine();
177+
const warn = vi.fn();
178+
const trigger = new RecordChangeTrigger(engine, { info: () => {}, warn, debug: () => {} });
179+
180+
trigger.start(binding({ event: 'record-after-updated' }), async () => {});
181+
182+
expect(hooks).toHaveLength(0);
183+
expect(warn).toHaveBeenCalledTimes(1);
184+
const msg = String(warn.mock.calls[0][0]);
185+
expect(msg).toMatch(/unsupported trigger event/i);
186+
expect(msg).toMatch(/record-after-updated/);
187+
});
188+
149189
it('binds BOTH afterInsert and afterUpdate for record-after-write (create OR update, #3427)', () => {
150190
const { engine, hooks } = fakeEngine();
151191
const trigger = new RecordChangeTrigger(engine, silentLogger());

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,23 @@ export class RecordChangeTrigger implements FlowTrigger {
175175
// a double-dispatch.
176176
const hookEvents = triggerTypeToHookEvents(binding.event);
177177
if (hookEvents.length === 0) {
178-
this.logger.warn(
179-
`[record-change] flow '${binding.flowName}' has unsupported trigger event '${binding.event ?? '(none)'}' — not bound`,
180-
);
178+
// Array-form triggerType (`['record-after-create', 'record-after-delete']`)
179+
// reaches here via the engine, which forwards the raw array in `config`
180+
// and a joined `event` string that maps to no hook. Multi-event arrays are
181+
// unsupported (#3457); give a targeted message steering to the supported
182+
// shapes rather than the generic unknown-token line (#3481).
183+
const rawTriggerType = (binding.config as { triggerType?: unknown } | undefined)?.triggerType;
184+
if (Array.isArray(rawTriggerType)) {
185+
this.logger.warn(
186+
`[record-change] flow '${binding.flowName}' has an ARRAY trigger event ${JSON.stringify(rawTriggerType)} — ` +
187+
`multi-event arrays are not supported, so the flow is NOT bound and will never fire. ` +
188+
`For "created or updated" use a single 'record-after-write'; for any other combination author one flow per event (#3457).`,
189+
);
190+
} else {
191+
this.logger.warn(
192+
`[record-change] flow '${binding.flowName}' has unsupported trigger event '${binding.event ?? '(none)'}' — not bound`,
193+
);
194+
}
181195
return;
182196
}
183197

0 commit comments

Comments
 (0)