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

claude added 2 commits August 5, 2026 18:17
…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

vercel Bot commented Aug 5, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 5, 2026 6:20pm

Request Review

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

github-actions Bot commented Aug 5, 2026

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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

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