✋ fix: Enable Submit on First Tool-Approval Decision (#14393)

client/src/components/Chat/Messages/Content/ApprovalContext.tsx

@@ -50,6 +50,13 @@ export function useAskSubmitStatus(): {
 }
 
 interface ApprovalContextValue {
+  /**
+   * Bumped whenever registrations or decisions change. Decisions live in refs
+   * (synchronous reads), so this is what makes the context value a NEW reference
+   * on each change; without it, consumers never re-render and never re-read
+   * `isReady`/`getLeadToolCallId` after an update.
+   */
+  version: number;
   /** Record (or clear) a card's decision for its tool_call within an action. */
   setDecision: (
     actionId: string,
@@ -88,6 +95,7 @@ export const useApprovalContext = (): ApprovalContextValue => {
 };
 
 const FALLBACK: ApprovalContextValue = {
+  version: 0,
   setDecision: () => undefined,
   getDecision: () => undefined,
   getDecisions: () => [],
@@ -120,11 +128,14 @@ const isExpiredError = (error: unknown): boolean => {
  * and the cards only render inside a live chat view where those providers exist.
  */
 export default function ApprovalProvider({ children }: { children: React.ReactNode }) {
-  /** actionId → (tool_call_id → resolution). Mutable ref + a version bump so
-   *  reads are synchronous for `isReady`/submit while renders stay cheap. */
+  /** actionId → (tool_call_id → resolution). Mutable refs so reads are
+   *  synchronous for `isReady`/submit; `version` is threaded into the context
+   *  value so each bump produces a new value reference and consumers re-render
+   *  (the callbacks alone are referentially stable, so without it a bump would
+   *  never propagate past the memoized value). */
   const decisionsRef = useRef(new Map<string, Map<string, Agents.ToolApprovalResolution>>());
   const registeredRef = useRef(new Map<string, Set<string>>());
-  const [, bump] = useState(0);
+  const [version, bump] = useState(0);
   const rerender = useCallback(() => bump((v) => v + 1), []);
   const [statusByAction, setStatusByAction] = useState<Record<string, ActionStatus>>({});
 
@@ -220,6 +231,7 @@ export default function ApprovalProvider({ children }: { children: React.ReactNo
 
   const value = useMemo<ApprovalContextValue>(
     () => ({
+      version,
       setDecision,
       getDecision,
       getDecisions,
@@ -232,6 +244,7 @@ export default function ApprovalProvider({ children }: { children: React.ReactNo
       setStatus,
     }),
     [
+      version,
       setDecision,
       getDecision,
       getDecisions,

client/src/components/Chat/Messages/Content/__tests__/ToolApproval.test.tsx

@@ -0,0 +1,105 @@
+import React from 'react';
+import { RecoilRoot } from 'recoil';
+import { fireEvent, render, screen } from '@testing-library/react';
+import type { Agents } from 'librechat-data-provider';
+import ApprovalProvider from '../ApprovalContext';
+import ToolApproval from '../ToolApproval';
+
+jest.mock('~/hooks', () => ({
+  useLocalize: () => (key: string, values?: Record<string | number, string | number>) => {
+    if (key === 'com_ui_submit_decisions') {
+      return `Submit ${values?.[0]} decisions`;
+    }
+    const map: Record<string, string> = {
+      com_ui_approve: 'Approve',
+      com_ui_reject: 'Reject',
+      com_ui_edit: 'Edit',
+      com_ui_respond: 'Respond',
+      com_ui_submit: 'Submit',
+      com_ui_submitting: 'Submitting',
+      com_ui_invalid_json: 'Invalid JSON',
+      com_ui_reject_reason_placeholder: 'Reason',
+      com_ui_tool_response_placeholder: 'Response',
+    };
+    return map[key] ?? key;
+  },
+}));
+
+jest.mock('~/data-provider', () => ({
+  useSubmitToolApprovalMutation: () => ({ mutate: jest.fn() }),
+  useSubmitAskAnswerMutation: () => ({ mutate: jest.fn() }),
+}));
+
+jest.mock('~/Providers/ChatContext', () => ({
+  ChatContext: jest.requireActual('react').createContext(null),
+}));
+
+const approval = (
+  allowed: Agents.ToolApprovalDecisionType[] = ['approve', 'reject'],
+): NonNullable<Agents.ToolCall['approval']> => ({
+  actionId: 'action-1',
+  allowed_decisions: allowed,
+});
+
+const renderCards = (cards: React.ReactNode) =>
+  render(
+    <RecoilRoot>
+      <ApprovalProvider>{cards}</ApprovalProvider>
+    </RecoilRoot>,
+  );
+
+describe('ToolApproval', () => {
+  test('enables Submit immediately after Approve is the first decision (#14390)', () => {
+    renderCards(<ToolApproval approval={approval()} toolCallId="call-1" args={{ a: 1 }} />);
+
+    const submit = screen.getByRole('button', { name: 'Submit' });
+    expect(submit).toBeDisabled();
+
+    fireEvent.click(screen.getByRole('button', { name: 'Approve' }));
+
+    expect(submit).toBeEnabled();
+  });
+
+  test('deselecting the active decision disables Submit again', () => {
+    renderCards(<ToolApproval approval={approval()} toolCallId="call-1" args={{ a: 1 }} />);
+
+    const approve = screen.getByRole('button', { name: 'Approve' });
+    fireEvent.click(approve);
+    expect(screen.getByRole('button', { name: 'Submit' })).toBeEnabled();
+
+    fireEvent.click(approve);
+    expect(screen.getByRole('button', { name: 'Submit' })).toBeDisabled();
+  });
+
+  test('a respond decision only counts once its text is non-empty', () => {
+    renderCards(<ToolApproval approval={approval(['respond'])} toolCallId="call-1" args={{}} />);
+
+    fireEvent.click(screen.getByRole('button', { name: 'Respond' }));
+    const submit = screen.getByRole('button', { name: 'Submit' });
+    expect(submit).toBeDisabled();
+
+    fireEvent.change(screen.getByRole('textbox', { name: 'Respond' }), {
+      target: { value: 'use the staging table' },
+    });
+    expect(submit).toBeEnabled();
+  });
+
+  test('multiple paused calls share one Submit that requires every decision', () => {
+    renderCards(
+      <>
+        <ToolApproval approval={approval()} toolCallId="call-1" args={{ a: 1 }} />
+        <ToolApproval approval={approval()} toolCallId="call-2" args={{ b: 2 }} />
+      </>,
+    );
+
+    const submit = screen.getByRole('button', { name: 'Submit 2 decisions' });
+    expect(submit).toBeDisabled();
+
+    const [approveFirst, approveSecond] = screen.getAllByRole('button', { name: 'Approve' });
+    fireEvent.click(approveFirst);
+    expect(submit).toBeDisabled();
+
+    fireEvent.click(approveSecond);
+    expect(submit).toBeEnabled();
+  });
+});