Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions .changeset/array-form-triggertype-not-silent.md
Original file line number Diff line number Diff line change
@@ -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.
39 changes: 39 additions & 0 deletions packages/lint/src/validate-flow-trigger-readiness.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> }).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<string, unknown> }).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<string, unknown> }).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({
Expand Down
28 changes: 28 additions & 0 deletions packages/lint/src/validate-flow-trigger-readiness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down Expand Up @@ -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')) {
Expand Down
28 changes: 28 additions & 0 deletions packages/services/service-automation/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> } | 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<string, unknown>).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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down