Skip to content

Commit b508244

Browse files
os-zhuangclaude
andauthored
feat(spec,service-automation,devx): 未声明 resumeAuthority 的 pausing 节点类型不再静默 fail-open (#5561) (#5725)
#3801 的 resume 闸门按「暂停在哪个节点」判权,所以它只覆盖作者记得声明 resumeAuthority 的 pausing 类型。而 `.default('any')` 让这个遗漏根本无法被观测: Zod 在 defineActionDescriptor 里就把键填上,「作者选了 'any'」与「作者从未想过」 产出逐字节相同的描述符。#3823 就是这样发生的 —— ADR-0044 把 revise 边指向通用 `wait`,`wait` 的 'any' 本身正确,而站在 service 所有位置上的那次暂停继承了一个 没人选过的 fail-open 值。 本次落地维护者裁决的第一步(两处探测 + 声明补齐,均非 breaking): - spec: resumeAuthority 去掉 .default('any') 改 .optional() —— 缺失即缺失。 运行时语义零变化,engine 的 `?? 'any'` 原样保留(它本来就在做默认值的事), 所以未声明者今天仍然可被通用路由 resume,只是不再无声。 - service-automation: registerNodeExecutor 对 supportsPause 且未声明 resumeAuthority 的描述符按类型去重打一次响亮 warn,文本写明两个合法值、 并说明显式声明 'any' 即可消音且不改变任何行为(避免把本就开放的节点逼向 'service')。screen / wait / subflow / map 四个 builtin 补上显式 resumeAuthority: 'any' —— 它们各自语义本就正确,只是此前在继承而非声明, 所以告警今天零点名,只捕未来的遗漏。 - devx: 新增 check:resume-authority-declared 静态门(AST 扫 defineActionDescriptor 字面量),仓内 pausing 描述符漏声明直接 CI 红。 #3823 是我们自己仓内的漏声明,PR 时刻才是该告诉作者的时刻;只扫发货源码, 测试夹具故意保留未声明形状以便测告警本身。 第二步(缺失即 fail-closed)仍挂 #5561 等 breaking 窗口 —— 经此改动后它收缩成 resolveResumeAuthority 里那一个表达式,而不再需要 schema 迁移。 supportsPause 自身是运行时零强制的声明(#5703):暂停的事实来自 execute() 返回 suspend: true。声明它的执行器漏声明 supportsPause 时,告警与静态门都看不见 —— 这一点在门的报错文本、告警的 TSDoc 与 spec 字段注释里都写明了。 Refs #5561, #3801, #3823, #3853, #5703; ADR-0044 修正案。 Claude-Session: https://claude.ai/code/session_01BWS4heBoAitLmzCLhcYdbK Co-authored-by: Claude <noreply@anthropic.com>
1 parent e6db317 commit b508244

15 files changed

Lines changed: 860 additions & 11 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
automation: `ActionDescriptor.resumeAuthority` no longer defaults to `'any'` — an
6+
omission is now a distinct, reportable fact (#5561, from #3823)
7+
8+
The #3801 resume gate keys on the suspended node, so it covers a pausing node type
9+
exactly when that type's author declared who may resume it. The schema default made
10+
that impossible to check: Zod filled the key inside `defineActionDescriptor`, so
11+
"the author chose `'any'`" and "the author never considered it" produced
12+
byte-identical descriptors. #3823 is what the erasure cost — ADR-0044 pointed an
13+
approval's revise edge at a generic `wait`, `wait` is legitimately `'any'`, and the
14+
pause standing in a service-owned position inherited a fail-open value nobody chose.
15+
16+
The field is now optional with no default, and absent means absent. Two seams read
17+
it: `AutomationEngine.registerNodeExecutor` warns once per node type when a
18+
`supportsPause` descriptor omits it, and the new `check:resume-authority-declared`
19+
gate fails CI on an omission in this repo's own executors.
20+
21+
**Not a behaviour change.** The engine already resolved the value with `?? 'any'`,
22+
so an undeclared pausing type is still raw-resumable exactly as before — loudly now
23+
instead of silently. Nothing needs migrating: an executor that declared
24+
`resumeAuthority` keeps its value, and one that omitted it keeps today's semantics
25+
and gains a warning telling it to state its intent. Making omission mean
26+
*fail-closed* is a breaking change still tracked on #5561 for a version window that
27+
allows it; it is now a one-expression change rather than a schema migration.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
automation: a pausing node type that never declares `resumeAuthority` is now named
6+
at registration, and the four pausing built-ins declare theirs (#5561)
7+
8+
`registerNodeExecutor` warns once per node type (per engine instance) when a
9+
descriptor declares `supportsPause: true` and omits `resumeAuthority` — the state in
10+
which the #3801 resume gate silently treats every pause that type creates as
11+
raw-resumable through the generic resume route. The line names the two legal values
12+
and says that declaring `'any'` explicitly silences it and changes no behaviour, so
13+
a node whose pause really is open to the route is not pushed toward `'service'` to
14+
quieten a log.
15+
16+
`screen`, `wait`, `subflow` and `map` now declare `resumeAuthority: 'any'`
17+
explicitly. Each was already correct on its own terms — it was inheriting the value
18+
rather than stating it — so the warning names nothing on a stock boot today and only
19+
catches future omissions. Authority resolution is unchanged: `resolveResumeAuthority`
20+
still resolves an absent value to `'any'`.

.github/workflows/lint.yml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,24 @@ jobs:
381381
- name: Engine test-double contract gate
382382
run: pnpm check:engine-double-contract
383383

384+
# Resume-authority declaration gate (#5561, from #3823). The #3801 resume
385+
# gate keys on the SUSPENDED NODE, so it covers a pausing node type exactly
386+
# when that type's author remembered to declare `resumeAuthority`. #3823 is
387+
# what forgetting costs: ADR-0044 pointed a revise edge at a generic `wait`,
388+
# `wait` is legitimately 'any', and a pause standing in a service-owned
389+
# position inherited that value with nobody choosing it — an unaudited
390+
# resubmit plus a destroyed remote run. Until #5561 removed the schema
391+
# default the omission was not even observable (Zod filled the key, so it
392+
# parsed byte-identically to an explicit 'any'); now a pausing descriptor
393+
# that never states its authority cannot merge. Shipped sources only —
394+
# fixtures deliberately construct the omission to test the engine's
395+
# registration warning. Static AST, no build needed, so it belongs in this
396+
# job. Runs its own --self-test first: the detector can be broken while
397+
# every descriptor is fine, and a scan that stops matching would report OK
398+
# while reading nothing (#4868's family).
399+
- name: Resume-authority declaration gate
400+
run: pnpm check:resume-authority-declared
401+
384402
typecheck:
385403
name: TypeScript Type Check
386404
runs-on: ubuntu-latest

content/docs/references/automation/node-executor.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ Canonical cross-paradigm action/node descriptor (ADR-0018)
7777
| **needsOutbox** | `boolean` || Dispatch via service-messaging outbox (retry/idempotency/dead-letter) |
7878
| **isAsync** | `boolean` || Suspends the flow awaiting an external reply |
7979
| **handlerContract** | `Enum<'none' \| 'pure'>` || Effect contract for author-supplied code this action invokes: 'none' (invokes none) or 'pure' (must not write — it returns a value and the flow graph persists it) |
80-
| **resumeAuthority** | `Enum<'any' \| 'service'>` | | Who may resume a run this node suspended: 'any' (the generic resume route) or 'service' (only the owning service, e.g. approvals) |
80+
| **resumeAuthority** | `Enum<'any' \| 'service'>` | optional | Who may resume a run this node suspended: 'any' (the generic resume route) or 'service' (only the owning service, e.g. approvals). Deliberately has no default — an omission is a distinct, reportable fact, and a pausing node type that omits it is warned about at registration (#5561) |
8181
| **maturity** | `Enum<'ga' \| 'beta' \| 'reserved'>` || Runtime maturity: ga (shipped), beta, or reserved (contract only — designers grey this out) |
8282
| **source** | `Enum<'builtin' \| 'plugin'>` || builtin = platform baseline; plugin = third-party contributed |
8383
| **deprecated** | `boolean` || Deprecated alias kept for back-compat |

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
"check:type-check-coverage": "node scripts/check-type-check-coverage.mjs --self-test && node scripts/check-type-check-coverage.mjs",
6161
"check:driver-conformance": "node scripts/check-driver-conformance.mjs --self-test && node scripts/check-driver-conformance.mjs",
6262
"check:engine-double-contract": "node scripts/check-engine-double-contract.mjs --self-test && node scripts/check-engine-double-contract.mjs",
63+
"check:resume-authority-declared": "node scripts/check-resume-authority-declared.mjs --self-test && node scripts/check-resume-authority-declared.mjs",
6364
"check:stall-guard": "node scripts/run-with-stall-guard.mjs --self-test"
6465
},
6566
"keywords": [

packages/services/service-automation/src/builtin/map-node.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v
5252
// Each item's subflow may pause, so the map suspends and resumes per item.
5353
supportsPause: true,
5454
isAsync: true,
55+
// As with `subflow`, `'any'` here is not the authority that applies: the
56+
// #3801 gate follows the `map:` correlation to the in-flight item's child
57+
// run and judges that node instead — judging the loop rather than the item
58+
// is precisely the hole #3853 closed. Stated rather than inherited (#5561).
59+
resumeAuthority: 'any',
5560
// Structured config form for the flow designer (ADR-0018). Mirrors the
5661
// objectui hardcoded `map` field group field-for-field, so the online
5762
// (schema-driven) form matches the offline one (objectui #2670 Phase 3 /

packages/services/service-automation/src/builtin/screen-nodes.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext
4848
icon: 'window', category: 'human', source: 'builtin',
4949
// Human-input nodes suspend the flow awaiting input.
5050
supportsPause: true, isAsync: true,
51+
// The generic resume route IS this node's intended door: the flow-runner
52+
// collects the inputs and hands them back as the continuation, so there
53+
// is no service decision to route around (#3801). Stated rather than
54+
// inherited from a default — an omission is now a reported fact (#5561).
55+
resumeAuthority: 'any',
5156
// Designer form (ADR-0018, #3304) — mirrors objectui's hardcoded `screen`
5257
// field group: flat input list OR an object form, plus title/description.
5358
// `visibleWhen` is bare CEL (xExpression), `defaults` a free-form keyValue

packages/services/service-automation/src/builtin/subflow-node.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ export function registerSubflowNode(engine: AutomationEngine, ctx: PluginContext
5252
// A child that suspends (approval/screen/wait) suspends this node too —
5353
// the parent run pauses here and resumes when the child completes.
5454
supportsPause: true,
55+
// `'any'` on this node is not the authority that ends up applying: the
56+
// #3801 gate follows the `subflow:` correlation down to the CHILD and
57+
// judges the node the signal actually lands on (#3853), so a parent parked
58+
// above a pending approval is still refused. Stated rather than inherited
59+
// so the omission-warning's silence here is a decision, not a gap (#5561).
60+
resumeAuthority: 'any',
5561
}),
5662
async execute(node, variables, context) {
5763
// #4343 — the contract is parsed before anything runs, the same seam the

packages/services/service-automation/src/builtin/wait-node.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,12 @@ export function registerWaitNode(engine: AutomationEngine, ctx: PluginContext):
165165
// Durable pause — the run suspends and resumes later (timer/signal).
166166
supportsPause: true,
167167
isAsync: true,
168+
// An external producer is *meant* to resume a signal wait, so the generic
169+
// route is the door (#3801). Stated rather than inherited from a default:
170+
// #3823 is what inheriting it costs — ADR-0044 pointed a revise edge at a
171+
// generic `wait`, and the pause in that service-owned position took this
172+
// value without anyone choosing it (#5561).
173+
resumeAuthority: 'any',
168174
}),
169175
async execute(node, variables, _context) {
170176
// `waitEventConfig` is the whole contract (`FlowNodeSchema`, flow.zod.ts).

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

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1138,6 +1138,15 @@ export class AutomationEngine implements IAutomationService {
11381138
* and a module-level flag would report only whichever engine ran first.
11391139
*/
11401140
private nodeTypeSealOmissionWarned = false;
1141+
/**
1142+
* Node types already named by {@link warnIfResumeAuthorityUndeclared}
1143+
* (#5561). Per **instance** and per **type**, for the same reason
1144+
* {@link nodeTypeSealOmissionWarned} is per instance: a hot-reload or a
1145+
* multi-tenant host re-registers the same executor repeatedly, and one
1146+
* omission must read as one finding rather than as a log that grows with
1147+
* uptime.
1148+
*/
1149+
private readonly resumeAuthorityOmissionWarned = new Set<string>();
11411150
private triggers = new Map<string, FlowTrigger>();
11421151
/**
11431152
* Flows currently wired to a trigger, keyed by flow name → the trigger
@@ -1334,11 +1343,60 @@ export class AutomationEngine implements IAutomationService {
13341343
);
13351344
}
13361345
this.actionDescriptors.set(descriptorType, executor.descriptor);
1346+
this.warnIfResumeAuthorityUndeclared(executor.descriptor);
13371347
}
13381348

13391349
this.logger.info(`Node executor registered: ${executor.type}`);
13401350
}
13411351

1352+
/**
1353+
* Name a pausing node type that never declared WHO may resume the pauses it
1354+
* creates (#5561, the tracking item ADR-0044's amendment deferred).
1355+
*
1356+
* `resumeAuthority` carries no schema default precisely so this warning can
1357+
* exist: with `.default('any')` an omission parsed into a descriptor
1358+
* byte-identical to an author's explicit `'any'`, so the fact was gone
1359+
* before the engine ever saw the object. Absent now means absent, and a
1360+
* pausing type that leaves it absent is fail-open by omission rather than
1361+
* by decision — #3823 is what that costs (a revise pause standing in a
1362+
* service-owned position inherited `wait`'s legitimate `'any'`, and a raw
1363+
* resume walked past an unrecorded decision).
1364+
*
1365+
* **What it asserts, and why that is safe here.** Only the static fact that
1366+
* THIS descriptor omits the key — a property of the object being registered,
1367+
* fixed at authoring time, which no later registration can contradict. It
1368+
* reads no registry and draws no conclusion from anything being absent from
1369+
* one, so it is not the shape AGENTS.md "Startup registry reads" forbids and
1370+
* needs no seal flag (contrast {@link warnIfNodeTypeVocabularyNeverSealed},
1371+
* which reports a missing CALL for the same reason). Whether the omission
1372+
* *matters* at run time is deliberately not judged: the engine still
1373+
* resolves absent to `'any'` ({@link resolveResumeAuthority}), so nothing
1374+
* about today's behaviour changes.
1375+
*
1376+
* **Blind spot, stated up front:** the trigger is `supportsPause`, itself a
1377+
* declaration no execution path enforces (#5703) — a run pauses because
1378+
* `execute()` returned `suspend: true`. An executor that suspends while
1379+
* leaving `supportsPause` false is therefore fail-open AND silent here.
1380+
* `check:resume-authority-declared` catches this repo's own executors at
1381+
* authoring time; #5703 tracks the runtime half.
1382+
*/
1383+
private warnIfResumeAuthorityUndeclared(descriptor: ActionDescriptor): void {
1384+
if (descriptor.supportsPause !== true) return;
1385+
if (descriptor.resumeAuthority !== undefined) return;
1386+
if (this.resumeAuthorityOmissionWarned.has(descriptor.type)) return;
1387+
this.resumeAuthorityOmissionWarned.add(descriptor.type);
1388+
this.logger.warn(
1389+
`[automation] node type '${descriptor.type}' declares supportsPause but never declares ` +
1390+
`resumeAuthority, so the #3801 resume gate treats every pause it creates as raw-resumable ` +
1391+
`through the generic route (POST /automation/:name/runs/:runId/resume) — fail-open by omission ` +
1392+
`rather than by decision, which is how #3823 walked past an unrecorded approval decision. ` +
1393+
`Declare it on the descriptor: 'any' if that route IS the intended door (a screen's collected ` +
1394+
`inputs, a signal wait's external producer), or 'service' if resuming is the tail of a decision ` +
1395+
`some service must authorize and record first. Declaring 'any' explicitly silences this and ` +
1396+
`changes no behaviour. Reported once per node type per engine.`,
1397+
);
1398+
}
1399+
13421400
/**
13431401
* Register a **deprecated alias** of a canonical node type (ADR-0018 M3).
13441402
*
@@ -2704,8 +2762,16 @@ export class AutomationEngine implements IAutomationService {
27042762
* snapshotting at alias-registration time) also keeps it correct whichever
27052763
* order the two register in. No alias of a pausing type exists today; this
27062764
* keeps it from becoming a hole the day one does.
2765+
*
2766+
* The `?? 'any'` is load-bearing in a second way since #5561: with no schema
2767+
* default on `resumeAuthority`, an undeclared descriptor arrives with the key
2768+
* absent and this is the one place that resolves it. It resolves fail-OPEN,
2769+
* exactly as the removed default did — step one of #5561 changed nothing
2770+
* here, it only made the omission audible at registration. Flipping this
2771+
* fallback to `'service'` is the breaking half still tracked on #5561, and
2772+
* it is this single expression.
27072773
*/
2708-
private resolveResumeAuthority(nodeType: string): ActionDescriptor['resumeAuthority'] {
2774+
private resolveResumeAuthority(nodeType: string): NonNullable<ActionDescriptor['resumeAuthority']> {
27092775
let descriptor = this.actionDescriptors.get(nodeType);
27102776
for (let hop = 0; descriptor?.aliasOf && hop < AutomationEngine.MAX_ALIAS_HOPS; hop++) {
27112777
const canonical = this.actionDescriptors.get(descriptor.aliasOf);

0 commit comments

Comments
 (0)