From 7d7b0f0fc79d98e4af3c0102b647c1ee68c947e4 Mon Sep 17 00:00:00 2001
From: Danny Avila <danny@librechat.ai>
Date: Tue, 7 Jul 2026 12:03:12 -0400
Subject: [PATCH] fix: Codex round 3 + real Skip semantics
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Skip now ANSWERS instead of hiding UI (danny): it resumes the run with a
  decline notice ('The user chose not to answer this question.') so the model
  moves on — a client-side dismiss left the run paused until expiry, a hung
  turn. × / Escape remain pure dismiss (switch to the inline card surface).
- P1 (resumed approval tool indices): resumed tool_calls steps whose
  tool_call id matches a seeded UNRESOLVED part now rebind to that seeded
  slot instead of offsetting — the original part resolves in place (output
  attaches) and no duplicate appears; message steps keep the offset, so the
  text-loss fix stands. createContentIndexOffsetHandlers now takes the seed
  array; resolved seeded calls are not rebind targets.
- P2 (stale selection across questions): selection state resets when the
  live actionId changes; the vestigial inline-Other state ('other' selection
  + text atom) is gone — the composer owns free-form.
- P2 (Redis abort path loses the args stamp): the abort route re-stamps the
  question onto the ask tool_call in the reconstructed abort content, so a
  Stop-abandoned question persists with its question intact.
- P2 (malformed args crash): parseAskUserQuestionArgs normalizes untrusted
  shapes (options: {} / non-string entries) instead of throwing in render.
---
 api/server/controllers/agents/client.js       |  2 +-
 api/server/routes/agents/index.js             | 11 ++-
 .../Chat/Input/AskUserQuestionPopover.tsx     |  4 +-
 client/src/hooks/Input/useAskAnswerMode.ts    | 88 ++++++++++---------
 client/src/utils/approval.ts                  | 24 ++++-
 packages/api/src/agents/hitl/resume.spec.ts   | 47 ++++++++--
 packages/api/src/agents/hitl/resume.ts        | 45 ++++++++--
 7 files changed, 161 insertions(+), 60 deletions(-)

diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js
index d8c528fd91..7cb265464e 100644
--- a/api/server/controllers/agents/client.js
+++ b/api/server/controllers/agents/client.js
@@ -1920,7 +1920,7 @@ class AgentClient extends BaseClient {
         // type mismatch, is silently dropped against) the pre-pause content.
         customHandlers: createContentIndexOffsetHandlers(
           this.options.eventHandlers,
-          Array.isArray(seedContent) ? seedContent.length : 0,
+          Array.isArray(seedContent) ? seedContent : [],
         ),
         requestBody: config.configurable.requestBody,
         user: createSafeUser(this.options.req?.user),
diff --git a/api/server/routes/agents/index.js b/api/server/routes/agents/index.js
index f479ab4b3d..a4221a3c37 100644
--- a/api/server/routes/agents/index.js
+++ b/api/server/routes/agents/index.js
@@ -8,6 +8,7 @@ const {
   toClientPendingAction,
   isHITLEnabled,
   deleteAgentCheckpoint,
+  attachAskUserQuestionArgs,
 } = require('@librechat/api');
 const { createSseStreamTelemetry } = require('@librechat/api/telemetry');
 const { logger } = require('@librechat/data-schemas');
@@ -318,7 +319,15 @@ router.post('/chat/abort', configMiddleware, async (req, res) => {
       abortResult.jobData?.responseMessageId &&
       hasPersistableAbortContent(abortResult.content)
     ) {
-      const { jobData, content, text } = abortResult;
+      const { jobData, text } = abortResult;
+      let { content } = abortResult;
+      // Redis reconstructs abort content from the graph/chunk log, which never saw
+      // the pause-time args stamp applied to the in-process contentParts — re-stamp
+      // here so a question abandoned via Stop persists with its question intact.
+      const abortedPayload = job.metadata?.pendingAction?.payload;
+      if (abortedPayload?.type === 'ask_user_question' && Array.isArray(content)) {
+        content = attachAskUserQuestionArgs(content, abortedPayload.question);
+      }
       const responseMessage = {
         messageId: jobData.responseMessageId,
         parentMessageId: jobData.userMessage.messageId,
diff --git a/client/src/components/Chat/Input/AskUserQuestionPopover.tsx b/client/src/components/Chat/Input/AskUserQuestionPopover.tsx
index 880be0d70b..87fc0bdc3a 100644
--- a/client/src/components/Chat/Input/AskUserQuestionPopover.tsx
+++ b/client/src/components/Chat/Input/AskUserQuestionPopover.tsx
@@ -14,7 +14,7 @@ import { cn } from '~/utils';
  */
 function AskUserQuestionPopoverContent({ conversationId }: { conversationId: string }) {
   const localize = useLocalize();
-  const { liveAsk, active, options, selected, setSelected, canSubmit, submit, dismiss } =
+  const { liveAsk, active, options, selected, setSelected, canSubmit, submit, skip, dismiss } =
     useAskAnswerMode(conversationId);
 
   if (!active || !liveAsk) {
@@ -61,7 +61,7 @@ function AskUserQuestionPopoverContent({ conversationId }: { conversationId: str
           </button>
         ))}
         <div className="flex items-center justify-end gap-2 p-2">
-          <Button size="sm" variant="outline" onClick={dismiss}>
+          <Button size="sm" variant="outline" onClick={() => skip()}>
             {localize('com_ui_skip')}
           </Button>
           <Button size="sm" variant="submit" disabled={!canSubmit} onClick={() => submit()}>
diff --git a/client/src/hooks/Input/useAskAnswerMode.ts b/client/src/hooks/Input/useAskAnswerMode.ts
index f641b22174..b1c17d13d8 100644
--- a/client/src/hooks/Input/useAskAnswerMode.ts
+++ b/client/src/hooks/Input/useAskAnswerMode.ts
@@ -1,7 +1,11 @@
-import { useCallback, useMemo } from 'react';
+import { useCallback, useEffect, useMemo } from 'react';
 import { atom, useRecoilState } from 'recoil';
+import {
+  ASK_USER_DECLINED_ANSWER,
+  findLiveAskUserQuestion,
+  splitOtherOption,
+} from '~/utils/approval';
 import { useResumeSubmit } from '~/components/Chat/Messages/Content/ApprovalContext';
-import { findLiveAskUserQuestion, splitOtherOption } from '~/utils/approval';
 import { useGetMessagesByConvoId } from '~/data-provider';
 
 /** Dismissed action ids — recoil so every consumer reacts. */
@@ -10,18 +14,12 @@ const dismissedAskActionsAtom = atom<string[]>({
   default: [],
 });
 
-/** Current selection: an option index, the inline free-form row, or nothing. */
-const askAnswerSelectionAtom = atom<number | 'other' | null>({
+/** Currently highlighted option row, or nothing. */
+const askAnswerSelectionAtom = atom<number | null>({
   key: 'askAnswerModeSelection',
   default: null,
 });
 
-/** Text typed into the popover's inline "Other" input. */
-const askAnswerOtherTextAtom = atom<string>({
-  key: 'askAnswerModeOtherText',
-  default: '',
-});
-
 /**
  * First-class "answer mode" for a live `ask_user_question` pause, with
  * select-then-confirm semantics (borrowed from Claude Code's AskUserQuestion
@@ -43,7 +41,6 @@ export default function useAskAnswerMode(conversationId?: string | null) {
   );
   const [dismissedIds, setDismissedIds] = useRecoilState(dismissedAskActionsAtom);
   const [selected, setSelected] = useRecoilState(askAnswerSelectionAtom);
-  const [otherText, setOtherText] = useRecoilState(askAnswerOtherTextAtom);
   const { submitAskAnswer } = useResumeSubmit();
 
   const dismissed = liveAsk != null && dismissedIds.includes(liveAsk.actionId);
@@ -53,6 +50,12 @@ export default function useAskAnswerMode(conversationId?: string | null) {
     [liveAsk],
   );
 
+  /** Selection is per-question: a new pause must never inherit a stale
+   *  highlight whose Enter would submit the previous question's choice. */
+  useEffect(() => {
+    setSelected(null);
+  }, [liveAsk?.actionId, setSelected]);
+
   const dismiss = useCallback(() => {
     if (liveAsk) {
       setDismissedIds((prev) =>
@@ -61,32 +64,17 @@ export default function useAskAnswerMode(conversationId?: string | null) {
     }
   }, [liveAsk, setDismissedIds]);
 
-  const canSubmit =
-    active &&
-    (typeof selected === 'number'
-      ? options[selected] != null
-      : selected === 'other' && otherText.trim().length > 0);
+  const canSubmit = active && typeof selected === 'number' && options[selected] != null;
 
   /** Fires the confirmed selection; returns true when an answer was sent. */
   const submit = useCallback((): boolean => {
-    if (!liveAsk || !canSubmit) {
+    if (!liveAsk || !canSubmit || typeof selected !== 'number') {
       return false;
     }
-    const answer = typeof selected === 'number' ? options[selected].value : otherText.trim();
-    submitAskAnswer(liveAsk.actionId, answer);
+    submitAskAnswer(liveAsk.actionId, options[selected].value);
     setSelected(null);
-    setOtherText('');
     return true;
-  }, [
-    liveAsk,
-    canSubmit,
-    selected,
-    options,
-    otherText,
-    submitAskAnswer,
-    setSelected,
-    setOtherText,
-  ]);
+  }, [liveAsk, canSubmit, selected, options, submitAskAnswer, setSelected]);
 
   /** Composer text answers the question directly; true when consumed. */
   const submitText = useCallback(
@@ -98,13 +86,26 @@ export default function useAskAnswerMode(conversationId?: string | null) {
       if (trimmed.length > 0) {
         submitAskAnswer(liveAsk.actionId, trimmed);
         setSelected(null);
-        setOtherText('');
       }
       return true;
     },
-    [active, liveAsk, submitAskAnswer, setSelected, setOtherText],
+    [active, liveAsk, submitAskAnswer, setSelected],
   );
 
+  /**
+   * Explicitly decline: resumes the run with a canned notice so the model
+   * knows the user chose not to answer. A client-side dismiss alone would
+   * leave the run paused until expiry — a hung turn.
+   */
+  const skip = useCallback((): boolean => {
+    if (!active || !liveAsk) {
+      return false;
+    }
+    submitAskAnswer(liveAsk.actionId, ASK_USER_DECLINED_ANSWER);
+    setSelected(null);
+    return true;
+  }, [active, liveAsk, submitAskAnswer, setSelected]);
+
   /** Selection steering from the empty composer; true when consumed. */
   const handleComposerKeyDown = useCallback(
     (e: React.KeyboardEvent<HTMLTextAreaElement>): boolean => {
@@ -121,25 +122,27 @@ export default function useAskAnswerMode(conversationId?: string | null) {
         }
         return false;
       }
-      const rowCount = options.length + 1; // + the inline Other row
-      const asIndex = (value: number | 'other' | null): number =>
-        value === 'other' ? options.length : (value ?? -1);
-      const fromIndex = (index: number): number | 'other' =>
-        index >= options.length ? 'other' : index;
+      if (options.length === 0) {
+        if (e.key === 'Escape') {
+          dismiss();
+          return true;
+        }
+        return false;
+      }
       const digit = Number.parseInt(e.key, 10);
-      if (!Number.isNaN(digit) && digit >= 1 && digit <= Math.min(rowCount, 9)) {
+      if (!Number.isNaN(digit) && digit >= 1 && digit <= Math.min(options.length, 9)) {
         e.preventDefault();
-        setSelected(fromIndex(digit - 1));
+        setSelected(digit - 1);
         return true;
       }
       if (e.key === 'ArrowDown') {
         e.preventDefault();
-        setSelected(fromIndex((asIndex(selected) + 1) % rowCount));
+        setSelected(((selected ?? -1) + 1) % options.length);
         return true;
       }
       if (e.key === 'ArrowUp') {
         e.preventDefault();
-        setSelected(fromIndex((asIndex(selected) - 1 + rowCount) % rowCount));
+        setSelected(((selected ?? 0) - 1 + options.length) % options.length);
         return true;
       }
       if (e.key === 'Enter' && !e.shiftKey && canSubmit) {
@@ -164,11 +167,10 @@ export default function useAskAnswerMode(conversationId?: string | null) {
     dismiss,
     selected,
     setSelected,
-    otherText,
-    setOtherText,
     canSubmit,
     submit,
     submitText,
+    skip,
     handleComposerKeyDown,
     /** Model-supplied "Other"-style label, folded into the inline input. */
     otherLabel,
diff --git a/client/src/utils/approval.ts b/client/src/utils/approval.ts
index c197b3dbd9..0b9cb2de57 100644
--- a/client/src/utils/approval.ts
+++ b/client/src/utils/approval.ts
@@ -10,6 +10,13 @@ import type { Agents, TMessage, TMessageContentParts } from 'librechat-data-prov
  */
 export const ASK_USER_QUESTION = 'ask_user_question' as const;
 
+/**
+ * Answer sent when the user explicitly skips a question: the run must resume
+ * (a client-side dismiss would leave it paused until expiry — a hung turn),
+ * and the model needs to know the user declined rather than answered.
+ */
+export const ASK_USER_DECLINED_ANSWER = 'The user chose not to answer this question.';
+
 /** Shape of the synthetic content part carrying an ask-user pending action. */
 export interface AskUserQuestionPart {
   type: typeof ASK_USER_QUESTION;
@@ -203,7 +210,22 @@ export function parseAskUserQuestionArgs(
   ) {
     return null;
   }
-  return parsed as unknown as Agents.AskUserQuestionRequest;
+  const request = parsed as { question: string; description?: unknown; options?: unknown };
+  /** Model/persisted args are untrusted — normalize instead of crashing the
+   *  message render on shapes like `options: {}` or non-string entries. */
+  const options = Array.isArray(request.options)
+    ? request.options.filter(
+        (option): option is Agents.AskUserQuestionOption =>
+          option != null &&
+          typeof (option as { label?: unknown }).label === 'string' &&
+          typeof (option as { value?: unknown }).value === 'string',
+      )
+    : undefined;
+  return {
+    question: request.question,
+    description: typeof request.description === 'string' ? request.description : undefined,
+    options: options && options.length > 0 ? options : undefined,
+  };
 }
 
 /**
diff --git a/packages/api/src/agents/hitl/resume.spec.ts b/packages/api/src/agents/hitl/resume.spec.ts
index 0fc146e25b..6adf562841 100644
--- a/packages/api/src/agents/hitl/resume.spec.ts
+++ b/packages/api/src/agents/hitl/resume.spec.ts
@@ -174,6 +174,7 @@ describe('findIncompleteDecisions', () => {
 });
 
 describe('createContentIndexOffsetHandlers', () => {
+  const textSeed = (n: number) => Array.from({ length: n }, () => ({ type: 'text' }));
   const makeRecorder = () => {
     const calls: Array<{ event: string; data: unknown }> = [];
     const handler = { handle: (event: string, data: unknown) => void calls.push({ event, data }) };
@@ -183,13 +184,13 @@ describe('createContentIndexOffsetHandlers', () => {
   it('returns the input untouched for offset 0 / undefined handlers', () => {
     const { handler } = makeRecorder();
     const handlers = { on_run_step: handler };
-    expect(createContentIndexOffsetHandlers(handlers, 0)).toBe(handlers);
-    expect(createContentIndexOffsetHandlers(undefined, 3)).toBeUndefined();
+    expect(createContentIndexOffsetHandlers(handlers, [])).toBe(handlers);
+    expect(createContentIndexOffsetHandlers(undefined, textSeed(3))).toBeUndefined();
   });
 
   it('shifts ON_RUN_STEP index by the offset without mutating the original payload', () => {
     const { calls, handler } = makeRecorder();
-    const wrapped = createContentIndexOffsetHandlers({ on_run_step: handler }, 3)!;
+    const wrapped = createContentIndexOffsetHandlers({ on_run_step: handler }, textSeed(3))!;
     const runStep = { id: 'step_1', index: 0, stepDetails: { type: 'message_creation' } };
     wrapped.on_run_step.handle('on_run_step', runStep as never);
     expect((calls[0].data as { index: number }).index).toBe(3);
@@ -198,7 +199,7 @@ describe('createContentIndexOffsetHandlers', () => {
 
   it('shifts ON_AGENT_UPDATE nested index', () => {
     const { calls, handler } = makeRecorder();
-    const wrapped = createContentIndexOffsetHandlers({ on_agent_update: handler }, 2)!;
+    const wrapped = createContentIndexOffsetHandlers({ on_agent_update: handler }, textSeed(2))!;
     wrapped.on_agent_update.handle('on_agent_update', {
       agent_update: { index: 1, runId: 'r' },
     } as never);
@@ -210,7 +211,7 @@ describe('createContentIndexOffsetHandlers', () => {
     const deltaHandler = { handle: jest.fn() };
     const wrapped = createContentIndexOffsetHandlers(
       { on_run_step: handler, on_message_delta: deltaHandler },
-      5,
+      textSeed(5),
     )!;
     expect(wrapped.on_message_delta).toBe(deltaHandler);
     // Deltas carry no index — they resolve through the (shifted) stepMap entry.
@@ -218,9 +219,43 @@ describe('createContentIndexOffsetHandlers', () => {
     expect(deltaHandler.handle).toHaveBeenCalledTimes(1);
   });
 
+  it('rebinds a resumed tool step to its seeded unresolved slot by tool_call id', () => {
+    const { calls, handler } = makeRecorder();
+    const seed = [
+      { type: 'text' },
+      { type: 'tool_call', tool_call: { id: 'tc_paused', output: '' } },
+      { type: 'tool_call', tool_call: { id: 'tc_done', output: 'already resolved' } },
+    ];
+    const wrapped = createContentIndexOffsetHandlers({ on_run_step: handler }, seed)!;
+
+    // The paused call's re-execution must land on its seeded slot (1), not 0+3.
+    wrapped.on_run_step.handle('on_run_step', {
+      id: 'step_t',
+      index: 0,
+      stepDetails: { type: 'tool_calls', tool_calls: [{ id: 'tc_paused' }] },
+    } as never);
+    expect((calls[0].data as { index: number }).index).toBe(1);
+
+    // A resolved seeded call is NOT a rebind target — a fresh same-id step offsets.
+    wrapped.on_run_step.handle('on_run_step', {
+      id: 'step_u',
+      index: 1,
+      stepDetails: { type: 'tool_calls', tool_calls: [{ id: 'tc_done' }] },
+    } as never);
+    expect((calls[1].data as { index: number }).index).toBe(4);
+
+    // Message steps always offset.
+    wrapped.on_run_step.handle('on_run_step', {
+      id: 'step_m',
+      index: 2,
+      stepDetails: { type: 'message_creation' },
+    } as never);
+    expect((calls[2].data as { index: number }).index).toBe(5);
+  });
+
   it('leaves a runStep without a numeric index unshifted (defensive)', () => {
     const { calls, handler } = makeRecorder();
-    const wrapped = createContentIndexOffsetHandlers({ on_run_step: handler }, 4)!;
+    const wrapped = createContentIndexOffsetHandlers({ on_run_step: handler }, textSeed(4))!;
     const weird = { id: 'step_x' };
     wrapped.on_run_step.handle('on_run_step', weird as never);
     expect(calls[0].data).toBe(weird);
diff --git a/packages/api/src/agents/hitl/resume.ts b/packages/api/src/agents/hitl/resume.ts
index 015e313f09..043c13f177 100644
--- a/packages/api/src/agents/hitl/resume.ts
+++ b/packages/api/src/agents/hitl/resume.ts
@@ -143,23 +143,56 @@ export function findIncompleteDecisions(
  */
 export function createContentIndexOffsetHandlers(
   handlers: Record<string, EventHandler> | undefined,
-  offset: number,
+  seedContent: Array<{ type?: string; tool_call?: { id?: string; output?: unknown } }> = [],
 ): Record<string, EventHandler> | undefined {
+  const offset = seedContent.length;
   if (handlers == null || !(offset > 0)) {
     return handlers;
   }
 
+  /**
+   * Resumed tool steps for calls the PAUSED turn already rendered must land
+   * back on their seeded slot — not a fresh offset slot — or the original
+   * part stays unresolved while a duplicate completed one appears after the
+   * seed (and its output never attaches). Map unresolved seeded tool_calls by
+   * id so the resume pass's re-execution (approval flows re-run the approved
+   * tool; ask re-runs its body) rebinds to the right index.
+   */
+  const seededToolCallIndex = new Map<string, number>();
+  seedContent.forEach((part, index) => {
+    const toolCall = part?.tool_call;
+    const unresolved =
+      typeof toolCall?.output !== 'string' || (toolCall.output as string).length === 0;
+    if (part?.type === 'tool_call' && typeof toolCall?.id === 'string' && unresolved) {
+      seededToolCallIndex.set(toolCall.id, index);
+    }
+  });
+
   const wrapped: Record<string, EventHandler> = { ...handlers };
 
   const runStepHandler = handlers[GraphEvents.ON_RUN_STEP];
   if (runStepHandler) {
     wrapped[GraphEvents.ON_RUN_STEP] = {
       handle: (event, data, metadata, graph) => {
-        const runStep = data as { index?: number } | undefined;
-        const shifted =
-          runStep != null && typeof runStep.index === 'number'
-            ? { ...runStep, index: runStep.index + offset }
-            : data;
+        const runStep = data as
+          | {
+              index?: number;
+              stepDetails?: { type?: string; tool_calls?: Array<{ id?: string }> };
+            }
+          | undefined;
+        if (runStep == null || typeof runStep.index !== 'number') {
+          return runStepHandler.handle(event, data, metadata, graph);
+        }
+        const seededIndex =
+          runStep.stepDetails?.type === 'tool_calls'
+            ? runStep.stepDetails.tool_calls
+                ?.map((call) => (call.id ? seededToolCallIndex.get(call.id) : undefined))
+                .find((index) => index != null)
+            : undefined;
+        const shifted = {
+          ...runStep,
+          index: seededIndex ?? runStep.index + offset,
+        };
         return runStepHandler.handle(event, shifted as typeof data, metadata, graph);
       },
     };