fix(approvals): decision outputs reach both decision surfaces (#2955)

.changeset/decision-outputs-reach-both-decision-surfaces.md

@@ -0,0 +1,53 @@
+---
+"@object-ui/app-shell": minor
+---
+
+fix(approvals): decision outputs reach both decision surfaces (objectui#2955, framework#3447 P2)
+
+An approval node can ask the approver for structured data with their decision
+(`decisionOutputs`) — typically to route the next node's approvers, which the
+flow then reads as `vars.<nodeId>.<key>`. The server has shipped this since
+framework#3447 P2 and surfaces the typed declaration on the request row
+(`decision_output_defs`), 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 objectui#2831 and the drawer really did synthesize a
+`lookup` param per declared output — but it spelled the picker target
+`referenceTo`, and `resolveActionParams()` (which every collected param passes
+through before the dialog renders it) rebuilds an inline param from a fixed key
+list, reading the target from `reference`. The target was dropped there, and
+`paramToField()` degrades a targetless picker to a plain text input — so a
+`position` output rendered as a box labelled "<label> 的记录 ID". The approver
+had to go find the record id somewhere else and paste it back. `user`-typed
+outputs were unaffected (that widget needs no target), which is why this
+survived: `department` / `position` / `team` were 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 (`ActionRunner` collects from `actionParams`).
+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
+declaring `decisionOutputs` got no inputs either, so the flow resumed with
+`vars.<node>.<key>` missing — the next node's `expression` approver then failed
+with `EXPRESSION_FAILED`, or fell through to `onEmptyApprovers`, 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 under `outputs` on the decide call. The
+comment box works again as a side effect, and it is a real textarea (the param
+resolver drops `multiline`, so the intent has to ride the type).
+
+The widget mapping now lives in one place (`utils/decisionOutputParams`), so
+the two surfaces cannot drift apart again, and the round trip through param
+resolution — the stage that actually broke — is pinned by tests.
+
+**And a `required` output is now enforced at the field.** The spec grew
+`decisionOutputs[].required` (the platform half of this issue, shipping in
+`@objectstack/spec` + `@objectstack/plugin-approvals`) — the server rejects an
+approve carrying no value for one, before any write. The dialog marks those params
+required, so the approver is stopped at the empty field with the Confirm button
+refusing rather than by a 400 after the round trip. Only on APPROVE: the server
+never requires them on a reject (the run leaves down the reject edge, where
+nothing reads the outputs), so the two dialogs differ in exactly that flag. On a
+backend that predates the field nothing is required, which is the behavior above
+unchanged.

packages/app-shell/src/hooks/useRecordApprovals.decisionOutputs.test.tsx

@@ -0,0 +1,106 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * What the record header actually POSTs when an approver decides
+ * (objectui#2955).
+ *
+ * The hook used to send `{ actorId, comment }` and nothing else, so an approval
+ * node that declared `decisionOutputs` lost them on this surface: the flow
+ * resumed with `vars.<node>.<key>` missing, and the next node's `expression`
+ * approver either faulted (`EXPRESSION_FAILED`) or fell through to
+ * `onEmptyApprovers`. Nothing surfaced to the approver.
+ */
+
+import { describe, it, expect, beforeEach, vi } from 'vitest';
+import { renderHook, waitFor, act } from '@testing-library/react';
+import { useRecordApprovals } from './useRecordApprovals';
+
+const PENDING = {
+  id: 'req_1',
+  process_name: 'flow:showcase_dynamic_approval',
+  object_name: 'showcase_announcement',
+  record_id: 'a1',
+  status: 'pending',
+  pending_approvers: ['user_1'],
+  decision_output_defs: [
+    { key: 'parallel_positions', label: '并行会审岗位', type: 'position', multiple: true },
+  ],
+};
+
+/** POST bodies captured per decide call. */
+let posts: Array<{ url: string; body: any }>;
+
+beforeEach(() => {
+  posts = [];
+  vi.stubGlobal(
+    'fetch',
+    vi.fn(async (url: string, init?: RequestInit) => {
+      if (init?.method === 'POST') {
+        posts.push({ url: String(url), body: JSON.parse(String(init.body)) });
+        return { ok: true, json: async () => ({ request: { ...PENDING, status: 'approved' } }) } as any;
+      }
+      return { ok: true, json: async () => ({ data: [PENDING] }) } as any;
+    }),
+  );
+});
+
+const mount = async () => {
+  const view = renderHook(() => useRecordApprovals('showcase_announcement', 'a1', 'user_1'));
+  await waitFor(() => expect(view.result.current.canDecide).toBe(true));
+  return view;
+};
+
+describe('useRecordApprovals — decision outputs in the decide body (objectui#2955)', () => {
+  it('posts the collected outputs under the nested `outputs` key', async () => {
+    const { result } = await mount();
+    await act(async () => {
+      await result.current.approve({
+        comment: 'ok',
+        outputs: { parallel_positions: ['pos_1', 'pos_2'] },
+      });
+    });
+    expect(posts).toHaveLength(1);
+    expect(posts[0].url).toContain('/approvals/requests/req_1/approve');
+    expect(posts[0].body).toEqual({
+      actorId: 'user_1',
+      comment: 'ok',
+      outputs: { parallel_positions: ['pos_1', 'pos_2'] },
+    });
+  });
+
+  it('rejects with outputs too — a send-back can route the next round as well', async () => {
+    const { result } = await mount();
+    await act(async () => {
+      await result.current.reject({ outputs: { parallel_positions: ['pos_1'] } });
+    });
+    expect(posts[0].url).toContain('/approvals/requests/req_1/reject');
+    expect(posts[0].body).toEqual({
+      actorId: 'user_1',
+      outputs: { parallel_positions: ['pos_1'] },
+    });
+  });
+
+  it('omits `outputs` entirely when the node declares none', async () => {
+    // The unchanged body for every approval without `decisionOutputs` — the
+    // service must not start seeing an empty object where nothing was sent.
+    const { result } = await mount();
+    await act(async () => {
+      await result.current.approve({ comment: 'ok', outputs: {} });
+    });
+    expect(posts[0].body).toEqual({ actorId: 'user_1', comment: 'ok' });
+    expect('outputs' in posts[0].body).toBe(false);
+  });
+
+  it('surfaces the node declaration so the header can build its pickers', async () => {
+    const { result } = await mount();
+    expect(result.current.pendingRequest?.decision_output_defs).toEqual(
+      PENDING.decision_output_defs,
+    );
+  });
+});

packages/app-shell/src/hooks/useRecordApprovals.ts

@@ -20,6 +20,7 @@
 
 import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { bearerAuthHeaders } from '../utils/authToken';
+import type { DecisionOutputDef } from '../utils/decisionOutputParams';
 
 export interface ApprovalRequestLite {
   id: string;
@@ -44,6 +45,19 @@ export interface ApprovalRequestLite {
    * have allowed. See {@link recordLockedByApproval}.
    */
   lock_record?: boolean;
+  /**
+   * Structured data THIS pending node asks the approver to submit with their
+   * decision (framework#3447 P2) — the flow reads it back as
+   * `vars.<nodeId>.<key>`, typically to route the next node's approvers.
+   *
+   * `decision_output_defs` carries the typed declaration (`type` /`multiple`),
+   * `decision_outputs` the bare key list a pre-typed backend sends; both are
+   * absent on a backend that predates the feature. The record header collects
+   * them exactly like the Approval Center does — a decision that silently
+   * skipped them left the next node reading a missing key (objectui#2955).
+   */
+  decision_outputs?: string[] | null;
+  decision_output_defs?: DecisionOutputDef[] | null;
 }
 
 /**
@@ -71,11 +85,20 @@ interface UseRecordApprovalsResult {
   latestRequest: ApprovalRequestLite | null;
   /** The current user is among the pending approvers and may record a decision. */
   canDecide: boolean;
-  approve: (input?: { comment?: string }) => Promise<ApprovalRequestLite | undefined>;
-  reject: (input?: { comment?: string }) => Promise<ApprovalRequestLite | undefined>;
+  approve: (input?: DecisionInput) => Promise<ApprovalRequestLite | undefined>;
+  reject: (input?: DecisionInput) => Promise<ApprovalRequestLite | undefined>;
   refresh: () => Promise<void>;
 }
 
+/**
+ * What an approver submits with a decision: the free-text comment, plus the
+ * node's declared decision outputs keyed by their declared `key` (objectui#2955).
+ */
+export interface DecisionInput {
+  comment?: string;
+  outputs?: Record<string, any>;
+}
+
 function apiBase() {
   const url = (import.meta as any).env?.VITE_SERVER_URL || '';
   return `${String(url).replace(/\/$/, '')}/api/v1`;
@@ -162,15 +185,21 @@ export function useRecordApprovals(
     && (pendingRequest.pending_approvers ?? []).includes(currentUserId);
 
   const decide = useCallback(
-    async (decision: 'approve' | 'reject', input?: { comment?: string }) => {
+    async (decision: 'approve' | 'reject', input?: DecisionInput) => {
       if (!pendingRequest) throw new Error('No pending request');
+      const outputs = input?.outputs && Object.keys(input.outputs).length > 0 ? input.outputs : undefined;
       const out = await fetchJson<{ request?: ApprovalRequestLite }>(
         `/approvals/requests/${encodeURIComponent(pendingRequest.id)}/${decision}`,
         {
           method: 'POST',
           body: JSON.stringify({
             ...(currentUserId ? { actorId: currentUserId } : {}),
             ...(input?.comment ? { comment: input.comment } : {}),
+            // The node's declared decision outputs, under the same nested key
+            // the Approval Center's `type:'api'` decide actions post
+            // (objectui#2955). Omitted entirely when nothing was collected, so
+            // a node without `decisionOutputs` posts the body it always did.
+            ...(outputs ? { outputs } : {}),
           }),
         },
       );
@@ -180,8 +209,8 @@ export function useRecordApprovals(
     [pendingRequest, currentUserId, refresh],
   );
 
-  const approve = useCallback((input?: { comment?: string }) => decide('approve', input), [decide]);
-  const reject = useCallback((input?: { comment?: string }) => decide('reject', input), [decide]);
+  const approve = useCallback((input?: DecisionInput) => decide('approve', input), [decide]);
+  const reject = useCallback((input?: DecisionInput) => decide('reject', input), [decide]);
 
   return {
     loading,