Skip to content

Commit 25784cf

Browse files
os-zhuangclaude
andauthored
fix(automation,approvals): 节点类型校验推迟到插件词汇表封闭之后 —— approval flow 不再被误报「运行时会失败」 (#4791)
* 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 * docs(automation): flows.mdx 的节点 `type` 校验时机改为两阶段描述 (#4771) 表格原文说节点 type「is checked against the live action registry at `registerFlow()`」。本 PR 之后这句不再为真:启动期注册的 flow 在词汇表封闭 (所有插件 start() 完毕)时统一校验,封闭之后的注册才回到即时校验。顺带写明 这条检查是 warn 而非拒绝,以及执行时的 NO_EXECUTOR 后果。 同文件其余提到 `registerFlow()` 的句子(config 键、条件表达式、ADR-0031 区域) 描述的是没有改动的校验,保持原样。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f2eb850 commit 25784cf

10 files changed

Lines changed: 601 additions & 34 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+
即时校验)。

content/docs/automation/flows.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ Each node performs a specific action in the flow.
137137
| Property | Type | Required | Description |
138138
| :--- | :--- | :--- | :--- |
139139
| `id` | `string` || Unique node identifier |
140-
| `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()` |
140+
| `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` |
141141
| `label` | `string` || Display label |
142142
| `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) |
143143
| `connectorConfig` | `object` | optional | `{ connectorId, actionId, input }` for a `connector_action` node |
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)