fix(automation,approvals): 屏幕流 resume 服务端校验 + 审批越权可审计 (#4477, #4466, #4469, #4470) - #4502
Merged
Merged
Conversation
…ared fields (#4477) Changeset for the screen-resume field contract landed in the preceding commit (which the container restart forced out as a WIP rescue): `resume` now refuses a bag that violates the suspended screen's declared `fields` — a missing `required` value the caller WAS asked for, or a key the screen never declared — with the new `INVALID_SCREEN_INPUT` code, mapped to 400 by the automation domain route. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
…as an override (#4466) `sys_approval_action` had no override column, so an admin overriding a properly staffed approver slate wrote a row byte-for-byte identical to the designated approver approving normally. A reader of the timeline saw `approve` by the admin and could not tell which had happened; the bypassed approver's `409 INVALID_STATE` was the only trace, and only if they happened to try. The platform knows at decision time — it took the `isOverrideActor` branch to admit the call — so this was dropped information, not unavailable information. Adds `sys_approval_action.via_override` (optional boolean), set on exactly the actions admitted by that branch: `decideNode`'s approve/reject and `reassign`'s admin rescue. Surfaced on `ApprovalActionRow`, returned by `listActions`, and added to `highlightFields` + two grid views. `true` = admitted only by the override branch. `false` = checked, not an override (an admin who IS a slot holder approves normally). Absent = a row predating the column — "not recorded" is not the claim "not an override", so `rowFromAction` maps `null` to `undefined`. Additive and nullable: no migration needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 6 package(s): 116 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…mpt the file from ADR-0112 D1 `check:error-code-casing` flagged `ScreenInputIssue.code`'s `'required' | 'unknown_field'` literal union as a lowercase `error.code`. It is not one: these are FIELD-ADDRESSED validator codes (ADR-0114 D2, ADR-0112 D6) carried inside the refusal's message, while the refusal's own machine code is the SCREAMING `INVALID_SCREEN_INPUT` the engine returns. Typed as the shared `FieldErrorCode` for the reason `ActionParamIssue` gives — a screen field, an action param and a record column must not drift into three vocabularies for the same two conditions — and registered in the checker's EXEMPT_FILES with its reason, beside the `action-params.zod.ts` precedent it mirrors exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
…ess pinning memory (#4469, #4470) #4469 — `releaseDeadRunRequests` scans `status: 'pending'`, and the very step that zombifies a request is the one that takes it OUT of `pending`: breaking it removed it from the only sweeper's field of view. Its oracle could not have answered anyway — `getRun` reads the execution LOG, which returns null for a perfectly alive suspended run after a restart. Adds `ApprovalService.inspectStrandedRequests()`, a READ-ONLY inspection over `approved`/`rejected`/`returned` rows that uses BOTH oracles and reports only rows failing both: `hasSuspendedRun(runId) === false` (no live pause) AND `getRun(runId) == null` (no terminal history row either). A store that THROWS is skipped and counted as `undetermined`, never condemned — an outage means unknown. It never rewrites a decision that really happened; it reports which requests are stuck at which step and what the mirrored status field still reads, and rides the existing sweep clock so the finding surfaces without an operator going looking. #4470 — `packages/verify/src/harness.ts` pinned `suspendedRunStore: 'memory'`, making the DB-backed path structurally unreachable from every dogfood/e2e fixture. Engine-side persistence was unit-tested against a fake table and the approval chain e2e-tested wholly in memory, while the assembly between them was covered by nothing — the seam #4420 grew in. The harness now boots the plugin's own `'auto'` default (a fixture opts out explicitly), and gains `databaseFile` so two sequential boots over one file are a genuine cold start. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
…tains (#4470) CI showed the harness store change (`'memory'` → the plugin's own `'auto'`) regressed nothing: 38 dogfood files and 239 tests passed, and the single failure was this file's own cold-boot assertion. Investigating that failure found a real blocker, so the proof is rewritten around what can be established rather than around what would be convenient. A second `bootStack` over the same `databaseFile` reads a database whose tables exist but whose ROWS are gone — ordinary records do not survive it either, which is what identifies it as a harness/driver persistence gap rather than a defect in the suspended-run store. #4470's third bullet is therefore documented as a KNOWN GAP in the file header, not faked with a weakened assertion. What is asserted instead is the substance of the issue's first two bullets, and all of it is a fact rather than the absence of an error: - `sys_automation_run` exists and is queryable (the table #4420 was missing); - the pause lands as a real `paused` row, read back by id, carrying every field a rehydration needs — `node_id`, the `node_type` the resume gate keys on, a parseable `variables_json` holding the trigger input, `steps_json`, and the `screen_json` a resumed pause validates against; - resuming CONSUMES that row and leaves the `run_`-prefixed terminal history row in its place — the zombie shape #4420 produced; - the screen contract (#4477) is enforced against the persisted `screen_json`, and a refusal leaves the durable row intact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
Contributor
Author
|
更正正文末尾的「文档核对」一节 —— 那段现在是过期的（写着「尚未核对完成」）。两篇手写文档已经核对并改好，提交在
我不会自行 dequeue 别人做出的合并决定。仍在重试推送；若本 PR 先行合并，这 2 个提交将以一个紧随其后的小 PR 落地，内容仅为上述两篇文档 + 两份 changeset，不含任何代码改动（四个缺陷的修复与回归测试都已在本 PR 内）。 （另外，正文里 #4477 一节的证据表已在早些时候更正过一次：初版附的「 Generated by Claude Code |
Contributor
Author
|
跟进 PR 已开：#4517 —— 内容就是上一条评论里说的那两个提交（两篇手写文档 + #4469/#4470 的两份 changeset），从 Generated by Claude Code |
This was referenced Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v17 验收缺陷汇总 #4482 中「屏幕流校验 + 审批完整性」一卡的四个缺陷。
1. #4477 —— 屏幕流
resume完全没有服务端校验（本卡最重要）根因.
screen节点的config.fields是一份完整的输入契约：作者声明了键、声明了哪些required、并用visibleWhen声明了什么时候才会问这个字段。渲染那一半一直是对的 —— 暂停结果和GET …/runs/:runId/screen都原样带着required与visibleWhen。校验那一半根本不存在：POST …/runs/:runId/resume把收到的任意 bag 直接折进流程变量，于是绕过对话框、直接打resume的客户端，不受作者写下的任何一条required约束 —— 缺必填字段、以及未声明的键，都能让整个 run 以success: true跑完。屏幕流是唯一一处「声明的字段契约就是全部契约」的地方 —— screen 节点背后没有对象 schema 能在下游兜住一个坏 bag。平台在这条线上的其它每一处都已经强制执行了对应契约：action params（ADR-0104 D2）、记录写入（ADR-0113）、审批
decisionOutputs（#3447）。这里补上的就是屏幕流那一格，并且刻意做成同一个形状。改动.
packages/services/service-automation/src/screen-input-contract.ts—— 纯校验函数（返回 issue 列表、不抛异常），处置权归调用方，与validateActionParams同构。AutomationEngine.resumeInternal在消费挂起之前调用refuseInvalidScreenInput，所以被拒绝的提交不改变任何状态：暂停仍然活着，合法提交照样能落。AutomationResult.code取值'INVALID_SCREEN_INPUT'，automation domain 路由映射为 400，错误文案形如Invalid screen input: … — declared fields: 'kind', 'escalation_reason'（把已声明的键列出来便于调用方自我纠正，与 审批流无法在「运行中」动态确定审批人——审批人在提交时即被 $trigger 快照锁定（动态路由 / 动态会签受阻） #3447 的做法一致）。visibleWhen的处理. 先用本次提交的值（叠在 run 变量快照之上）求值，所以隐藏字段的required不会触发 —— 否则就是把 #3528 那个死锁搬到服务端：卡在一个用户根本没被展示过的字段上。求值失败的谓词记 warn 并按隐藏处理而非按可见处理：客户端才是「用户看到了什么」的权威，一个坏谓词不构成「该字段在屏幕上」的证据。刻意保留 pass-through 的三种形状（与
enforceActionParams对无params的 action 一致）：object-form 屏幕（fields天生为空，记录由客户端走正常写入路径落库，对象自己的required由 ADR-0113 强制）、纯消息屏幕（没声明键就不约束任何键）、以及signal.output（那是节点输出命名空间，属于审批式 resume 信封，不是本契约该管的）。验证证据.
flow-durable-suspend.dogfood.test.ts经真实 Hono 路由注入打POST /automation/flow_durable_suspend/runs/:runId/resume，body{"inputs":{}}（缺必填resolution）得到 400，响应体包含resolution；随后同一个 run 提交{"inputs":{"resolution":"ok"}}仍然成功 —— 证明拒绝没有消费掉暂停。该用例在 Dogfood 分片 1 的 CI 运行中通过。packages/services/service-automation/src/builtin/screen-resume-validation.test.ts，15 例，用的就是 issue 里那份两字段规格（kind无条件必填、escalation_reason条件必填）。撤掉修复后 9 例失败，覆盖 issue 表格的四行、拒绝后暂停仍可续跑、一次报出全部违规、以及visibleWhen的各条边界。Fixes #4477
2. #4466 —— 管理员越过指定审批人名单被记成普通审批
根因.
sys_approval_action根本没有 override 列，所以「管理员越过一份已配齐的审批人名单」和「就是名单里那位审批人正常审批」在字节层面完全相同。看时间线的人只看到管理员的一条approve，无从判断他究竟是审批人还是越过了审批人；被绕过的那位后来收到的409 INVALID_STATE是唯一痕迹，而且只有他碰巧去点了才存在。平台在决策时是知道的 —— 正是走了isOverrideActor分支才放行的这次调用 —— 所以这是被丢弃的信息，不是拿不到的信息。改动.
sys_approval_action新增via_override（可选 boolean），只在被那条分支放行的动作上置位：decideNode的 approve/reject，以及reassign的管理员救援。经ApprovalActionRow.via_override暴露、由listActions返回，并加入对象的highlightFields与两个 grid 列表视图；四个语言包的字段标签/说明已翻译（非直接抄英文原文）。三种取值刻意区分开：
true—— 操作者不在名单里，只凭越权分支被放行。false—— 检查过，不是越权。同时是指定审批人的管理员算正常审批：这个标记记的是「哪条分支放行了这次调用」，不是「操作者有没有管理员权限」。把两者合并，会让每一位管理员的正常决策都读起来像绕过名单。rowFromAction把null映射为undefined而非false。关于 schema 变更（任务边界要求的评估）. 这是纯增量、可空的一列，不移除也不重命名任何可作者化的键，不改公开导出的既有形状（只新增一个可选字段），不需要数据迁移 —— 存量行保持可用，读作「未记录」。因此判断它落在授权范围内（修 bug），没有做破坏性改动。需要知会的是：它是一次可观察的行为变化 ——
listActions的响应和sys_approval_action的表格视图会多出一个消费方此前没见过的字段，且sys_approval_action会在下次 schema sync 时多一列。changeset 正文里已按要求写明定级理由。验证证据 / 回归测试.
packages/plugins/plugin-approvals/src/approval-override-audit.test.ts，9 例，复现 issue 的场景（名单只解析出一位指定用户u9，管理员root不在其中）。撤掉修复后 7 例失败。其中一例直接断言 issue 的核心论断：越权行与普通行不再是同一份字节（两行action相同而via_override不同）。另有一例钉住「同时是审批人的管理员记false」，一例钉住旧行读作undefined。Fixes #4466
3. #4469 —— 存量僵尸审批请求无人认领
根因，以及为什么它能长期无声. #4460 修的是不再产生新僵尸；已经卡住的行没有任何机制发现。故障形态（#4420）是：请求行已翻成
approved/rejected/returned，而它的flow_run_id指向的 run 已不存在 —— 决策落库了，流程永远不动。releaseDeadRunRequests扫不到它们，原因值得原样写下来：它只扫status: 'pending'，而把请求推入僵尸态的那一步，正是把它推出pending的那一步。弄坏它的动作同时把它移出了唯一一个会去看它的清扫器的视野。而且即便它去看了也答不上来：它的存活性判据是
getRun，读的是执行日志环形缓冲 —— 重启后对一个活着的挂起 run 同样返回null。它把null当作「还活着」（保守，且对它自己要做的事是正确的）—— 这恰恰意味着它没有能力说出「这个 run 是真的没了」。改动. 新增
ApprovalService.inspectStrandedRequests()，同时用两个判据，两个都不通过才报告：hasSuspendedRun(runId) === false—— 直接问挂起存储，没有活的暂停。该方法在库读不到时抛错，这种情况被跳过并计入undetermined，绝不判死：存储不可读 ≠ run 不存在，一次抖动不能被发布成一条丢失的 run。getRun(runId) == null—— 也没有终态历史行（sys_automation_run里run_前缀的行）。跑完的 run 不算悬空；既不在挂起、也没跑完的才算。只报告，绝不改写. 不改任何状态、不取消任何 run。决策是真实发生过的 —— 有人真的批了或驳了 —— 自动回滚会让审计与事实不符。报告里带上运维决策所需的东西：哪些请求卡在哪一级、以及业务记录上镜像的状态字段现在读作什么（通常就是用户正盯着的那个过期值）。是重跑后续动作还是重新发起，这是它无法替人做的判断。
它挂在既有的 SLA/死信清扫时钟上，所以运维不必知道要去找，日志里就会浮出来。
recalled刻意排除：撤回本就是有意放弃它的 run（recall明确容忍一个无法 resume 的 run），把它们报出来只会让真正的发现被淹没。验证证据 / 回归测试.
packages/plugins/plugin-approvals/src/stranded-request-inspection.test.ts，13 例，全部通过。关键几条：approved的僵尸，releaseDeadRunRequests()返回{ scanned: 0, released: 0 }（它的扫描集是空的）。requestId / status / runId / nodeId / flowName / objectName / recordId，以及过期的镜像状态（决策说approved，记录仍读pending）。hasSuspendedRun抛错、getRun抛错 —— 都跳过且undetermined === 1，所以「0 stranded」永远不会被误读成「一切正常」。hasSuspendedRun、或根本没接 automation → 不给结论（没有判据就不下判决）。撤销验证：把存活性判据取反（
if (!suspended) continue）后该套件失败。Fixes #4469
4. #4470 —— 端到端验证从构造上绕开挂起 run 的持久化路径
根因.
packages/verify/src/harness.ts挂载自动化插件时写死suspendedRunStore: 'memory'。这不是「测得不够」，是测不到：DB-backed 挂起存储从构造上就不可能被任何 dogfood/e2e fixture 触及。覆盖率上因此有一条整齐的缝：suspended-run-store.test.ts用假表跑 suspend → restart → resume）；#4420 恰好就长在这条缝里：store 挂在一张没建的表上，每次写盘失败都被吞进一条 warn，挂起照常「成功」，直到下次重启才暴露。
改动. harness 改用插件自己的
'auto'默认值（objectstack dev/serve得到的同一套装配）；automation选项现在也接受{ suspendedRunStore: 'auto' | 'memory' }，想要旧行为的 fixture 必须显式要；新增databaseFile让数据库可以落到文件而非:memory:。换成真实持久化路径后，dogfood 里到底什么失败了 —— 逐条判断.
这是本项修复真正的价值所在，所以把过程写全：
在 Dogfood 分片 1 的 CI 运行里，换 store 之后 38 个测试文件、239 个用例全部通过，唯一失败的是我自己新增的那个文件里的一条断言。也就是说：
'memory'是为了跑得快/省事，不是因为持久化在这个环境跑不通。持久化在这里跑得通。那条失败的诊断过程：新 kernel 用同一个
databaseFile起来后，sys_automation_run里查不到刚才那行paused。进一步用探针确认 —— 连普通的suspend_note记录也不在，而表结构是在的。这就把归因定死了：不是挂起存储的问题，是 harness/驱动层「文件数据库在进程内重启后不保留行」的问题。据此我没有把断言改松去凑绿，而是：现在断言的是事实，不是「没报错」（
packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts+ 新 fixture）：sys_automation_run真的存在且可查 —— 就是 [automation/approvals] 进程重启后审批决策静默失效:挂起 flow run 仍只存内存(#1518 标记 COMPLETED 但 17.0.0-rc.1 未生效),approve 落库却永不推进且零报错 #4420 缺的那张表；paused，按 id 读回来，且带着 rehydrate 所需的每一个字段：node_id、resume 闸门（automation: the generic run-resume route needs an authorization gate keyed on the suspended node #3801）所依赖的node_type、可解析且含触发输入的variables_json、steps_json，以及被 resume 校验所依赖的screen_json（含required标记）；run_前缀下留下completed的终态历史行 —— 正是 [automation/approvals] 进程重启后审批决策静默失效:挂起 flow run 仍只存内存(#1518 标记 COMPLETED 但 17.0.0-rc.1 未生效),approve 落库却永不推进且零报错 #4420 会产生的僵尸形态的反面；screen_json生效，且拒绝之后那行仍在、合法提交仍能落。Fixes #4470
顺带发现（按 AGENTS.md #10 单独立 issue，未认领）
AutomationEngine.getSuspendedScreen只读内存热缓存（方法是同步的，没能力问存储），所以进程重启后，一个resume还能打通的挂起 run，GET …/runs/:runId/screen却 404。修它要把公开契约上的方法改成异步，属于协议破坏性变更，没有顺手改。这条正是本 PR 的新 e2e 第一次跑就抓出来的。bootStack打同一个databaseFile，表在、行没了（普通记录也一样）。它挡住了 [verify] 端到端验证从构造上绕开了挂起 run 的持久化路径(harness 写死 suspendedRunStore: 'memory') #4470 第三条「冷启动后 resume」的断言。闸门
pnpm --filter @objectstack/service-automation testpnpm --filter @objectstack/plugin-approvals test@objectstack/objectqldist 未构建而无法加载，与本改动无关；CI 全新安装构建，不受影响）screen-resume-validation.test.tsapproval-override-audit.test.tsstranded-request-inspection.test.tspnpm lintcheck:error-code-casingScreenInputIssue.code改用共享FieldErrorCode，并按 ADR-0112 D6 在 EXEMPT_FILES 注册理由，与action-params.zod.ts先例一致）pnpm --filter @objectstack/spec check:generatedcheck-i18n-bundles文档核对.
content/docs/automation/flows.mdx与content/docs/automation/approvals.mdx尚未核对完成 —— 前者需要说明resume现在会以 400INVALID_SCREEN_INPUT拒绝不合契约的提交，后者需要说明审批动作多了via_override。这两处将在合并前补上；content/docs/references/是自动生成的，不会手改。