Skip to content

fix(approvals): the ADR-0044 revise window is a service-owned node type (#3823) - #5560

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-3823-revise-pause-typed
Aug 5, 2026
Merged

fix(approvals): the ADR-0044 revise window is a service-owned node type (#3823)#5560
os-zhuang merged 2 commits into
mainfrom
claude/issue-3823-revise-pause-typed

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#3823

维护者已批准方案 A(2026-08-05 裁决评论):把 revise 暂停改为 typed / service-owned,使 #3822 既有的 resumeAuthority 类型门直接覆盖它,零新机制。本 PR 按「专用节点类型」落地。

前提复核(先于实现)

在当前 origin/main(229d29e)上按 2026-07-28 实证评论重写了 repro(真引擎 + 真 ApprovalService),三段实录完全复现,前提成立:

[A] resume code: undefined ← 未被拒绝(wait 是 resumeAuthority 'any')
[A] round-2 request opened: areq_2bb1… | round1: areq_8ef9…
[A] audit trail of round 1: [ 'submit', 'revise' ] ← 永远没有 'resubmit' 行
[B] service.resubmit refusal: DUPLICATE_REQUEST: another approval request is already pending on fin_expense/x1
[B] suspended after service refusal: 1 ← 服务在消费 suspension 之前就拒绝了
[B] raw resume result: {"success":false,"error":"Node 'review' failed: … DUPLICATE_REQUEST …"}
[B] suspended runs after raw resume: 0 ← suspension 已被消费,run 永久死亡
[B] round-1 status: returned ← 审批不可再解

repro 是临时文件,已删除;它的两个断言以正式用例的形式留在 approval-revise.test.ts

形状与理由

  • APPROVAL_REVISE_NODE_TYPE(approval_revise),在 spec/automation/approval.zod.ts 声明,由 plugin-approvals 与 approval 节点同一处注册(registerApprovalNode 内调用),descriptor 声明 resumeAuthority: 'service' / supportsPause / isAsync / category: 'human'引擎一行未改 —— 门本来就按「暂停节点的注册类型」判定,这正是裁决的验收标准。
  • 不带 config:窗口只是图上的位置,没有 signal 也没有 timer,所以不声明 configSchema(与 wait / subflow / decision 同为 schemaless),不凭空造一个没有读者的可编写面。executor 入场不武装任何东西,因此故意没有配 onSuspensionReleased(与 wait 的 timer 一次性钩子相反,wait 定时唤醒 job 在 resume 没能消费掉暂停时也会自我取消 —— store 短暂不可达即丢掉这一次唤醒,run 挂到下次重启才被捞回 #5529 的先例在此不适用),代码里写明了这一点。
  • 为什么不是「approval 节点自挂起」:那样会跳过作者的 revise 边 —— 该分支上的节点(notify、状态更新)不再执行,窗口也从画布与 run log 上消失,而这正是 ADR-0044 当初选择通用 wait 要的性质。错的只是复用
  • 两道拒绝,都带处方:ApprovalService.sendBack任何写入之前拒绝 revise 边指向非 approval_revise 的图(与既有「无 revise 边」检查同一处);flow-approval-revise-target-not-service-owned(@objectstack/lint,severity error)在编写期拒绝 —— 走的是 lintFlowPatterns 这条已经接好的注册项(gating、三个 CLI 命令 + runtime publish gate),所以连 lint 的接线都没有新增。该 rule 升到 error 符合该模块自陈的门槛(「运行时会拒绝」),理由写在 rule 旁。

约束 1:approve → screen 的 UI 推进不受影响

resumeAuthority 只挂在 revise 窗口这一个类型上,screen / wait 一律不动。新增用例 leaves an approve-branch screen resumable through the generic route screen executor 钉住:approval → approve → screen,decide 后 run 停在 collect,随后 automation.resume(runId, { variables: … })(无 service marker)成功推进到终点。同一用例里也断言 wait 的 descriptor 仍不是 'service'

约束 2:向后兼容(明确写出)

按现行 D3 写好的存量 flow(revise 边 → 作者放置的普通 wait):

  • 继续注册、继续运行,审批照旧可决(approve / reject / recall / reassign 全不受影响);
  • 变化的只有 send-back 被拒绝,报文点名节点、当前类型与一处修复(type: 'wait'type: 'approval_revise',并删掉 waitEventConfig);重新发布该 flow 会报 lint error。用例 refuses send-back into a bare wait node before anything mutates 钉住「拒绝发生在任何写入之前」:请求仍是 pending,只有 submit 一行审计,run 仍停在 approval 节点。
  • 升级前就已经停在旧窗口里的 run:SuspendedRun.nodeType 是暂停时刻记录、读取时 recorded-first(fix(automation,approvals): gate the generic run-resume route on the suspended node (#3801) #3822 有意如此,避免 republish 把活 run 的节点改型),所以这些 run 保持原样,由 resubmitrecall 正常排空 —— 不做追溯改型。

为什么不加 ADR-0087 D2 conversion(考虑过并否决,ADR 里记了):它不会是无损改名,而是依赖拓扑的语义重写 —— timer 风味的 wait 会静默丢掉 timer,被另一条入边共用的 wait 会连那条路径一起变成 service-only,这两种破坏 conversion 看不见。可测量的存量也指向同一结论:Studio designer 还不能画 revise 边(ADR-0044 自己的 follow-up),cloud 仓库没有任何 revise flow,本仓库唯一一个是 showcase(本 PR 一并迁移)。一处响亮的拒绝 + 一个 token 的修复,胜过日后还要退休的容忍层 —— 对读诊断信息的 AI 作者尤其如此。

一处有意的收窄:revise 边的直接目标必须是窗口。想写 revise → notify → 窗口 的图会被拒绝,而不是去做「该分支上所有可达暂停都是 service-owned」的不定边界分析;send-back 本身已经通知提交人,这个写法背后没有丢失的能力。

测试

pnpm --filter @objectstack/plugin-approvals test → Test Files 19 passed (19) Tests 446 passed (446)
pnpm --filter @objectstack/lint test → Test Files 59 passed (59) Tests 1287 passed (1287)
pnpm --filter @objectstack/service-automation test → Test Files 57 passed (57) Tests 695 passed (695)
pnpm --filter @objectstack/spec test → Test Files 314 passed (314) Tests 8010 passed (8010)
typecheck (spec / plugin-approvals / lint) → 全部 Done
pnpm --filter @objectstack/spec check:generated → api-surface 因新增导出而 stale,已 gen:api-surface 并提交;其余 9 项 ✓
node scripts/check-nul-bytes.mjs / check-adr-anchors.mjs → OK
eslint(改动的源文件) → 无输出

反向验证(方向先定后跑):把 descriptor 的 resumeAuthority'service' 改回 'any'(只改这一处,保持 flow 仍用新类型,以隔离「类型门」这一条论断),预期新增的门用例转红 —— 结果如预期:

× declares resumeAuthority: service on the revise-window node type
× refuses a raw resume of a run parked in the revise window
expected { success: true, … } to match object { success: false, code: 'PERMISSION_DENIED' }
× a raw resume can no longer destroy the run when a pending request collides
Tests 3 failed | 15 passed (18)

第一条是 descriptor 断言(与 flow 无关),另两条正是实证评论里的行为回归 —— raw resume 重新成功。其余用例(legacy-wait 拒绝、screen 推进)自带各自的 flow,不受该反转影响,保持绿色,这点如实记录而非硬凑成「全红」。

另外把迁移后的 showcase flow 直接喂给 lint 验证:lintFlowPatterns 零 finding(另有两条既有的 approval-approvers-may-resolve-empty info,与本改动无关)。

packages/spec/src/automation/approval.zod.ts(常量 + APPROVAL_BRANCH_LABELS.revise 文档)、packages/plugins/plugin-approvals/(新 approval-revise-node.tsapproval-node.ts 注册、approval-service.tsassertReviseEdge、index 导出)、packages/lint/(rule + 导出 + 测试)、examples/app-showcase(迁移)、skills/objectstack-automation/(SKILL.md 与 revise-loop eval)、content/docs/automation/approvals.mdxdocs/adr/0044-*.md(amendment 增补 Implementation 段:落地形状、否决 conversion 的理由、向后兼容),changeset 覆盖 spec / plugin-approvals / lint(均 minor)。

同包排队中的 #5048 / #4792 的面未触碰;service-automation 一个字节未改(引擎侧零改动本身就是裁决的验收标准)。

packages/spec/authorable-surface.base.json 在本地 gen:schema 运行时会把 baseRev 前推并补 3 个与本单无关的 EmailServiceConfig key —— 已还原,不夹带进本 PR(其 gate 通过,只提示 "trails the merge base by 3 key(s)")。


🤖 Generated with Claude Code

https://claude.ai/code/session_01BWS4heBoAitLmzCLhcYdbK


Generated by Claude Code

…pe (#3823)
Send-back parked the run on an ordinary `wait` node the flow author placed.
`wait` is `resumeAuthority: 'any'` — correctly, for a signal wait — so the
#3801 type-keyed resume gate could not see that the pause was service-owned:
a raw `POST /automation/:name/runs/:runId/resume` with an empty body walked
the resubmit back-edge into the approval node with no submitter check and no
`resubmit` audit row, and when a request was already pending on the record it
consumed the suspension before the re-entry failed — destroying the run.
The revise pause becomes its own node type instead:
- `APPROVAL_REVISE_NODE_TYPE` (`approval_revise`), registered by
plugin-approvals alongside the `approval` node, declaring
`resumeAuthority: 'service'`. No engine change: the existing gate covers any
node type that declares service ownership. No config — the window ends on
the submitter's resubmit, never on a signal or timer.
- `ApprovalService.sendBack` refuses a `revise` edge whose target is not that
node type, before any mutation.
- `flow-approval-revise-target-not-service-owned` (severity `error`, via the
already-wired `lintFlowPatterns` entry) rejects the old shape at authoring
time — CLI commands and the runtime metadata publish gate.
ADR-0044's amendment gains an implementation section: what shipped, why the
approval node does not re-suspend itself, why no ADR-0087 conversion was added
for the legacy shape, and what upgrading a legacy flow costs (one token).
Showcase, skill guidance, skill eval and docs migrated with it.
Fixes#3823
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BWS4heBoAitLmzCLhcYdbK
@vercel

vercelBot commented Aug 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 5, 2026 6:20pm

Request Review

@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 5, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/lint, @objectstack/plugin-approvals, @objectstack/spec.

109 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/ai/agents.mdx(via @objectstack/spec)
  • content/docs/ai/skills-reference.mdx(via @objectstack/spec)
  • content/docs/ai/skills.mdx(via @objectstack/spec)
  • content/docs/api/client-sdk.mdx(via @objectstack/spec)
  • content/docs/api/environment-routing.mdx(via @objectstack/spec)
  • content/docs/api/error-catalog.mdx(via @objectstack/spec)
  • content/docs/api/error-handling-client.mdx(via @objectstack/spec)
  • content/docs/api/error-handling-server.mdx(via @objectstack/spec)
  • content/docs/api/index.mdx(via @objectstack/spec)
  • content/docs/automation/approvals.mdx(via @objectstack/plugin-approvals, @objectstack/spec)
  • content/docs/automation/connectors.mdx(via @objectstack/spec)
  • content/docs/automation/flows.mdx(via @objectstack/spec)
  • content/docs/automation/hook-bodies.mdx(via @objectstack/lint, packages/spec)
  • content/docs/automation/hooks.mdx(via @objectstack/spec)
  • content/docs/automation/index.mdx(via @objectstack/spec)
  • content/docs/automation/webhooks.mdx(via @objectstack/spec)
  • content/docs/automation/workflows.mdx(via @objectstack/spec)
  • content/docs/concepts/architecture.mdx(via @objectstack/spec)
  • content/docs/concepts/design-principles.mdx(via packages/spec)
  • content/docs/concepts/index.mdx(via @objectstack/spec)
  • content/docs/concepts/metadata-driven.mdx(via @objectstack/spec)
  • content/docs/concepts/metadata-lifecycle.mdx(via packages/spec)
  • content/docs/concepts/north-star.mdx(via @objectstack/spec)
  • content/docs/data-modeling/analytics.mdx(via @objectstack/spec)
  • content/docs/data-modeling/drivers.mdx(via @objectstack/spec)
  • content/docs/data-modeling/external-datasources.mdx(via @objectstack/spec)
  • content/docs/data-modeling/field-types.mdx(via @objectstack/spec)
  • content/docs/data-modeling/fields.mdx(via @objectstack/spec)
  • content/docs/data-modeling/formulas.mdx(via @objectstack/spec)
  • content/docs/data-modeling/index.mdx(via @objectstack/spec)
  • content/docs/data-modeling/objects.mdx(via @objectstack/spec)
  • content/docs/data-modeling/queries.mdx(via @objectstack/spec)
  • content/docs/data-modeling/schema-design.mdx(via @objectstack/spec)
  • content/docs/data-modeling/seed-data.mdx(via @objectstack/spec)
  • content/docs/data-modeling/validation-rules.mdx(via @objectstack/spec)
  • content/docs/data-modeling/validation.mdx(via @objectstack/spec)
  • content/docs/deployment/cli.mdx(via @objectstack/spec)
  • content/docs/deployment/tenancy-modes.mdx(via @objectstack/spec)
  • content/docs/deployment/troubleshooting.mdx(via @objectstack/spec)
  • content/docs/deployment/validating-metadata.mdx(via @objectstack/spec)
  • content/docs/getting-started/build-with-claude-code.mdx(via @objectstack/spec)
  • content/docs/getting-started/common-patterns.mdx(via @objectstack/spec)
  • content/docs/getting-started/examples.mdx(via @objectstack/spec)
  • content/docs/getting-started/quick-reference.mdx(via @objectstack/spec)
  • content/docs/getting-started/quick-start.mdx(via @objectstack/spec)
  • content/docs/getting-started/your-first-project.mdx(via @objectstack/spec)
  • content/docs/kernel/cluster.mdx(via @objectstack/spec)
  • content/docs/kernel/contracts/auth-service.mdx(via packages/spec)
  • content/docs/kernel/contracts/cache-service.mdx(via packages/spec)
  • content/docs/kernel/contracts/data-engine.mdx(via @objectstack/spec)
  • content/docs/kernel/contracts/index.mdx(via @objectstack/spec)
  • content/docs/kernel/contracts/metadata-service.mdx(via packages/spec)
  • content/docs/kernel/contracts/storage-service.mdx(via packages/spec)
  • content/docs/kernel/index.mdx(via packages/spec)
  • content/docs/kernel/runtime-services/email-service.mdx(via packages/spec)
  • content/docs/kernel/runtime-services/index.mdx(via packages/spec)
  • content/docs/kernel/runtime-services/queue-service.mdx(via packages/spec)
  • content/docs/kernel/runtime-services/sharing-service.mdx(via packages/spec)
  • content/docs/kernel/runtime-services/sms-service.mdx(via packages/spec)
  • content/docs/kernel/runtime-services/storage-service.mdx(via packages/spec)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/plugin-approvals, @objectstack/spec)
  • content/docs/kernel/services.mdx(via @objectstack/spec)
  • content/docs/permissions/authorization.mdx(via @objectstack/lint, @objectstack/spec)
  • content/docs/permissions/permission-sets.mdx(via @objectstack/spec)
  • content/docs/permissions/permissions-matrix.mdx(via @objectstack/spec)
  • content/docs/permissions/positions.mdx(via @objectstack/spec)
  • content/docs/permissions/rls.mdx(via @objectstack/spec)
  • content/docs/permissions/sharing-rules.mdx(via @objectstack/spec)
  • content/docs/plugins/adding-a-metadata-type.mdx(via @objectstack/spec)
  • content/docs/plugins/development.mdx(via @objectstack/spec)
  • content/docs/plugins/index.mdx(via @objectstack/spec)
  • content/docs/plugins/packages.mdx(via @objectstack/plugin-approvals, @objectstack/spec)
  • content/docs/protocol/backward-compatibility.mdx(via @objectstack/spec)
  • content/docs/protocol/diagram.mdx(via packages/spec)
  • content/docs/protocol/kernel/config-resolution.mdx(via @objectstack/spec)
  • content/docs/protocol/kernel/http-protocol.mdx(via @objectstack/spec)
  • content/docs/protocol/kernel/i18n-standard.mdx(via @objectstack/spec)
  • content/docs/protocol/kernel/index.mdx(via @objectstack/spec)
  • content/docs/protocol/kernel/lifecycle.mdx(via @objectstack/spec)
  • content/docs/protocol/kernel/plugin-spec.mdx(via @objectstack/spec)
  • content/docs/protocol/knowledge.mdx(via @objectstack/spec)
  • content/docs/protocol/objectql/index.mdx(via @objectstack/spec)
  • content/docs/protocol/objectql/query-syntax.mdx(via @objectstack/spec)
  • content/docs/protocol/objectql/schema.mdx(via @objectstack/spec)
  • content/docs/protocol/objectql/security.mdx(via packages/spec)
  • content/docs/protocol/objectql/state-machine.mdx(via @objectstack/spec)
  • content/docs/protocol/objectui/actions.mdx(via @objectstack/spec)
  • content/docs/protocol/objectui/concept.mdx(via @objectstack/spec)
  • content/docs/protocol/objectui/index.mdx(via @objectstack/spec)
  • content/docs/protocol/objectui/layout-dsl.mdx(via @objectstack/spec)
  • content/docs/protocol/objectui/record-alert.mdx(via @objectstack/spec)
  • content/docs/protocol/objectui/widget-contract.mdx(via @objectstack/spec)
  • content/docs/releases/implementation-status.mdx(via @objectstack/plugin-approvals, @objectstack/spec)
  • content/docs/releases/index.mdx(via @objectstack/spec)
  • content/docs/releases/v12.mdx(via @objectstack/spec)
  • content/docs/releases/v13.mdx(via @objectstack/spec)
  • content/docs/releases/v16.mdx(via @objectstack/spec)
  • content/docs/releases/v17.mdx(via @objectstack/lint, @objectstack/spec)
  • content/docs/releases/v9.mdx(via @objectstack/plugin-approvals, @objectstack/spec)
  • content/docs/ui/actions.mdx(via @objectstack/spec)
  • content/docs/ui/apps.mdx(via @objectstack/spec)
  • content/docs/ui/create-vs-edit-form.mdx(via @objectstack/spec)
  • content/docs/ui/dashboards.mdx(via @objectstack/spec)
  • content/docs/ui/forms.mdx(via @objectstack/spec)
  • content/docs/ui/index.mdx(via @objectstack/spec)
  • content/docs/ui/public-data-collection.mdx(via @objectstack/spec)
  • content/docs/ui/setup-app.mdx(via @objectstack/spec)
  • content/docs/ui/translations.mdx(via @objectstack/spec)
  • content/docs/ui/views.mdx(via @objectstack/spec)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

automation: the revise-window wait pause is service-owned but type-keyed gating can't see it

2 participants

@os-zhuang@claude