From 8b278e35b3dee68630ddd6373af89509f0067d27 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 03:14:51 +0000 Subject: [PATCH 1/2] test(spec): pin `DecisionOutputDef.required` at the schema level (#4525) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #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 — cd6b9f202 (`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 cd6b9f202). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5 --- packages/spec/src/automation/approval.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/spec/src/automation/approval.test.ts b/packages/spec/src/automation/approval.test.ts index 645bb4e89e..a41efdb888 100644 --- a/packages/spec/src/automation/approval.test.ts +++ b/packages/spec/src/automation/approval.test.ts @@ -396,6 +396,38 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { expect(unknownKeyIssue(DecisionOutputDefSchema, { key: 'k', widget: 'user' })!.message) .toContain('`widget` → `type`'); }); + + /** + * #4525: `required` is the one decision-output key the RUNTIME enforces — + * `ApprovalService.decide()` refuses an approve whose required outputs are + * blank. The spec has declared it since objectui#2955, but nothing at the + * schema level pinned it: the schema is `.strict()`, so dropping the key + * would turn every author's `required: true` into an unknown-key rejection, + * and the only guard against that was `authorable-surface.json` — a + * REGENERATED baseline, which a `gen:schema` run silently rewrites. These + * cases fail loudly instead, which is what "declared = enforced" needs on + * the declaring side. + */ + it('accepts `required` — the key the runtime enforces (#4525)', () => { + expect(DecisionOutputDefSchema.parse({ key: 'next_reviewers', required: true })) + .toEqual({ key: 'next_reviewers', required: true }); + expect(DecisionOutputDefSchema.parse({ key: 'note', required: false })) + .toEqual({ key: 'note', required: false }); + }); + + it('leaves `required` optional — an unflagged output stays unflagged', () => { + // Absent must stay absent rather than defaulting to `false`: the parsed + // shape is what `normalizeDecisionOutputs` ships to every decision UI. + expect(DecisionOutputDefSchema.parse({ key: 'note' })).toEqual({ key: 'note' }); + }); + + it('rejects a non-boolean `required` instead of coercing it', () => { + // A truthy string is exactly how an AI-authored flow would spell it; the + // runtime compares `d.required === true`, so a coerced 'true' would + // declare a constraint the server then never enforces. + expect(() => DecisionOutputDefSchema.parse({ key: 'note', required: 'true' })).toThrow(); + expect(() => DecisionOutputDefSchema.parse({ key: 'note', required: 1 })).toThrow(); + }); }); it('publishes additionalProperties:false through the JSON schema (Studio + registerFlow)', () => { From 0b8d1427f0a3bb6abf2ccb9357c70a794eec228f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 03:16:59 +0000 Subject: [PATCH 2/2] =?UTF-8?q?chore:=20empty=20changeset=20=E2=80=94=20th?= =?UTF-8?q?is=20PR=20releases=20nothing=20(#4525)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Check Changeset" gate counts changesets ADDED by the PR relative to base, so pointing at cd6b9f202'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 Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5 --- .changeset/decision-output-required-pin.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changeset/decision-output-required-pin.md diff --git a/.changeset/decision-output-required-pin.md b/.changeset/decision-output-required-pin.md new file mode 100644 index 0000000000..0e649d668c --- /dev/null +++ b/.changeset/decision-output-required-pin.md @@ -0,0 +1,4 @@ +--- +--- + +Test-only pin for `DecisionOutputDef.required` (#4525) — releases nothing.