fix: preserve tool run and answer state

This commit is contained in:
Marco Beretta 2026-08-02 03:13:27 +02:00
parent 14e6f1cc07
commit 43b1be085a
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
8 changed files with 220 additions and 28 deletions

View file

@ -92,6 +92,14 @@ function AskUserQuestionSingle({
/** Live pause: share the hook's checked set so the composer's Enter and
* this card submit exactly what the card displays. */
const checkedIndices = isLivePause ? answerMode.checked : localChecked;
const answerValue = isLivePause ? answerMode.answerText : answer;
const setAnswerValue = (value: string) => {
if (isLivePause) {
answerMode.setAnswerText(value);
return;
}
setAnswer(value);
};
const toggleIndex = (index: number) => {
if (isLivePause) {
answerMode.toggleChecked(index);
@ -102,7 +110,7 @@ function AskUserQuestionSingle({
);
};
const trimmed = answer.trim();
const trimmed = answerValue.trim();
const canSubmit = multiSelect
? checkedIndices.length > 0 || trimmed.length > 0
: trimmed.length > 0;
@ -207,16 +215,16 @@ function AskUserQuestionSingle({
)}
<TextareaAutosize
value={answer}
value={answerValue}
disabled={locked}
onChange={(e) => {
setAnswer(e.target.value);
setAnswerValue(e.target.value);
setAskAnswerDraft(actionId, e.target.value);
}}
minRows={2}
maxRows={12}
placeholder={otherLabel ?? localize('com_ui_your_answer')}
className="w-full resize-none rounded-lg border border-border-light bg-surface-chat px-3 py-2 text-sm text-text-primary placeholder:text-text-secondary focus:outline-none"
className="w-full resize-none rounded-lg border border-border-light bg-surface-chat px-3 py-2 text-sm text-text-primary placeholder:text-text-secondary focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-text-primary"
aria-label={localize('com_ui_your_answer')}
/>

View file

@ -23,6 +23,8 @@ import {
import SubagentCall, { SUBAGENT_TICKER_THROTTLE_MS } from '../SubagentCall';
import { MessageContext } from '~/Providers/MessageContext';
const mockMCPServerNames: string[] = [];
jest.mock('~/hooks', () => ({
useLocalize:
() =>
@ -59,14 +61,17 @@ jest.mock('lucide-react', () => ({
jest.mock('~/Providers', () => ({ useAgentsMapContext: () => ({}) }));
jest.mock('~/components/Share/MessageIcon', () => ({ __esModule: true, default: () => null }));
jest.mock('~/hooks/MCP', () => ({ useMCPServerNames: () => [] }));
jest.mock('~/hooks/MCP', () => ({ useMCPServerNames: () => mockMCPServerNames }));
jest.mock('~/utils', () => ({
...jest.requireActual('~/utils/toolLabels'),
cn: (...classes: unknown[]) => classes.filter(Boolean).join(' '),
logger: { log: jest.fn() },
}));
afterEach(() => jest.useRealTimers());
afterEach(() => {
jest.useRealTimers();
mockMCPServerNames.length = 0;
});
function foldEvents(events: SubagentUpdateEvent[]): {
contentParts: SubagentContentPart[];
@ -230,6 +235,42 @@ describe('SubagentCall', () => {
expect(rendered.getSelection()?.durable).toBeUndefined();
});
it('preserves a configured MCP server boundary in the live ticker', () => {
mockMCPServerNames.push('Google_mcp_Workspace');
renderWithState({
toolCallId: 'call_mcp_ticker',
initialProgress: 0.3,
isSubmitting: true,
progress: progressFromEvents({
subagentRunId: 'run_a',
subagentType: 'self',
status: 'run_step',
events: [
{
runId: 'p',
subagentRunId: 'run_a',
subagentType: 'self',
phase: 'run_step',
data: {
stepDetails: {
type: 'tool_calls',
tool_calls: [
{
id: 'c1',
name: 'search_documents_mcp_Google_mcp_Workspace',
},
],
},
},
timestamp: '',
} as SubagentUpdateEvent,
],
}),
});
expect(screen.getByText('Google_mcp_Workspace')).toBeInTheDocument();
});
it('refreshes a long ticker preview only after the throttle window', () => {
jest.useFakeTimers();
const progressFor = (text: string) =>
@ -401,4 +442,5 @@ describe('SubagentCall', () => {
expect(rendered.getSelection()?.durable).toBeUndefined();
expect(rendered.getSelection()?.legacyOutput).toBe(output);
});
});

View file

@ -269,9 +269,9 @@ export default function ToolCallGroup({
/** For a single-tool group, lead with the tool's own (capitalized) label
* instead of the generic "Used 1 tool — name", which reads awkwardly. */
const singleToolLabel = useMemo(() => {
const raw = getToolDisplayLabel(toolMetadata[0]?.name ?? '', localize);
const raw = getToolDisplayLabel(toolMetadata[0]?.name ?? '', localize, mcpServerNames);
return raw ? raw.charAt(0).toUpperCase() + raw.slice(1) : '';
}, [toolMetadata, localize]);
}, [toolMetadata, localize, mcpServerNames]);
const autoExpand = useRecoilValue(store.autoExpandTools);
/** A labeled activity block is summarized by its header, so it collapses

View file

@ -1,13 +1,23 @@
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 AskUserQuestion from '../AskUserQuestion';
const mockSubmitAnswer = jest.fn();
const mockSetAnswerText = jest.fn();
let mockPopoverVisible = false;
let mockCollapsed = false;
let mockLiveActionId: string | null = null;
let mockChecked: number[] = [];
let mockAnswerText = '';
jest.mock('~/hooks', () => ({
useLocalize: () => (key: string) => {
const labels: Record<string, string> = {
com_ui_your_answer: 'Your answer',
com_ui_skip: 'Skip',
com_ui_submit: 'Submit',
com_ui_submitting: 'Submitting',
};
@ -18,14 +28,16 @@ jest.mock('~/hooks', () => ({
jest.mock('~/hooks/Input/useAskAnswerMode', () => ({
__esModule: true,
default: () => ({
popoverVisible: false,
collapsed: false,
popoverVisible: mockPopoverVisible,
collapsed: mockCollapsed,
expand: jest.fn(),
liveAsk: null,
checked: [],
liveAsk: mockLiveActionId == null ? null : { actionId: mockLiveActionId },
checked: mockChecked,
toggleChecked: jest.fn(),
submitOption: jest.fn(),
submitAnswer: jest.fn(),
submitAnswer: mockSubmitAnswer,
answerText: mockAnswerText,
setAnswerText: mockSetAnswerText,
}),
}));
@ -39,18 +51,32 @@ jest.mock('~/store/agents', () => ({
}));
jest.mock('~/Providers/ChatContext', () => ({
ChatContext: jest.requireActual('react').createContext(null),
ChatContext: jest.requireActual('react').createContext({
conversation: { conversationId: 'conversation-1' },
}),
}));
const tree = (key: string) => (
const tree = (
key: string,
question: Agents.AskUserQuestionRequest = { question: 'Which environment?' },
) => (
<RecoilRoot>
<ApprovalProvider>
<AskUserQuestion key={key} actionId="ask-1" question={{ question: 'Which environment?' }} />
<AskUserQuestion key={key} actionId="ask-1" question={question} />
</ApprovalProvider>
</RecoilRoot>
);
describe('AskUserQuestion', () => {
beforeEach(() => {
jest.clearAllMocks();
mockPopoverVisible = false;
mockCollapsed = false;
mockLiveActionId = null;
mockChecked = [];
mockAnswerText = '';
});
test('restores a typed answer after the card remounts inside the same message', () => {
const view = render(tree('direct'));
fireEvent.change(screen.getByRole('textbox', { name: 'Your answer' }), {
@ -61,4 +87,26 @@ describe('AskUserQuestion', () => {
expect(screen.getByRole('textbox', { name: 'Your answer' })).toHaveValue('Use staging first');
});
test('submits checked options together with free text carried from the composer', () => {
mockCollapsed = true;
mockLiveActionId = 'ask-1';
mockChecked = [0];
mockAnswerText = 'carried free-form answer';
render(
tree('live', {
question: 'Choose a source',
multiSelect: true,
options: [{ label: 'Public data', value: 'public' }],
}),
);
expect(screen.getByRole('textbox', { name: 'Your answer' })).toHaveValue(
'carried free-form answer',
);
fireEvent.click(screen.getByRole('button', { name: 'Submit' }));
expect(mockSubmitAnswer).toHaveBeenCalledWith(['public', 'carried free-form answer']);
});
});

View file

@ -7,6 +7,8 @@ import { scheduleMessageContentLayoutReconcile } from '~/hooks';
import ToolCallGroup from '../ToolCallGroup';
import { ToolAuthWarning } from '../auth';
const mockMCPServerNames: string[] = [];
jest.mock('~/hooks', () => ({
useLocalize: () => (key: string, values?: Record<string | number, string>) => {
if (key === 'com_ui_ran_n_actions') {
@ -79,10 +81,9 @@ jest.mock('~/hooks', () => ({
}));
jest.mock('~/hooks/MCP', () => {
const mcpServerNames: string[] = [];
return {
useMCPIconMap: () => new Map(),
useMCPServerNames: () => mcpServerNames,
useMCPServerNames: () => mockMCPServerNames,
};
});
@ -111,7 +112,14 @@ jest.mock('~/utils/approval', () => ({
jest.mock('~/utils', () => ({
cn: (...classes: Array<string | false | null | undefined>) => classes.filter(Boolean).join(' '),
getToolDisplayLabel: (name: string) => {
getToolDisplayLabel: (name: string, _localize: unknown, knownServerNames?: readonly string[]) => {
const configuredServer = knownServerNames?.find((server) => name.endsWith(`_mcp_${server}`));
if (configuredServer) {
return configuredServer;
}
if (name.includes('_mcp_')) {
return name.slice(name.lastIndexOf('_mcp_') + '_mcp_'.length);
}
if (
['execute_code', 'bash_tool', 'run_tools_with_code', 'run_tools_with_bash'].includes(name)
) {
@ -255,6 +263,7 @@ describe('ToolCallGroup image hoisting', () => {
beforeEach(() => {
mockScheduleMessageContentLayoutReconcile.mockClear();
mockMCPServerNames.length = 0;
});
it('renders an AttachmentGroup outside the collapsible container with all attachments', () => {
@ -654,6 +663,22 @@ describe('ToolCallGroup image hoisting', () => {
);
});
it('preserves a configured MCP server boundary in a single-tool label', () => {
mockMCPServerNames.push('Google_mcp_Workspace');
renderGroup({
...baseProps,
parts: [
{
part: makePart('mcp-1', 'result', 'search_documents_mcp_Google_mcp_Workspace'),
idx: 0,
},
],
lastContentIdx: 0,
});
expect(screen.getByRole('button', { name: /^Google_mcp_Workspace$/ })).toBeInTheDocument();
});
it('summarizes repeated completed web searches as an outcome and count', () => {
const searchParts = Array.from({ length: 9 }, (_, idx) => ({
part: makePart(`w${idx}`, 'result', 'web_search'),

View file

@ -20,11 +20,17 @@ jest.mock('react-router-dom', () => ({
jest.mock('@librechat/client', () => {
const React = jest.requireActual<typeof import('react')>('react');
const ReactDOM = jest.requireActual<typeof import('react-dom')>('react-dom');
return {
OGDialog: ({ open, children }: { open: boolean; children: ReactNode }) =>
open ? React.createElement('div', null, children) : null,
open
? ReactDOM.createPortal(
React.createElement('div', null, children),
globalThis.document.body,
)
: null,
OGDialogContent: ({ children }: { children: ReactNode }) =>
React.createElement('div', null, children),
React.createElement('div', { role: 'dialog', 'data-state': 'open' }, children),
Spinner: () => React.createElement('div', { 'data-testid': 'spinner' }),
useToastContext: () => ({
showToast: mockShowToast,
@ -67,7 +73,13 @@ jest.mock('~/utils', () => ({
}));
function getFileInput(container: HTMLElement): HTMLInputElement {
const input = container.querySelector<HTMLInputElement>('input[type="file"]');
/** Prefer the local render container used by the lightweight dialog mock,
* then the active portal. Exiting Headless UI portals can leave older file
* inputs in `document.body`; a document-wide first/last match is unstable. */
const selector = 'input[type="file"][accept=".zip,.skill,.md"]';
const input =
container.querySelector<HTMLInputElement>(selector) ??
document.querySelector<HTMLInputElement>(`[role="dialog"][data-state="open"] ${selector}`);
if (input == null) {
throw new Error('Upload input was not rendered');
}
@ -102,6 +114,18 @@ describe('UploadSkillDialog', () => {
expect(screen.getByText('File size must not exceed 1.06 MB')).toBeInTheDocument();
});
it('targets the current upload input when an exiting portal still has one', () => {
const staleInput = document.createElement('input');
staleInput.type = 'file';
staleInput.accept = '.zip,.skill,.md';
document.body.appendChild(staleInput);
const { container } = render(<UploadSkillDialog isOpen={true} setIsOpen={mockSetIsOpen} />);
expect(getFileInput(container)).not.toBe(staleInput);
staleInput.remove();
});
it('rejects files above the configured skill import limit before upload', () => {
const { container } = render(<UploadSkillDialog isOpen={true} setIsOpen={mockSetIsOpen} />);
const file = new File([new Uint8Array(1024 * 1024 + 1)], 'too-large.skill', {

View file

@ -3,6 +3,7 @@ const mockResetComposer = jest.fn();
const mockGetComposerText = jest.fn(() => 'answer from A');
const mockSetSelected = jest.fn();
const mockSetChecked = jest.fn();
const mockSetAnswerDraft = jest.fn();
let mockSaveDrafts = false;
jest.mock('~/data-provider', () => ({ useGetMessagesByConvoId: jest.fn() }));
@ -16,7 +17,10 @@ jest.mock('~/Providers', () => ({
getValues: mockGetComposerText,
}),
}));
jest.mock('~/utils', () => ({ getAskAnswerDraftId: (id: string) => `draft-${id}` }));
jest.mock('~/utils', () => ({
getAskAnswerDraftId: (id: string) => `draft-${id}`,
morphTransition: (update: () => void) => update(),
}));
jest.mock('recoil', () => ({
atom: (cfg: unknown) => cfg,
useRecoilState: (state: { key?: string }) => {
@ -26,13 +30,16 @@ jest.mock('recoil', () => ({
if (state.key === 'askAnswerModeChecked') {
return [[], mockSetChecked];
}
if (state.key === 'askAnswerModeText') {
return [{ actionId: null, text: '' }, mockSetAnswerDraft];
}
return [[], jest.fn()];
},
useRecoilValue: () => mockSaveDrafts,
}));
jest.mock('~/store', () => ({ __esModule: true, default: { saveDrafts: 'saveDrafts' } }));
import { renderHook } from '@testing-library/react';
import { act, renderHook } from '@testing-library/react';
import { useGetMessagesByConvoId } from '~/data-provider';
import { findLiveAskUserQuestion } from '~/utils/approval';
import useAskAnswerMode from './useAskAnswerMode';
@ -122,6 +129,18 @@ describe('useAskAnswerMode', () => {
expect(result.current.liveAsk).toBeNull();
});
it('carries the composer answer into shared card state when collapsed', () => {
mockUseGetMessages.mockReturnValue({ data: liveAsk });
const { result } = renderHook(() => useAskAnswerMode('conversation-1'));
act(() => result.current.collapse());
expect(mockSetAnswerDraft).toHaveBeenCalledWith({
actionId: 'a1',
text: 'answer from A',
});
});
it('does not let a delayed answer success clear the composer or selection after navigation', () => {
let finishAnswer: (() => void) | undefined;
mockUseGetMessages.mockReturnValue({ data: liveAsk });

View file

@ -40,6 +40,12 @@ const askAnswerCheckedAtom = atom<number[]>({
default: [],
});
/** Free-form answer handed between the composer and the in-message card. */
const askAnswerTextAtom = atom<{ actionId: string | null; text: string }>({
key: 'askAnswerModeText',
default: { actionId: null, text: '' },
});
/**
* First-class "answer mode" for a live `ask_user_question` pause. Clicking an
* option submits it immediately (multi-select clicks toggle instead, confirmed
@ -73,6 +79,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
const [collapsedIds, setCollapsedIds] = useRecoilState(collapsedAskActionsAtom);
const [selected, setSelected] = useRecoilState(askAnswerSelectionAtom);
const [checked, setChecked] = useRecoilState(askAnswerCheckedAtom);
const [answerDraft, setAnswerDraft] = useRecoilState(askAnswerTextAtom);
const saveDrafts = useRecoilValue<boolean>(store.saveDrafts);
const { submitAskAnswer } = useResumeSubmit();
/** Recoil-backed so the lock/status works from the composer, which renders
@ -147,6 +154,15 @@ export default function useAskAnswerMode(conversationId?: string | null) {
() => splitOtherOption(batchMode ? undefined : liveAsk?.question.options),
[batchMode, liveAsk],
);
const answerText = answerDraft.actionId === liveAsk?.actionId ? answerDraft.text : '';
const setAnswerText = useCallback(
(text: string) => {
if (liveAsk) {
setAnswerDraft({ actionId: liveAsk.actionId, text });
}
},
[liveAsk, setAnswerDraft],
);
/** Selection state is per-question: a new pause must never inherit a stale
* highlight (or checks) whose Enter would submit the previous question's
@ -162,13 +178,15 @@ export default function useAskAnswerMode(conversationId?: string | null) {
* which morphTransition's synchronous flush requires. */
const collapse = useCallback(() => {
if (liveAsk) {
morphTransition(() =>
const composerAnswer = formContext?.getValues('text') ?? answerText;
morphTransition(() => {
setAnswerDraft({ actionId: liveAsk.actionId, text: composerAnswer });
setCollapsedIds((prev) =>
prev.includes(liveAsk.actionId) ? prev : [...prev, liveAsk.actionId],
),
);
);
});
}
}, [liveAsk, setCollapsedIds]);
}, [liveAsk, formContext, answerText, setAnswerDraft, setCollapsedIds]);
const expand = useCallback(() => {
if (liveAsk) {
@ -231,6 +249,11 @@ export default function useAskAnswerMode(conversationId?: string | null) {
}
setSelected(null);
setChecked([]);
setAnswerDraft((current) =>
current.actionId === submittedActionId
? { actionId: submittedActionId, text: '' }
: current,
);
if (
(consumedComposerText || (wasActive && saveDrafts)) &&
currentScope.formContext?.getValues('text') === submittedComposerText
@ -251,6 +274,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
submitAskAnswer,
setSelected,
setChecked,
setAnswerDraft,
],
);
@ -481,6 +505,8 @@ export default function useAskAnswerMode(conversationId?: string | null) {
setSelected,
checked,
toggleChecked,
answerText,
setAnswerText,
canSubmit,
submit,
submitOption,