Skip to content

Commit 77034a0

Browse files
committed
fix(automation,approvals): defer flow node-type validation until the plugin vocabulary is closed (#4771)
ADR-0018 makes the flow node-type vocabulary open and runtime-extensible: a plugin contributes types via registerNodeExecutor() from its own init()/start(). AutomationServicePlugin.start() pulls flows from the ObjectQL registry and validated each node type on the spot — ~0.8s before ApprovalsServicePlugin registered the `approval` executor. Every showcase cold boot therefore asserted that eight ADR-0019 approval flows "will fail at execution time", and all eight were false. Worse, a deployment that genuinely lacks the approvals plugin produced the identical eight, so the warning could not distinguish the two. - AutomationEngine.sealNodeTypeVocabulary() declares the vocabulary closed and runs the authoritative audit, warning once per offending flow. Idempotent. - AutomationEngine.getUnknownNodeTypeAudit() exposes the same finding as state (mirrors getTriggerBindingAudit) for hosts that read the engine, not the log. - AutomationServicePlugin seals at kernel:bootstrapped — strictly after every plugin's start() AND every kernel:ready handler (its own registers more flows). - After sealing, registerFlow warns inline again: a Studio publish / dev reload into a running server IS judged against a complete vocabulary. Timing fix, not a mute. - Disabled flows (obsolete/invalid) are skipped: a flow that cannot run cannot fail at run time. Same defect class, second half: ApprovalsServicePlugin logged "no automation engine — approval node not registered" at info while dev defaults to warn, so the real degradation was invisible exactly when it happened (#4632). Now warn, naming consequence and remedy; the catch is narrowed to the service lookup so a failure inside registerApprovalNode surfaces as itself; and the service-exists-but-takes-no-executors branch, which logged nothing at all, warns too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
1 parent 9f601e8 commit 77034a0

9 files changed

Lines changed: 600 additions & 33 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/service-automation": minor
3+
"@objectstack/plugin-approvals": patch
4+
---
5+
6+
fix(automation,approvals): 节点类型校验推迟到插件贡献完成之后 —— approval flow 不再被误报"运行时会失败" (#4771)
7+
8+
showcase 每次冷启都打印 8 条断言:这些 flow "will fail at execution time"。8 条全是假的。
9+
`AutomationServicePlugin.start()` 从 ObjectQL registry 拉起 flow 并**当场**校验节点类型,而
10+
`ApprovalsServicePlugin.start()` 在 0.8 秒后才注册 `approval` 执行器 —— 校验器在词汇表还没
11+
成型的时候就下了结论。
12+
13+
真正的代价不是噪音,是信号丢失:**真的没装 approvals 插件**的部署会得到一模一样的 8 条告警,
14+
所以这条 warn 无法区分"健康"和"坏掉",信噪比为 0。
15+
16+
ADR-0018 明确把节点词汇表定义为**开放、可运行时扩展**的(插件通过
17+
`registerNodeExecutor(type)` 贡献类型)。因此校验只在词汇表**封闭**的那一刻才成立:
18+
19+
- `AutomationEngine.sealNodeTypeVocabulary()` —— 宣告词汇表封闭,对**所有**已注册 flow 跑一次
20+
权威校验,每个有问题的 flow warn 一条。`AutomationServicePlugin``kernel:bootstrapped`
21+
调用它(严格晚于每个插件的 `start()` 和每个 `kernel:ready` handler —— 本插件自己的
22+
`kernel:ready` 还会再注册一批 flow,别的插件也可能在它的 `kernel:ready` 里贡献执行器)。
23+
- `AutomationEngine.getUnknownNodeTypeAudit(): UnknownNodeTypeAuditEntry[]` —— 同一发现的
24+
**状态**形态,供 host(CLI 启动摘要、健康检查)直接读,而不是去 grep 日志。与
25+
`getTriggerBindingAudit()` 同一套路。
26+
- 封闭之后 `registerFlow` **恢复即时告警**:Studio 发布 / dev reload 进正在运行的服务器时,
27+
词汇表确实是完整的,那句断言此时为真。所以这是时序修复,不是把告警静音。
28+
29+
告警文案也随之改成它现在能承诺的事:"Every plugin has started, so nothing will register them
30+
now — these nodes fail at execution time with NO_EXECUTOR",并给出补救动作。
31+
32+
一并修掉同一缺陷类的另一半:`ApprovalsServicePlugin`**拿不到 automation 引擎**时,把
33+
"`approval` 节点没注册"记成 `info` —— 而 dev 的默认日志级别是 `warn`,于是**真降级发生时反而
34+
看不见**(#4632:静默降级必须响亮)。现在是 `warn`,写明后果(该部署里每个 ADR-0019 approval
35+
flow 都会以 NO_EXECUTOR 失败)和补救(装 `@objectstack/service-automation`)。`catch` 同时收窄
36+
到"服务查找"这一步,`registerApprovalNode` 内部真出错时会以自己的身份抛出,而不再被贴上
37+
"no automation engine" 的错误标签;`automation` 服务存在但不接受节点执行器的分支从前**一条日志
38+
都不打**,现在同样 warn。
39+
40+
**嵌入式 host 注意**:直接 `new AutomationEngine()` 而不经过 `AutomationServicePlugin` 的宿主,
41+
需要在自己的插件都装好之后调用一次 `sealNodeTypeVocabulary()`,才能拿到这条告警(以及之后的
42+
即时校验)。
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #4771 (second half) — the missing-automation degradation must be LOUD.
5+
*
6+
* `ApprovalsServicePlugin.start()` contributes the ADR-0019 `approval` node
7+
* executor to the flow engine. When there is no engine to contribute it to,
8+
* every approval flow in the deployment is dead on arrival — and that fact was
9+
* logged at `info`, while `os dev` runs at the default `warn` level. The one
10+
* line that mattered was invisible in exactly the deployment where it was true
11+
* (and #4632 already ruled that a silent degradation is a defect, not a style).
12+
*
13+
* The mirror-image assertion matters just as much: when the engine IS present
14+
* the executor is registered and nothing is warned about, because the pair is
15+
* what makes the log line diagnostic rather than decorative.
16+
*/
17+
18+
import { describe, it, expect } from 'vitest';
19+
import { ApprovalsServicePlugin } from './approvals-plugin.js';
20+
import { APPROVAL_NODE_TYPE } from '@objectstack/spec/automation';
21+
22+
/** Minimal ObjectQL stand-in — enough for start() to build the service. */
23+
function fakeObjectql() {
24+
return {
25+
async find() { return []; },
26+
async insert(_o: string, d: any) { return { ...d }; },
27+
async update(_o: string, d: any) { return { ...d }; },
28+
async delete() { return { affected: 0 }; },
29+
};
30+
}
31+
32+
function makeCtx(services: Record<string, unknown>) {
33+
const logs = { info: [] as string[], warn: [] as string[] };
34+
const ctx: any = {
35+
getService: (name: string) => {
36+
if (!(name in services)) throw new Error(`[Kernel] Service '${name}' not found`);
37+
return services[name];
38+
},
39+
registerService: () => {},
40+
logger: {
41+
info: (msg: string) => logs.info.push(msg),
42+
warn: (msg: string) => logs.warn.push(msg),
43+
error: () => {},
44+
debug: () => {},
45+
},
46+
};
47+
return { ctx, logs };
48+
}
49+
50+
describe('ApprovalsServicePlugin — missing automation engine is reported at warn (#4771)', () => {
51+
it('WARNS (not info) and names the consequence when no automation service exists', async () => {
52+
const { ctx, logs } = makeCtx({ objectql: fakeObjectql() });
53+
await new ApprovalsServicePlugin({ disableAutoHooks: true }).start(ctx);
54+
55+
// The whole point: visible at the default dev log level.
56+
const warned = logs.warn.filter((m) => m.includes('no automation engine'));
57+
expect(warned).toHaveLength(1);
58+
expect(warned[0]).toContain(APPROVAL_NODE_TYPE);
59+
expect(warned[0]).toMatch(/NOT registered/);
60+
// It carries the remedy, not just the symptom.
61+
expect(warned[0]).toMatch(/@objectstack\/service-automation/);
62+
// …and it is no longer buried under a level dev never prints.
63+
expect(logs.info.some((m) => m.includes('no automation engine'))).toBe(false);
64+
});
65+
66+
it('WARNS when an automation service exists but cannot take node executors', async () => {
67+
// A foreign/older `automation` service degrades identically — pre-fix this
68+
// branch logged nothing at all, at any level.
69+
const { ctx, logs } = makeCtx({ objectql: fakeObjectql(), automation: { resume: async () => undefined } });
70+
await new ApprovalsServicePlugin({ disableAutoHooks: true }).start(ctx);
71+
72+
expect(logs.warn.filter((m) => m.includes('no automation engine'))).toHaveLength(1);
73+
});
74+
75+
it('registers the `approval` executor and says nothing when the engine is present', async () => {
76+
const registered: string[] = [];
77+
const automation = {
78+
registerNodeExecutor: (e: { type: string }) => registered.push(e.type),
79+
resume: async () => undefined,
80+
};
81+
const { ctx, logs } = makeCtx({ objectql: fakeObjectql(), automation });
82+
await new ApprovalsServicePlugin({ disableAutoHooks: true }).start(ctx);
83+
84+
expect(registered).toEqual([APPROVAL_NODE_TYPE]);
85+
expect(logs.warn.filter((m) => m.includes('no automation engine'))).toEqual([]);
86+
});
87+
});

packages/plugins/plugin-approvals/src/approvals-plugin.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -274,14 +274,30 @@ export class ApprovalsServicePlugin implements Plugin {
274274
// present. The node lets a flow suspend on an approval and resume on
275275
// decision; the service is wired to the same engine so `decide()` can
276276
// resume the suspended run.
277+
//
278+
// #4771 — the degradation must be LOUD (#4632). This used to be one
279+
// try/catch logging at `info`, and dev's default log level is `warn`: the
280+
// one line that says "every `approval` node in this deployment is dead"
281+
// was invisible in exactly the deployment where it is true, while the flow
282+
// registration was warning about `approval` in the deployments where it is
283+
// false. The catch is also narrowed to the service *lookup*, so a genuine
284+
// failure inside registerApprovalNode surfaces as itself instead of being
285+
// relabelled "no automation engine".
286+
let automation: ApprovalAutomationSurface | undefined;
277287
try {
278-
const automation = ctx.getService<ApprovalAutomationSurface>('automation');
279-
if (automation && typeof automation.registerNodeExecutor === 'function') {
280-
this.service.attachAutomation(automation);
281-
registerApprovalNode(automation, this.service, ctx.logger);
282-
}
288+
automation = ctx.getService<ApprovalAutomationSurface>('automation');
283289
} catch {
284-
ctx.logger.info('ApprovalsServicePlugin: no automation engine — approval node not registered');
290+
automation = undefined; // no automation service registered in this stack
291+
}
292+
if (automation && typeof automation.registerNodeExecutor === 'function') {
293+
this.service.attachAutomation(automation);
294+
registerApprovalNode(automation, this.service, ctx.logger);
295+
} else {
296+
ctx.logger.warn(
297+
'ApprovalsServicePlugin: no automation engine — the `approval` flow node is NOT registered. '
298+
+ 'Every ADR-0019 approval flow in this deployment fails at execution time with NO_EXECUTOR. '
299+
+ 'Add @objectstack/service-automation to the stack to enable them.',
300+
);
285301
}
286302
}
287303

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

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2316,16 +2316,35 @@ describe('Action Descriptor Registry (ADR-0018)', () => {
23162316
expect(warnings.some(w => w.includes('send_sms'))).toBe(false);
23172317
});
23182318

2319-
it('registers the flow but warns when a node type has no executor or descriptor', () => {
2319+
it('registers the flow and reports the unknown type once the vocabulary is sealed (#4771)', () => {
23202320
const warnings: string[] = [];
23212321
const engine = new AutomationEngine(createCapturingLogger(warnings));
23222322

23232323
// Soft-fail per ADR-0018: register but warn (a temporarily-absent
2324-
// plugin should not block flow registration).
2324+
// plugin should not block flow registration). The warning is deferred
2325+
// to the moment the vocabulary can no longer grow — during boot an
2326+
// unknown type means "no plugin has registered it YET" (#4771).
23252327
expect(() => engine.registerFlow('plugin_node_flow', baseFlow('not_a_real_type'))).not.toThrow();
2328+
expect(warnings.some(w => w.includes('not_a_real_type'))).toBe(false);
2329+
2330+
const audit = engine.sealNodeTypeVocabulary();
2331+
expect(audit).toEqual([
2332+
expect.objectContaining({ flowName: 'plugin_node_flow', unknownTypes: ['not_a_real_type'] }),
2333+
]);
23262334
expect(warnings.some(w => w.includes('not_a_real_type'))).toBe(true);
23272335
});
23282336

2337+
it('warns INLINE for a flow registered after the vocabulary is sealed (#4771)', () => {
2338+
const warnings: string[] = [];
2339+
const engine = new AutomationEngine(createCapturingLogger(warnings));
2340+
2341+
// Post-boot registration (Studio publish / dev reload) is judged against
2342+
// a complete vocabulary, so the assertion is true and immediate.
2343+
engine.sealNodeTypeVocabulary();
2344+
engine.registerFlow('plugin_node_flow', baseFlow('not_a_real_type'));
2345+
expect(warnings.filter(w => w.includes('not_a_real_type'))).toHaveLength(1);
2346+
});
2347+
23292348
it('does not warn for the structural start/end node types', () => {
23302349
const warnings: string[] = [];
23312350
const engine = new AutomationEngine(createCapturingLogger(warnings));
@@ -2339,9 +2358,46 @@ describe('Action Descriptor Registry (ADR-0018)', () => {
23392358
],
23402359
edges: [{ id: 'e1', source: 'start', target: 'end' }],
23412360
});
2361+
engine.sealNodeTypeVocabulary();
23422362
expect(warnings.filter(w => w.includes('no registered executor'))).toHaveLength(0);
23432363
});
23442364

2365+
it('stays quiet about a DISABLED flow — a flow that cannot run cannot fail (#4771)', () => {
2366+
const warnings: string[] = [];
2367+
const engine = new AutomationEngine(createCapturingLogger(warnings));
2368+
2369+
// `status: 'obsolete'` unbinds the flow and guards execute(), so
2370+
// asserting a run-time failure for it would be the same false claim
2371+
// this check was moved to stop making.
2372+
engine.registerFlow('retired_flow', { ...baseFlow('not_a_real_type'), status: 'obsolete' });
2373+
expect(engine.sealNodeTypeVocabulary()).toEqual([]);
2374+
expect(warnings.filter(w => w.includes('not_a_real_type'))).toHaveLength(0);
2375+
});
2376+
2377+
it('seals idempotently — a second seal never re-reports the same finding (#4771)', () => {
2378+
const warnings: string[] = [];
2379+
const engine = new AutomationEngine(createCapturingLogger(warnings));
2380+
engine.registerFlow('plugin_node_flow', baseFlow('not_a_real_type'));
2381+
2382+
expect(engine.sealNodeTypeVocabulary()).toHaveLength(1);
2383+
expect(engine.sealNodeTypeVocabulary()).toHaveLength(1); // still reports as STATE…
2384+
expect(warnings.filter(w => w.includes('not_a_real_type'))).toHaveLength(1); // …but warns once
2385+
});
2386+
2387+
it('says nothing about a type a plugin registered AFTER the flow (the #4771 false alarm)', () => {
2388+
const warnings: string[] = [];
2389+
const engine = new AutomationEngine(createCapturingLogger(warnings));
2390+
2391+
// Exactly the showcase cold-boot order: flows are pulled first, the
2392+
// contributing plugin starts second. Pre-fix this warned "will fail at
2393+
// execution time" about a node type that was registered 0.8s later.
2394+
engine.registerFlow('plugin_node_flow', baseFlow('approval'));
2395+
engine.registerNodeExecutor({ type: 'approval', async execute() { return { success: true }; } });
2396+
2397+
expect(engine.sealNodeTypeVocabulary()).toEqual([]);
2398+
expect(warnings.filter(w => w.includes('approval'))).toHaveLength(0);
2399+
});
2400+
23452401
it('publishes a descriptor into the registry when an executor declares one', () => {
23462402
const engine = new AutomationEngine(createTestLogger());
23472403
engine.registerNodeExecutor({

0 commit comments

Comments
 (0)