Skip to content

Commit 6720008

Browse files
os-zhuangclaude
andauthored
feat(approvals): dynamic decision-output fields + expression approver editing (framework#3447 P2) (#2827)
Approve/reject dialogs synthesize one input per author-declared decision- output key (row.decision_outputs — per-request, so not a static action param); the api handler folds the dotted outputs.<key> params into the nested outputs body the decide route expects. The flow designer renders an expression approver's value as a CEL input (mono + syntax check) with the closed-root placeholder instead of a dead free-text reference box; the static fallback descriptor gains the Expression type, resolveAs and onEmptyApprovers. Browser-dogfooded end to end against a framework backend: dialog field renders, outputs nest, and the co-sign stage resolves the picked reviewers. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent df6697f commit 6720008

6 files changed

Lines changed: 139 additions & 15 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
"@object-ui/app-shell": minor
3+
"@object-ui/console": minor
4+
---
5+
6+
feat(approvals): dynamic decision-output fields + expression approver editing (framework#3447 P2)
7+
8+
- The approve/reject dialogs now render one input per author-declared decision-output key (the row's `decision_outputs`, per-request so it can't be a static action param). DeclaredActionsBar synthesizes `outputs.<key>` params; the api handler folds them into the nested `outputs` body the decide route expects. Blank optional outputs are omitted.
9+
- The flow designer's approval-node approver list renders an `expression`-type approver's value as a CEL expression input (mono + syntax check) instead of a dead free-text reference box, with a placeholder teaching the three legal roots (`current.*` / `trigger.*` / `vars.*`). Flow-scope pickers are deliberately not wired in — approval expressions have their own closed root set, and offering flow-scope paths would teach exactly the spelling the runtime rejects.
10+
- Static fallback descriptor gains the `Expression (CEL)` approver type, the expression-only `resolveAs` column, and the node-level `onEmptyApprovers` policy select (the online form derives all of these from the engine's published configSchema).

apps/console/src/services/approvalsApi.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,14 @@ export interface ApprovalRequestRow {
104104
};
105105
/** ADR-0044 revision round on this (run, node): absent/1 = first round. */
106106
round?: number;
107+
/**
108+
* framework#3447 — the node's author-declared decision-output keys
109+
* (`config.decisionOutputs`). DeclaredActionsBar synthesizes one input per
110+
* key on the approve/reject dialogs (`outputs.<key>` params, folded into a
111+
* nested `outputs` body by the api handler); the flow receives them as
112+
* `<nodeId>.<key>` variables. Absent when the node declares none.
113+
*/
114+
decision_outputs?: string[];
107115
}
108116

109117
/**

packages/app-shell/src/hooks/useConsoleActionRuntime.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,21 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons
297297
: undefined;
298298
const body: Record<string, any> = wrap ? { [wrap]: resolvedParams } : { ...resolvedParams };
299299

300+
// #3447: decision outputs. DeclaredActionsBar synthesizes one param per
301+
// author-declared output key, named `outputs.<key>` (the key set is
302+
// per-request, so it can't be a static action param). Fold the dotted
303+
// params into the nested `outputs` object the approvals decide route
304+
// expects. Scoped to the `outputs.` prefix — a generic dotted-key fold
305+
// could reinterpret existing actions' literal param names.
306+
for (const k of Object.keys(body)) {
307+
if (k.startsWith('outputs.') && k.length > 'outputs.'.length) {
308+
const value = body[k];
309+
delete body[k];
310+
if (value === undefined || value === '') continue; // blank optional output → omit
311+
(body.outputs ??= {})[k.slice('outputs.'.length)] = value;
312+
}
313+
}
314+
300315
if (rowRecord && action.recordIdParam) {
301316
const rowField = action.recordIdField || 'id';
302317
const rowValue = rowRecord[rowField];

packages/app-shell/src/views/DeclaredActionsBar.tsx

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,26 @@ const DeclaredActionButton: React.FC<{
123123
// input), and reserve `params` for the `_rowRecord` stash the api handler
124124
// reads for `{id}` interpolation + record-id injection.
125125
const { params: rawParams, ...rest } = action as ActionDef & { params?: unknown };
126+
// #3447: an approval decision may carry author-declared structured
127+
// outputs. The key set is PER-REQUEST (each approval node declares its
128+
// own `decisionOutputs`, surfaced on the row as `decision_outputs`), so
129+
// it cannot be a static action param — synthesize one text param per key
130+
// for the decide actions. Params are named `outputs.<key>`; the api
131+
// handler folds them into the nested `outputs` body the decide route
132+
// expects. Comma-separated values are legal (the service accepts CSV for
133+
// multi-id outputs).
134+
const declaredOutputKeys: string[] =
135+
Array.isArray(recordData.decision_outputs)
136+
&& /\/(approve|reject)$/.test(String((action as any).target ?? ''))
137+
? (recordData.decision_outputs as unknown[]).map(String)
138+
: [];
139+
const outputParams = declaredOutputKeys.map((key) => ({
140+
name: `outputs.${key}`,
141+
label: key.split(/[_-]+/).filter(Boolean).map(w => w.charAt(0).toUpperCase() + w.slice(1)).join(' '),
142+
type: 'text' as const,
143+
required: false,
144+
helpText: 'Handed to the flow as a decision output. Comma-separate multiple values.',
145+
}));
126146
const dispatch: any = {
127147
...rest,
128148
// Localized copies ride the dispatch: the runner reads `label` for the
@@ -141,8 +161,9 @@ const DeclaredActionButton: React.FC<{
141161
objectName,
142162
params: { _rowRecord: record },
143163
};
144-
if (Array.isArray(rawParams) && rawParams.length > 0) {
145-
dispatch.actionParams = rawParams;
164+
const staticParams = Array.isArray(rawParams) ? rawParams : [];
165+
if (staticParams.length > 0 || outputParams.length > 0) {
166+
dispatch.actionParams = [...staticParams, ...outputParams];
146167
}
147168
await execute(dispatch as ActionDef);
148169
} finally {

packages/app-shell/src/views/metadata-admin/inspectors/FlowObjectListField.tsx

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -261,18 +261,58 @@ export function FlowObjectListField({
261261
disabled={disabled}
262262
/>
263263
) : col.kind === 'reference' ? (
264-
<div className="flex-1">
265-
<ReferenceCombobox
266-
resolved={resolveRefKind(col.ref, (k) => row.values[k])}
267-
value={typeof row.values[col.key] === 'string' ? (row.values[col.key] as string) : ''}
268-
onCommit={(v) => setCell(row.id, col.key, typeof v === 'string' ? v : '')}
269-
onBlur={() => flush(rows)}
270-
placeholder={col.placeholder}
271-
disabled={disabled}
272-
context={context}
273-
showHint={false}
274-
/>
275-
</div>
264+
(() => {
265+
const resolved = resolveRefKind(col.ref, (k) => row.values[k]);
266+
const disc = col.ref?.kindFrom
267+
? String(row.values[col.ref.kindFrom] ?? '')
268+
: '';
269+
// #3447: `expression` is a discriminator value, not a
270+
// reference kind — an approver whose sibling `type` is
271+
// 'expression' authors a CEL expression over the approval
272+
// roots (current/trigger/vars). Render the expression
273+
// input (mono + syntax check) instead of a dead free-text
274+
// reference box. The flow-scope picker/roots are
275+
// deliberately NOT wired in: approval expressions have
276+
// their own root set, and offering flow-scope paths here
277+
// (record.x, bare fields) would teach exactly the
278+
// spelling the runtime rejects; root validation runs
279+
// server-side (os lint + node-entry pre-check).
280+
if (resolved === undefined && disc === 'expression') {
281+
const raw = typeof row.values[col.key] === 'string' ? (row.values[col.key] as string) : '';
282+
return (
283+
<div className="flex-1 space-y-1">
284+
<VariableTextInput
285+
mode="expression"
286+
mono
287+
value={raw}
288+
onValueChange={(v) => setCell(row.id, col.key, v)}
289+
onBlur={() => flush(rows)}
290+
onKeyDown={(e) => {
291+
if (e.key === 'Enter') (e.target as HTMLInputElement).blur();
292+
}}
293+
groups={[]}
294+
placeholder={col.placeholder ?? 'current.<field> · trigger.<field> · vars.<node>.<key>'}
295+
disabled={disabled}
296+
/>
297+
<FlowExprIssue value={raw} role="value" />
298+
</div>
299+
);
300+
}
301+
return (
302+
<div className="flex-1">
303+
<ReferenceCombobox
304+
resolved={resolved}
305+
value={typeof row.values[col.key] === 'string' ? (row.values[col.key] as string) : ''}
306+
onCommit={(v) => setCell(row.id, col.key, typeof v === 'string' ? v : '')}
307+
onBlur={() => flush(rows)}
308+
placeholder={col.placeholder}
309+
disabled={disabled}
310+
context={context}
311+
showHint={false}
312+
/>
313+
</div>
314+
);
315+
})()
276316
) : col.kind === 'select' ? (
277317
(() => {
278318
const current =

packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,12 +539,18 @@ const FLOW_NODE_CONFIG: Record<string, FlowConfigField[]> = {
539539
{ value: 'manager', label: 'Manager' },
540540
{ value: 'field', label: 'Field' },
541541
{ value: 'queue', label: 'Queue' },
542+
// #3447: CEL over current.* / trigger.* / vars.*, resolved at node
543+
// entry — the value cell switches to the expression input.
544+
{ value: 'expression', label: 'Expression (CEL)' },
542545
],
543546
},
544547
{
545548
// Polymorphic: the picker follows the row's `type`. `manager` takes no
546549
// value (resolved from the submitter's manager_id) so it stays unmapped
547-
// → free text; unmapped/empty types likewise fall back to free text.
550+
// → free text; unmapped/empty types likewise fall back to free text —
551+
// except `expression` (#3447), which the cell special-cases into the
552+
// CEL expression input (it is a discriminator value, not a reference
553+
// kind, so it deliberately has no `map` entry).
548554
key: 'value',
549555
label: 'Value',
550556
kind: 'reference',
@@ -568,6 +574,19 @@ const FLOW_NODE_CONFIG: Record<string, FlowConfigField[]> = {
568574
},
569575
},
570576
},
577+
{
578+
// #3447: expression-only — how the expression's resolved ids expand
579+
// into people. Dead config on other types (linted server-side).
580+
key: 'resolveAs',
581+
label: 'Resolve as',
582+
kind: 'select',
583+
options: [
584+
{ value: 'user', label: 'User ids (default)' },
585+
{ value: 'department', label: 'Department ids → members' },
586+
{ value: 'position', label: 'Position names → holders' },
587+
{ value: 'team', label: 'Team ids → members' },
588+
],
589+
},
571590
{
572591
// Group label for `per_group` sign-off (#3266): approvers sharing a
573592
// label form one group; the node advances when EACH group reaches
@@ -601,6 +620,17 @@ const FLOW_NODE_CONFIG: Record<string, FlowConfigField[]> = {
601620
placeholder: 'approval_status',
602621
help: 'Business-object field to mirror request status onto (pending/approved/rejected). Should be readonly.',
603622
}),
623+
// #3447: empty-slate policy — load-bearing for expression approvers, whose
624+
// slate is runtime data and may legitimately resolve to nobody.
625+
cfg('onEmptyApprovers', 'If no approver resolves', 'select', {
626+
options: [
627+
{ value: 'admin_rescue', label: 'Hold for admin takeover (default)' },
628+
{ value: 'fail', label: 'Fail the node (config bug)' },
629+
{ value: 'auto_approve', label: 'Auto-approve (waves through!)' },
630+
],
631+
defaultValue: 'admin_rescue',
632+
help: 'What an empty resolved approver slate does at node entry. Auto-approve silently waves the record through — opt in deliberately.',
633+
}),
604634
// Per-node SLA escalation (spec ApprovalEscalationSchema, nested under
605635
// config.escalation). Sub-fields reveal once escalation is enabled.
606636
{ id: 'escalation.enabled', path: ['config', 'escalation', 'enabled'], label: 'SLA escalation', kind: 'boolean', defaultValue: 'false', help: 'Escalate when a decision is not recorded within the timeout.' },

0 commit comments

Comments
 (0)