🙋 fix: Stop answered ask_user_question card from reopening its popover (#14297)

client/src/hooks/Input/useAskAnswerMode.ts

@@ -94,8 +94,14 @@ export default function useAskAnswerMode(conversationId?: string | null) {
    * suppressing it behind an open popover would strand the user at a locked
    * card with no explanation. (`error`, unlike `expired`, stays active: it is
    * retryable — see the composer-preserving submit path.)
+   *
+   * A `submitted` question is likewise done — the run has resumed and there is
+   * nothing left to answer. The chat card already self-hides on it; without the
+   * same test here a card that outlives its strip (a resurrected copy, or a
+   * submit whose store write couldn't run) holds the popover open over an
+   * answered question with every option greyed out.
    */
-  const active = liveAsk != null && !dismissed && status !== 'expired';
+  const active = liveAsk != null && !dismissed && status !== 'expired' && status !== 'submitted';
   const collapsed = active && collapsedIds.includes(liveAsk.actionId);
   /** The popover renders only while expanded; collapse keeps `active` (and the
    *  composer's answer role) but hands the question display to the chat card. */

client/src/hooks/SSE/__tests__/useStepHandler.spec.ts

@@ -18,6 +18,7 @@ import type {
   Agents,
 } from 'librechat-data-provider';
 import { subagentProgressByToolCallId } from '~/store/subagents';
+import { resolveAskUserQuestionPart } from '~/utils/approval';
 import useStepHandler from '~/hooks/SSE/useStepHandler';
 
 /** `Constants` is a heterogeneous enum (`string | number`); annotate as
@@ -1899,6 +1900,117 @@ describe('useStepHandler', () => {
       consoleSpy.mockRestore();
     });
 
+    it('drops an answered ask card when the resumed segment streams AROUND its slot', () => {
+      /**
+       * The first event after a resume re-renders the ask tool_call at ITS OWN
+       * index, never the card's, so the slot displacement above cannot fire.
+       * This handler's cached copy still holds the card the answer-submit
+       * stripped from the store; writing it back reopened the popover over an
+       * answered question with its options locked.
+       */
+      const askPart = {
+        type: 'ask_user_question',
+        ask_user_question: { actionId: 'a2', question: { question: 'Which env?' } },
+      } as unknown as TMessageContentParts;
+      const askToolCall = {
+        type: ContentTypes.TOOL_CALL,
+        tool_call: {
+          id: 'ask-call-1',
+          name: 'ask_user_question',
+          args: '{"question":"Which env?"}',
+          type: ToolCallTypes.TOOL_CALL,
+        },
+      } as unknown as TMessageContentParts;
+      /** Card appended at the END (index 2), ask tool_call at index 1. */
+      const paused = createResponseMessage({
+        content: [{ type: ContentTypes.TEXT, text: 'Let me ask.' }, askToolCall, askPart],
+      });
+
+      const { result } = renderHook(() => useStepHandler(createHookParams()));
+      act(() => {
+        result.current.syncStepMessage(paused);
+      });
+
+      /** The user answers: the store copy is stripped + stamped, and the action
+       *  is recorded as answered. The handler's cached copy is untouched. */
+      const answered = resolveAskUserQuestionPart(paused, 'a2', 'prod');
+      mockGetMessages.mockReturnValue([answered]);
+
+      /** Resume replays the ask tool_call's completion at index 1, not 2. */
+      const askCompletion = createToolCallRunStep({
+        id: 'step-ask',
+        index: 1,
+        stepDetails: {
+          type: StepTypes.TOOL_CALLS,
+          tool_calls: [
+            {
+              id: 'ask-call-1',
+              name: 'ask_user_question',
+              args: '{"question":"Which env?"}',
+              output: 'prod',
+              type: ToolCallTypes.TOOL_CALL,
+            },
+          ],
+        },
+      } as Partial<Agents.RunStep>);
+
+      act(() => {
+        result.current.stepHandler(
+          { event: StepEvents.ON_RUN_STEP, data: askCompletion },
+          createSubmission(),
+        );
+      });
+
+      const lastCall = mockSetMessages.mock.calls[mockSetMessages.mock.calls.length - 1];
+      const updated = (lastCall[0] as TMessage[]).find((m) => m.messageId === 'response-msg-1');
+      const content = (updated?.content ?? []) as Array<{ type?: string }>;
+      expect(content.some((part) => part?.type === 'ask_user_question')).toBe(false);
+      /** The real content is untouched: only the answered card is dropped. */
+      expect(content[1]).toMatchObject({ type: ContentTypes.TOOL_CALL });
+    });
+
+    it('keeps a still-live ask card when an event streams around its slot', () => {
+      /** Only ANSWERED cards are droppable — a late event racing a live pause
+       *  must not take the question away before the user can respond. */
+      const askPart = {
+        type: 'ask_user_question',
+        ask_user_question: { actionId: 'a3', question: { question: 'Live?' } },
+      } as unknown as TMessageContentParts;
+      const responseMessage = createResponseMessage({
+        content: [{ type: ContentTypes.TEXT, text: 'Pre-pause' }, askPart],
+      });
+      mockGetMessages.mockReturnValue([responseMessage]);
+
+      const { result } = renderHook(() => useStepHandler(createHookParams()));
+      act(() => {
+        result.current.syncStepMessage(responseMessage);
+      });
+
+      act(() => {
+        result.current.stepHandler(
+          { event: StepEvents.ON_RUN_STEP, data: createRunStep({ index: 0 }) },
+          createSubmission(),
+        );
+      });
+      act(() => {
+        result.current.stepHandler(
+          {
+            event: StepEvents.ON_MESSAGE_DELTA,
+            data: {
+              id: 'step-1',
+              delta: { content: [{ type: ContentTypes.TEXT, text: ' more' }] },
+            } as Agents.MessageDeltaEvent,
+          },
+          createSubmission(),
+        );
+      });
+
+      const lastCall = mockSetMessages.mock.calls[mockSetMessages.mock.calls.length - 1];
+      const updated = (lastCall[0] as TMessage[]).find((m) => m.messageId === 'response-msg-1');
+      const content = (updated?.content ?? []) as Array<{ type?: string }>;
+      expect(content.some((part) => part?.type === 'ask_user_question')).toBe(true);
+    });
+
     it('should warn on content type mismatch and not overwrite', () => {
       const consoleSpy = jest.spyOn(console, 'warn').mockImplementation();
 

client/src/hooks/SSE/useStepHandler.ts

@@ -27,8 +27,8 @@ import {
   initSubagentAggregatorState,
   initSubagentTickerState,
 } from '~/utils/subagentContent';
+import { isAskUserQuestionPart, isAnsweredAskUserQuestionPart } from '~/utils/approval';
 import { subagentProgressByToolCallId, sandboxStartingByToolCallId } from '~/store';
-import { isAskUserQuestionPart } from '~/utils/approval';
 import { MESSAGE_UPDATE_INTERVAL } from '~/common';
 
 type TUseStepHandler = {
@@ -388,6 +388,17 @@ export default function useStepHandler({
      */
     if (isAskUserQuestionPart(updatedContent[index])) {
       updatedContent = updatedContent.filter((part) => !isAskUserQuestionPart(part));
+    } else if (updatedContent.some(isAnsweredAskUserQuestionPart)) {
+      /**
+       * An ALREADY-ANSWERED card the resumed segment streams around rather than
+       * into: the first event after the resume re-renders the ask tool_call at
+       * ITS OWN index, so the slot test above never fires and this handler's
+       * cached copy — which still holds the card the answer-submit stripped from
+       * the store — gets written back, reopening the popover with its options
+       * locked. Only cards the user actually answered are dropped, so an event
+       * racing a still-live pause can't take its card down.
+       */
+      updatedContent = updatedContent.filter((part) => !isAnsweredAskUserQuestionPart(part));
     }
 
     if (!updatedContent[index] && contentType !== ContentTypes.TOOL_CALL) {

client/src/utils/approval.spec.ts

@@ -11,6 +11,7 @@ import {
   resolveAskUserQuestionPart,
   getSubmittedAskAnswer,
   findLiveAskUserQuestion,
+  isAnsweredAskUserQuestionPart,
   splitOtherOption,
 } from './approval';
 
@@ -391,6 +392,52 @@ describe('findLiveAskUserQuestion', () => {
     expect(findLiveAskUserQuestion(null)).toBeNull();
     expect(findLiveAskUserQuestion(undefined)).toBeNull();
   });
+
+  /**
+   * The strip on answer-submit is a store write, so any holder of an older copy
+   * of the message (the SSE step handler's in-flight cache, a replayed event)
+   * can put the card back. Honouring a resurrected card reopened the popover
+   * over a question the user had already answered, options greyed out.
+   */
+  it('ignores a resurrected card for an answered question', () => {
+    const paused = applyPendingAction(msg({ content: [] }), askAction({ actionId: 'a-answered' }));
+    expect(findLiveAskUserQuestion([paused])?.actionId).toBe('a-answered');
+
+    resolveAskUserQuestionPart(paused, 'a-answered', 'Ada');
+
+    // `paused` is the pre-answer copy — exactly what a stale cache writes back.
+    expect(findLiveAskUserQuestion([paused])).toBeNull();
+  });
+
+  it('falls back to an older live question when the newest is answered', () => {
+    const older = applyPendingAction(msg({ content: [] }), askAction({ actionId: 'a-live' }));
+    const newer = applyPendingAction(
+      { ...msg({ content: [] }), messageId: 'm2' },
+      askAction({ actionId: 'a-done' }),
+    );
+    resolveAskUserQuestionPart(newer, 'a-done', 'Ada');
+
+    expect(findLiveAskUserQuestion([older, newer])?.actionId).toBe('a-live');
+  });
+});
+
+describe('isAnsweredAskUserQuestionPart', () => {
+  it('marks only cards whose question was actually answered', () => {
+    const live = applyPendingAction(msg({ content: [] }), askAction({ actionId: 'a-open' }));
+    const answered = applyPendingAction(msg({ content: [] }), askAction({ actionId: 'a-closed' }));
+    resolveAskUserQuestionPart(answered, 'a-closed', 'Ada');
+
+    expect(isAnsweredAskUserQuestionPart(answered.content?.[0])).toBe(true);
+    expect(isAnsweredAskUserQuestionPart(live.content?.[0])).toBe(false);
+    expect(isAnsweredAskUserQuestionPart(textPart('hi'))).toBe(false);
+    expect(isAnsweredAskUserQuestionPart(undefined)).toBe(false);
+  });
+
+  it('stays false for a question whose resolve found no matching card', () => {
+    const paused = applyPendingAction(msg({ content: [] }), askAction({ actionId: 'a-other' }));
+    resolveAskUserQuestionPart(paused, 'a-missing', 'Ada');
+    expect(isAnsweredAskUserQuestionPart(paused.content?.[0])).toBe(false);
+  });
 });
 
 describe('applyPendingAction — unsupported type', () => {

client/src/utils/approval.ts

@@ -272,11 +272,28 @@ export function removeAskUserQuestionPart(message: TMessage, actionId: string):
  */
 const submittedAskAnswers = new Map<string, string>();
 
+/**
+ * Ask actions the user has answered this session. Same rationale as
+ * {@link submittedAskAnswers}: the SSE step handler evolves its own cached copy
+ * of the streaming message, so the store-level strip below can't reach it —
+ * writing that copy back would resurrect an answered card. Keyed by `actionId`
+ * and only ever added to by {@link resolveAskUserQuestionPart}, so a step event
+ * racing a still-LIVE pause can never mistake its card for an answered one.
+ */
+const answeredAskActionIds = new Set<string>();
+
 /** The locally-submitted answer for an ask tool_call, if any. */
 export function getSubmittedAskAnswer(toolCallId: string | undefined): string | undefined {
   return toolCallId ? submittedAskAnswers.get(toolCallId) : undefined;
 }
 
+/** Whether `part` is an ask card whose question the user already answered. */
+export const isAnsweredAskUserQuestionPart = (
+  part: Partial<TMessageContentParts> | undefined,
+): boolean =>
+  isAskUserQuestionPart(part) &&
+  answeredAskActionIds.has((part as unknown as AskUserQuestionPart)[ASK_USER_QUESTION].actionId);
+
 /**
  * Resolve an answered ask-user-question pause on the client, mirroring the
  * server's resume-time stamp so the durable Q&A card shows the answer the
@@ -303,6 +320,7 @@ export function resolveAskUserQuestionPart(
   if (!syntheticPart) {
     return message;
   }
+  answeredAskActionIds.add(actionId);
 
   let patched = false;
   const nextContent: TMessageContentParts[] = [];
@@ -381,6 +399,11 @@ export function splitOtherOption(options: Agents.AskUserQuestionOption[] | undef
  * messages — the newest synthetic part wins. Drives the composer popover: the
  * part exists exactly while a pause is live (applied on `on_pending_action`,
  * stripped when the answer submits), so its presence IS the popover signal.
+ *
+ * Answered cards are skipped rather than assumed absent: the strip is a store
+ * write, and any holder of an older message copy (the SSE step handler's
+ * in-flight cache, a replayed event) can put one back. Honouring it would
+ * reopen the popover on a question the user already answered.
  */
 export function findLiveAskUserQuestion(
   messages: TMessage[] | null | undefined,
@@ -396,7 +419,7 @@ export function findLiveAskUserQuestion(
     }
     for (let j = content.length - 1; j >= 0; j--) {
       const part = content[j];
-      if (isAskUserQuestionPart(part)) {
+      if (isAskUserQuestionPart(part) && !isAnsweredAskUserQuestionPart(part)) {
         const ask = (part as unknown as AskUserQuestionPart)[ASK_USER_QUESTION];
         return { actionId: ask.actionId, question: ask.question, messageId: message.messageId };
       }