diff --git a/api/package.json b/api/package.json index 05d4488392..3181046fc1 100644 --- a/api/package.json +++ b/api/package.json @@ -46,7 +46,7 @@ "@azure/storage-blob": "^12.30.0", "@google/genai": "^2.8.0", "@keyv/redis": "^4.3.3", - "@librechat/agents": "^3.4.4", + "@librechat/agents": "^3.4.5", "@librechat/api": "*", "@librechat/data-schemas": "*", "@microsoft/microsoft-graph-client": "^3.0.7", diff --git a/api/server/controllers/agents/__tests__/resume.spec.js b/api/server/controllers/agents/__tests__/resume.spec.js index 1fa910dbc9..bce83e1264 100644 --- a/api/server/controllers/agents/__tests__/resume.spec.js +++ b/api/server/controllers/agents/__tests__/resume.spec.js @@ -166,6 +166,20 @@ function makeAskUserJob(overrides = {}) { return job; } +function makeAskUserBatchJob(overrides = {}) { + const job = makeToolApprovalJob(overrides); + job.metadata.pendingAction.payload = { + type: 'ask_user_question', + question: { question: 'Which environment?' }, + questions: [ + { id: 'environment', question: 'Which environment?' }, + { id: 'window', question: 'Which time window?' }, + ], + tool_call_id: 'tc1', + }; + return job; +} + /** A mock reconstructed client for the post-ACK path. */ function makeClient(overrides = {}) { return { @@ -629,6 +643,30 @@ describe('ResumeAgentController (POST /agents/chat/resume)', () => { expect(mockGenerationJobManager.approvals.resolve).not.toHaveBeenCalled(); }); + it('400 when a batched answer omits a question or includes an unknown id', async () => { + mockGenerationJobManager.getJob.mockResolvedValue(makeAskUserBatchJob()); + const missing = await post({ + conversationId: CONVO_ID, + actionId: ACTION_ID, + agent_id: AGENT_ID, + endpoint: 'agents', + answers: { environment: 'staging' }, + }); + expect(missing.status).toBe(400); + expect(missing.body.error).toMatch(/every question/i); + + mockGenerationJobManager.getJob.mockResolvedValue(makeAskUserBatchJob()); + const extra = await post({ + conversationId: CONVO_ID, + actionId: ACTION_ID, + agent_id: AGENT_ID, + endpoint: 'agents', + answers: { environment: 'staging', window: '7d', region: 'us-east-2' }, + }); + expect(extra.status).toBe(400); + expect(extra.body.error).toMatch(/unknown question id/i); + }); + it('400 on an unsupported pending-action type', async () => { const job = makeToolApprovalJob(); job.metadata.pendingAction.payload = { type: 'totally_unknown' }; @@ -1313,6 +1351,26 @@ describe('ResumeAgentController (POST /agents/chat/resume)', () => { ); }); + it('resumes a batched ask_user_question with answers keyed by question id', async () => { + mockGenerationJobManager.getJob.mockResolvedValue(makeAskUserBatchJob()); + const answers = { environment: 'staging', window: '7d' }; + const res = await post({ + conversationId: CONVO_ID, + actionId: ACTION_ID, + agent_id: AGENT_ID, + endpoint: 'agents', + answers, + }); + expect(res.status).toBe(200); + await settled; + await flush(); + + const client = await mockInitializeClient.mock.results[0].value.then((r) => r.client); + expect(client.resumeCompletion).toHaveBeenCalledWith( + expect.objectContaining({ resumeValue: { answers } }), + ); + }); + it('generates a title for a first-turn pause before completing the stream', async () => { const job = makeToolApprovalJob(); job.metadata.userMessage.parentMessageId = Constants.NO_PARENT; diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index 0673f6a31a..48a1063ab2 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -2371,7 +2371,9 @@ class AgentClient extends BaseClient { if (interrupt.payload?.type === 'ask_user_question' && Array.isArray(this.contentParts)) { const stamped = attachAskUserQuestionArgs( this.contentParts, - interrupt.payload.question, + Array.isArray(interrupt.payload.questions) + ? { questions: interrupt.payload.questions } + : interrupt.payload.question, interrupt.payload.tool_call_id, ); if (stamped !== this.contentParts) { diff --git a/api/server/controllers/agents/resume.js b/api/server/controllers/agents/resume.js index 0d375d804b..bd057e8fbd 100644 --- a/api/server/controllers/agents/resume.js +++ b/api/server/controllers/agents/resume.js @@ -4,7 +4,7 @@ const { GenerationJobManager, isPendingActionStale, mapToolApprovalResolutions, - mapAskUserAnswer, + resolveAskUserQuestionResume, attachAskUserQuestionAnswer, findUndecidedToolCalls, findDisallowedDecisions, @@ -41,13 +41,6 @@ function sendGenerationJson(res, status, body, generationProtocolVersion) { return res.status(status).json({ ...body, generationProtocolVersion }); } -/** - * Upper bound on an `ask_user_question` answer (characters). Generous for any real - * reply typed into the question card while still bounding what a crafted POST can - * inject into the resumed run's ToolMessage. - */ -const MAX_ASK_ANSWER_LENGTH = 16_000; - /** * How long a resume waits on best-effort steering bookkeeping before answering * anyway. The approval is already consumed by that point, so a stalled Redis @@ -231,15 +224,7 @@ function resolveResumeValue(pendingAction, body) { return { resumeValue: mapToolApprovalResolutions(resolutions) }; } if (payload?.type === 'ask_user_question') { - if (typeof body.answer !== 'string' || body.answer.length === 0) { - return { status: 400, error: 'An answer is required' }; - } - // The answer becomes a ToolMessage the model must ingest — bound it like any - // other user-controlled wire field rather than trusting the client. - if (body.answer.length > MAX_ASK_ANSWER_LENGTH) { - return { status: 400, error: 'Answer exceeds the maximum length' }; - } - return { resumeValue: mapAskUserAnswer({ answer: body.answer }) }; + return resolveAskUserQuestionResume(payload, body); } return { status: 400, error: 'Unsupported pending action type' }; } @@ -908,10 +893,15 @@ const ResumeAgentController = async (req, res, next, initializeClient, addTitle) // no completion event ever fires for this tool — without this the saved part is // an empty "cancelled-looking" tool call. See attachAskUserQuestionAnswer. if (pendingAction.payload?.type === 'ask_user_question') { + const batched = Array.isArray(pendingAction.payload.questions); + const request = batched + ? { questions: pendingAction.payload.questions } + : pendingAction.payload.question; + const output = batched ? JSON.stringify({ answers: req.body.answers }) : req.body.answer; seedContent = attachAskUserQuestionAnswer( seedContent, - pendingAction.payload.question, - req.body.answer, + request, + output, pendingAction.payload.tool_call_id, ); } diff --git a/api/server/middleware/moderateText.js b/api/server/middleware/moderateText.js index 3356f75921..9aedf3bd51 100644 --- a/api/server/middleware/moderateText.js +++ b/api/server/middleware/moderateText.js @@ -1,5 +1,10 @@ const axios = require('axios'); -const { isEnabled, getReferencedQuotes, mergeQuotedText } = require('@librechat/api'); +const { + isEnabled, + getReferencedQuotes, + mergeQuotedText, + serializeAskUserAnswerVariants, +} = require('@librechat/api'); const { logger } = require('@librechat/data-schemas'); const { ErrorTypes } = require('librechat-data-provider'); const denyRequest = require('./denyRequest'); @@ -32,6 +37,18 @@ async function moderateText(req, res, next) { if (safeText.length > 0) { inputs.push(safeText); } + if ( + req.body.answers != null && + typeof req.body.answers === 'object' && + !Array.isArray(req.body.answers) + ) { + for (const answer of Object.values(req.body.answers)) { + if (typeof answer === 'string' && answer.length > 0) { + inputs.push(answer); + } + } + inputs.push(...serializeAskUserAnswerVariants(req.body.answers)); + } const quotes = getReferencedQuotes(req.body.quotes); if (quotes != null) { inputs.push(...quotes); diff --git a/api/server/routes/agents/index.js b/api/server/routes/agents/index.js index 39d69a2353..36b5c0d334 100644 --- a/api/server/routes/agents/index.js +++ b/api/server/routes/agents/index.js @@ -658,7 +658,13 @@ router.post('/chat/abort', configMiddleware, async (req, res, next) => { expectedCreatedAt: job.createdAt, transformAbortContent: (content) => abortedAskPayload?.type === 'ask_user_question' && Array.isArray(content) - ? attachAskUserQuestionArgs(content, abortedAskPayload.question) + ? attachAskUserQuestionArgs( + content, + Array.isArray(abortedAskPayload.questions) + ? { questions: abortedAskPayload.questions } + : abortedAskPayload.question, + abortedAskPayload.tool_call_id, + ) : content, /** Persist every parent-row prerequisite before publishing the ordinary * abort FINAL. That frame can immediately drain a queued follow-up, whose diff --git a/client/src/components/Chat/Input/AskUserQuestionPopover.tsx b/client/src/components/Chat/Input/AskUserQuestionPopover.tsx index 1df592e213..a855e572ee 100644 --- a/client/src/components/Chat/Input/AskUserQuestionPopover.tsx +++ b/client/src/components/Chat/Input/AskUserQuestionPopover.tsx @@ -2,6 +2,7 @@ import { memo, useEffect, useRef } from 'react'; import { useWatch } from 'react-hook-form'; import { Button } from '@librechat/client'; import { Check, ChevronDown, CornerDownLeft, TriangleAlert, X } from 'lucide-react'; +import AskUserQuestions from '~/components/Chat/Messages/Content/AskUserQuestions'; import useAskAnswerMode from '~/hooks/Input/useAskAnswerMode'; import { useChatFormContext } from '~/Providers'; import { useLocalize } from '~/hooks'; @@ -33,9 +34,56 @@ function AskUserQuestionPopoverContent({ return null; } + if (ask.liveAsk.questions != null && ask.liveAsk.questions.length > 0) { + return ; + } + return ; } +function AskUserQuestionsPopoverPanel({ ask }: { ask: ReturnType }) { + const localize = useLocalize(); + const { liveAsk, collapse, dismiss } = ask; + const questions = liveAsk?.questions; + if (liveAsk == null || questions == null || questions.length === 0) { + return null; + } + + return ( +
+
+
+

+ {localize( + questions.length === 1 ? 'com_ui_asking_questions_one' : 'com_ui_asking_questions', + { 0: questions.length }, + )} +

+
+ + +
+
+ +
+
+ ); +} + /** * Split from the gate above so the per-keystroke `useWatch` subscription only * exists while the popover is actually visible — the invisible popover was diff --git a/client/src/components/Chat/Input/ChatForm.tsx b/client/src/components/Chat/Input/ChatForm.tsx index 4ff0927d91..fe57678719 100644 --- a/client/src/components/Chat/Input/ChatForm.tsx +++ b/client/src/components/Chat/Input/ChatForm.tsx @@ -173,6 +173,9 @@ const ChatForm = memo(function ChatForm({ }, []); const answerMode = useAskAnswerMode(conversationId); + const answerPlaceholder = answerMode.batchMode + ? localize('com_ui_answer_questions_above') + : (answerMode.otherLabel ?? localize('com_ui_something_else')); useAutoSave({ files, @@ -386,9 +389,7 @@ const ChatForm = memo(function ChatForm({ setIsScrollable, disabled: disableInputs, // The composer IS the free-form answer box while a question pause is live. - placeholder: answerMode.active - ? (answerMode.otherLabel ?? localize('com_ui_something_else')) - : placeholder, + placeholder: answerMode.active ? answerPlaceholder : placeholder, // Enter stays live during a run when it can steer/queue instead of send. allowSubmitWhileGenerating: steering.duringRunActive, onDuringRunModifier: steering.duringRunActive ? handleDuringRunModifier : undefined, @@ -595,7 +596,11 @@ const ChatForm = memo(function ChatForm({ textAreaRef as React.MutableRefObject ).current = e; }} - disabled={disableInputs || isNotAppendable} + disabled={ + disableInputs || + isNotAppendable || + (answerMode.active && answerMode.batchMode) + } onPaste={handlePaste} onKeyDown={(e) => { // Answer mode consumes option-navigation keys from the @@ -698,6 +703,7 @@ const ChatForm = memo(function ChatForm({ filesLoading || disableInputs || isNotAppendable || + (answerMode.active && answerMode.batchMode) || (isSubmitting && !answerMode.active) } /> diff --git a/client/src/components/Chat/Messages/Content/ApprovalContext.tsx b/client/src/components/Chat/Messages/Content/ApprovalContext.tsx index 2b54a4664e..bb61c75a18 100644 --- a/client/src/components/Chat/Messages/Content/ApprovalContext.tsx +++ b/client/src/components/Chat/Messages/Content/ApprovalContext.tsx @@ -381,9 +381,15 @@ export function useResumeSubmit() { ); const submitAskAnswer = useCallback( - (actionId: string, answer: string, opts?: { onSuccess?: () => void }) => { + ( + actionId: string, + resolution: string | Record, + opts?: { onSuccess?: () => void }, + ) => { const fields = buildResumeFields(); - if (!fields || answer.length === 0) { + const isBatch = typeof resolution !== 'string'; + const hasAnswer = isBatch ? Object.keys(resolution).length > 0 : resolution.length > 0; + if (!fields || !hasAnswer) { return; } if (submittingAskActionIdsRef.current.has(actionId)) { @@ -392,7 +398,11 @@ export function useResumeSubmit() { submittingAskActionIdsRef.current.add(actionId); setAskStatus(actionId, 'submitting'); askMutation.mutate( - { ...fields, actionId, answer }, + { + ...fields, + actionId, + ...(isBatch ? { answers: resolution } : { answer: resolution }), + }, { onSuccess: () => { setAskStatus(actionId, 'submitted'); @@ -408,7 +418,7 @@ export function useResumeSubmit() { if (messages && chatContext?.setMessages) { let changed = false; const next = messages.map((message) => { - const resolved = resolveAskUserQuestionPart(message, actionId, answer); + const resolved = resolveAskUserQuestionPart(message, actionId, resolution); if (resolved !== message) { changed = true; } diff --git a/client/src/components/Chat/Messages/Content/AskUserQuestion.tsx b/client/src/components/Chat/Messages/Content/AskUserQuestion.tsx index 7e5d76da2f..b35cb1ecdc 100644 --- a/client/src/components/Chat/Messages/Content/AskUserQuestion.tsx +++ b/client/src/components/Chat/Messages/Content/AskUserQuestion.tsx @@ -6,6 +6,7 @@ import { useApprovalContext, useAskSubmitStatus, useResumeSubmit } from './Appro import useAskAnswerMode from '~/hooks/Input/useAskAnswerMode'; import { ChatContext } from '~/Providers/ChatContext'; import { splitOtherOption } from '~/utils/approval'; +import AskUserQuestions from './AskUserQuestions'; import { useLocalize } from '~/hooks'; /** @@ -20,9 +21,39 @@ import { useLocalize } from '~/hooks'; export default function AskUserQuestion({ actionId, question, + questions, }: { actionId: string; question: Agents.AskUserQuestionRequest; + questions?: Agents.AskUserQuestionBatchItem[]; +}) { + const conversationId = useContext(ChatContext)?.conversation?.conversationId; + const answerMode = useAskAnswerMode(conversationId); + const isLivePause = answerMode.liveAsk?.actionId === actionId; + if (questions != null && questions.length > 0) { + if (answerMode.popoverVisible && isLivePause) { + return null; + } + return ( + + ); + } + return ; +} + +function AskUserQuestionSingle({ + actionId, + question, + answerMode, +}: { + actionId: string; + question: Agents.AskUserQuestionRequest; + answerMode: ReturnType; }) { const localize = useLocalize(); const { getAskAnswerDraft, setAskAnswerDraft } = useApprovalContext(); @@ -37,8 +68,6 @@ export default function AskUserQuestion({ * chevron re-expands it) or dismissed (and in contexts without a * ChatContext, where the popover can't exist). */ - const conversationId = useContext(ChatContext)?.conversation?.conversationId; - const answerMode = useAskAnswerMode(conversationId); const { popoverVisible, collapsed, expand, liveAsk } = answerMode; const isLivePause = liveAsk?.actionId === actionId; diff --git a/client/src/components/Chat/Messages/Content/AskUserQuestionCall.tsx b/client/src/components/Chat/Messages/Content/AskUserQuestionCall.tsx index 535cf93338..a3d622ab9c 100644 --- a/client/src/components/Chat/Messages/Content/AskUserQuestionCall.tsx +++ b/client/src/components/Chat/Messages/Content/AskUserQuestionCall.tsx @@ -1,5 +1,10 @@ import { MessageCircleQuestion, TriangleAlert } from 'lucide-react'; -import { getSubmittedAskAnswer, parseAskUserQuestionArgs } from '~/utils/approval'; +import type { Agents } from 'librechat-data-provider'; +import { + getSubmittedAskAnswer, + parseAskUserQuestionArgs, + parseAskUserQuestionsArgs, +} from '~/utils/approval'; import AskUserQuestionProgress from './AskUserQuestionProgress'; import EmptyText from './Parts/EmptyText'; import { useLocalize } from '~/hooks'; @@ -29,6 +34,7 @@ export default function AskUserQuestionCall({ }) { const localize = useLocalize(); const question = parseAskUserQuestionArgs(args); + const batch = parseAskUserQuestionsArgs(args); /** * The part's own output arrives from the server only at finalize, and the * streaming handler's message copy can overwrite the optimistic store stamp @@ -36,7 +42,24 @@ export default function AskUserQuestionCall({ * Q&A record never blinks out while the resumed segment streams. */ const effectiveOutput = output.length > 0 ? output : (getSubmittedAskAnswer(toolCallId) ?? ''); - const answered = effectiveOutput.length > 0 && !failed; + const batchAnswers = (() => { + if (batch == null || effectiveOutput.length === 0) { + return null; + } + try { + const parsed = JSON.parse(effectiveOutput) as { answers?: unknown }; + return parsed.answers != null && typeof parsed.answers === 'object' + ? (parsed.answers as Record) + : null; + } catch { + return null; + } + })(); + const answered = + !failed && + (batch != null + ? batch.questions.every((item) => typeof batchAnswers?.[item.id] === 'string') + : effectiveOutput.length > 0); /** * While the turn is live and unanswered, the INTERACTIVE card (rendered from @@ -65,6 +88,68 @@ export default function AskUserQuestionCall({ ) : null; + if (batch != null) { + let statusLabel = localize('com_ui_asking'); + if (failed) { + statusLabel = localize('com_ui_question_failed'); + } else if (answered) { + statusLabel = localize('com_ui_asked'); + } + return ( + <> +
+
+ {failed ? ( +
+
+ {batch.questions.map((item, index) => { + const answer = batchAnswers?.[item.id]; + return ( +
0 ? 'border-t border-border-light pt-3' : ''}> + {item.header != null && ( +

{item.header}

+ )} +

+ {item.question} +

+ {item.description != null && ( +

+ {item.description} +

+ )} + {typeof answer === 'string' && ( +

+ + {localize('com_ui_you_answered')} + {' '} + {formatAnswerLabel(item, answer)} +

+ )} + {typeof answer !== 'string' && !failed && ( +

+ {localize('com_ui_question_unanswered')} +

+ )} +
+ ); + })} + {failed && ( +

+ {localize('com_ui_question_failed_description')} +

+ )} +
+
+ {resumingCursor} + + ); + } + if (failed) { return ( <> @@ -98,17 +183,8 @@ export default function AskUserQuestionCall({ * such a value into fragments that relabel as options the user never * picked. When any segment misses, show the raw answer untouched. */ - const exactLabel = question?.options?.find((option) => option.value === effectiveOutput)?.label; - const mappedMultiLabel = (() => { - if (exactLabel != null || question?.multiSelect !== true || question.options == null) { - return null; - } - const labels = effectiveOutput - .split(', ') - .map((segment) => question.options?.find((option) => option.value === segment)?.label); - return labels.every((label) => label != null) ? labels.join(', ') : null; - })(); - const answerLabel = exactLabel ?? mappedMultiLabel ?? effectiveOutput; + const answerLabel = + question == null ? effectiveOutput : formatAnswerLabel(question, effectiveOutput); return ( <> @@ -142,3 +218,14 @@ export default function AskUserQuestionCall({ ); } + +function formatAnswerLabel(question: Agents.AskUserQuestionRequest, answer: string): string { + const exactLabel = question.options?.find((option) => option.value === answer)?.label; + if (exactLabel != null || question.multiSelect !== true || question.options == null) { + return exactLabel ?? answer; + } + const labels = answer + .split(', ') + .map((segment) => question.options?.find((option) => option.value === segment)?.label); + return labels.every((label) => label != null) ? labels.join(', ') : answer; +} diff --git a/client/src/components/Chat/Messages/Content/AskUserQuestionProgress.tsx b/client/src/components/Chat/Messages/Content/AskUserQuestionProgress.tsx index 228d9bda0b..9e921fb36d 100644 --- a/client/src/components/Chat/Messages/Content/AskUserQuestionProgress.tsx +++ b/client/src/components/Chat/Messages/Content/AskUserQuestionProgress.tsx @@ -1,9 +1,9 @@ import { useContext } from 'react'; import { MessageCircleQuestion } from 'lucide-react'; +import parseJsonField, { parseJsonFieldOccurrences } from './Parts/parseJsonField'; import { collectLiveAskToolCallIds } from '~/utils/approval'; import { useGetMessagesByConvoId } from '~/data-provider'; import { ChatContext } from '~/Providers/ChatContext'; -import parseJsonField from './Parts/parseJsonField'; import { useLocalize } from '~/hooks'; /** @@ -13,9 +13,8 @@ import { useLocalize } from '~/hooks'; * interactive card). Without it a long question, many options, or several * parallel questions leave a dead gap after the last streamed token. * - * The question text streams in live: `question` is the schema's first (and - * only required) property, so providers emit it as the first args key and - * `parseJsonField`'s partial-JSON path can render it delta by delta. + * The first question text streams in live from either the legacy top-level + * field or the first item in the batched `questions` array. * * Mounted only for a live, unanswered call ({@link AskUserQuestionCall} * gates on `isSubmitting`), so the live-ask subscription below never runs @@ -35,7 +34,23 @@ export default function AskUserQuestionProgress({ enabled, select: collectLiveAskToolCallIds, }); - const question = parseJsonField(args, 'question'); + const legacyQuestion = parseJsonField(args, 'question'); + const batchQuestion = (() => { + if (typeof args === 'string') { + return parseJsonFieldOccurrences(args, 'question')[0] ?? ''; + } + const questions = args?.questions; + if (!Array.isArray(questions)) { + return ''; + } + const first = questions[0]; + if (first == null || typeof first !== 'object') { + return ''; + } + const firstQuestion = (first as { question?: unknown }).question; + return typeof firstQuestion === 'string' ? firstQuestion : ''; + })(); + const question = legacyQuestion || batchQuestion; /** * THIS call's pause went interactive: the popover (or the interactive card) diff --git a/client/src/components/Chat/Messages/Content/AskUserQuestions.tsx b/client/src/components/Chat/Messages/Content/AskUserQuestions.tsx new file mode 100644 index 0000000000..078c4ea24b --- /dev/null +++ b/client/src/components/Chat/Messages/Content/AskUserQuestions.tsx @@ -0,0 +1,136 @@ +import { Button, TextareaAutosize } from '@librechat/client'; +import { Check, ChevronUp, TriangleAlert } from 'lucide-react'; +import type { Agents } from 'librechat-data-provider'; +import useAskQuestionsForm from '~/hooks/Input/useAskQuestionsForm'; +import { splitOtherOption } from '~/utils/approval'; +import { useLocalize } from '~/hooks'; +import { cn } from '~/utils'; + +export default function AskUserQuestions({ + actionId, + questions, + className, + onExpand, +}: { + actionId: string; + questions: Agents.AskUserQuestionBatchItem[]; + className?: string; + onExpand?: () => void; +}) { + const localize = useLocalize(); + const form = useAskQuestionsForm(actionId, questions); + + if (form.status === 'submitted') { + return null; + } + + return ( +
+ {onExpand != null && ( +
+ +
+ )} +
+ {questions.map((question, questionIndex) => { + const { choices, otherLabel } = splitOtherOption(question.options); + const selected = Object.hasOwn(form.state.selected, question.id) + ? form.state.selected[question.id] + : []; + const text = Object.hasOwn(form.state.text, question.id) + ? form.state.text[question.id] + : ''; + return ( +
0 && 'border-t border-border-light')} + > + + {question.header ?? localize('com_ui_question_number', { 0: questionIndex + 1 })} + +

+ {question.question} +

+ {question.description != null && question.description.length > 0 && ( +

+ {question.description} +

+ )} + {choices.length > 0 && ( +
+ {choices.map((option) => { + const isSelected = selected.includes(option.value); + return ( + + ); + })} +
+ )} + form.setText(question, event.target.value)} + minRows={1} + maxRows={6} + placeholder={otherLabel ?? localize('com_ui_your_answer')} + className="mt-2 w-full resize-none rounded-md border border-border-light bg-surface-primary p-2 text-sm" + aria-label={`${question.question} ${localize('com_ui_your_answer')}`} + /> +
+ ); + })} +
+ {(form.status === 'error' || form.status === 'expired') && ( +
+
+ )} +
+ + +
+
+ ); +} diff --git a/client/src/components/Chat/Messages/Content/Part.tsx b/client/src/components/Chat/Messages/Content/Part.tsx index 082bad9bc4..3e636da8ff 100644 --- a/client/src/components/Chat/Messages/Content/Part.tsx +++ b/client/src/components/Chat/Messages/Content/Part.tsx @@ -70,6 +70,7 @@ const Part = memo(function Part({ key={askUserQuestion.ask_user_question.actionId} actionId={askUserQuestion.ask_user_question.actionId} question={askUserQuestion.ask_user_question.question} + questions={askUserQuestion.ask_user_question.questions} /> ); } diff --git a/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionCall.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionCall.test.tsx index 6909e2904e..2fd34dab71 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionCall.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionCall.test.tsx @@ -24,6 +24,10 @@ jest.mock('~/utils/approval', () => ({ } return args ?? null; }, + parseAskUserQuestionsArgs: (args: string | Record | undefined) => { + const parsed = typeof args === 'string' ? JSON.parse(args) : args; + return Array.isArray(parsed?.questions) ? parsed : null; + }, })); jest.mock('../AskUserQuestionProgress', () => ({ @@ -113,4 +117,47 @@ describe('AskUserQuestionCall', () => { expect(screen.getByText(output)).toBeInTheDocument(); expect(screen.queryByText("Question wasn't shown")).not.toBeInTheDocument(); }); + + test('renders each question and answer from one completed batch', () => { + render( + , + ); + + expect(screen.getByText('Environment')).toBeInTheDocument(); + expect(screen.getByText('Where should this run?')).toBeInTheDocument(); + expect(screen.getByText('Choose the deployment target.')).toBeInTheDocument(); + expect(screen.getByText('Staging')).toBeInTheDocument(); + expect(screen.getByText('Which window?')).toBeInTheDocument(); + expect(screen.getByText('7d')).toBeInTheDocument(); + }); + + test('renders a failed batch without implying the user declined to answer', () => { + render( + , + ); + + expect(screen.getByText("Question wasn't shown")).toBeInTheDocument(); + expect( + screen.getByText("The agent couldn't show this question and may retry automatically."), + ).toBeInTheDocument(); + expect(screen.queryByText('No answer was given')).not.toBeInTheDocument(); + }); }); diff --git a/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionProgress.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionProgress.test.tsx index 5f96dbc67f..0f87cf23a9 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionProgress.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionProgress.test.tsx @@ -51,6 +51,28 @@ describe('AskUserQuestionProgress', () => { expect(screen.getByText('Café or "bar"')).toBeInTheDocument(); }); + test('streams the first question from partial batched args', () => { + render( + , + ); + + expect(screen.getByText('Which environ')).toBeInTheDocument(); + }); + + test('reads the first question from settled batched args', () => { + render( + , + ); + + expect(screen.getByText('Which environment?')).toBeInTheDocument(); + }); + test('renders a skeleton line before any question text streams', () => { render(); diff --git a/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestions.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestions.test.tsx new file mode 100644 index 0000000000..3c19cb2c4f --- /dev/null +++ b/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestions.test.tsx @@ -0,0 +1,113 @@ +import React from 'react'; +import { RecoilRoot } from 'recoil'; +import { fireEvent, render, screen } from '@testing-library/react'; +import AskUserQuestions from '../AskUserQuestions'; + +const mockSubmitAskAnswer = jest.fn(); + +jest.mock('~/components/Chat/Messages/Content/ApprovalContext', () => ({ + useAskSubmitStatus: () => ({ getAskStatus: () => 'idle' }), + useResumeSubmit: () => ({ submitAskAnswer: mockSubmitAskAnswer }), +})); + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string, values?: Record) => { + const labels: Record = { + com_ui_question_number: `Question ${values?.[0] ?? ''}`, + com_ui_your_answer: 'Your answer', + com_ui_skip: 'Skip', + com_ui_submit: 'Submit', + com_ui_submitting: 'Submitting', + }; + return labels[key] ?? key; + }, +})); + +const questions = [ + { + id: 'environment', + header: 'Environment', + question: 'Where should this run?', + options: [ + { label: 'Staging', value: 'staging' }, + { label: 'Production', value: 'production' }, + ], + }, + { id: 'window', question: 'Which time window?' }, +]; + +describe('AskUserQuestions', () => { + beforeEach(() => mockSubmitAskAnswer.mockClear()); + + test('submits one answer map after every question is complete', () => { + render( + + + , + ); + + const submit = screen.getByRole('button', { name: 'Submit' }); + expect(submit).toBeDisabled(); + + fireEvent.click(screen.getByRole('button', { name: 'Staging' })); + fireEvent.change(screen.getByRole('textbox', { name: /Which time window/ }), { + target: { value: 'Last seven days' }, + }); + expect(submit).toBeEnabled(); + fireEvent.click(submit); + + expect(mockSubmitAskAnswer).toHaveBeenCalledWith( + 'ask-batch', + { + environment: 'staging', + window: 'Last seven days', + }, + expect.objectContaining({ onSuccess: expect.any(Function) }), + ); + }); + + test('retains partial answers across surface remounts', () => { + const view = render( + + + , + ); + fireEvent.change(screen.getByRole('textbox', { name: /Which time window/ }), { + target: { value: 'Today' }, + }); + + view.rerender( + + + , + ); + + expect(screen.getByRole('textbox', { name: /Which time window/ })).toHaveValue('Today'); + }); + + test('supports question ids inherited by ordinary objects', () => { + render( + + + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'Yes' })); + fireEvent.click(screen.getByRole('button', { name: 'Submit' })); + + expect(mockSubmitAskAnswer).toHaveBeenCalledWith( + 'ask-prototype-id', + { constructor: 'yes' }, + expect.objectContaining({ onSuccess: expect.any(Function) }), + ); + }); +}); diff --git a/client/src/data-provider/SSE/mutations.ts b/client/src/data-provider/SSE/mutations.ts index d6bb83667e..39e051e8b2 100644 --- a/client/src/data-provider/SSE/mutations.ts +++ b/client/src/data-provider/SSE/mutations.ts @@ -132,8 +132,10 @@ export function useSubmitToolApprovalMutation() { export interface SubmitAskAnswerParams extends ResumeAgentFields { actionId: string; - /** Free-form answer to the agent's ask-user question. */ - answer: string; + /** Free-form answer for a legacy single-question pause. */ + answer?: string; + /** Answers keyed by question id for a batched pause. */ + answers?: Record; } /** @@ -141,11 +143,12 @@ export interface SubmitAskAnswerParams extends ResumeAgentFields { * POSTs to the shared resume route; the continuation streams over the existing SSE. */ export const submitAskAnswer = async (params: SubmitAskAnswerParams): Promise => { - const { actionId, answer, ...fields } = params; + const { actionId, answer, answers, ...fields } = params; return postGenerationRequest(`${apiBaseUrl()}/api/agents/chat/resume`, { ...buildResumeBase(fields), actionId, - answer, + ...(answer != null && { answer }), + ...(answers != null && { answers }), }); }; diff --git a/client/src/hooks/Input/useAskAnswerMode.spec.ts b/client/src/hooks/Input/useAskAnswerMode.spec.ts index 148fb45565..d2ffce31f5 100644 --- a/client/src/hooks/Input/useAskAnswerMode.spec.ts +++ b/client/src/hooks/Input/useAskAnswerMode.spec.ts @@ -75,6 +75,27 @@ describe('useAskAnswerMode', () => { expect(result.current.popoverVisible).toBe(false); }); + it('keeps batch mode active while reserving answers for the bounded form', () => { + mockUseGetMessages.mockReturnValue({ + data: { + ...liveAsk, + questions: [ + { id: 'environment', question: 'Which environment?' }, + { id: 'window', question: 'Which window?' }, + ], + }, + }); + + const { result } = renderHook(() => useAskAnswerMode('conversation-1')); + + expect(result.current.active).toBe(true); + expect(result.current.batchMode).toBe(true); + expect(result.current.options).toEqual([]); + expect(result.current.draftId).toBeNull(); + expect(result.current.submitText('must stay out of the normal send path')).toBe(true); + expect(mockSubmitAskAnswer).not.toHaveBeenCalled(); + }); + it('disables the query and forces liveAsk null for a new (unsaved) conversation', () => { mockUseGetMessages.mockReturnValue({ data: liveAsk }); diff --git a/client/src/hooks/Input/useAskAnswerMode.ts b/client/src/hooks/Input/useAskAnswerMode.ts index d13d0205f2..436b9433aa 100644 --- a/client/src/hooks/Input/useAskAnswerMode.ts +++ b/client/src/hooks/Input/useAskAnswerMode.ts @@ -131,14 +131,16 @@ export default function useAskAnswerMode(conversationId?: string | null) { /** The popover renders only while expanded; collapse keeps `active` (and the * composer's answer role) but hands the question display to the chat card. */ const popoverVisible = active && !collapsed; - const multiSelect = liveAsk != null && liveAsk.question.multiSelect === true; + const batchMode = (liveAsk?.questions?.length ?? 0) > 0; + const multiSelect = !batchMode && liveAsk != null && liveAsk.question.multiSelect === true; /** Answer-phase draft key: handed to useAutoSave so the composer drafts * under the question's own key while answer mode is live, leaving the * conversation draft untouched until the swap-back restores it. */ - const draftId = active && liveAsk != null ? getAskAnswerDraftId(liveAsk.actionId) : null; + const draftId = + active && liveAsk != null && !batchMode ? getAskAnswerDraftId(liveAsk.actionId) : null; const { choices: options, otherLabel } = useMemo( - () => splitOtherOption(liveAsk?.question.options), - [liveAsk], + () => splitOtherOption(batchMode ? undefined : liveAsk?.question.options), + [batchMode, liveAsk], ); /** Selection state is per-question: a new pause must never inherit a stale @@ -296,13 +298,16 @@ export default function useAskAnswerMode(conversationId?: string | null) { if (!active || !liveAsk) { return false; } + if (batchMode) { + return true; + } const trimmed = text.trim(); if (trimmed.length > 0) { submitValues(multiSelect ? [...checkedValues(), trimmed] : [trimmed], true); } return true; }, - [active, liveAsk, multiSelect, checkedValues, submitValues], + [active, liveAsk, batchMode, multiSelect, checkedValues, submitValues], ); /** @@ -446,6 +451,7 @@ export default function useAskAnswerMode(conversationId?: string | null) { return { active, + batchMode, liveAsk, options, dismissed, diff --git a/client/src/hooks/Input/useAskQuestionsForm.ts b/client/src/hooks/Input/useAskQuestionsForm.ts new file mode 100644 index 0000000000..b0c0d897f5 --- /dev/null +++ b/client/src/hooks/Input/useAskQuestionsForm.ts @@ -0,0 +1,118 @@ +import { useCallback, useMemo } from 'react'; +import { atomFamily, useRecoilState, useResetRecoilState } from 'recoil'; +import type { Agents } from 'librechat-data-provider'; +import { + useAskSubmitStatus, + useResumeSubmit, +} from '~/components/Chat/Messages/Content/ApprovalContext'; +import { ASK_USER_DECLINED_ANSWER } from '~/utils/approval'; + +interface AskQuestionsFormState { + text: Record; + selected: Record; +} + +const askQuestionsFormState = atomFamily({ + key: 'askQuestionsFormState', + default: { text: {}, selected: {} }, +}); + +function ownValue(record: Record, key: string): T | undefined { + return Object.hasOwn(record, key) ? record[key] : undefined; +} + +export default function useAskQuestionsForm( + actionId: string, + questions: Agents.AskUserQuestionBatchItem[], +) { + const [state, setState] = useRecoilState(askQuestionsFormState(actionId)); + const resetState = useResetRecoilState(askQuestionsFormState(actionId)); + const { submitAskAnswer } = useResumeSubmit(); + const { getAskStatus } = useAskSubmitStatus(); + const status = getAskStatus(actionId); + const locked = status === 'submitting' || status === 'submitted' || status === 'expired'; + + const setText = useCallback( + (question: Agents.AskUserQuestionBatchItem, value: string) => { + setState((previous) => ({ + text: { ...previous.text, [question.id]: value }, + selected: + question.multiSelect === true || value.length === 0 + ? previous.selected + : { ...previous.selected, [question.id]: [] }, + })); + }, + [setState], + ); + + const selectOption = useCallback( + (question: Agents.AskUserQuestionBatchItem, value: string) => { + setState((previous) => { + const current = ownValue(previous.selected, question.id) ?? []; + let selected = [value]; + if (question.multiSelect === true) { + selected = current.includes(value) + ? current.filter((item) => item !== value) + : [...current, value]; + } + return { + text: + question.multiSelect === true ? previous.text : { ...previous.text, [question.id]: '' }, + selected: { ...previous.selected, [question.id]: selected }, + }; + }); + }, + [setState], + ); + + const answers = useMemo(() => { + const resolved = Object.create(null) as Record; + for (const question of questions) { + const text = ownValue(state.text, question.id)?.trim() ?? ''; + const selected = ownValue(state.selected, question.id) ?? []; + const values = question.multiSelect === true ? [...selected] : []; + if (text.length > 0) { + values.push(text); + } else if (question.multiSelect !== true && selected[0] != null) { + values.push(selected[0]); + } + if (values.length > 0) { + resolved[question.id] = values.join(', '); + } + } + return resolved; + }, [questions, state]); + + const canSubmit = + !locked && questions.every((question) => (ownValue(answers, question.id)?.length ?? 0) > 0); + + const submit = useCallback(() => { + if (!canSubmit) { + return false; + } + submitAskAnswer(actionId, answers, { onSuccess: resetState }); + return true; + }, [actionId, answers, canSubmit, resetState, submitAskAnswer]); + + const skip = useCallback(() => { + if (locked) { + return false; + } + const declined = Object.fromEntries( + questions.map((question) => [question.id, ASK_USER_DECLINED_ANSWER]), + ); + submitAskAnswer(actionId, declined, { onSuccess: resetState }); + return true; + }, [actionId, locked, questions, resetState, submitAskAnswer]); + + return { + state, + status, + locked, + canSubmit, + setText, + selectOption, + submit, + skip, + }; +} diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index a376a1302c..0415a65708 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1687,6 +1687,10 @@ "com_ui_quality": "Quality", "com_ui_question_failed": "Question wasn't shown", "com_ui_question_failed_description": "The agent couldn't show this question and may retry automatically.", + "com_ui_question_number": "Question {{0}}", + "com_ui_answer_questions_above": "Answer the questions above", + "com_ui_asking_questions_one": "Asking {{0}} question", + "com_ui_asking_questions": "Asking {{0}} questions", "com_ui_question_unanswered": "No answer was given", "com_ui_queue": "Queue", "com_ui_queue_send": "Queue message for after the response", diff --git a/client/src/utils/approval.spec.ts b/client/src/utils/approval.spec.ts index 02a313ff22..099028673d 100644 --- a/client/src/utils/approval.spec.ts +++ b/client/src/utils/approval.spec.ts @@ -8,6 +8,7 @@ import { findPendingActionMessageIndex, removeAskUserQuestionPart, parseAskUserQuestionArgs, + parseAskUserQuestionsArgs, resolveAskUserQuestionPart, getSubmittedAskAnswer, findLiveAskUserQuestion, @@ -295,6 +296,50 @@ describe('parseAskUserQuestionArgs', () => { }); }); +describe('parseAskUserQuestionsArgs', () => { + it('parses a complete batch and preserves headers and options', () => { + const parsed = parseAskUserQuestionsArgs({ + questions: [ + { + id: 'environment', + header: 'Environment', + question: 'Which environment?', + options: [{ label: 'Staging', value: 'staging' }], + }, + { id: 'window', question: 'Which window?' }, + ], + }); + expect(parsed?.questions).toHaveLength(2); + expect(parsed?.questions[0]).toMatchObject({ id: 'environment', header: 'Environment' }); + }); + + it('rejects malformed and duplicate-id batches', () => { + expect(parseAskUserQuestionsArgs({ questions: [] })).toBeNull(); + expect( + parseAskUserQuestionsArgs({ + questions: [ + { id: 'same', question: 'First?' }, + { id: 'same', question: 'Second?' }, + ], + }), + ).toBeNull(); + }); + + it('trims valid headers and omits whitespace-only or oversized headers', () => { + const parsed = parseAskUserQuestionsArgs({ + questions: [ + { id: 'trimmed', header: ' Context ', question: 'First?' }, + { id: 'blank', header: ' ', question: 'Second?' }, + { id: 'large', header: 'x'.repeat(81), question: 'Third?' }, + ], + }); + + expect(parsed?.questions[0].header).toBe('Context'); + expect(parsed?.questions[1].header).toBeUndefined(); + expect(parsed?.questions[2].header).toBeUndefined(); + }); +}); + describe('removeAskUserQuestionPart', () => { it('strips the synthetic part for the matching actionId and keeps everything else', () => { const withCard = applyPendingAction(msg({ content: [textPart('hello')] }), askAction()); @@ -400,6 +445,43 @@ describe('resolveAskUserQuestionPart', () => { expect(content[0]?.tool_call?.output).toBe('us-east'); expect(content[1]?.tool_call?.output).toBeUndefined(); }); + + it('stamps one batched tool call with structured args and answers', () => { + const base = msg({ + content: [ + { + type: 'tool_call', + tool_call: { id: 'tc-batch', name: 'ask_user_question', args: '', type: 'tool_call' }, + } as unknown as TMessageContentParts, + ], + }); + const questions = [ + { id: 'environment', question: 'Which environment?' }, + { id: 'window', question: 'Which window?' }, + ]; + const withCard = applyPendingAction( + base, + askAction({ + actionId: 'a-batch', + payload: { + type: 'ask_user_question', + question: questions[0], + questions, + tool_call_id: 'tc-batch', + }, + }), + ); + const resolved = resolveAskUserQuestionPart(withCard, 'a-batch', { + environment: 'staging', + window: '7d', + }); + const toolCall = (resolved.content as Array<{ tool_call?: Record }>)[0] + ?.tool_call as Record; + expect(JSON.parse(toolCall.args as string)).toEqual({ questions }); + expect(JSON.parse(toolCall.output as string)).toEqual({ + answers: { environment: 'staging', window: '7d' }, + }); + }); }); describe('splitOtherOption', () => { diff --git a/client/src/utils/approval.ts b/client/src/utils/approval.ts index b6cdb146e0..2fb4f87440 100644 --- a/client/src/utils/approval.ts +++ b/client/src/utils/approval.ts @@ -16,6 +16,9 @@ export const ASK_USER_QUESTION = 'ask_user_question' as const; * 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.'; +const ASK_USER_QUESTION_ID_PATTERN = /^[A-Za-z][A-Za-z0-9_-]{0,63}$/; +const MAX_ASK_USER_QUESTIONS = 4; +const MAX_ASK_USER_QUESTION_HEADER_LENGTH = 80; /** Shape of the synthetic content part carrying an ask-user pending action. */ export interface AskUserQuestionPart { @@ -23,6 +26,7 @@ export interface AskUserQuestionPart { [ASK_USER_QUESTION]: { actionId: string; question: Agents.AskUserQuestionRequest; + questions?: Agents.AskUserQuestionBatchItem[]; /** The ask tool call that raised the pause (present from * `@librechat/agents` > 3.3.8) — lets the answer stamp target the exact * tool-call part in multi-ask turns. */ @@ -178,6 +182,7 @@ function applyAskUserQuestion( [ASK_USER_QUESTION]: { actionId, question: payload.question, + ...(payload.questions != null && { questions: payload.questions }), ...(payload.tool_call_id != null && { tool_call_id: payload.tool_call_id }), }, } as unknown as TMessageContentParts; @@ -246,6 +251,60 @@ export function parseAskUserQuestionArgs( }; } +/** Parse and validate the batched form of an ask-user tool call. */ +export function parseAskUserQuestionsArgs( + args: string | Record | undefined, +): Agents.AskUserQuestionsRequest | null { + let parsed: unknown = args; + if (typeof args === 'string') { + if (args.trim().length === 0) { + return null; + } + try { + parsed = JSON.parse(args); + } catch { + return null; + } + } + const rawQuestions = + parsed != null && typeof parsed === 'object' + ? (parsed as { questions?: unknown }).questions + : undefined; + if ( + !Array.isArray(rawQuestions) || + rawQuestions.length === 0 || + rawQuestions.length > MAX_ASK_USER_QUESTIONS + ) { + return null; + } + const questions: Agents.AskUserQuestionBatchItem[] = []; + const ids = new Set(); + for (const item of rawQuestions) { + if (item == null || typeof item !== 'object') { + return null; + } + const id = (item as { id?: unknown }).id; + const request = parseAskUserQuestionArgs(item as Record); + if ( + typeof id !== 'string' || + !ASK_USER_QUESTION_ID_PATTERN.test(id) || + ids.has(id) || + request == null + ) { + return null; + } + ids.add(id); + const rawHeader = (item as { header?: unknown }).header; + const header = typeof rawHeader === 'string' ? rawHeader.trim() : ''; + questions.push({ + id, + ...(header.length > 0 && header.length <= MAX_ASK_USER_QUESTION_HEADER_LENGTH && { header }), + ...request, + }); + } + return { questions }; +} + /** * Removes the synthetic ask-user-question part for `actionId` from a message. * Pure — returns the same message reference when nothing matched. @@ -318,7 +377,7 @@ export const isAnsweredAskUserQuestionPart = ( export function resolveAskUserQuestionPart( message: TMessage, actionId: string, - answer: string, + resolution: string | Record, ): TMessage { const content = message.content; if (!Array.isArray(content)) { @@ -337,6 +396,13 @@ export function resolveAskUserQuestionPart( * id — several ask cards in one turn each resolve their own part. Absent * (older server/SDK), the newest-unanswered fallback below applies. */ const targetToolCallId = syntheticPart[ASK_USER_QUESTION].tool_call_id; + const questions = syntheticPart[ASK_USER_QUESTION].questions; + const args = + questions != null + ? JSON.stringify({ questions }) + : JSON.stringify(syntheticPart[ASK_USER_QUESTION].question); + const output = + typeof resolution === 'string' ? resolution : JSON.stringify({ answers: resolution }); let patched = false; const nextContent: TMessageContentParts[] = []; @@ -369,13 +435,13 @@ export function resolveAskUserQuestionPart( ...(part as object), tool_call: { ...toolCall, - ...(hasArgs ? {} : { args: JSON.stringify(syntheticPart[ASK_USER_QUESTION].question) }), - output: answer, + ...(hasArgs ? {} : { args }), + output, progress: 1, }, } as TMessageContentParts; if (typeof toolCall.id === 'string' && toolCall.id.length > 0) { - submittedAskAnswers.set(toolCall.id, answer); + submittedAskAnswers.set(toolCall.id, output); } patched = true; break; @@ -424,9 +490,12 @@ export function splitOtherOption(options: Agents.AskUserQuestionOption[] | undef * 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, -): { actionId: string; question: Agents.AskUserQuestionRequest; messageId: string } | null { +export function findLiveAskUserQuestion(messages: TMessage[] | null | undefined): { + actionId: string; + question: Agents.AskUserQuestionRequest; + questions?: Agents.AskUserQuestionBatchItem[]; + messageId: string; +} | null { if (!Array.isArray(messages)) { return null; } @@ -440,7 +509,12 @@ export function findLiveAskUserQuestion( const part = content[j]; 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 }; + return { + actionId: ask.actionId, + question: ask.question, + questions: ask.questions, + messageId: message.messageId, + }; } } } diff --git a/e2e/setup/fake-model.js b/e2e/setup/fake-model.js index 12debcfffa..021e6ced64 100644 --- a/e2e/setup/fake-model.js +++ b/e2e/setup/fake-model.js @@ -1172,11 +1172,17 @@ function askUserQuestionResponses(label, toolNames) { id: `call_e2e_ask_user_question_${label}`, name: ASK_USER_QUESTION_TOOL_NAME, args: { - question: `Which environment should Bombadil use for ${label}?`, - description: 'This deterministic pause exercises the HITL answer and resume lifecycle.', - options: [ - { label: 'Staging', value: 'staging' }, - { label: 'Production', value: 'production' }, + questions: [ + { + id: 'environment', + question: `Which environment should Bombadil use for ${label}?`, + description: + 'This deterministic pause exercises the HITL answer and resume lifecycle.', + options: [ + { label: 'Staging', value: 'staging' }, + { label: 'Production', value: 'production' }, + ], + }, ], }, type: 'tool_call', @@ -1716,8 +1722,13 @@ function deferredHitlInvocationResponse({ graph, messages, options, runManager } id: askCallId, name: ASK_USER_QUESTION_NAME, args: { - question: `Continue deferred schema check ${label}?`, - options: [{ label: `Continue ${label}`, value: `continue-${label}` }], + questions: [ + { + id: 'confirmation', + question: `Continue deferred schema check ${label}?`, + options: [{ label: `Continue ${label}`, value: `continue-${label}` }], + }, + ], }, type: 'tool_call', }, diff --git a/e2e/specs/mock/deferred-tools-hitl.spec.ts b/e2e/specs/mock/deferred-tools-hitl.spec.ts index cdf58d4bda..3b993eebda 100644 --- a/e2e/specs/mock/deferred-tools-hitl.spec.ts +++ b/e2e/specs/mock/deferred-tools-hitl.spec.ts @@ -29,7 +29,7 @@ type MCPToolsResponse = { type AskResumeBody = { actionId?: string; agent_id?: string; - answer?: string; + answers?: Record; conversationId?: string; endpoint?: string; }; @@ -141,19 +141,22 @@ test.describe('deferred tools across HITL resume', () => { name: new RegExp(`${escapeRegExp(optionLabel)}$`), }); await expect(option).toBeVisible(); + await option.click(); + const submit = page.getByRole('button', { name: 'Submit', exact: true }); + await expect(submit).toBeEnabled(); const [resumeRequest, resumeResponse] = await Promise.all([ page.waitForRequest(isResumeRequest), page.waitForResponse( (candidate) => isResumeRequest(candidate.request()) && candidate.status() === 200, ), - option.click(), + submit.click(), ]); const conversationId = conversationPath.replace('/c/', ''); const body = resumeRequest.postDataJSON() as AskResumeBody; expect(body.actionId).toBeTruthy(); expect(body.agent_id).toBe(agentId); - expect(body.answer).toBe(answer); + expect(body.answers).toEqual({ confirmation: answer }); expect(body.conversationId).toBe(conversationId); expect(body.endpoint).toBe('agents'); expect(resumeResponse.ok()).toBeTruthy(); diff --git a/package-lock.json b/package-lock.json index 4ea985826a..c54a9a82f1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -63,7 +63,7 @@ "@azure/storage-blob": "^12.30.0", "@google/genai": "^2.8.0", "@keyv/redis": "^4.3.3", - "@librechat/agents": "^3.4.4", + "@librechat/agents": "^3.4.5", "@librechat/api": "*", "@librechat/data-schemas": "*", "@microsoft/microsoft-graph-client": "^3.0.7", @@ -10628,9 +10628,9 @@ } }, "node_modules/@librechat/agents": { - "version": "3.4.4", - "resolved": "https://registry.npmjs.org/@librechat/agents/-/agents-3.4.4.tgz", - "integrity": "sha512-Sv1+my/28jqjV+Dz+RQjxM53itVVcQNOfI9hJ4puxpdjZpDYCPDQzUJT73d9yvI5IO1NoDYA8wULlIJJ5FCI9w==", + "version": "3.4.5", + "resolved": "https://registry.npmjs.org/@librechat/agents/-/agents-3.4.5.tgz", + "integrity": "sha512-o3wSWG3P639i0ApRZ5uax88yds8ogNJEe2dUuD6727XRAwOm9dcrXmx2ioPCIZzF6cOT/iiBFL3xia0wWRSOdQ==", "license": "MIT", "dependencies": { "@anthropic-ai/sdk": "^0.115.0", @@ -42849,7 +42849,7 @@ "@azure/storage-blob": "^12.30.0", "@google/genai": "^2.8.0", "@keyv/redis": "^4.3.3", - "@librechat/agents": "^3.4.4", + "@librechat/agents": "^3.4.5", "@librechat/data-schemas": "*", "@modelcontextprotocol/sdk": "^1.29.0", "@opentelemetry/api": "^1.9.0", diff --git a/packages/api/package.json b/packages/api/package.json index b77b2188d6..e7b107f44a 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -113,7 +113,7 @@ "@azure/storage-blob": "^12.30.0", "@google/genai": "^2.8.0", "@keyv/redis": "^4.3.3", - "@librechat/agents": "^3.4.4", + "@librechat/agents": "^3.4.5", "@librechat/data-schemas": "*", "@modelcontextprotocol/sdk": "^1.29.0", "@opentelemetry/api": "^1.9.0", diff --git a/packages/api/src/agents/hitl/askUserQuestionTool.spec.ts b/packages/api/src/agents/hitl/askUserQuestionTool.spec.ts index 75a7e6e654..eaaf66d41e 100644 --- a/packages/api/src/agents/hitl/askUserQuestionTool.spec.ts +++ b/packages/api/src/agents/hitl/askUserQuestionTool.spec.ts @@ -5,11 +5,8 @@ import { createAskUserQuestionTool, } from './askUserQuestionTool'; -/** - * Contract-shape coverage. The runtime behavior (interrupt from the tool body, - * durable pause, answer round-trip over the real resume controller) is covered - * end-to-end in `api/server/controllers/agents/__tests__/askUserQuestion.e2e.spec.js`. - */ +const question = (id = 'environment') => ({ id, question: 'Deploy where?' }); + describe('ask_user_question tool contract', () => { test('the name matches the SDK interrupt discriminator the pipeline keys on', () => { expect(ASK_USER_QUESTION_TOOL_NAME).toBe('ask_user_question'); @@ -17,61 +14,58 @@ describe('ask_user_question tool contract', () => { expect(createAskUserQuestionTool().name).toBe(ASK_USER_QUESTION_TOOL_NAME); }); - describe('zod schema (the wire shape the client card receives)', () => { - test('accepts a bare question', () => { - expect(askUserQuestionToolSchema.parse({ question: 'Deploy where?' })).toEqual({ - question: 'Deploy where?', - }); - }); - - test('accepts question + description + options', () => { + describe('zod schema', () => { + test('accepts one to four related questions', () => { const input = { - question: 'Deploy where?', - description: 'Two environments are configured.', - options: [ - { label: 'Staging', value: 'staging' }, - { label: 'Production', value: 'production' }, + questions: [ + { + ...question(), + header: 'Environment', + description: 'Two environments are configured.', + options: [ + { label: 'Staging', value: 'staging' }, + { label: 'Production', value: 'production' }, + ], + }, + { ...question('window'), question: 'Which time window?', multiSelect: true }, ], }; expect(askUserQuestionToolSchema.parse(input)).toEqual(input); }); - test('rejects an empty question', () => { - expect(askUserQuestionToolSchema.safeParse({ question: '' }).success).toBe(false); + test('rejects empty, oversized, duplicate-id, and unsafe-id batches', () => { + expect(askUserQuestionToolSchema.safeParse({ questions: [] }).success).toBe(false); + expect( + askUserQuestionToolSchema.safeParse({ + questions: Array.from({ length: 5 }, (_, index) => question(`q${index}`)), + }).success, + ).toBe(false); + expect( + askUserQuestionToolSchema.safeParse({ questions: [question(), question()] }).success, + ).toBe(false); + expect( + askUserQuestionToolSchema.safeParse({ questions: [question('time.window')] }).success, + ).toBe(false); }); - test('rejects an over-cap question (model-generated text is bounded)', () => { - expect(askUserQuestionToolSchema.safeParse({ question: 'x'.repeat(2001) }).success).toBe( - false, - ); - }); - - test('rejects more than 12 options', () => { - const options = Array.from({ length: 13 }, (_, i) => ({ - label: `opt ${i}`, - value: `v${i}`, + test('rejects over-cap question and option content', () => { + expect( + askUserQuestionToolSchema.safeParse({ + questions: [{ ...question(), question: 'x'.repeat(2001) }], + }).success, + ).toBe(false); + const options = Array.from({ length: 13 }, (_, index) => ({ + label: `opt ${index}`, + value: `v${index}`, })); - expect(askUserQuestionToolSchema.safeParse({ question: 'pick', options }).success).toBe( - false, - ); - }); - - test('rejects an option missing label or value', () => { expect( askUserQuestionToolSchema.safeParse({ - question: 'pick', - options: [{ label: 'only label' }], - }).success, - ).toBe(false); - expect( - askUserQuestionToolSchema.safeParse({ - question: 'pick', - options: [{ label: '', value: 'v' }], + questions: [{ ...question(), options }], }).success, ).toBe(false); }); - test('rejects an overlong option label with guidance and records the real tool call', async () => { + test('records an overlong option label against the real tool call', async () => { const validationErrors = new Map(); await expect( createAskUserQuestionTool(validationErrors).invoke({ @@ -79,94 +73,52 @@ describe('ask_user_question tool contract', () => { name: ASK_USER_QUESTION_TOOL_NAME, type: 'tool_call', args: { - question: 'How should I get the data?', - options: [{ label: 'x'.repeat(281), value: 'public-data' }], + questions: [ + { + ...question(), + options: [{ label: 'x'.repeat(281), value: 'public-data' }], + }, + ], }, }), ).rejects.toThrow( 'Option labels must be 280 characters or fewer. Shorten the label and retry.', ); expect(validationErrors).toEqual( - new Map([['tool-1', { fieldPath: 'options[0].label', isLengthLimit: true }]]), + new Map([['tool-1', { fieldPath: 'questions[0].options[0].label', isLengthLimit: true }]]), ); }); - test('accepts multiSelect as an optional boolean and rejects other types', () => { - const input = { - question: 'Which apply?', - options: [ - { label: 'A', value: 'a' }, - { label: 'B', value: 'b' }, - ], - multiSelect: true, - }; - expect(askUserQuestionToolSchema.parse(input)).toEqual(input); - expect( - askUserQuestionToolSchema.safeParse({ question: 'pick', multiSelect: 'yes' }).success, - ).toBe(false); + test('accepts checkpointed legacy arguments without advertising them', async () => { + const promise = createAskUserQuestionTool().invoke({ + id: 'legacy-call', + name: ASK_USER_QUESTION_TOOL_NAME, + type: 'tool_call', + args: { question: 'Continue the pre-deploy run?' }, + }); + + await expect(promise).rejects.toThrow('No configurable found in config'); + expect(AskUserQuestionToolDefinition.schema.required).toEqual(['questions']); }); }); - describe('registry definition (schema-only twin)', () => { - test('question is required; options items require label + value', () => { - expect(AskUserQuestionToolDefinition.schema.required).toEqual(['question']); - expect(AskUserQuestionToolDefinition.schema.properties.options.items.required).toEqual([ - 'label', - 'value', - ]); + describe('registry definition', () => { + test('mirrors the nested batch schema and caps', () => { + const questions = AskUserQuestionToolDefinition.schema.properties.questions; + expect(AskUserQuestionToolDefinition.schema.required).toEqual(['questions']); + expect(questions.minItems).toBe(1); + expect(questions.maxItems).toBe(4); + expect(questions.items.required).toEqual(['id', 'question']); + expect(questions.items.properties.options.items.required).toEqual(['label', 'value']); + expect(questions.items.properties.options.items.properties.label.maxLength).toBe(280); }); - test('multiSelect is declared as an optional boolean in both schemas', () => { - expect(AskUserQuestionToolDefinition.schema.properties.multiSelect.type).toBe('boolean'); - expect(AskUserQuestionToolDefinition.schema.required).not.toContain('multiSelect'); - expect(AskUserQuestionToolDefinition.description).toContain('multiSelect'); - }); - - test('definition caps agree with the zod schema caps', () => { - const { properties } = AskUserQuestionToolDefinition.schema; - expect( - askUserQuestionToolSchema.safeParse({ question: 'x'.repeat(properties.question.maxLength) }) - .success, - ).toBe(true); - expect( - askUserQuestionToolSchema.safeParse({ - question: 'x'.repeat(properties.question.maxLength + 1), - }).success, - ).toBe(false); - const atCap = Array.from({ length: properties.options.maxItems }, (_, i) => ({ - label: `l${i}`, - value: `v${i}`, - })); - expect( - askUserQuestionToolSchema.safeParse({ question: 'pick', options: atCap }).success, - ).toBe(true); - const labelMax = properties.options.items.properties.label.maxLength; - expect( - askUserQuestionToolSchema.safeParse({ - question: 'pick', - options: [{ label: 'x'.repeat(labelMax), value: 'v' }], - }).success, - ).toBe(true); - expect( - AskUserQuestionToolDefinition.schema.properties.options.items.properties.label.maxLength, - ).toBe(280); - expect( - askUserQuestionToolSchema.safeParse({ - question: 'pick', - options: [{ label: 'x'.repeat(labelMax + 1), value: 'v' }], - }).success, - ).toBe(false); - }); - - test('descriptions match between the instance, the definition, and the constant name', () => { + test('describes one batched interaction and forbids sibling calls', () => { const instance = createAskUserQuestionTool(); expect(instance.description).toBe(AskUserQuestionToolDefinition.description); - expect(instance.description).toContain('exactly ONE question per turn'); + expect(instance.description).toContain('one to four related'); + expect(instance.description).toContain('ONE tool call'); expect(instance.description).toContain('NEVER call this tool in parallel'); - expect(instance.description).toContain('option label within 280 characters'); - expect( - AskUserQuestionToolDefinition.schema.properties.options.items.properties.label.description, - ).toContain('Maximum 280 characters'); }); }); }); diff --git a/packages/api/src/agents/hitl/askUserQuestionTool.ts b/packages/api/src/agents/hitl/askUserQuestionTool.ts index b281158676..07dead25f3 100644 --- a/packages/api/src/agents/hitl/askUserQuestionTool.ts +++ b/packages/api/src/agents/hitl/askUserQuestionTool.ts @@ -1,6 +1,11 @@ import { z } from 'zod'; -import { askUserQuestion } from '@librechat/agents'; import { tool } from '@librechat/agents/langchain/tools'; +import { + ASK_USER_QUESTION_ID_PATTERN, + MAX_ASK_USER_QUESTIONS, + askUserQuestion, + askUserQuestions, +} from '@librechat/agents'; import type { DynamicStructuredTool } from '@librechat/agents/langchain/tools'; import type { ToolInputValidationError } from '../toolValidation'; import { recordToolInputValidationError } from '../toolValidation'; @@ -20,6 +25,7 @@ export const ASK_USER_QUESTION_TOOL_NAME = 'ask_user_question'; */ const QUESTION_MAX = 2000; const DESCRIPTION_MAX = 4000; +const HEADER_MAX = 80; const OPTION_LABEL_MAX = 280; const OPTION_VALUE_MAX = 500; const OPTIONS_MAX = 12; @@ -32,10 +38,12 @@ const OPTION_LABEL_MAX_ERROR = 'Shorten the label and retry.'; const ASK_USER_QUESTION_DESCRIPTION = [ - 'Ask the user a clarifying question and pause the run until they answer; their answer is', - "returned as this tool's result. Use it only when you are genuinely blocked on a decision", - 'you cannot resolve from the conversation or your other tools. Ask exactly ONE question per', - 'turn, and NEVER call this tool in parallel with any other tool call. When the realistic', + 'Ask the user one to four related clarifying questions and pause the run until they answer;', + "their answers are returned as this tool's result, keyed by each question id. Use it only", + 'when you are genuinely blocked on decisions you cannot resolve from the conversation or your', + 'other tools. Put every related question in this ONE tool call, and NEVER call this tool in', + 'parallel with any other tool call. Use stable, concise ids matching', + '[A-Za-z][A-Za-z0-9_-]{0,63}. When the realistic', 'answers are enumerable, provide 2-6 concise options; set multiSelect to true only when', 'several options may sensibly apply at once (the selected option values are returned joined', `by ", "). Keep every option label within ${OPTION_LABEL_MAX} characters and put supporting`, @@ -46,12 +54,14 @@ const ASK_USER_QUESTION_DESCRIPTION = [ ].join(' '); /** - * Mirrors the SDK's `AskUserQuestionRequest` (question / description? / options?) — the - * validated input is passed to `askUserQuestion()` unchanged, so this schema IS the wire - * shape the client card receives inside the pendingAction payload. + * Mirrors the SDK's `AskUserQuestionsRequest`; the validated input is passed to + * `askUserQuestions()` unchanged, so this schema is the wire shape inside the + * pending-action payload. */ -export const askUserQuestionToolSchema: z.ZodObject< +const askUserQuestionItemSchema: z.ZodObject< { + id: z.ZodString; + header: z.ZodOptional; question: z.ZodString; description: z.ZodOptional; options: z.ZodOptional< @@ -61,11 +71,21 @@ export const askUserQuestionToolSchema: z.ZodObject< }, 'strip' > = z.object({ + id: z + .string() + .regex(ASK_USER_QUESTION_ID_PATTERN) + .describe('Unique answer key for this question.'), + header: z + .string() + .min(1) + .max(HEADER_MAX) + .optional() + .describe('Optional short heading shown above the question.'), question: z .string() .min(1) .max(QUESTION_MAX) - .describe('The single clarifying question to ask the user.'), + .describe('One clarifying question to ask the user.'), description: z .string() .max(DESCRIPTION_MAX) @@ -97,6 +117,27 @@ export const askUserQuestionToolSchema: z.ZodObject< .describe('Allow the user to pick several options; their values are returned joined by ", ".'), }); +const askUserQuestionsArraySchema: z.ZodEffects> = z + .array(askUserQuestionItemSchema) + .min(1) + .max(MAX_ASK_USER_QUESTIONS) + .refine( + (questions) => new Set(questions.map((question) => question.id)).size === questions.length, + { + message: 'Question ids must be unique.', + }, + ) + .describe('One to four related questions presented to the user in one interaction.'); + +const legacyAskUserQuestionToolSchema = askUserQuestionItemSchema.omit({ id: true, header: true }); + +export const askUserQuestionToolSchema: z.ZodObject< + { questions: typeof askUserQuestionsArraySchema }, + 'strip' +> = z.object({ + questions: askUserQuestionsArraySchema, +}); + export type AskUserQuestionToolInput = z.infer; /** Explicit shape of {@link AskUserQuestionToolDefinition} (isolatedDeclarations). */ @@ -106,22 +147,51 @@ export interface AskUserQuestionToolDefinitionShape { schema: { type: 'object'; properties: { - question: { type: 'string'; minLength: number; maxLength: number; description: string }; - description: { type: 'string'; maxLength: number; description: string }; - options: { + questions: { type: 'array'; + minItems: number; maxItems: number; description: string; items: { type: 'object'; properties: { - label: { type: 'string'; minLength: number; maxLength: number; description: string }; - value: { type: 'string'; minLength: number; maxLength: number; description: string }; + id: { type: 'string'; pattern: string; description: string }; + header: { type: 'string'; minLength: number; maxLength: number; description: string }; + question: { + type: 'string'; + minLength: number; + maxLength: number; + description: string; + }; + description: { type: 'string'; maxLength: number; description: string }; + options: { + type: 'array'; + maxItems: number; + description: string; + items: { + type: 'object'; + properties: { + label: { + type: 'string'; + minLength: number; + maxLength: number; + description: string; + }; + value: { + type: 'string'; + minLength: number; + maxLength: number; + description: string; + }; + }; + required: string[]; + }; + }; + multiSelect: { type: 'boolean'; description: string }; }; required: string[]; }; }; - multiSelect: { type: 'boolean'; description: string }; }; required: string[]; }; @@ -138,57 +208,78 @@ export const AskUserQuestionToolDefinition: AskUserQuestionToolDefinitionShape = schema: { type: 'object', properties: { - question: { - type: 'string', - minLength: 1, - maxLength: QUESTION_MAX, - description: 'The single clarifying question to ask the user.', - }, - description: { - type: 'string', - maxLength: DESCRIPTION_MAX, - description: 'Optional context rendered alongside the question (why you are asking).', - }, - options: { + questions: { type: 'array', - maxItems: OPTIONS_MAX, - description: - 'Optional pre-defined choices (2-6 recommended). Omit to require a free-form answer.', + minItems: 1, + maxItems: MAX_ASK_USER_QUESTIONS, + description: 'One to four related questions presented in one interaction.', items: { type: 'object', properties: { - label: { + id: { type: 'string', - minLength: 1, - maxLength: OPTION_LABEL_MAX, - description: OPTION_LABEL_DESCRIPTION, + pattern: ASK_USER_QUESTION_ID_PATTERN.source, + description: 'Unique answer key for this question.', }, - value: { + header: { type: 'string', minLength: 1, - maxLength: OPTION_VALUE_MAX, - description: 'Value returned as the answer if this option is picked.', + maxLength: HEADER_MAX, + description: 'Optional short heading shown above the question.', + }, + question: { + type: 'string', + minLength: 1, + maxLength: QUESTION_MAX, + description: 'One clarifying question to ask the user.', + }, + description: { + type: 'string', + maxLength: DESCRIPTION_MAX, + description: 'Optional context rendered alongside the question.', + }, + options: { + type: 'array', + maxItems: OPTIONS_MAX, + description: 'Optional pre-defined choices. Omit for free-form only.', + items: { + type: 'object', + properties: { + label: { + type: 'string', + minLength: 1, + maxLength: OPTION_LABEL_MAX, + description: OPTION_LABEL_DESCRIPTION, + }, + value: { + type: 'string', + minLength: 1, + maxLength: OPTION_VALUE_MAX, + description: 'Value returned if this option is picked.', + }, + }, + required: ['label', 'value'], + }, + }, + multiSelect: { + type: 'boolean', + description: 'Allow several option values for this question.', }, }, - required: ['label', 'value'], + required: ['id', 'question'], }, }, - multiSelect: { - type: 'boolean', - description: - 'Allow the user to pick several options; their values are returned joined by ", ".', - }, }, - required: ['question'], + required: ['questions'], }, }; /** * Create the `ask_user_question` tool instance. The func calls the SDK's - * `askUserQuestion()` helper, which raises a LangGraph `interrupt()` — on the first + * `askUserQuestions()` helper, which raises a LangGraph `interrupt()` — on the first * pass execution unwinds (the run pauses; `run.getInterrupt().payload.type === * 'ask_user_question'`), and on the resume pass it returns the host-supplied - * `{ answer }`, which becomes the ToolMessage content the model sees. + * `{ answers }`, which becomes the ToolMessage content the model sees. * * Requirements at the run level (wired in `agents/run.ts`): a checkpointer must be * attached (the interrupt must be durable to be resumable) and the tool must be @@ -201,26 +292,27 @@ export const AskUserQuestionToolDefinition: AskUserQuestionToolDefinitionShape = * from the top on the resume pass, and sibling tools in the same batch re-execute — * which is why the description forbids parallel calls. */ -/** - * `askUserQuestion` with the optional attribution argument shipped in - * `@librechat/agents` > 3.3.8 (interrupt payload gains `tool_call_id`, letting - * the host stamp the question/answer onto the exact tool-call part in - * multi-ask turns). The pinned release still types the single-arg form; the - * extra argument is ignored at runtime by older versions. Drop this alias once - * the dependency pin includes the two-arg signature. - */ -const askUserQuestionWithId = askUserQuestion as ( - question: AskUserQuestionToolInput, - options?: { toolCallId?: string }, -) => ReturnType; - export function createAskUserQuestionTool( validationErrorsByToolCallId?: Map, ): DynamicStructuredTool { + /** Kept out of the provider-facing definition. A graph rebuilt during a + * rolling deploy can still rerun checkpointed legacy `{ question, ... }` + * arguments through this schema and consume its retained `{ answer }` resume. */ + const legacyAskTool = tool( + async (input, config?: { toolCall?: { id?: string } }) => { + const resolution = askUserQuestion(input, { toolCallId: config?.toolCall?.id }); + return JSON.stringify(resolution); + }, + { + name: ASK_USER_QUESTION_TOOL_NAME, + description: ASK_USER_QUESTION_DESCRIPTION, + schema: legacyAskUserQuestionToolSchema, + }, + ); const askTool = tool( async (input: AskUserQuestionToolInput, config?: { toolCall?: { id?: string } }) => { - const { answer } = askUserQuestionWithId(input, { toolCallId: config?.toolCall?.id }); - return answer; + const resolution = askUserQuestions(input, { toolCallId: config?.toolCall?.id }); + return JSON.stringify(resolution); }, { name: ASK_USER_QUESTION_TOOL_NAME, @@ -233,8 +325,16 @@ export function createAskUserQuestionTool( * the tool boundary so the thrown validation error can be correlated with * the real call ID without inferring failure from persisted output text. */ const invoke = askTool.invoke.bind(askTool); - askTool.invoke = async (input, config) => { + askTool.invoke = (async (input, config) => { try { + const candidate = + typeof input === 'object' && input != null && 'args' in input ? input.args : input; + if (legacyAskUserQuestionToolSchema.safeParse(candidate).success) { + return await legacyAskTool.invoke( + input as unknown as Parameters[0], + config, + ); + } return await invoke(input, config); } catch (error) { const toolCallId = @@ -242,7 +342,7 @@ export function createAskUserQuestionTool( recordToolInputValidationError(validationErrorsByToolCallId, error, toolCallId); throw error; } - }; + }) as typeof askTool.invoke; return askTool; } diff --git a/packages/api/src/agents/hitl/resume.spec.ts b/packages/api/src/agents/hitl/resume.spec.ts index 992b223c18..d0779a5a36 100644 --- a/packages/api/src/agents/hitl/resume.spec.ts +++ b/packages/api/src/agents/hitl/resume.spec.ts @@ -9,6 +9,9 @@ import type { Agents } from 'librechat-data-provider'; import { mapToolApprovalResolutions, mapAskUserAnswer, + mapAskUserAnswers, + serializeAskUserAnswerVariants, + resolveAskUserQuestionResume, findUndecidedToolCalls, findDisallowedDecisions, findIncompleteDecisions, @@ -70,6 +73,105 @@ describe('mapAskUserAnswer', () => { }); }); +describe('mapAskUserAnswers', () => { + it('preserves answers keyed by question id', () => { + expect(mapAskUserAnswers({ answers: { environment: 'staging', window: '7d' } })).toEqual({ + answers: { environment: 'staging', window: '7d' }, + }); + }); +}); + +describe('serializeAskUserAnswerVariants', () => { + it('covers every bounded downstream answer-map ordering with the resolution wrapper', () => { + expect(serializeAskUserAnswerVariants({ second: '456', first: '123' })).toEqual([ + '{"answers":{"second":"456","first":"123"}}', + '{"answers":{"first":"123","second":"456"}}', + ]); + }); + + it('does not expand invalid or unbounded answer maps', () => { + expect(serializeAskUserAnswerVariants(['one'])).toEqual([]); + expect( + serializeAskUserAnswerVariants({ + one: '1', + two: '2', + three: '3', + four: '4', + five: '5', + }), + ).toEqual([]); + }); +}); + +describe('resolveAskUserQuestionResume', () => { + const payload: Agents.AskUserQuestionInterruptPayload = { + type: 'ask_user_question', + question: { question: 'Where and when?' }, + questions: [ + { id: 'environment', question: 'Where?' }, + { id: 'window', question: 'When?' }, + ], + }; + + test('validates and maps a complete batch', () => { + expect( + resolveAskUserQuestionResume(payload, { + answers: { environment: 'staging', window: '7d' }, + }), + ).toEqual({ resumeValue: { answers: { environment: 'staging', window: '7d' } } }); + }); + + test('rejects missing, unknown, and array-shaped answers', () => { + expect(resolveAskUserQuestionResume(payload, { answers: { environment: 'staging' } })).toEqual({ + status: 400, + error: 'Answers are required for every question', + }); + expect( + resolveAskUserQuestionResume(payload, { + answers: { environment: 'staging', window: '7d', region: 'us-east-2' }, + }), + ).toEqual({ status: 400, error: 'Answers contain an unknown question id' }); + expect(resolveAskUserQuestionResume(payload, { answers: ['staging', '7d'] })).toEqual({ + status: 400, + error: 'Answers are required for every question', + }); + }); + + test('rejects invalid batches and oversized answers', () => { + expect( + resolveAskUserQuestionResume( + { ...payload, questions: [{ id: 'bad id', question: 'Invalid?' }] }, + { answers: { 'bad id': 'yes' } }, + ), + ).toEqual({ status: 400, error: 'The pending question batch is invalid' }); + expect( + resolveAskUserQuestionResume( + { + ...payload, + questions: [{ id: undefined, question: 'Invalid?' }], + } as unknown as Agents.AskUserQuestionInterruptPayload, + { answers: { undefined: 'yes' } }, + ), + ).toEqual({ status: 400, error: 'The pending question batch is invalid' }); + expect( + resolveAskUserQuestionResume(payload, { + answers: { environment: 'x'.repeat(16_001), window: '7d' }, + }), + ).toEqual({ status: 400, error: 'An answer exceeds the maximum length' }); + }); + + test('preserves legacy single-answer validation and mapping', () => { + const legacy = { ...payload, questions: undefined }; + expect(resolveAskUserQuestionResume(legacy, { answer: 'staging' })).toEqual({ + resumeValue: { answer: 'staging' }, + }); + expect(resolveAskUserQuestionResume(legacy, {})).toEqual({ + status: 400, + error: 'An answer is required', + }); + }); +}); + describe('findUndecidedToolCalls', () => { const payload: Agents.ToolApprovalInterruptPayload = { type: 'tool_approval', @@ -463,6 +565,22 @@ describe('attachAskUserQuestionAnswer', () => { attachAskUserQuestionAnswer(content as never, question as never, 'x', 'tc_missing'), ).toBe(content); }); + + it('stamps batched args and structured answers onto one ask tool call', () => { + const request = { + questions: [ + { id: 'environment', question: 'Which env?' }, + { id: 'window', question: 'Which window?' }, + ], + }; + const output = JSON.stringify({ answers: { environment: 'staging', window: '7d' } }); + const next = attachAskUserQuestionAnswer([askPart()] as never, request, output); + const toolCall = (next[0] as { tool_call: { args: string; output: string } }).tool_call; + expect(JSON.parse(toolCall.args)).toEqual(request); + expect(JSON.parse(toolCall.output)).toEqual({ + answers: { environment: 'staging', window: '7d' }, + }); + }); }); describe('attachAskUserQuestionArgs (pause-time stamp)', () => { @@ -500,4 +618,20 @@ describe('attachAskUserQuestionArgs (pause-time stamp)', () => { ); expect((next[1] as { tool_call: { args: string } }).tool_call.args).toBe(''); }); + + it('stamps the complete batched request at pause time', () => { + const content = [ + { type: 'tool_call', tool_call: { id: 'tc1', name: 'ask_user_question', args: '' } }, + ]; + const request = { + questions: [ + { id: 'environment', question: 'Which env?' }, + { id: 'window', question: 'Which window?' }, + ], + }; + const next = attachAskUserQuestionArgs(content as never, request); + expect(JSON.parse((next[0] as { tool_call: { args: string } }).tool_call.args)).toEqual( + request, + ); + }); }); diff --git a/packages/api/src/agents/hitl/resume.ts b/packages/api/src/agents/hitl/resume.ts index 99ca6a4ce6..40c2ab2466 100644 --- a/packages/api/src/agents/hitl/resume.ts +++ b/packages/api/src/agents/hitl/resume.ts @@ -3,6 +3,7 @@ import type { ToolApprovalDecision, ToolApprovalDecisionMap, AskUserQuestionResolution, + AskUserQuestionsResolution, EventHandler, RunStep, } from '@librechat/agents'; @@ -57,6 +58,112 @@ export function mapAskUserAnswer( return { answer: resolution.answer }; } +/** Translate batched ask-user wire answers into the SDK's resume value. */ +export function mapAskUserAnswers( + resolution: Agents.AskUserQuestionsResolution, +): AskUserQuestionsResolution { + return { answers: resolution.answers }; +} + +const MAX_ASK_ANSWER_LENGTH = 16_000; +const ASK_QUESTION_ID_PATTERN = /^[A-Za-z][A-Za-z0-9_-]{0,63}$/; +const MAX_ASK_QUESTIONS = 4; + +/** + * Serialize every ordering a validated batch can take after the SDK rebuilds + * its answer map in question order. Batches are capped at four questions, so + * this remains bounded at 24 candidates and lets pre-controller PII/moderation + * checks inspect the exact ToolMessage even for a crafted key order. + */ +export function serializeAskUserAnswerVariants(answers: unknown): string[] { + if (answers == null || typeof answers !== 'object' || Array.isArray(answers)) { + return []; + } + const entries = Object.entries(answers); + if ( + entries.length === 0 || + entries.length > MAX_ASK_QUESTIONS || + entries.some(([, value]) => typeof value !== 'string') + ) { + return []; + } + + const variants: string[] = []; + const visit = (remaining: Array<[string, unknown]>, ordered: Array<[string, unknown]>) => { + if (remaining.length === 0) { + const normalized = Object.create(null) as Record; + for (const [key, value] of ordered) { + normalized[key] = value; + } + variants.push(JSON.stringify({ answers: normalized })); + return; + } + for (let index = 0; index < remaining.length; index++) { + visit( + [...remaining.slice(0, index), ...remaining.slice(index + 1)], + [...ordered, remaining[index]], + ); + } + }; + visit(entries, []); + return variants; +} + +interface AskUserResumeBody { + answer?: unknown; + answers?: unknown; +} + +type AskUserResumeResult = + | { resumeValue: AskUserQuestionResolution | AskUserQuestionsResolution } + | { status: 400; error: string }; + +/** Validate an ask-user resume payload and translate it to the SDK contract. */ +export function resolveAskUserQuestionResume( + payload: Agents.AskUserQuestionInterruptPayload, + body: AskUserResumeBody, +): AskUserResumeResult { + if (!Array.isArray(payload.questions)) { + if (typeof body.answer !== 'string' || body.answer.length === 0) { + return { status: 400, error: 'An answer is required' }; + } + if (body.answer.length > MAX_ASK_ANSWER_LENGTH) { + return { status: 400, error: 'Answer exceeds the maximum length' }; + } + return { resumeValue: mapAskUserAnswer({ answer: body.answer }) }; + } + + if (payload.questions.length === 0 || payload.questions.length > MAX_ASK_QUESTIONS) { + return { status: 400, error: 'The pending question batch is invalid' }; + } + if (body.answers == null || typeof body.answers !== 'object' || Array.isArray(body.answers)) { + return { status: 400, error: 'Answers are required for every question' }; + } + + const submittedAnswers = body.answers as Record; + const answers: Record = Object.create(null); + const expectedIds = new Set(); + for (const question of payload.questions) { + const { id } = question; + if (typeof id !== 'string' || !ASK_QUESTION_ID_PATTERN.test(id) || expectedIds.has(id)) { + return { status: 400, error: 'The pending question batch is invalid' }; + } + expectedIds.add(id); + const answer = Object.getOwnPropertyDescriptor(submittedAnswers, id)?.value; + if (typeof answer !== 'string' || answer.length === 0) { + return { status: 400, error: 'Answers are required for every question' }; + } + if (answer.length > MAX_ASK_ANSWER_LENGTH) { + return { status: 400, error: 'An answer exceeds the maximum length' }; + } + answers[id] = answer; + } + if (Object.keys(submittedAnswers).some((id) => !expectedIds.has(id))) { + return { status: 400, error: 'Answers contain an unknown question id' }; + } + return { resumeValue: mapAskUserAnswers({ answers }) }; +} + /** * Validate that a set of resolutions covers exactly the tool calls a pending * `tool_approval` action is waiting on. Returns the list of `tool_call_id`s that @@ -350,8 +457,8 @@ export function attachAskUserQuestionAnswer< TPart extends { type?: string; tool_call?: { id?: string; name?: string; output?: unknown } }, >( content: TPart[], - question: Agents.AskUserQuestionRequest, - answer: string, + request: Agents.AskUserQuestionRequest | Agents.AskUserQuestionsRequest, + output: string, toolCallId?: string, ): TPart[] { const index = findAskPartIndex( @@ -368,8 +475,8 @@ export function attachAskUserQuestionAnswer< ...part, tool_call: { ...part.tool_call, - args: JSON.stringify(question), - output: answer, + args: JSON.stringify(request), + output, progress: 1, }, }; @@ -391,7 +498,11 @@ export function attachAskUserQuestionArgs< type?: string; tool_call?: { id?: string; name?: string; args?: unknown; output?: unknown }; }, ->(content: TPart[], question: Agents.AskUserQuestionRequest, toolCallId?: string): TPart[] { +>( + content: TPart[], + request: Agents.AskUserQuestionRequest | Agents.AskUserQuestionsRequest, + toolCallId?: string, +): TPart[] { const index = findAskPartIndex(content, toolCallId, (part) => { const toolCall = part.tool_call; const hasArgs = @@ -406,6 +517,6 @@ export function attachAskUserQuestionArgs< } const part = content[index]; const next = [...content]; - next[index] = { ...part, tool_call: { ...part.tool_call, args: JSON.stringify(question) } }; + next[index] = { ...part, tool_call: { ...part.tool_call, args: JSON.stringify(request) } }; return next; } diff --git a/packages/api/src/agents/memory.spec.ts b/packages/api/src/agents/memory.spec.ts index 22f24380ac..be7843fdf4 100644 --- a/packages/api/src/agents/memory.spec.ts +++ b/packages/api/src/agents/memory.spec.ts @@ -70,17 +70,13 @@ jest.mock('~/utils', () => ({ const { createSafeUser } = jest.requireMock('~/utils'); -jest.mock('@librechat/agents', () => { - const actual = jest.requireActual('@librechat/agents'); - return { - Run: { - create: jest.fn(() => ({ +beforeEach(() => { + jest.spyOn(Run, 'create').mockImplementation( + () => + ({ processStream: jest.fn(() => Promise.resolve('success')), - })), - }, - Providers: actual.Providers, - GraphEvents: actual.GraphEvents, - }; + }) as never, + ); }); function createTestUser(overrides: Partial = {}): IUser { diff --git a/packages/api/src/agents/toolValidation.spec.ts b/packages/api/src/agents/toolValidation.spec.ts index c6d6325a6e..ace05e744d 100644 --- a/packages/api/src/agents/toolValidation.spec.ts +++ b/packages/api/src/agents/toolValidation.spec.ts @@ -30,6 +30,24 @@ describe('getToolInputValidationDetails', () => { expect(JSON.stringify(details)).not.toContain('Shorten the label'); }); + test('classifies an overlong option label nested in a question batch', () => { + const validationError = parseToolInputValidationError( + new Error( + 'Received tool input did not match expected schema\n' + + '✖ String must contain at most 120 character(s)\n' + + ' → at questions[0].options[0].label', + ), + ); + + expect( + getToolInputValidationDetails({ tool_call: { name: 'ask_user_question' } }, validationError), + ).toEqual({ + toolName: 'ask_user_question', + reason: 'option_label_too_long', + fieldPath: 'questions[0].options[0].label', + }); + }); + test('classifies other schema failures without requiring a field path', () => { expect( getToolInputValidationDetails( diff --git a/packages/api/src/agents/toolValidation.ts b/packages/api/src/agents/toolValidation.ts index 7445cc0fec..03a534e7db 100644 --- a/packages/api/src/agents/toolValidation.ts +++ b/packages/api/src/agents/toolValidation.ts @@ -1,6 +1,6 @@ const TOOL_INPUT_SCHEMA_ERROR = 'Received tool input did not match expected schema'; const SCHEMA_ERROR_PATH_PATTERN = /(?:→|->)\s+at\s+([A-Za-z0-9_.[\]-]{1,120})/; -const ASK_OPTION_LABEL_PATH_PATTERN = /^options\[\d+\]\.label$/; +const ASK_OPTION_LABEL_PATH_PATTERN = /^(?:questions\[\d+\]\.)?options\[\d+\]\.label$/; const OPTION_LABEL_LIMIT_PATTERN = /(?:at most \d+|\d+ characters or fewer)/i; interface CompletedToolCall { diff --git a/packages/api/src/middleware/messageFilterPii.spec.ts b/packages/api/src/middleware/messageFilterPii.spec.ts index 093a68e66a..f81cf176c7 100644 --- a/packages/api/src/middleware/messageFilterPii.spec.ts +++ b/packages/api/src/middleware/messageFilterPii.spec.ts @@ -115,6 +115,45 @@ describe('messageFilterPii middleware', () => { expect(capturedRes.status).toBe(400); }); + it('rejects a batched ask-user answer containing a blocked token', () => { + const { capturedRes, nextCalls } = runMiddleware( + {}, + { answers: { environment: 'staging', credentials: `the key is ${SK}` } }, + ); + expect(nextCalls).toBe(0); + expect(capturedRes.status).toBe(400); + }); + + it('rejects a blocked pattern spanning serialized batch answers', () => { + const { capturedRes, nextCalls } = runMiddleware( + { + starterPatterns: [], + customPatterns: [{ id: 'split', label: 'Split token', regex: '123[^0-9]+456' }], + }, + { answers: { first: '123', second: '456' } }, + ); + expect(nextCalls).toBe(0); + expect(capturedRes.status).toBe(400); + }); + + it('rejects the normalized ToolMessage ordering when request keys arrive out of order', () => { + const { capturedRes, nextCalls } = runMiddleware( + { + starterPatterns: [], + customPatterns: [ + { + id: 'ordered', + label: 'Ordered token', + regex: '\\{"answers":\\{"first":"123","second":"456"\\}\\}', + }, + ], + }, + { answers: { second: '456', first: '123' } }, + ); + expect(nextCalls).toBe(0); + expect(capturedRes.status).toBe(400); + }); + it('rejects a tool-approval decision responseText containing a blocked token', () => { const { capturedRes, nextCalls } = runMiddleware( {}, diff --git a/packages/api/src/middleware/messageFilterPii.ts b/packages/api/src/middleware/messageFilterPii.ts index 85094ce368..87b5959035 100644 --- a/packages/api/src/middleware/messageFilterPii.ts +++ b/packages/api/src/middleware/messageFilterPii.ts @@ -8,6 +8,7 @@ import type { Response as ServerResponse, } from 'express'; import type { MessageFilterPiiConfig } from 'librechat-data-provider'; +import { serializeAskUserAnswerVariants } from '../agents/hitl/resume'; import { getReferencedQuotes, mergeQuotedText } from '../utils/quotes'; /** @@ -196,6 +197,18 @@ export function createMessageFilterPii(options: CreateMessageFilterPiiOptions): if (typeof req.body?.answer === 'string' && req.body.answer.length > 0) { candidates.push(req.body.answer); } + if ( + req.body?.answers != null && + typeof req.body.answers === 'object' && + !Array.isArray(req.body.answers) + ) { + for (const answer of Object.values(req.body.answers)) { + if (typeof answer === 'string' && answer.length > 0) { + candidates.push(answer); + } + } + candidates.push(...serializeAskUserAnswerVariants(req.body.answers)); + } if (Array.isArray(req.body?.decisions)) { for (const decision of req.body.decisions) { if (typeof decision?.responseText === 'string' && decision.responseText.length > 0) { diff --git a/packages/data-provider/src/types/agents.ts b/packages/data-provider/src/types/agents.ts index 1543c6cdfe..2fa9c7b43e 100644 --- a/packages/data-provider/src/types/agents.ts +++ b/packages/data-provider/src/types/agents.ts @@ -380,10 +380,25 @@ export namespace Agents { multiSelect?: boolean; } + /** One independently answerable question in a batched clarification. */ + export interface AskUserQuestionBatchItem extends AskUserQuestionRequest { + /** Batch-unique identifier used to map the submitted answer. */ + id: string; + /** Optional short heading rendered above the question. */ + header?: string; + } + + /** Input shape for one tool call that asks several related questions. */ + export interface AskUserQuestionsRequest { + questions: AskUserQuestionBatchItem[]; + } + /** Interrupt payload for an ask-user-question pause. */ export interface AskUserQuestionInterruptPayload { type: 'ask_user_question'; question: AskUserQuestionRequest; + /** Present for a batched clarification; `question` remains the first-item fallback. */ + questions?: AskUserQuestionBatchItem[]; /** * The ask tool call that raised this interrupt (mirrors the SDK field, * present from `@librechat/agents` > 3.3.8). Lets the question/answer @@ -478,6 +493,11 @@ export namespace Agents { answer: string; } + /** Wire format for a batched ask-user-question response. */ + export interface AskUserQuestionsResolution { + answers: Record; + } + export interface ExtendedMessageContent { type?: string; text?: string;