test(spec): pin DecisionOutputDef.required at the schema level (#4525) - #4561
Merged
Conversation
#4525 asked the spec to model `required` on `DecisionOutputDef` so the contract declares what `ApprovalService.decide()` already enforces. The modelling itself landed three days before the issue was filed — cd6b9f2 (`feat(approvals): decisionOutputs may be declared required`, objectui#2955) added the Zod key, its TSDoc, the `strictUnknownKeyError` known-key entry, the `normalizeDecisionOutputs` pass-through, the authorable-surface baseline row and the generated docs table. The issue was recorded from objectui's ledger burn-down, whose derived-type comment ("the spec does not model it yet") was written against an older spec and was already stale. What was genuinely missing is the pin. `DecisionOutputDefSchema` is `.strict()`, so dropping the key would turn every author's `required: true` into an unknown-key rejection — and the only thing standing in the way was `authorable-surface.json`, a REGENERATED baseline that a `gen:schema` run rewrites without comment. Verified by mutation: commenting the key out left all 40 pre-existing cases green, including the `normalizeDecisionOutputs` ones, because that normalizer is hand-written and never consults the schema. Three cases close it — the key parses and round-trips, it stays optional (absent must not become `required: false`; the parsed shape is what ships to every decision UI on `decision_output_defs`), and a non-boolean is rejected rather than coerced. That last one is the AI-authoring case: the runtime compares `d.required === true`, so a coerced `'true'` would declare a constraint the server then never enforces — the same declared-vs-enforced gap #4525 exists to close, one layer down. No behaviour change; no changeset (the feature shipped with its own in cd6b9f2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckNo hand-written docs reference the 0 changed package(s). ✅ |
The "Check Changeset" gate counts changesets ADDED by the PR relative to base, so pointing at cd6b9f2's `.changeset/required-decision-outputs.md` (which shipped the feature itself) does not satisfy it. An empty-frontmatter changeset is the sanctioned form for a PR that bumps no package: the diff is one test file pinning a spec key that already exists, so there is nothing for a consumer to read in a CHANGELOG. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5
This was referenced Aug 2, 2026
os-zhuang
marked this pull request as ready for review
August 2, 2026 03:24
os-zhuang
enabled auto-merge
August 2, 2026 03:24
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
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.
Fixes #4525
结论先行:维护者拍板的选项 1 早已落地,issue 的前提是过期的
#4525 要求「spec 建模
DecisionOutputDef.required,让契约声明服务端已经在强制的约束」。核查后确认:这件事在 issue 提出前 3 天就已经合并进main了。落地提交是
cd6b9f202——feat(approvals): decisionOutputs may be declared required (objectui#2955),2026-07-29。它一次性补齐了整套:packages/spec/src/automation/approval.zod.ts:518required: z.boolean().optional()decisionOutputUnknownKeyError.knownKeysrequirednormalizeDecisionOutputs...(e.required === true ? ... )authorable-surface.json:2232automation/DecisionOutputDef:requiredcontent/docs/references/automation/approval.mdx:119plugin-approvals/src/approval-service.ts:1773-1783.changeset/required-decision-outputs.mdissue 是从 objectui 台账燃尽批次 3 记录下来的,而 objectui 派生类型里那句注释「the spec does not model it yet」本身是对着更早版本的 spec 写的,记录时就已经过期。所以
DecisionOutputDef的 spec 建模没有任何增量可做。那本 PR 做了什么:补上真正缺失的那颗钉子
建模有了,钉子没有。这是一处真实的薄弱点:
DecisionOutputDefSchema是.strict()的 —— 一旦required被摘掉,作者写的required: true会立刻变成 unknown-key 报错。而唯一拦着这件事的,只有authorable-surface.json这张由gen:schema重新生成的基线 —— 它会被无声改写,不会喊。变异测试实证了这个缺口:把 Zod 里的
required一行注释掉后,既有 40 条用例全绿,包括那几条normalizeDecisionOutputs的 —— 因为那个归一化函数是手写的,从不读 schema。补的三条用例:
required能解析并原样回传 —— 摘掉即红(上方即为实证输出)。required: false。归一化后的形状就是decision_output_defs发给每个决策 UI 的东西。d.required === true,一个被强转的'true'会声明一条服务端根本不会执行的约束,正是 spec 是否该建模DecisionOutputDef.required?服务端今天就在强制它,而 spec 未声明 #4525 想关掉的「声明 ≠ 强制」,只不过下沉了一层。无行为变更;不加 changeset(功能本体的 changeset 已随
cd6b9f202发出)。验证
生成物零漂移(本 PR 只动测试文件,符合预期)。
范围外
objectui 侧
DecisionOutputDef extends SpecDecisionOutputDef { required?: boolean }现在已经是纯冗余——spec 早就有这个键了,该接口可以塌缩成纯 re-export,其__tests__/spec-symbol-parity.test.ts:246那条断言(Exclude<keyof …> === 'required')需要同步反转。按派发约定,objectui 不在本 PR 范围内,由 PM 另开 follow-up。🤖 Generated with Claude Code
Generated by Claude Code