diff --git a/.changeset/flow-node-type-audit-at-boot-close.md b/.changeset/flow-node-type-audit-at-boot-close.md new file mode 100644 index 0000000000..da2691ea59 --- /dev/null +++ b/.changeset/flow-node-type-audit-at-boot-close.md @@ -0,0 +1,42 @@ +--- +"@objectstack/service-automation": minor +"@objectstack/plugin-approvals": patch +--- + +fix(automation,approvals): 节点类型校验推迟到插件贡献完成之后 —— approval flow 不再被误报"运行时会失败" (#4771) + +showcase 每次冷启都打印 8 条断言:这些 flow "will fail at execution time"。8 条全是假的。 +`AutomationServicePlugin.start()` 从 ObjectQL registry 拉起 flow 并**当场**校验节点类型,而 +`ApprovalsServicePlugin.start()` 在 0.8 秒后才注册 `approval` 执行器 —— 校验器在词汇表还没 +成型的时候就下了结论。 + +真正的代价不是噪音,是信号丢失:**真的没装 approvals 插件**的部署会得到一模一样的 8 条告警, +所以这条 warn 无法区分"健康"和"坏掉",信噪比为 0。 + +ADR-0018 明确把节点词汇表定义为**开放、可运行时扩展**的(插件通过 +`registerNodeExecutor(type)` 贡献类型)。因此校验只在词汇表**封闭**的那一刻才成立: + +- `AutomationEngine.sealNodeTypeVocabulary()` —— 宣告词汇表封闭,对**所有**已注册 flow 跑一次 + 权威校验,每个有问题的 flow warn 一条。`AutomationServicePlugin` 在 `kernel:bootstrapped` + 调用它(严格晚于每个插件的 `start()` 和每个 `kernel:ready` handler —— 本插件自己的 + `kernel:ready` 还会再注册一批 flow,别的插件也可能在它的 `kernel:ready` 里贡献执行器)。 +- `AutomationEngine.getUnknownNodeTypeAudit(): UnknownNodeTypeAuditEntry[]` —— 同一发现的 + **状态**形态,供 host(CLI 启动摘要、健康检查)直接读,而不是去 grep 日志。与 + `getTriggerBindingAudit()` 同一套路。 +- 封闭之后 `registerFlow` **恢复即时告警**:Studio 发布 / dev reload 进正在运行的服务器时, + 词汇表确实是完整的,那句断言此时为真。所以这是时序修复,不是把告警静音。 + +告警文案也随之改成它现在能承诺的事:"Every plugin has started, so nothing will register them +now — these nodes fail at execution time with NO_EXECUTOR",并给出补救动作。 + +一并修掉同一缺陷类的另一半:`ApprovalsServicePlugin` 在**拿不到 automation 引擎**时,把 +"`approval` 节点没注册"记成 `info` —— 而 dev 的默认日志级别是 `warn`,于是**真降级发生时反而 +看不见**(#4632:静默降级必须响亮)。现在是 `warn`,写明后果(该部署里每个 ADR-0019 approval +flow 都会以 NO_EXECUTOR 失败)和补救(装 `@objectstack/service-automation`)。`catch` 同时收窄 +到"服务查找"这一步,`registerApprovalNode` 内部真出错时会以自己的身份抛出,而不再被贴上 +"no automation engine" 的错误标签;`automation` 服务存在但不接受节点执行器的分支从前**一条日志 +都不打**,现在同样 warn。 + +**嵌入式 host 注意**:直接 `new AutomationEngine()` 而不经过 `AutomationServicePlugin` 的宿主, +需要在自己的插件都装好之后调用一次 `sealNodeTypeVocabulary()`,才能拿到这条告警(以及之后的 +即时校验)。 diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index b5b045ed8e..28e90a4392 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -137,7 +137,7 @@ Each node performs a specific action in the flow. | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | `id` | `string` | ✅ | Unique node identifier | -| `type` | `string` | ✅ | Node type — a built-in id from the table above **or** a plugin-registered one. Per ADR-0018 the spec does not gate this with a closed enum; it is checked against the live action registry at `registerFlow()` | +| `type` | `string` | ✅ | Node type — a built-in id from the table above **or** a plugin-registered one. Per ADR-0018 the spec does not gate this with a closed enum; it is checked against the live action registry once that registry is complete — plugins contribute node types while they start, so flows registered during boot are checked in one pass when the vocabulary closes (all plugins started), and anything registered after that (Studio publish, dev reload) is checked immediately. Unknown types warn, never reject; executing one fails with `NO_EXECUTOR` | | `label` | `string` | ✅ | Display label | | `config` | `object` | optional | Type-specific configuration — the registered executor's `configSchema` owns its shape. Keys that schema does not declare are rejected at `registerFlow()`, and the built-in executors `parse()` the value against their Zod contract before running (#4277) | | `connectorConfig` | `object` | optional | `{ connectorId, actionId, input }` for a `connector_action` node | diff --git a/packages/plugins/plugin-approvals/src/approval-node-degradation.test.ts b/packages/plugins/plugin-approvals/src/approval-node-degradation.test.ts new file mode 100644 index 0000000000..792b1d5d53 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/approval-node-degradation.test.ts @@ -0,0 +1,87 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #4771 (second half) — the missing-automation degradation must be LOUD. + * + * `ApprovalsServicePlugin.start()` contributes the ADR-0019 `approval` node + * executor to the flow engine. When there is no engine to contribute it to, + * every approval flow in the deployment is dead on arrival — and that fact was + * logged at `info`, while `os dev` runs at the default `warn` level. The one + * line that mattered was invisible in exactly the deployment where it was true + * (and #4632 already ruled that a silent degradation is a defect, not a style). + * + * The mirror-image assertion matters just as much: when the engine IS present + * the executor is registered and nothing is warned about, because the pair is + * what makes the log line diagnostic rather than decorative. + */ + +import { describe, it, expect } from 'vitest'; +import { ApprovalsServicePlugin } from './approvals-plugin.js'; +import { APPROVAL_NODE_TYPE } from '@objectstack/spec/automation'; + +/** Minimal ObjectQL stand-in — enough for start() to build the service. */ +function fakeObjectql() { + return { + async find() { return []; }, + async insert(_o: string, d: any) { return { ...d }; }, + async update(_o: string, d: any) { return { ...d }; }, + async delete() { return { affected: 0 }; }, + }; +} + +function makeCtx(services: Record) { + const logs = { info: [] as string[], warn: [] as string[] }; + const ctx: any = { + getService: (name: string) => { + if (!(name in services)) throw new Error(`[Kernel] Service '${name}' not found`); + return services[name]; + }, + registerService: () => {}, + logger: { + info: (msg: string) => logs.info.push(msg), + warn: (msg: string) => logs.warn.push(msg), + error: () => {}, + debug: () => {}, + }, + }; + return { ctx, logs }; +} + +describe('ApprovalsServicePlugin — missing automation engine is reported at warn (#4771)', () => { + it('WARNS (not info) and names the consequence when no automation service exists', async () => { + const { ctx, logs } = makeCtx({ objectql: fakeObjectql() }); + await new ApprovalsServicePlugin({ disableAutoHooks: true }).start(ctx); + + // The whole point: visible at the default dev log level. + const warned = logs.warn.filter((m) => m.includes('no automation engine')); + expect(warned).toHaveLength(1); + expect(warned[0]).toContain(APPROVAL_NODE_TYPE); + expect(warned[0]).toMatch(/NOT registered/); + // It carries the remedy, not just the symptom. + expect(warned[0]).toMatch(/@objectstack\/service-automation/); + // …and it is no longer buried under a level dev never prints. + expect(logs.info.some((m) => m.includes('no automation engine'))).toBe(false); + }); + + it('WARNS when an automation service exists but cannot take node executors', async () => { + // A foreign/older `automation` service degrades identically — pre-fix this + // branch logged nothing at all, at any level. + const { ctx, logs } = makeCtx({ objectql: fakeObjectql(), automation: { resume: async () => undefined } }); + await new ApprovalsServicePlugin({ disableAutoHooks: true }).start(ctx); + + expect(logs.warn.filter((m) => m.includes('no automation engine'))).toHaveLength(1); + }); + + it('registers the `approval` executor and says nothing when the engine is present', async () => { + const registered: string[] = []; + const automation = { + registerNodeExecutor: (e: { type: string }) => registered.push(e.type), + resume: async () => undefined, + }; + const { ctx, logs } = makeCtx({ objectql: fakeObjectql(), automation }); + await new ApprovalsServicePlugin({ disableAutoHooks: true }).start(ctx); + + expect(registered).toEqual([APPROVAL_NODE_TYPE]); + expect(logs.warn.filter((m) => m.includes('no automation engine'))).toEqual([]); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/approvals-plugin.ts b/packages/plugins/plugin-approvals/src/approvals-plugin.ts index d51cc8331c..def48c277f 100644 --- a/packages/plugins/plugin-approvals/src/approvals-plugin.ts +++ b/packages/plugins/plugin-approvals/src/approvals-plugin.ts @@ -274,14 +274,30 @@ export class ApprovalsServicePlugin implements Plugin { // present. The node lets a flow suspend on an approval and resume on // decision; the service is wired to the same engine so `decide()` can // resume the suspended run. + // + // #4771 — the degradation must be LOUD (#4632). This used to be one + // try/catch logging at `info`, and dev's default log level is `warn`: the + // one line that says "every `approval` node in this deployment is dead" + // was invisible in exactly the deployment where it is true, while the flow + // registration was warning about `approval` in the deployments where it is + // false. The catch is also narrowed to the service *lookup*, so a genuine + // failure inside registerApprovalNode surfaces as itself instead of being + // relabelled "no automation engine". + let automation: ApprovalAutomationSurface | undefined; try { - const automation = ctx.getService('automation'); - if (automation && typeof automation.registerNodeExecutor === 'function') { - this.service.attachAutomation(automation); - registerApprovalNode(automation, this.service, ctx.logger); - } + automation = ctx.getService('automation'); } catch { - ctx.logger.info('ApprovalsServicePlugin: no automation engine — approval node not registered'); + automation = undefined; // no automation service registered in this stack + } + if (automation && typeof automation.registerNodeExecutor === 'function') { + this.service.attachAutomation(automation); + registerApprovalNode(automation, this.service, ctx.logger); + } else { + ctx.logger.warn( + 'ApprovalsServicePlugin: no automation engine — the `approval` flow node is NOT registered. ' + + 'Every ADR-0019 approval flow in this deployment fails at execution time with NO_EXECUTOR. ' + + 'Add @objectstack/service-automation to the stack to enable them.', + ); } } diff --git a/packages/services/service-automation/src/engine.test.ts b/packages/services/service-automation/src/engine.test.ts index c5206442c8..7dd98b1b09 100644 --- a/packages/services/service-automation/src/engine.test.ts +++ b/packages/services/service-automation/src/engine.test.ts @@ -2316,16 +2316,35 @@ describe('Action Descriptor Registry (ADR-0018)', () => { expect(warnings.some(w => w.includes('send_sms'))).toBe(false); }); - it('registers the flow but warns when a node type has no executor or descriptor', () => { + it('registers the flow and reports the unknown type once the vocabulary is sealed (#4771)', () => { const warnings: string[] = []; const engine = new AutomationEngine(createCapturingLogger(warnings)); // Soft-fail per ADR-0018: register but warn (a temporarily-absent - // plugin should not block flow registration). + // plugin should not block flow registration). The warning is deferred + // to the moment the vocabulary can no longer grow — during boot an + // unknown type means "no plugin has registered it YET" (#4771). expect(() => engine.registerFlow('plugin_node_flow', baseFlow('not_a_real_type'))).not.toThrow(); + expect(warnings.some(w => w.includes('not_a_real_type'))).toBe(false); + + const audit = engine.sealNodeTypeVocabulary(); + expect(audit).toEqual([ + expect.objectContaining({ flowName: 'plugin_node_flow', unknownTypes: ['not_a_real_type'] }), + ]); expect(warnings.some(w => w.includes('not_a_real_type'))).toBe(true); }); + it('warns INLINE for a flow registered after the vocabulary is sealed (#4771)', () => { + const warnings: string[] = []; + const engine = new AutomationEngine(createCapturingLogger(warnings)); + + // Post-boot registration (Studio publish / dev reload) is judged against + // a complete vocabulary, so the assertion is true and immediate. + engine.sealNodeTypeVocabulary(); + engine.registerFlow('plugin_node_flow', baseFlow('not_a_real_type')); + expect(warnings.filter(w => w.includes('not_a_real_type'))).toHaveLength(1); + }); + it('does not warn for the structural start/end node types', () => { const warnings: string[] = []; const engine = new AutomationEngine(createCapturingLogger(warnings)); @@ -2339,9 +2358,46 @@ describe('Action Descriptor Registry (ADR-0018)', () => { ], edges: [{ id: 'e1', source: 'start', target: 'end' }], }); + engine.sealNodeTypeVocabulary(); expect(warnings.filter(w => w.includes('no registered executor'))).toHaveLength(0); }); + it('stays quiet about a DISABLED flow — a flow that cannot run cannot fail (#4771)', () => { + const warnings: string[] = []; + const engine = new AutomationEngine(createCapturingLogger(warnings)); + + // `status: 'obsolete'` unbinds the flow and guards execute(), so + // asserting a run-time failure for it would be the same false claim + // this check was moved to stop making. + engine.registerFlow('retired_flow', { ...baseFlow('not_a_real_type'), status: 'obsolete' }); + expect(engine.sealNodeTypeVocabulary()).toEqual([]); + expect(warnings.filter(w => w.includes('not_a_real_type'))).toHaveLength(0); + }); + + it('seals idempotently — a second seal never re-reports the same finding (#4771)', () => { + const warnings: string[] = []; + const engine = new AutomationEngine(createCapturingLogger(warnings)); + engine.registerFlow('plugin_node_flow', baseFlow('not_a_real_type')); + + expect(engine.sealNodeTypeVocabulary()).toHaveLength(1); + expect(engine.sealNodeTypeVocabulary()).toHaveLength(1); // still reports as STATE… + expect(warnings.filter(w => w.includes('not_a_real_type'))).toHaveLength(1); // …but warns once + }); + + it('says nothing about a type a plugin registered AFTER the flow (the #4771 false alarm)', () => { + const warnings: string[] = []; + const engine = new AutomationEngine(createCapturingLogger(warnings)); + + // Exactly the showcase cold-boot order: flows are pulled first, the + // contributing plugin starts second. Pre-fix this warned "will fail at + // execution time" about a node type that was registered 0.8s later. + engine.registerFlow('plugin_node_flow', baseFlow('approval')); + engine.registerNodeExecutor({ type: 'approval', async execute() { return { success: true }; } }); + + expect(engine.sealNodeTypeVocabulary()).toEqual([]); + expect(warnings.filter(w => w.includes('approval'))).toHaveLength(0); + }); + it('publishes a descriptor into the registry when an executor declares one', () => { const engine = new AutomationEngine(createTestLogger()); engine.registerNodeExecutor({ diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 994968bc8d..0d56bc3bcb 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -537,6 +537,25 @@ export const MAX_PERSISTED_HISTORY_STEPS = 200; */ export type RunSummaryLogLevel = 'info' | 'debug' | 'off'; +/** + * One flow whose node types the (sealed) vocabulary does not cover — the + * ADR-0018 §M1 audit entry produced by + * {@link AutomationEngine.getUnknownNodeTypeAudit}. + * + * Structured rather than a log line because the finding has two consumers with + * different needs: the plugin warns per entry at `kernel:bootstrapped`, and a + * host (CLI startup summary, a health endpoint) reads the state off the engine + * — the same split as {@link AutomationEngine.getTriggerBindingAudit}. + */ +export interface UnknownNodeTypeAuditEntry { + /** Flow that references the unknown type(s). */ + flowName: string; + /** Node `type` values no executor and no action descriptor covers. */ + unknownTypes: string[]; + /** The full vocabulary the audit judged against, for the operator's benefit. */ + knownTypes: string[]; +} + /** Construction options for {@link AutomationEngine}. */ export interface AutomationEngineOptions { /** @@ -1023,6 +1042,15 @@ export class AutomationEngine implements IAutomationService { private flowVersionHistory = new Map>(); private nodeExecutors = new Map(); private actionDescriptors = new Map(); + /** + * Whether the node-type vocabulary can still grow implicitly (#4771). + * `false` for the whole boot — ADR-0018 lets plugins contribute node types + * from their own `init()`/`start()`, so an unknown type seen while flows + * are being pulled means "not registered YET", not "will fail". The host + * flips it via {@link sealNodeTypeVocabulary} once every plugin has + * started; only then is an unknown type a finding worth warning about. + */ + private nodeTypeVocabularySealed = false; private triggers = new Map(); /** * Flows currently wired to a trigger, keyed by flow name → the trigger @@ -1783,12 +1811,13 @@ export class AutomationEngine implements IAutomationService { // Execution takes the parsed shape (schema defaults materialized). const { parsed } = this.canonicalizeStoredFlow(name, definition); - // ADR-0018 §M1 — validate node types against the live action registry. - // The protocol no longer gates `type` with a closed enum; membership is - // checked here instead. Soft-fail (warn, don't throw): a flow authored - // against a plugin that is currently disabled should still register, and - // executeNode() already throws NO_EXECUTOR at run time for unknown types. - this.validateNodeTypes(name, parsed); + // ADR-0018 §M1 — node types are validated against the live action + // registry, but NOT here: the check moved to the moment the vocabulary + // is closed (see the end of this method and {@link + // sealNodeTypeVocabulary}). The protocol no longer gates `type` with a + // closed enum; membership is checked at that seam instead, and stays + // soft-fail — a flow authored against a currently-absent plugin must + // still register, and executeNode() throws NO_EXECUTOR at run time. // #4277 — REJECT config keys the node's descriptor does not declare // (the tightened #4059 warning; a `visibleIf` typo used to register in @@ -1833,6 +1862,25 @@ export class AutomationEngine implements IAutomationService { } this.logger.info(`Flow registered: ${name} (version ${parsed.version})`); + // ADR-0018 §M1 node-type check, inline — but ONLY once the vocabulary + // is closed (#4771). During boot the registry is still filling up + // (plugins contribute executors from their own init()/start(), which + // runs after the flow pull), so a verdict here would judge a world that + // has not finished forming — that is what warned about every showcase + // `approval` flow 0.8s before the `approval` executor existed. The + // authoritative boot pass runs in sealNodeTypeVocabulary(); after it, + // every later registration (Studio publish, dev reload, a runtime + // registerFlow) IS against a complete vocabulary, so it warns at once. + // Placed after the enable/disable resolution above so it can honor the + // same "a flow that cannot run cannot fail" rule as the audit. + if (this.nodeTypeVocabularySealed && this.flowEnabled.get(name) !== false) { + const known = this.knownNodeTypes(); + const unknownTypes = this.unknownNodeTypes(parsed, known); + if (unknownTypes.length > 0) { + this.warnUnknownNodeTypes({ flowName: name, unknownTypes, knownTypes: [...known] }); + } + } + // Re-bind in case the definition changed its trigger, then (re)activate. this.deactivateFlowTrigger(name); if (this.flowEnabled.get(name) !== false) { @@ -3302,36 +3350,108 @@ export class AutomationEngine implements IAutomationService { } /** - * Validate each node's `type` against the live action registry (ADR-0018). + * The node-type vocabulary this engine can validate against (ADR-0018). * A type is known if it is structural (start/end), has a registered - * executor, or has a published action descriptor. Unknown types are - * warned about (not rejected) so flows authored against a temporarily - * absent plugin still register; the runtime surfaces a hard NO_EXECUTOR - * error if such a node is actually executed. + * executor, or has a published action descriptor. + */ + private knownNodeTypes(): Set { + return new Set([ + ...FLOW_STRUCTURAL_NODE_TYPES, + ...this.nodeExecutors.keys(), + ...this.actionDescriptors.keys(), + ]); + } + + /** + * The node types one flow references that the CURRENT vocabulary does not + * cover. Pure — it reports, it never logs; the callers below decide whether + * the answer is authoritative yet. * * Covers nodes inside ADR-0031 regions (#4389). A node in a `loop` body is * as executable as one beside it, so leaving regions out meant the warning * that exists to predict NO_EXECUTOR went quiet on exactly the nodes whose * failure is hardest to place at run time. */ - private validateNodeTypes(flowName: string, flow: FlowParsed): void { - const known = new Set([ - ...FLOW_STRUCTURAL_NODE_TYPES, - ...this.nodeExecutors.keys(), - ...this.actionDescriptors.keys(), - ]); - const unknown = [...new Set( + private unknownNodeTypes(flow: FlowParsed, known: Set): string[] { + return [...new Set( collectFlowGraphs(flow) .flatMap(g => g.nodes.map(n => n.type)) .filter(t => !known.has(t)), )]; - if (unknown.length > 0) { - this.logger.warn( - `Flow '${flowName}' references node type(s) with no registered executor or descriptor: ` + - `${unknown.join(', ')}. They will fail at execution time unless a plugin registers them. ` + - `Registered types: ${[...known].join(', ') || '(none)'}`, - ); + } + + /** + * Unknown-node-type audit over every ENABLED registered flow, against the + * live registry (ADR-0018 §M1). Empty when every node type is covered. + * + * Disabled flows (`status: 'obsolete'`/`'invalid'`, or toggled off) are + * skipped for the same reason the whole check moved: the finding asserts a + * *run-time* failure, and a flow that cannot run cannot fail. Mirrors + * {@link getTriggerBindingAudit}, which skips them too. + * + * Read this only where the vocabulary is complete — see + * {@link sealNodeTypeVocabulary} for why "complete" is a moment in the boot + * sequence and not a property of the engine. + */ + getUnknownNodeTypeAudit(): UnknownNodeTypeAuditEntry[] { + const known = this.knownNodeTypes(); + const audit: UnknownNodeTypeAuditEntry[] = []; + for (const [flowName, flow] of this.flows) { + if (this.flowEnabled.get(flowName) === false) continue; + const unknownTypes = this.unknownNodeTypes(flow, known); + if (unknownTypes.length > 0) { + audit.push({ flowName, unknownTypes, knownTypes: [...known] }); + } } + return audit; + } + + /** + * Declare the node-type vocabulary CLOSED and run the authoritative + * unknown-type audit, warning once per offending flow (#4771). + * + * Why this is a separate act, rather than a check inside `registerFlow`: + * ADR-0018 makes the node vocabulary **open and runtime-extensible** — a + * plugin contributes types via `registerNodeExecutor()` during its own + * `init()`/`start()`. Flows, meanwhile, are registered from the boot pull + * long before the last plugin has started. Validating at registration + * therefore judged a world that had not finished forming: every ADR-0019 + * `approval` flow in the showcase was warned about as "will fail at + * execution time" ~0.8s before `ApprovalsServicePlugin` registered the + * `approval` executor. Eight false alarms per cold boot, phrased as an + * assertion — and a deployment that genuinely lacks the plugin produced + * the identical eight, so the signal could not tell the two apart. + * + * The host calls this once the vocabulary can no longer grow implicitly + * (`AutomationServicePlugin` does it at `kernel:bootstrapped`, strictly + * after every plugin's `start()` and every `kernel:ready` handler). From + * then on `registerFlow` validates inline again — a flow published into a + * running server (Studio publish, dev reload) IS being registered against + * a complete vocabulary, and its unknown types deserve an immediate warn. + * + * Idempotent: only the first call warns (a second one would re-report + * findings whose flows have not changed), and every call returns the + * current audit so a host can surface it its own way — the CLI startup + * summary reads engine state rather than scraping log lines. + */ + sealNodeTypeVocabulary(): UnknownNodeTypeAuditEntry[] { + const alreadySealed = this.nodeTypeVocabularySealed; + this.nodeTypeVocabularySealed = true; + const audit = this.getUnknownNodeTypeAudit(); + if (!alreadySealed) { + for (const entry of audit) this.warnUnknownNodeTypes(entry); + } + return audit; + } + + /** One warning per flow, shared by the boot audit and the post-seal path. */ + private warnUnknownNodeTypes(entry: UnknownNodeTypeAuditEntry): void { + this.logger.warn( + `Flow '${entry.flowName}' references node type(s) with no registered executor or descriptor: ` + + `${entry.unknownTypes.join(', ')}. Every plugin has started, so nothing will register them now — ` + + `these nodes fail at execution time with NO_EXECUTOR. Install/enable the plugin that contributes them. ` + + `Registered types: ${entry.knownTypes.join(', ') || '(none)'}`, + ); } /** diff --git a/packages/services/service-automation/src/flow-node-type-audit.test.ts b/packages/services/service-automation/src/flow-node-type-audit.test.ts new file mode 100644 index 0000000000..2c69feb1b3 --- /dev/null +++ b/packages/services/service-automation/src/flow-node-type-audit.test.ts @@ -0,0 +1,205 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #4771 — the ADR-0018 node-type check must run when the vocabulary is COMPLETE. + * + * Reported symptom: every `pnpm dev` (showcase) cold boot printed eight + * assertions that eight ADR-0019 approval flows "will fail at execution time", + * and all eight were false. `AutomationServicePlugin.start()` pulls the flows + * from the ObjectQL registry and validated each node type immediately; + * `ApprovalsServicePlugin.start()` registers the `approval` executor ~0.8s + * later. The check therefore judged a world that had not finished forming. + * + * The damage is not the noise, it is the loss of the signal: a deployment that + * genuinely lacks the approvals plugin produced the IDENTICAL eight warnings, + * so nothing about the output distinguished "fine" from "broken". Both + * populations are pinned here — the fix is only real if the two boots now say + * different things. + * + * The reproduction condition is asserted, not assumed: each test proves from + * the boot log that the flow was registered BEFORE the executor arrived, which + * is the window the pre-fix check fired in. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import type { Plugin, PluginContext } from '@objectstack/core'; +import { AutomationServicePlugin } from './plugin.js'; +import type { AutomationEngine } from './engine.js'; + +/** The showcase's shape: a flow whose only real node is an ADR-0019 `approval`. */ +const approvalFlow = (name: string) => ({ + name, + label: name, + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'signoff', type: 'approval', label: 'Sign-off', config: { approvers: [{ type: 'user', value: 'u1' }] } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'signoff' }, + { id: 'e2', source: 'signoff', target: 'end' }, + ], +}); + +/** + * Minimal `objectql` stand-in exposing the ONE seam the boot pull reads — + * `registry.listItems('flow')`. This is the path the showcase takes (inline app + * flows land in the registry at init), and the reason flows are registered + * during automation's own `start()`, i.e. before later plugins have started. + */ +function fakeObjectqlPlugin(flows: unknown[]): Plugin { + return { + name: 'fake-objectql', + version: '1.0.0', + async init(ctx: PluginContext) { + (ctx as unknown as { registerService(n: string, s: unknown): void }).registerService('objectql', { + registry: { + listItems: (type: string) => (type === 'flow' ? flows : []), + getObject: () => undefined, + }, + }); + }, + }; +} + +/** + * Stand-in for `ApprovalsServicePlugin`: contributes the `approval` executor + * from `start()`, exactly as the real one does + * (`approvals-plugin.ts` → `registerApprovalNode`). Registered after the + * automation plugin so it starts after it — the real boot order. + */ +function fakeApprovalsPlugin(): Plugin { + return { + name: 'fake-approvals', + version: '1.0.0', + async init() { /* the real plugin registers its schemas here */ }, + async start(ctx: PluginContext) { + const automation = ctx.getService('automation'); + automation.registerNodeExecutor({ + type: 'approval', + async execute() { return { success: true }; }, + }); + }, + }; +} + +/** Boot a kernel over the given flows, with or without the approvals-like plugin. */ +async function boot(opts: { flows: unknown[]; withApprovals: boolean }) { + const stdout: string[] = []; + // `warn`/`info` go to process.stdout (only error/fatal use stderr). + const spy = vi.spyOn(process.stdout, 'write').mockImplementation(((chunk: unknown) => { + stdout.push(String(chunk)); + return true; + }) as never); + const kernel = new LiteKernel(); + kernel.use(fakeObjectqlPlugin(opts.flows)); + kernel.use(new AutomationServicePlugin()); + if (opts.withApprovals) kernel.use(fakeApprovalsPlugin()); + try { + await kernel.bootstrap(); + } finally { + spy.mockRestore(); + } + const engine = kernel.getService('automation'); + const log = stdout.join(''); + return { + kernel, + engine, + log, + /** Node-type warnings emitted anywhere in the boot (the #4771 subject). */ + nodeTypeWarnings: stdout.filter((l) => l.includes('no registered executor or descriptor')), + }; +} + +/** The pre-fix false-alarm window: flow registered first, executor second. */ +function assertFlowRegisteredBeforeExecutor(log: string, flowName: string) { + const flowAt = log.indexOf(`Flow registered: ${flowName}`); + const executorAt = log.indexOf('Node executor registered: approval'); + expect(flowAt, 'the flow must have been registered during boot').toBeGreaterThan(-1); + expect(executorAt, 'the approval executor must have been registered during boot').toBeGreaterThan(-1); + expect( + flowAt, + 'reproduction condition: the flow is registered BEFORE the plugin contributes its node type', + ).toBeLessThan(executorAt); +} + +describe('#4771 — flow node-type validation waits for the plugin-contributed vocabulary', () => { + it('says NOTHING when the contributing plugin starts after the flow was registered', async () => { + const { kernel, engine, log, nodeTypeWarnings } = await boot({ + flows: [approvalFlow('showcase_expense_signoff'), approvalFlow('showcase_budget_approval')], + withApprovals: true, + }); + + // The order that produced the bug really did occur in this boot. + assertFlowRegisteredBeforeExecutor(log, 'showcase_expense_signoff'); + + // Acceptance criterion 1: a deployment WITH the plugin gets zero + // node-type warnings. Pre-fix this was two (eight in the showcase). + expect(nodeTypeWarnings).toEqual([]); + expect(log).not.toMatch(/will fail at execution time/); + expect(engine.getUnknownNodeTypeAudit()).toEqual([]); + + await kernel.shutdown(); + }); + + it('still warns — naming the flow and the type — when NO plugin provides the node', async () => { + const { kernel, engine, log, nodeTypeWarnings } = await boot({ + flows: [approvalFlow('showcase_expense_signoff'), approvalFlow('showcase_budget_approval')], + withApprovals: false, + }); + + // Acceptance criterion 2: the real defect is still reported, per flow, + // and the warning now carries its remedy. + expect(nodeTypeWarnings).toHaveLength(2); + expect(nodeTypeWarnings.join('')).toContain('showcase_expense_signoff'); + expect(nodeTypeWarnings.join('')).toContain('showcase_budget_approval'); + expect(nodeTypeWarnings.join('')).toMatch(/approval/); + expect(log).toMatch(/Install\/enable the plugin/); + + expect(engine.getUnknownNodeTypeAudit()).toEqual([ + expect.objectContaining({ flowName: 'showcase_expense_signoff', unknownTypes: ['approval'] }), + expect.objectContaining({ flowName: 'showcase_budget_approval', unknownTypes: ['approval'] }), + ]); + + await kernel.shutdown(); + }); + + it('the two deployments are DISTINGUISHABLE — the property the pre-fix warning lacked', async () => { + const flows = [approvalFlow('showcase_expense_signoff')]; + const installed = await boot({ flows, withApprovals: true }); + const missing = await boot({ flows, withApprovals: false }); + + // Before the fix both boots printed the identical warning, so the + // signal-to-noise ratio of this check was exactly zero: no operator + // could tell a healthy stack from one missing the approvals plugin. + expect(installed.nodeTypeWarnings.length).toBe(0); + expect(missing.nodeTypeWarnings.length).toBe(1); + expect(installed.nodeTypeWarnings).not.toEqual(missing.nodeTypeWarnings); + + await installed.kernel.shutdown(); + await missing.kernel.shutdown(); + }); + + it('warns immediately for a flow published into the RUNNING server (post-seal)', async () => { + // A Studio publish / dev reload after boot IS registering against a + // complete vocabulary, so deferral would be wrong there: the check goes + // back to inline, which is what makes this a timing fix and not a mute. + const { kernel, engine } = await boot({ flows: [], withApprovals: false }); + + const warnings: string[] = []; + const spy = vi.spyOn(process.stdout, 'write').mockImplementation(((chunk: unknown) => { + warnings.push(String(chunk)); + return true; + }) as never); + try { + engine.registerFlow('published_later', approvalFlow('published_later')); + } finally { + spy.mockRestore(); + } + + expect(warnings.filter((w) => w.includes('no registered executor or descriptor'))).toHaveLength(1); + await kernel.shutdown(); + }); +}); diff --git a/packages/services/service-automation/src/index.ts b/packages/services/service-automation/src/index.ts index a181a8cc7a..d0b997c77b 100644 --- a/packages/services/service-automation/src/index.ts +++ b/packages/services/service-automation/src/index.ts @@ -16,6 +16,7 @@ export type { SuspendedRunStore, RunRecord, StepLogEntry, + UnknownNodeTypeAuditEntry, } from './engine.js'; // Per-run summary (#4354): the fold that turns a run's step log into diff --git a/packages/services/service-automation/src/nested-region-parity.test.ts b/packages/services/service-automation/src/nested-region-parity.test.ts index 73d13c7d88..9fe551323d 100644 --- a/packages/services/service-automation/src/nested-region-parity.test.ts +++ b/packages/services/service-automation/src/nested-region-parity.test.ts @@ -290,7 +290,7 @@ describe('#4389 — registration validators cover region graphs', () => { }); }); - describe('validateNodeTypes (soft-fail)', () => { + describe('unknown node types (soft-fail)', () => { const unknownNode = [{ id: 'x', type: 'no_such_node_type', label: 'X' }]; const warningsFor = (nested: boolean) => { @@ -298,6 +298,11 @@ describe('#4389 — registration validators cover region graphs', () => { const engine = new AutomationEngine(recordingLogger(warnings)); registerLoopNode(engine, ctx()); engine.registerFlow('sweep', flowWith(nested, unknownNode)); + // #4771 — the verdict is delivered once the vocabulary is sealed (during + // boot an unknown type only means "not registered YET"). Region coverage + // is unchanged: the audit walks ADR-0031 regions exactly as the + // registration-time check did. + engine.sealNodeTypeVocabulary(); return warnings.filter(w => w.includes('no registered executor')); }; diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts index 603ef42b11..dd2755b378 100644 --- a/packages/services/service-automation/src/plugin.ts +++ b/packages/services/service-automation/src/plugin.ts @@ -842,6 +842,41 @@ export class AutomationServicePlugin implements Plugin { // startup summary. The warn matters for embedded hosts and tests. ctx.hook('kernel:bootstrapped', async () => { if (!this.engine) return; + + // ── ADR-0018 §M1 node-type audit, at the ONE moment it can be true + // (#4771). The vocabulary is open by design: a plugin contributes + // node types from its own init()/start() — ApprovalsServicePlugin + // registers `approval` in start() — while flows are pulled and + // registered earlier in THIS plugin's start(). Judging types at + // registration therefore warned "will fail at execution time" about + // every ADR-0019 approval flow ~0.8s before the executor that runs + // them showed up: eight false alarms per showcase cold boot, and a + // deployment genuinely missing the plugin emitted the identical + // eight, so the warning could not distinguish the two. + // + // kernel:bootstrapped — not kernel:ready — because the vocabulary + // is only closed after every kernel:ready handler has settled: this + // plugin's own handler registers more flows (syncFlowsFromProtocol), + // and a plugin that starts after us could still contribute an + // executor from its. That is precisely what the kernel documents + // this hook for (reconcile work consuming data produced by a later + // plugin's kernel:ready handler). Sealing also re-arms the inline + // check, so a Studio publish / dev reload into the RUNNING server — + // where the vocabulary really is complete — warns immediately again. + // + // The engine owns the warning text (one wording for this pass and + // for every post-seal registration); this hook only picks the + // moment. It returns the audit so a host can also read the finding + // as state rather than as log lines. + const unknownNodeTypeAudit = this.engine.sealNodeTypeVocabulary(); + if (unknownNodeTypeAudit.length > 0) { + ctx.logger.warn( + `[Automation] ${unknownNodeTypeAudit.length} flow(s) reference node types no installed plugin provides ` + + `(${[...new Set(unknownNodeTypeAudit.flatMap((e) => e.unknownTypes))].join(', ')}) — ` + + `install/enable the plugin that contributes them (e.g. 'approval' ⇐ @objectstack/plugin-approvals).`, + ); + } + const audit = this.engine.getTriggerBindingAudit(); for (const entry of audit) { ctx.logger.warn(