fix(approvals): decision outputs reach both decision surfaces (#2955) - #2961
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
An approval node's `decisionOutputs` (framework#3447 P2) never made it to the approver on either Console surface, though the server has shipped the typed declaration on the request row all along. The Approval Center DID synthesize a typed picker per declared output (#2831), but spelled the picker target `referenceTo`. Every collected param passes through `resolveActionParams()` first, and its inline branch rebuilds the param from a fixed key list, reading the target from `reference` — so the target was dropped and `paramToField()` degraded the targetless picker to a plain text input labelled "<label> 的记录 ID". `user` outputs were unaffected (that widget needs no target); `department`/`position`/`team` were all broken. The record header collected nothing at all: its Approve/Reject shipped their inputs under `collectParams`, a key nothing reads (ActionRunner collects from `actionParams`). No dialog had opened there since ADR-0019 — the comment was silently dropped on every record-page decision, and a node declaring `decisionOutputs` resumed the flow with `vars.<node>.<key>` missing, so the next node's `expression` approver faulted or fell through to `onEmptyApprovers` with no hint to anyone. The widget mapping now lives in `utils/decisionOutputParams` so the two surfaces cannot drift again; the header collects through `actionParams`, folds `outputs.<key>` into the decide body, and its comment box is a real textarea (the resolver drops `multiline`). Tests pin the round trip through param resolution — the stage that actually broke — not just the emitted shape. Verified end to end against the app-showcase runtime: an approval node declaring a `position`-typed multi-select renders a sys_position picker in both the Approval Center drawer and the record header, and deciding from the record page carries the outputs through to `vars.*` — the co-sign node's `expression` approver resolved to the user picked in the dialog. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The platform half of this issue gives `decisionOutputs` a `required` flag: the server rejects an approve that carries no value for one, before any write, so the run can never resume past the node with the key a downstream `expression` approver reads still missing. The console mirrors it where the approver actually is. A required output is marked required on the param it synthesizes, so the dialog flags the empty field and refuses Confirm instead of sending a decision the server will 400. APPROVE only, matching the server: a reject leaves down the reject edge where nothing reads the outputs, so the reject dialog offers the same fields without blocking on them — the two dialogs now differ in exactly that flag. Both decision surfaces get it from the one shared helper, and a backend that predates the field requires nothing, which is the behaviour of the previous commit unchanged. Verified against the showcase runtime (its dynamic-approval flow declares the flag): the record header's Approve refuses with "Next Reviewers 为必填项" and sends no request, its Reject offers the field unmarked, the Approval Center drawer agrees on both, and a filled approve routes the co-sign node to the picked user. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Only conflict: the import block in RecordDetailView — main added `interpretActionResponse` (#2967) on the line this branch added `decisionOutputParams`. Both kept; the two changes touch disjoint parts of the file (the action-envelope reader vs the approval decision actions). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Jul 31, 2026
os-zhuang
pushed a commit
that referenced
this pull request
Aug 1, 2026
) (#3128) Since #2961 made the record header's decision inputs live, Reject carried both `confirmText` and a collectable comment param. The ActionRunner chains confirm before param collection, so rejecting queued TWO dialogs — after "Continue" the request silently waited on a second, unexpected comment dialog, which downstream read as a dead button (zero requests, no toast). Drop `confirmText` from the Reject decision action: the param dialog is the confirmation surface, titled by the action and carrying the old confirm question (same `approvals.rejectConfirm` key) as its description. One decision, one dialog — matching Approve and the Approval Center. Verified end-to-end in a real Chromium harness: Reject → single dialog → Confirm → `reject_request` dispatched. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Fixes #2955.
An approval node can ask the approver for structured data with their decision (
decisionOutputs, framework#3447 P2) — typically to route the next node's approvers, which the flow reads asvars.<nodeId>.<key>. The server has shipped this all along and surfaces the typed declaration on the request row, but neither Console decision surface actually delivered it.① The Approval Center asked for a record id instead of showing a picker
The typed pickers landed in #2831 and the drawer really did synthesize a
lookupparam per declared output — but it spelled the picker targetreferenceTo, andresolveActionParams()(which every collected param passes through before the dialog renders it) rebuilds an inline param from a fixed key list, reading the target fromreference. The target was dropped there, andparamToField()degrades a targetless picker to a plain text input — so apositionoutput rendered as a box labelled 「 的记录 ID」 and the approver had to go find the record id elsewhere and paste it back.user-typed outputs were unaffected (that widget needs no target), which is why this survived review:department/position/teamwere the broken three.② The record header decided without collecting anything at all
Approve / Reject on the detail page shipped their inputs under
collectParams— a key nothing in the codebase reads (ActionRunnercollects fromactionParams). No dialog had opened on that surface since the ADR-0019 rework: the approver's comment was silently dropped on every record-page decision, and a node declaringdecisionOutputsgot no inputs either, so the flow resumed withvars.<node>.<key>missing — the next node'sexpressionapprover then failed withEXPRESSION_FAILEDor fell through toonEmptyApprovers, with nothing surfaced to the approver or the flow author.The header now collects through
actionParams, renders the node's declared outputs with the same pickers the Approval Center uses, and posts them underoutputs. The comment box works again as a side effect, and it is a real textarea (the param resolver dropsmultiline, so the intent has to ride the type).③
required(second commit — needs objectstack-ai/objectstack#3931)The spec gained
decisionOutputs[].required, enforced server-side. The dialog mirrors it: a required output is marked required, so the approver is stopped at the empty field with Confirm refusing, instead of by a 400 after the round trip. Approve only, matching the server — the reject dialog offers the same fields without blocking on them. A backend predating the flag requires nothing.Shape of the fix
The widget mapping now lives in one place (
utils/decisionOutputParams) so the two surfaces cannot drift apart again. Tests pin the round trip through param resolution — the stage that actually broke — not just the emitted shape; reverting either fix turns 11 assertions red.app-shellregression run: 55 files / 566 tests green, type-check clean, lint 0 errors.Verified end to end (app-showcase runtime + this branch's console)
positionmulti-select outputsys_positionpicker with live records, multi-select chips — no more "record ID" boxsys_approval_actionvars.*expressionapprover to the user picked in the dialog🤖 Generated with Claude Code