diff --git a/.changeset/engine-trigger-kind-from-spec-resolver.md b/.changeset/engine-trigger-kind-from-spec-resolver.md new file mode 100644 index 0000000000..8ba37bf8e4 --- /dev/null +++ b/.changeset/engine-trigger-kind-from-spec-resolver.md @@ -0,0 +1,51 @@ +--- +"@objectstack/service-automation": patch +--- + +refactor(service-automation): take the trigger KIND from spec's `resolveFlowTriggerKind` instead of a second private copy of the chain (#14328) + +No behaviour change and no API change — `patch` because nothing observable moves. +`resolveTriggerBinding` is `private`, no export is added or removed, no payload +key changes, and the kind reported for every flow is the kind reported before +(1,223 `service-automation` cases and 81 `trigger-record-change` cases green +unchanged, plus new pins across the whole precedence chain). What changes is that +one rule now has one home. + +**The defect.** `AutomationEngine.resolveTriggerBinding` hand-kept the chain that +decides which trigger a flow asks for — string `record-*` token, array form, +`timeRelative` descriptor, `schedule` cadence or `type: 'schedule'`, `type: 'api'` +or `triggerType: 'api'` — in parallel with `@objectstack/spec`'s +`resolveFlowTriggerKind`, the authoring-time mirror of that same rule. Both +authoring surfaces already read the spec one: `defineStack`'s trigger-capability +refusal and `@objectstack/lint`'s `validate-flow-trigger-readiness`. The engine +did not, and nothing pinned the two together. A branch added to one side leaves +`defineStack` accepting a stack the runtime leaves inert, or refusing one it would +arm — the drift the shared resolver was hoisted to prevent, reopened one layer +down. The two agreed on every string-form flow, so this was an observation rather +than a live defect; the harm was future drift. + +**The shape.** `resolveTriggerBinding` now takes its kind from +`resolveFlowTriggerKind(flow)` and keeps only the per-kind BINDING construction — +which start-node fields each trigger needs. `getTriggerBindingAudit` and the boot +banner therefore name the kind authoring named, by construction. + +**The one deliberate divergence is preserved, not unified.** The ARRAY form of +`triggerType` (`['record-after-create', 'record-after-delete']`) resolves to *no* +kind in spec — multi-event unions are unsupported (#3457), and reading the shape +as "asks for a record-change trigger" would have `defineStack` demand a capability +the flow can never use and would widen the lint rule's auto-triggered set. The +engine routes it to the record-change trigger anyway, from an explicit pre-check +that runs BEFORE the resolver, for one reason: so that trigger refuses it LOUDLY +at bind time (#3481) instead of the flow folding into "manual" and vanishing from +every surface. Pre-check *ordering* is load-bearing too — array form outranks +`timeRelative`, which the resolver, blind to the array, would otherwise answer for +a start node carrying both. + +**What now catches the drift.** Two guards, one static and one runtime. The +per-kind `switch` is exhaustive over `FlowTriggerKind` with a `never` default, so a +kind added to spec fails this package's type-check until its binding shape is +written; and a new case asserts every kind in `FLOW_TRIGGER_KINDS` is reachable +through the real engine. The preserved divergence is pinned on both sides: engine +routing and pre-check precedence in `service-automation`, and the refusal itself — +asserted as a refusal, on a binding the real engine produced — end-to-end against +the real trigger in `@objectstack/trigger-record-change`. diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 6f42d81869..00401ee104 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -20,6 +20,11 @@ import { } from './screen-input-contract.js'; import type { Logger } from '@objectstack/spec/contracts'; import { FlowSchema, FLOW_STRUCTURAL_NODE_TYPES, validateControlFlow, collectFlowGraphs, findRegionEntry, defineActionDescriptor } from '@objectstack/spec/automation'; +// [#14328] The ONE answer to "which trigger kind does this flow ask for?" — +// shared with `defineStack`'s trigger-capability refusal and `@objectstack/lint`'s +// `validate-flow-trigger-readiness`, so the runtime cannot drift from what +// authoring accepted. See `resolveTriggerBinding`. +import { resolveFlowTriggerKind } from '@objectstack/spec/automation'; import { resolveFlowNodeExpressions } from '@objectstack/spec/automation'; import { applyConversionsToFlow, type ConversionNotice, type ConversionConflictNotice } from '@objectstack/spec'; // [ADR-0126 §7.3] "Does a code package ship this flow?" for the subflow guard. @@ -2448,6 +2453,17 @@ export class AutomationEngine implements IAutomationService { * established by the showcase flows — is that the start node carries the * trigger details in its `config`: `{ objectName, triggerType, condition }` * for record-change, or a `schedule` descriptor for time-based flows. + * + * [#14328] WHICH KIND a flow asks for is not decided here: it is + * {@link resolveFlowTriggerKind}'s answer, the one `@objectstack/spec` + * export that `defineStack`'s trigger-capability refusal and + * `@objectstack/lint`'s `validate-flow-trigger-readiness` already read. + * This method keeps only the per-kind BINDING construction — which start-node + * fields each trigger needs. Before #14328 the chain was hand-kept here in + * parallel with the spec one, and nothing pinned them together: a branch + * added on one side left `defineStack` accepting a stack the runtime leaves + * inert, or refusing one it would arm. Now `getTriggerBindingAudit` and the + * boot banner name the kind authoring named, by construction. */ private resolveTriggerBinding( flowName: string, @@ -2456,20 +2472,7 @@ export class AutomationEngine implements IAutomationService { if (!flow) return undefined; const startNode = flow.nodes.find(n => n.type === 'start'); const config = (startNode?.config ?? {}) as Record; - const triggerType = typeof config.triggerType === 'string' ? config.triggerType : undefined; - - if (triggerType && triggerType.startsWith('record-')) { - return { - triggerType: 'record_change', - binding: { - flowName, - object: typeof config.objectName === 'string' ? config.objectName : undefined, - event: triggerType, - condition: (config.condition as FlowTriggerBinding['condition']) ?? undefined, - config, - }, - }; - } + const condition = (config.condition as FlowTriggerBinding['condition']) ?? undefined; // Array-form triggerType (e.g. ['record-after-create', 'record-after-delete']). // Multi-event unions are deliberately unsupported (#3457). But a non-string @@ -2483,6 +2486,18 @@ export class AutomationEngine implements IAutomationService { // 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. + // + // [#14328] This stays an explicit pre-check BEFORE `resolveFlowTriggerKind`, + // and that is the ONE deliberate divergence between the two — documented in + // that resolver's own header. It resolves array form to NO kind on purpose: + // reading it as "asks for a record-change trigger" would have `defineStack` + // demand a capability for a flow that can never use it and would widen the + // lint rule's auto-triggered set. The route below is a DIAGNOSTIC route, not + // a trigger the flow could fire on; folding it into the shared resolver would + // delete a standing decision and turn a loud refusal into silence. Pre-check + // ordering also preserves this method's own precedence exactly: array form + // outranks `timeRelative`/`schedule`, which the resolver — blind to it — + // would otherwise answer for a start node carrying both. if ( Array.isArray(config.triggerType) && config.triggerType.some((t) => typeof t === 'string' && (t as string).startsWith('record-')) @@ -2493,56 +2508,86 @@ export class AutomationEngine implements IAutomationService { 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, + condition, 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` - // because such a flow ALSO carries a `schedule` cadence (the sweep - // interval) — without this precedence it would bind to the plain schedule - // trigger and fire once with no record instead of once per record. - if (config.timeRelative != null && typeof config.timeRelative === 'object') { - const tr = config.timeRelative as Record; - return { - triggerType: 'time_relative', - binding: { - flowName, - object: - typeof tr.object === 'string' - ? tr.object - : typeof config.objectName === 'string' - ? config.objectName - : undefined, - schedule: config.schedule, - condition: (config.condition as FlowTriggerBinding['condition']) ?? undefined, - config, - }, - }; - } + const kind = resolveFlowTriggerKind(flow); + if (!kind) return undefined; - if (config.schedule != null || flow.type === 'schedule') { - return { - triggerType: 'schedule', - binding: { flowName, schedule: config.schedule, condition: (config.condition as FlowTriggerBinding['condition']) ?? undefined, config }, - }; - } + switch (kind) { + case 'record_change': + return { + triggerType: kind, + binding: { + flowName, + object: typeof config.objectName === 'string' ? config.objectName : undefined, + // A `record-*` kind means `config.triggerType` IS that string + // token — the resolver read it to answer; the narrowing is + // re-stated because the answer does not carry it back. + event: typeof config.triggerType === 'string' ? config.triggerType : undefined, + condition, + config, + }, + }; - // Inbound HTTP (ADR-0041 Tier 1): an `api` flow waits for an external - // POST. The concrete trigger (`@objectstack/trigger-api`) mounts the - // endpoint and enqueues; the binding's `config` carries the hook - // details (`hookId`, `secret`) from the start node. - if (flow.type === 'api' || triggerType === 'api') { - return { - triggerType: 'api', - binding: { flowName, 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. The resolver ranks it + // BEFORE `schedule` for the same reason this method did — such a flow + // ALSO carries a `schedule` cadence (the sweep interval), and without + // that precedence it would bind to the plain schedule trigger and fire + // once with no record instead of once per record. + case 'time_relative': { + const tr = (config.timeRelative ?? {}) as Record; + return { + triggerType: kind, + binding: { + flowName, + object: + typeof tr.object === 'string' + ? tr.object + : typeof config.objectName === 'string' + ? config.objectName + : undefined, + schedule: config.schedule, + condition, + config, + }, + }; + } + + case 'schedule': + return { + triggerType: kind, + binding: { flowName, schedule: config.schedule, condition, config }, + }; + + // Inbound HTTP (ADR-0041 Tier 1): an `api` flow waits for an external + // POST. The concrete trigger (`@objectstack/trigger-api`) mounts the + // endpoint and enqueues; the binding's `config` carries the hook + // details (`hookId`, `secret`) from the start node. + case 'api': + return { + triggerType: kind, + binding: { flowName, condition, config }, + }; - return undefined; + default: { + // [#14328] Exhaustive over `FlowTriggerKind`, and that is the point: + // a kind added to the spec resolver makes `kind` no longer `never` + // here, so THIS package's type-check fails until the binding shape + // for it is written — the drift is caught at the commit that opens + // it instead of showing up as a flow that arms nowhere. At run time + // (a spec build ahead of this one) fall back to today's behaviour — + // no binding — rather than throwing inside the boot audit. + const unhandledKind: never = kind; + void unhandledKind; + return undefined; + } + } } /** diff --git a/packages/services/service-automation/src/flow-trigger-kind-shared-resolver.test.ts b/packages/services/service-automation/src/flow-trigger-kind-shared-resolver.test.ts new file mode 100644 index 0000000000..bb749717bc --- /dev/null +++ b/packages/services/service-automation/src/flow-trigger-kind-shared-resolver.test.ts @@ -0,0 +1,288 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14328] The engine's trigger KIND is spec's answer, not a second copy of the + * chain. + * + * `AutomationEngine.resolveTriggerBinding` used to hand-keep the precedence that + * decides which trigger a flow asks for — `record-*` token, array form, + * `timeRelative`, `schedule` cadence / `type: 'schedule'`, `type: 'api'` / + * `triggerType: 'api'` — in parallel with `@objectstack/spec`'s + * `resolveFlowTriggerKind`, which `defineStack`'s trigger-capability refusal and + * `@objectstack/lint`'s `validate-flow-trigger-readiness` both read. Two + * hand-kept copies of one rule with nothing pinning them together: a branch added + * to one side leaves `defineStack` accepting a stack the runtime leaves inert, or + * refusing one it would arm. + * + * These cases drive the REAL engine — `registerFlow` then the public + * `getFlowRuntimeStates()` / `getTriggerBindingAudit()`, the same surfaces the CLI + * boot banner and the kernel:bootstrapped audit read — and assert a LITERAL kind + * per case. The literal is what discriminates: an `expect(engineKind).toBe( + * resolveFlowTriggerKind(flow))` alone is satisfied by two wrong answers that + * happen to agree, and by `undefined === undefined` for a case that silently + * stopped resolving. The equality against the resolver is asserted too, but as + * the second half of the pin, never as the whole of it. + * + * The last case is the coupling itself: every kind spec publishes in + * `FLOW_TRIGGER_KINDS` must be reachable through the engine. Spec adding a fifth + * kind reddens it — which is the drift this card closes. (The engine's `switch` + * also fails THIS package's type-check on that day, via its `never` default; + * this case is the runtime half of the same guard.) + * + * ⚠️ The ARRAY form is deliberately NOT covered here as an equality case: it is + * the one documented divergence — `resolveFlowTriggerKind` answers `undefined` + * for it while the engine routes it to the record-change trigger so that trigger + * can refuse it LOUDLY at bind time. Its preservation is pinned below (routing + + * precedence) and end-to-end, against the real trigger that emits the refusal, in + * `@objectstack/trigger-record-change`'s + * `array-form-refusal-end-to-end.test.ts` — the only place both halves can meet + * without inverting a package dependency. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { FLOW_TRIGGER_KINDS, resolveFlowTriggerKind } from '@objectstack/spec/automation'; +import { AutomationEngine } from './engine.js'; +import type { FlowTrigger, FlowTriggerBinding } from './engine.js'; + +function createTestLogger() { + return { debug() {}, info() {}, warn() {}, error() {} } as any; +} + +/** A minimal registrable flow whose start node carries `config`. */ +function flowWith( + name: string, + config: Record, + type: string = 'autolaunched', +) { + return { + name, + label: name, + type, + status: 'active', + nodes: [ + { id: 'start', type: 'start', label: 'Start', config }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end' }], + }; +} + +describe('[#14328] the engine takes its trigger kind from spec.resolveFlowTriggerKind', () => { + let engine: AutomationEngine; + beforeEach(() => { + engine = new AutomationEngine(createTestLogger()); + }); + + /** The kind the REAL engine resolved, read off a public surface. */ + function engineKind(name: string): string | undefined { + const row = engine.getFlowRuntimeStates().find((s) => s.name === name); + expect(row, `flow '${name}' is registered`).toBeDefined(); + return row!.triggerType; + } + + // Each row: the whole precedence chain, one literal kind each. `expected` is + // the pre-#14328 engine's answer, written out — so a unification that got a + // case wrong reddens here rather than being blessed by a resolver that agrees + // with itself. + const chain: Array<{ case: string; flow: ReturnType; expected: string }> = [ + { + case: 'string record-* token', + flow: flowWith('rc', { objectName: 'task', triggerType: 'record-after-update' }), + expected: 'record_change', + }, + { + case: 'timeRelative descriptor', + flow: flowWith('tr', { timeRelative: { object: 'task', field: 'due_date' } }), + expected: 'time_relative', + }, + { + case: 'timeRelative OUTRANKS its own schedule cadence (the sweep interval)', + flow: flowWith('tr_sched', { + timeRelative: { object: 'task', field: 'due_date' }, + schedule: { cron: '0 * * * *' }, + }), + expected: 'time_relative', + }, + { + case: 'schedule cadence', + flow: flowWith('sched', { schedule: { cron: '0 9 * * *' } }), + expected: 'schedule', + }, + { + case: "type: 'schedule' with no cadence on the start node", + flow: flowWith('sched_type', {}, 'schedule'), + expected: 'schedule', + }, + { + case: "type: 'api'", + flow: flowWith('api_type', {}, 'api'), + expected: 'api', + }, + { + case: "triggerType: 'api'", + flow: flowWith('api_token', { triggerType: 'api' }), + expected: 'api', + }, + ]; + + for (const row of chain) { + it(`resolves ${row.case} to '${row.expected}', and to what spec answers`, () => { + // `registerFlow` hands back the canonicalized flow it stored — the + // same object `this.flows` holds and the engine resolved against. + const stored = engine.registerFlow(row.flow.name, row.flow as never); + + // Half 1 — the literal. This is the half that can fail on its own. + expect(engineKind(row.flow.name)).toBe(row.expected); + + // Half 2 — and it is spec's answer, on that same stored flow, so a + // registration that rewrote the start node cannot hide behind the + // literal above. + expect(resolveFlowTriggerKind(stored)).toBe(row.expected); + }); + } + + it('resolves a flow with NO trigger declaration to no kind (manual / screen)', () => { + const stored = engine.registerFlow('manual', flowWith('manual', {}) as never); + + expect(engineKind('manual')).toBeUndefined(); + expect(resolveFlowTriggerKind(stored)).toBeUndefined(); + // …and it is absent from the silent-miss audit: nothing to bind. + expect(engine.getTriggerBindingAudit().map((a) => a.flowName)).not.toContain('manual'); + }); + + it('names the same kind on getTriggerBindingAudit — the surface the boot banner prints', () => { + // The card's stated payoff: the audit an admin reads names the kind + // `defineStack` named at authoring, by construction. + const stored = new Map(); + stored.set( + 'sched', + engine.registerFlow('sched', flowWith('sched', { schedule: { cron: '0 9 * * *' } }) as never), + ); + stored.set( + 'rc', + engine.registerFlow( + 'rc', + flowWith('rc', { objectName: 'task', triggerType: 'record-after-create' }) as never, + ), + ); + + const audit = engine.getTriggerBindingAudit(); + const byFlow = new Map(audit.map((a) => [a.flowName, a.triggerType])); + expect(byFlow.get('sched')).toBe('schedule'); + expect(byFlow.get('rc')).toBe('record_change'); + // Each row's kind is the authoring-time answer for the same flow. + for (const [name, kind] of byFlow) { + expect(resolveFlowTriggerKind(stored.get(name)), `audit row '${name}'`).toBe(kind); + } + }); + + it('reaches EVERY kind spec publishes in FLOW_TRIGGER_KINDS', () => { + // The coupling, as a runtime pin: a kind added to spec that the engine has + // no binding shape for resolves to `undefined` here and reddens this case. + // Without it, such a flow is simply inert — accepted by `defineStack`, + // armed by nothing, and invisible on every surface above. + const reached = new Set(); + for (const row of chain) { + engine.registerFlow(row.flow.name, row.flow as never); + const kind = engineKind(row.flow.name); + if (kind) reached.add(kind); + } + expect([...reached].sort()).toEqual([...FLOW_TRIGGER_KINDS].sort()); + }); +}); + +describe('[#14328] the ARRAY-form divergence is PRESERVED, not unified away', () => { + let engine: AutomationEngine; + beforeEach(() => { + engine = new AutomationEngine(createTestLogger()); + }); + + function arrayFlow(name: string, extra: Record = {}) { + return flowWith(name, { + objectName: 'task', + triggerType: ['record-after-create', 'record-after-delete'], + ...extra, + }); + } + + it('routes array form to record_change even though spec answers NO kind for it', () => { + const stored = engine.registerFlow('array_flow', arrayFlow('array_flow') as never); + + // The divergence, both sides of it, in one case. Spec deliberately answers + // `undefined` (multi-event unions are unsupported, #3457, and reading it as + // "asks for a record-change trigger" would have `defineStack` demand a + // capability the flow can never use). The engine deliberately routes it + // anyway, so `@objectstack/trigger-record-change` can refuse it LOUDLY at + // bind time (#3481) instead of the flow vanishing into "manual". + expect(resolveFlowTriggerKind(stored)).toBeUndefined(); + const row = engine.getFlowRuntimeStates().find((s) => s.name === 'array_flow'); + expect(row?.triggerType).toBe('record_change'); + }); + + it('keeps the array pre-check AHEAD of the resolver (it outranks timeRelative)', () => { + // Ordering is the whole reason the pre-check sits before the resolver call + // rather than beside the per-kind cases. A start node carrying BOTH an + // array `triggerType` and a `timeRelative` descriptor resolved to + // record_change before #14328; the resolver — blind to the array — answers + // `time_relative` for it. Moving the pre-check after the resolver call + // silently re-routes this flow and swaps the loud refusal for a sweep. + const stored = engine.registerFlow( + 'array_and_time_relative', + arrayFlow('array_and_time_relative', { + timeRelative: { object: 'task', field: 'due_date' }, + }) as never, + ); + + expect(resolveFlowTriggerKind(stored)).toBe('time_relative'); + const row = engine + .getFlowRuntimeStates() + .find((s) => s.name === 'array_and_time_relative'); + expect(row?.triggerType).toBe('record_change'); + }); + + it('hands the record-change trigger the raw array and a joined event token', () => { + // The two fields the refusal is built from: `config.triggerType` carries the + // raw array (so the trigger can name the offending shape) and `event` is the + // joined string that maps to NO hook (so the trigger's single-token mapper + // reports it verbatim and binds nothing). + // Typed as the real `FlowTrigger`, NOT `… as never`: the cast erases the + // contextual type for `start`, which leaves `binding` implicitly `any` + // (TS7006). This package has no `typecheck` script, so its `tsc --noEmit` + // runs only in the type-check DEBT lane — which compiles `src/**`, tests + // included, and is a shrink-only ratchet. Same shape as `engine.test.ts`. + const started: FlowTriggerBinding[] = []; + const trigger: FlowTrigger = { + type: 'record_change', + start: (binding) => { + started.push(binding); + }, + stop: () => {}, + }; + engine.registerTrigger(trigger); + engine.registerFlow('array_flow', arrayFlow('array_flow') as never); + + expect(started).toHaveLength(1); + expect(started[0].event).toBe('record-after-create,record-after-delete'); + expect((started[0].config as { triggerType?: unknown }).triggerType).toEqual([ + 'record-after-create', + 'record-after-delete', + ]); + }); + + it('leaves an array with NO record-* element to the resolver (not the record case)', () => { + // The pre-check is narrow on purpose: only an array containing a `record-*` + // token is the diagnostic route. Anything else falls through to spec's + // answer, which for this flow is the schedule cadence it also declares. + const stored = engine.registerFlow( + 'sched_array', + flowWith('sched_array', { + triggerType: ['schedule', 'manual'], + schedule: { cron: '0 9 * * *' }, + }) as never, + ); + + const row = engine.getFlowRuntimeStates().find((s) => s.name === 'sched_array'); + expect(row?.triggerType).toBe('schedule'); + expect(resolveFlowTriggerKind(stored)).toBe('schedule'); + }); +}); diff --git a/packages/triggers/trigger-record-change/src/array-form-refusal-end-to-end.test.ts b/packages/triggers/trigger-record-change/src/array-form-refusal-end-to-end.test.ts new file mode 100644 index 0000000000..b47d26a4de --- /dev/null +++ b/packages/triggers/trigger-record-change/src/array-form-refusal-end-to-end.test.ts @@ -0,0 +1,151 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14328] The array-form divergence, end to end: engine → REAL record-change + * trigger → the loud bind-time refusal. + * + * #14328 replaced the engine's hand-kept trigger-kind chain with + * `@objectstack/spec`'s `resolveFlowTriggerKind`. That resolver deliberately + * answers NO kind for an array `triggerType` (`['record-after-create', + * 'record-after-delete']`) — multi-event unions are unsupported (#3457), and + * reading the shape as "asks for a record-change trigger" would have + * `defineStack` demand a capability the flow can never use and would widen + * `@objectstack/lint`'s auto-triggered set. The engine routes it anyway, from an + * explicit pre-check that runs BEFORE the resolver, for one reason: so this + * trigger can refuse it LOUDLY at bind time (#3481) rather than the flow folding + * into "manual" and vanishing from every surface. + * + * "Unifying" that divergence away is the single way to get #14328 wrong, and the + * failure would be SILENT — the flow would still never fire, it would simply stop + * saying so. Nothing pinned the two halves together end to end: the engine-side + * routing is pinned in `@objectstack/service-automation` + * (`flow-trigger-kind-shared-resolver.test.ts`, `trigger-dispatch-observability.test.ts`) + * against a recording double, and the refusal is pinned here + * (`record-change-trigger.test.ts`) against a HAND-BUILT binding. Either can stay + * green while the join between them rots. This file asserts the REFUSAL ITSELF, + * on a binding the real engine produced from a registered flow. + * + * It lives in this package because this is the only side of the edge where both + * halves exist: `@objectstack/service-automation` is a devDependency here, and + * `@objectstack/trigger-record-change` is not a dependency of it (adding one + * would invert the edge the trigger plugin was written to avoid — see + * `plugin.ts`'s header). + * + * Resolution note, same as `reentrant-start-condition.test.ts`: this package's + * tests resolve `@objectstack/service-automation` through its `exports` to + * `dist/` (`check:test-source-alias`'s `KNOWN_UNALIASED_TEST_IMPORTS` entry for + * this package), so this file is a verdict on the BUILT engine — which is what + * the trigger loads in production, and CI builds the dependency closure before + * running it. The engine-side pre-check is pinned against source separately, in + * `service-automation`'s `flow-trigger-kind-shared-resolver.test.ts`. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { AutomationEngine } from '@objectstack/service-automation'; +import { RecordChangeTrigger, type RecordChangeDataEngine } from './record-change-trigger.js'; + +function createEngineLogger() { + return { debug() {}, info() {}, warn() {}, error() {} } as never; +} + +/** Fake ObjectQL engine: records the hooks the trigger registers. */ +function fakeDataEngine() { + const hooks: Array<{ event: string; object?: string | string[] }> = []; + const engine: RecordChangeDataEngine = { + registerHook(event, _handler, options) { + hooks.push({ event, object: options?.object }); + }, + unregisterHooksByPackage() { + return 0; + }, + }; + return { engine, hooks }; +} + +function arrayFormFlow(name: string) { + return { + name, + label: name, + type: 'autolaunched', + status: 'active', + nodes: [ + { + id: 'start', + type: 'start', + label: 'Start', + config: { + objectName: 'showcase_task', + triggerType: ['record-after-create', 'record-after-delete'], + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end' }], + }; +} + +describe('[#14328] array-form triggerType still draws the record-change trigger’s loud refusal', () => { + /** Real engine + real trigger; the trigger's own logger is the spy. */ + function wire() { + const warn = vi.fn(); + const { engine: dataEngine, hooks } = fakeDataEngine(); + const trigger = new RecordChangeTrigger(dataEngine, { + info: () => {}, + warn, + debug: () => {}, + }); + const automation = new AutomationEngine(createEngineLogger()); + automation.registerTrigger(trigger as never); + return { automation, trigger, warn, hooks }; + } + + it('refuses the array form by name, and registers NO hook for it', () => { + const { automation, warn, hooks } = wire(); + + automation.registerFlow('array_flow', arrayFormFlow('array_flow') as never); + + // ── The refusal itself, not merely "something happened" ────────────── + expect(warn, 'the trigger warned exactly once').toHaveBeenCalledTimes(1); + const msg = String(warn.mock.calls[0][0]); + expect(msg, 'names the offending flow').toMatch(/array_flow/); + expect(msg, 'names the shape as an ARRAY').toMatch(/array/i); + expect(msg, 'says the flow is NOT bound and will never fire').toMatch( + /NOT bound|never fire/i, + ); + expect(msg, 'steers to the supported single token').toMatch(/record-after-write/); + expect(msg, 'cites the standing decision').toMatch(/#3457/); + + // The refusal is a refusal: nothing was armed for this flow. + expect(hooks, 'no lifecycle hook registered for an array-form flow').toHaveLength(0); + }); + + it('gets there because the ENGINE routed it — the pre-check ahead of the shared resolver', () => { + const { automation, warn } = wire(); + automation.registerFlow('array_flow', arrayFormFlow('array_flow') as never); + + // If the array pre-check were folded into `resolveFlowTriggerKind`'s + // answer, the engine would resolve NO kind, hand the binding to nobody, + // and this trigger would never be called: no warn, and the flow silently + // reads as manual. That is the failure mode this case exists to catch — + // the assertion below is the join, and it fails on zero calls. + expect(warn).toHaveBeenCalled(); + expect( + automation.getFlowRuntimeStates().find((s) => s.name === 'array_flow')?.triggerType, + 'the engine named record_change for the array form', + ).toBe('record_change'); + }); + + it('leaves a legitimate single record-* token armed, not refused', () => { + // Anti-vacuity control: the refusal above is specific to the array shape, + // not something this wiring emits for every record-change flow. Without + // this, a trigger that refused EVERYTHING would pass the case above. + const { automation, warn, hooks } = wire(); + const flow = arrayFormFlow('ok_flow'); + (flow.nodes[0].config as Record).triggerType = 'record-after-update'; + + automation.registerFlow('ok_flow', flow as never); + + expect(warn, 'no refusal for a supported single token').not.toHaveBeenCalled(); + expect(hooks.map((h) => h.event)).toEqual(['afterUpdate']); + }); +});