From 7447fddfb2cd40a0f45da4829874812eeaa9a08a Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 15 Jul 2026 11:06:29 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=99=8A=20refactor:=20Clarify=20Ask=20Ques?= =?UTF-8?q?tion=20Schema=20Errors=20and=20Retry=20Guidance=20(#14279)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(agents): clarify ask question validation errors * fix(agents): narrow question failure detection * fix(agents): persist question validation failures * fix(agents): track question validation failures --- .../prompts/formatAgentMessages.spec.js | 2 + api/app/clients/prompts/formatMessages.js | 7 +- .../agents/__tests__/callbacks.spec.js | 112 ++++++++++++++++++ api/server/controllers/agents/callbacks.js | 33 ++++++ api/server/controllers/agents/client.js | 8 ++ .../services/Endpoints/agents/initialize.js | 8 +- .../Messages/Content/AskUserQuestionCall.tsx | 28 ++++- .../components/Chat/Messages/Content/Part.tsx | 1 + .../__tests__/AskUserQuestionCall.test.tsx | 68 +++++++++++ .../SSE/__tests__/useStepHandler.spec.ts | 2 + client/src/hooks/SSE/useStepHandler.ts | 6 + client/src/locales/en/translation.json | 2 + .../agents/hitl/askUserQuestionTool.spec.ts | 40 +++++++ .../src/agents/hitl/askUserQuestionTool.ts | 44 +++++-- packages/api/src/agents/index.ts | 1 + packages/api/src/agents/run.ts | 8 +- .../api/src/agents/toolValidation.spec.ts | 76 ++++++++++++ packages/api/src/agents/toolValidation.ts | 91 ++++++++++++++ packages/data-provider/src/types/agents.ts | 2 + 19 files changed, 525 insertions(+), 14 deletions(-) create mode 100644 client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionCall.test.tsx create mode 100644 packages/api/src/agents/toolValidation.spec.ts create mode 100644 packages/api/src/agents/toolValidation.ts diff --git a/api/app/clients/prompts/formatAgentMessages.spec.js b/api/app/clients/prompts/formatAgentMessages.spec.js index 5a4b6937ba..b5a9775974 100644 --- a/api/app/clients/prompts/formatAgentMessages.spec.js +++ b/api/app/clients/prompts/formatAgentMessages.spec.js @@ -55,6 +55,7 @@ describe('formatAgentMessages', () => { name: 'search', args: '{"query":"weather"}', output: 'The weather is sunny.', + inputValidationError: true, }, }, ], @@ -65,6 +66,7 @@ describe('formatAgentMessages', () => { expect(result[0]).toBeInstanceOf(AIMessage); expect(result[1]).toBeInstanceOf(ToolMessage); expect(result[0].tool_calls).toHaveLength(1); + expect(result[0].tool_calls[0]).not.toHaveProperty('inputValidationError'); expect(result[1].tool_call_id).toBe('123'); }); diff --git a/api/app/clients/prompts/formatMessages.js b/api/app/clients/prompts/formatMessages.js index 795c62c14f..29d4841b4c 100644 --- a/api/app/clients/prompts/formatMessages.js +++ b/api/app/clients/prompts/formatMessages.js @@ -201,7 +201,12 @@ const formatAgentMessages = (payload) => { } // Note: `tool_calls` list is defined when constructed by `AIMessage` class, and outputs should be excluded from it - const { output, args: _args, ...tool_call } = part.tool_call; + const { + output, + args: _args, + inputValidationError: _inputValidationError, + ...tool_call + } = part.tool_call; // TODO: investigate; args as dictionary may need to be provider-or-tool-specific let args = _args; try { diff --git a/api/server/controllers/agents/__tests__/callbacks.spec.js b/api/server/controllers/agents/__tests__/callbacks.spec.js index 20fcf54a6c..433e1dc2e8 100644 --- a/api/server/controllers/agents/__tests__/callbacks.spec.js +++ b/api/server/controllers/agents/__tests__/callbacks.spec.js @@ -8,6 +8,15 @@ jest.mock('nanoid', () => ({ jest.mock('@librechat/api', () => ({ sendEvent: jest.fn(), HOST_FILE_AUTHORING_ARTIFACT_KEY: '__librechat_file_authoring', + getToolInputValidationDetails: jest.fn((result, validationError) => + validationError != null + ? { + toolName: result.tool_call.name, + reason: 'option_label_too_long', + fieldPath: validationError.fieldPath, + } + : null, + ), isCodeSessionToolName: jest.fn((name) => ['execute_code', 'bash_tool', 'read_file'].includes(name), ), @@ -15,6 +24,7 @@ jest.mock('@librechat/api', () => ({ jest.mock('@librechat/data-schemas', () => ({ logger: { + debug: jest.fn(), error: jest.fn(), }, })); @@ -648,6 +658,108 @@ describe('createToolEndCallback', () => { }); }); +describe('tool input validation marker', () => { + it('marks the streamed result and persisted content part out of band', async () => { + const { GraphEvents, createContentAggregator } = jest.requireActual('@librechat/agents'); + const { getDefaultHandlers } = require('../callbacks'); + const { contentParts, aggregateContent, stepMap } = createContentAggregator(); + const toolInputValidationErrors = new Map([ + ['tool-1', { fieldPath: 'options[0].label', isLengthLimit: true }], + ]); + const handlers = getDefaultHandlers({ + res: { write: jest.fn() }, + contentParts, + stepMap, + aggregateContent, + toolInputValidationErrors, + toolEndCallback: jest.fn(), + collectedUsage: [], + }); + + aggregateContent({ + event: GraphEvents.ON_RUN_STEP, + data: { + id: 'step-1', + index: 0, + stepDetails: { + type: 'tool_calls', + tool_calls: [{ id: 'tool-1', name: 'ask_user_question', args: '{}' }], + }, + }, + }); + + const data = { + result: { + id: 'step-1', + tool_call: { + id: 'tool-1', + name: 'ask_user_question', + output: + 'Error processing tool: Received tool input did not match expected schema ' + + '→ at options[0].label', + }, + }, + }; + + await handlers[GraphEvents.ON_RUN_STEP_COMPLETED].handle( + GraphEvents.ON_RUN_STEP_COMPLETED, + data, + { run_id: 'run-1', thread_id: 'conversation-1' }, + ); + + expect(data.result.tool_call.inputValidationError).toBe(true); + expect(contentParts[0].tool_call.inputValidationError).toBe(true); + expect(toolInputValidationErrors.size).toBe(0); + }); + + it('does not mark successful output that resembles a schema error', async () => { + const { GraphEvents, createContentAggregator } = jest.requireActual('@librechat/agents'); + const { getDefaultHandlers } = require('../callbacks'); + const { contentParts, aggregateContent, stepMap } = createContentAggregator(); + const handlers = getDefaultHandlers({ + res: { write: jest.fn() }, + contentParts, + stepMap, + aggregateContent, + toolInputValidationErrors: new Map(), + toolEndCallback: jest.fn(), + collectedUsage: [], + }); + + aggregateContent({ + event: GraphEvents.ON_RUN_STEP, + data: { + id: 'step-1', + index: 0, + stepDetails: { + type: 'tool_calls', + tool_calls: [{ id: 'tool-1', name: 'ask_user_question', args: '{}' }], + }, + }, + }); + + const data = { + result: { + id: 'step-1', + tool_call: { + id: 'tool-1', + name: 'ask_user_question', + output: 'Received tool input did not match expected schema → at options[0].label', + }, + }, + }; + + await handlers[GraphEvents.ON_RUN_STEP_COMPLETED].handle( + GraphEvents.ON_RUN_STEP_COMPLETED, + data, + { run_id: 'run-1', thread_id: 'conversation-1' }, + ); + + expect(data.result.tool_call).not.toHaveProperty('inputValidationError'); + expect(contentParts[0].tool_call).not.toHaveProperty('inputValidationError'); + }); +}); + describe('isStreamWritable', () => { /* Direct parametric coverage of the predicate that gates SSE writes * in both the chat-completions and Open Responses callbacks. The diff --git a/api/server/controllers/agents/callbacks.js b/api/server/controllers/agents/callbacks.js index e006d96417..adc6086f70 100644 --- a/api/server/controllers/agents/callbacks.js +++ b/api/server/controllers/agents/callbacks.js @@ -23,6 +23,7 @@ const { HOST_FILE_AUTHORING_ARTIFACT_KEY, isCodeSessionToolName, shouldSignalSandboxStart, + getToolInputValidationDetails, } = require('@librechat/api'); const { processFileCitations } = require('~/server/services/Files/Citations'); const { processCodeOutput, runPreviewFinalize } = require('~/server/services/Files/Code/process'); @@ -314,6 +315,10 @@ function feedSubagentAggregator(aggregator, event) { * @param {Object} options - The options object. * @param {ServerResponse} options.res - The server response object. * @param {ContentAggregator} options.aggregateContent - Content aggregator function. + * @param {Array} [options.contentParts] - Aggregated message content parts. + * @param {Map} [options.stepMap] - Run steps keyed by step ID. + * @param {Map} [options.toolInputValidationErrors] + * Schema-validation errors keyed by tool-call ID at the execution error boundary. * @param {ToolEndCallback} options.toolEndCallback - Callback to use when tool ends. * @param {Array} options.collectedUsage - The list of collected usage metadata. * @param {string | null} [options.streamId] - The stream ID for resumable mode, or null for standard mode. @@ -330,6 +335,9 @@ function feedSubagentAggregator(aggregator, event) { function getDefaultHandlers({ res, aggregateContent, + contentParts = null, + stepMap = null, + toolInputValidationErrors = null, toolEndCallback, collectedUsage, collectedThoughtSignatures = null, @@ -441,7 +449,32 @@ function getDefaultHandlers({ * @param {GraphRunnableConfig['configurable']} [metadata] The runnable metadata. */ handle: async (event, data, metadata) => { + const toolCallId = data?.result?.tool_call?.id; + const validationError = + typeof toolCallId === 'string' ? toolInputValidationErrors?.get(toolCallId) : null; + const validationDetails = getToolInputValidationDetails(data?.result, validationError); + if (typeof toolCallId === 'string') { + toolInputValidationErrors?.delete(toolCallId); + } + if (validationDetails != null) { + if (data?.result?.tool_call != null) { + data.result.tool_call.inputValidationError = true; + } + logger.debug('[AgentToolValidation] Tool input rejected', { + ...validationDetails, + runId: metadata?.run_id, + conversationId: metadata?.thread_id, + agentId: metadata?.agent_id, + }); + } aggregateContent({ event, data }); + if (validationDetails != null) { + const runStep = stepMap?.get(data?.result?.id); + const toolCall = contentParts?.[runStep?.index]?.tool_call; + if (toolCall != null) { + toolCall.inputValidationError = true; + } + } if (data?.result != null) { await emitEvent(res, streamId, { event, data }); } else if (checkIfLastAgent(metadata?.last_agent_id, metadata?.langgraph_node)) { diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index a2ad4be54c..9bc1cf1fd5 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -143,6 +143,7 @@ class AgentClient extends BaseClient { subagentAggregatorsByToolCallId, contextUsageSink, usageEmitSink, + toolInputValidationErrors, ...clientOptions } = options; @@ -157,6 +158,11 @@ class AgentClient extends BaseClient { * persisted on `metadata.usage`. * @type {Array | undefined} */ this.usageEmitSink = usageEmitSink; + /** Schema-validation exceptions keyed by tool-call ID. The completion + * handler consumes these to distinguish execution failures from tool + * output that merely contains similar text. + * @type {Map | undefined} */ + this.toolInputValidationErrors = toolInputValidationErrors; /** @type {MessageContentComplex[]} */ this.contentParts = contentParts; /** @type {Array} */ @@ -1759,6 +1765,7 @@ class AgentClient extends BaseClient { // opts into the tool-approval wiring. Non-resumable callers (OpenAI-compat, Responses) // leave this off so an approval-gated tool can't pause where there's no resume path. hitlCapable: true, + toolInputValidationErrors: this.toolInputValidationErrors, // Mid-run steering: drain queued user messages at each tool-batch // boundary and inject them into graph state. The offset wrapper // shifts SDK content indices past any spliced steer parts. @@ -2089,6 +2096,7 @@ class AgentClient extends BaseClient { // The resumed run can pause AGAIN (another tool, a follow-up question), and this // controller owns that lifecycle, so it must keep the HITL wiring on the rebuilt run. hitlCapable: true, + toolInputValidationErrors: this.toolInputValidationErrors, // Steering stays live across a pause/resume cycle: steers queued while // the resumed segment runs drain at its tool-batch boundaries. steering: this.buildSteerWiring(streamId), diff --git a/api/server/services/Endpoints/agents/initialize.js b/api/server/services/Endpoints/agents/initialize.js index d94f48d6f6..2e4ad17c3a 100644 --- a/api/server/services/Endpoints/agents/initialize.js +++ b/api/server/services/Endpoints/agents/initialize.js @@ -135,7 +135,9 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { const collectedThoughtSignatures = {}; /** @type {ArtifactPromises} */ const artifactPromises = []; - const { contentParts, aggregateContent } = createContentAggregator(); + /** @type {Map} */ + const toolInputValidationErrors = new Map(); + const { contentParts, aggregateContent, stepMap } = createContentAggregator(); const toolEndCallback = createToolEndCallback({ req, res, artifactPromises, streamId }); /** Query accessible skill IDs once per run (shared across all agents). @@ -290,6 +292,9 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { const eventHandlers = getDefaultHandlers({ res, + contentParts, + stepMap, + toolInputValidationErrors, toolExecuteOptions, summarizationOptions, aggregateContent, @@ -1004,6 +1009,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { * them to persist the breakdown + usage rollup on the response message. */ contextUsageSink, usageEmitSink, + toolInputValidationErrors, }); if (streamId) { diff --git a/client/src/components/Chat/Messages/Content/AskUserQuestionCall.tsx b/client/src/components/Chat/Messages/Content/AskUserQuestionCall.tsx index 3c5f409a33..9b26d60a0c 100644 --- a/client/src/components/Chat/Messages/Content/AskUserQuestionCall.tsx +++ b/client/src/components/Chat/Messages/Content/AskUserQuestionCall.tsx @@ -1,4 +1,4 @@ -import { MessageCircleQuestion } from 'lucide-react'; +import { MessageCircleQuestion, TriangleAlert } from 'lucide-react'; import { getSubmittedAskAnswer, parseAskUserQuestionArgs } from '~/utils/approval'; import { useLocalize } from '~/hooks'; @@ -14,11 +14,13 @@ export default function AskUserQuestionCall({ output, toolCallId, isSubmitting = false, + failed = false, }: { args: string | Record | undefined; output: string; toolCallId?: string; isSubmitting?: boolean; + failed?: boolean; }) { const localize = useLocalize(); const question = parseAskUserQuestionArgs(args); @@ -29,7 +31,7 @@ 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; + const answered = effectiveOutput.length > 0 && !failed; /** * While the turn is live and unanswered, the INTERACTIVE card (rendered from @@ -39,10 +41,30 @@ export default function AskUserQuestionCall({ * so the record takes over immediately; an abandoned pause only shows its * "no answer" state after the turn is no longer submitting. */ - if (!answered && isSubmitting) { + if (!answered && !failed && isSubmitting) { return null; } + if (failed) { + return ( +
+
+
+ {question?.question != null && ( +

{question.question}

+ )} +

+ {localize('com_ui_question_failed_description')} +

+
+ ); + } + /** * Prefer the picked option's label over its wire value when they differ. * Multi-select answers are option values joined by ", " — map the segments diff --git a/client/src/components/Chat/Messages/Content/Part.tsx b/client/src/components/Chat/Messages/Content/Part.tsx index 18a8b472df..301d35532e 100644 --- a/client/src/components/Chat/Messages/Content/Part.tsx +++ b/client/src/components/Chat/Messages/Content/Part.tsx @@ -222,6 +222,7 @@ const Part = memo(function Part({ output={typeof toolCall.output === 'string' ? toolCall.output : ''} toolCallId={toolCall.id} isSubmitting={isSubmitting} + failed={'inputValidationError' in toolCall && toolCall.inputValidationError === true} /> ); } else if (toolCall.name === 'skill') { diff --git a/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionCall.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionCall.test.tsx new file mode 100644 index 0000000000..c206af61a0 --- /dev/null +++ b/client/src/components/Chat/Messages/Content/__tests__/AskUserQuestionCall.test.tsx @@ -0,0 +1,68 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import AskUserQuestionCall from '../AskUserQuestionCall'; + +const translations: Record = { + com_ui_asked: 'Asked', + com_ui_asking: 'Asking', + 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_unanswered: 'No answer was given', + com_ui_you_answered: 'You answered:', +}; + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => translations[key] ?? key, +})); + +jest.mock('~/utils/approval', () => ({ + getSubmittedAskAnswer: () => undefined, + parseAskUserQuestionArgs: (args: string | Record | undefined) => { + if (typeof args === 'string') { + return JSON.parse(args) as Record; + } + return args ?? null; + }, +})); + +describe('AskUserQuestionCall', () => { + const args = JSON.stringify({ + question: 'How would you like me to get the data?', + options: [{ label: 'Use public data', value: 'public' }], + }); + + test('renders a successful tool result as the user answer', () => { + render(); + + expect(screen.getByText('You answered:')).toBeInTheDocument(); + expect(screen.getByText('Use public data')).toBeInTheDocument(); + }); + + test('renders schema rejection as an internal question failure, not a user answer', () => { + const output = + 'Error processing tool: Received tool input did not match expected schema ' + + '✖ String must contain at most 120 character(s) → at options[0].label'; + + 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('You answered:')).not.toBeInTheDocument(); + expect(screen.queryByText(/Received tool input did not match expected schema/)).toBeNull(); + }); + + test('preserves a user answer that contains the complete schema error text', () => { + const output = + 'Error processing tool: Received tool input did not match expected schema ' + + '✖ String must contain at most 120 character(s) → at options[0].label'; + + render(); + + expect(screen.getByText('You answered:')).toBeInTheDocument(); + expect(screen.getByText(output)).toBeInTheDocument(); + expect(screen.queryByText("Question wasn't shown")).not.toBeInTheDocument(); + }); +}); diff --git a/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts b/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts index a3f153d423..f9c438c03c 100644 --- a/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts +++ b/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts @@ -1422,6 +1422,7 @@ describe('useStepHandler', () => { name: 'test_tool', args: '{}', output: 'Tool result output', + inputValidationError: true as const, type: ToolCallTypes.TOOL_CALL, }, }, @@ -1445,6 +1446,7 @@ describe('useStepHandler', () => { ); expect(toolCallContent?.tool_call?.output).toBe('Tool result output'); expect(toolCallContent?.tool_call?.progress).toBe(1); + expect(toolCallContent?.tool_call?.inputValidationError).toBe(true); }); it('signals skill authoring when a completed create_file call targets a skill path', () => { diff --git a/client/src/hooks/SSE/useStepHandler.ts b/client/src/hooks/SSE/useStepHandler.ts index 411d7e26d7..68106652db 100644 --- a/client/src/hooks/SSE/useStepHandler.ts +++ b/client/src/hooks/SSE/useStepHandler.ts @@ -490,6 +490,12 @@ export default function useStepHandler({ if (finalUpdate) { newToolCall.progress = 1; newToolCall.output = contentPart.tool_call.output; + if ( + 'inputValidationError' in contentPart.tool_call && + contentPart.tool_call.inputValidationError === true + ) { + Object.assign(newToolCall, { inputValidationError: true }); + } } updatedContent[index] = { diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 0a9730b33d..9ab54e1432 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1564,6 +1564,8 @@ "com_ui_provider_api_keys_description": "Manage API keys for endpoints configured to use a user-provided key.", "com_ui_provider_api_keys_not_set": "No key set", "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_unanswered": "No answer was given", "com_ui_queue": "Queue", "com_ui_queue_send": "Queue message for after the response", diff --git a/packages/api/src/agents/hitl/askUserQuestionTool.spec.ts b/packages/api/src/agents/hitl/askUserQuestionTool.spec.ts index 0d7a463e4b..02cf16b092 100644 --- a/packages/api/src/agents/hitl/askUserQuestionTool.spec.ts +++ b/packages/api/src/agents/hitl/askUserQuestionTool.spec.ts @@ -71,6 +71,26 @@ describe('ask_user_question tool contract', () => { ).toBe(false); }); + test('rejects an overlong option label with guidance and records the real tool call', async () => { + const validationErrors = new Map(); + await expect( + createAskUserQuestionTool(validationErrors).invoke({ + id: 'tool-1', + name: ASK_USER_QUESTION_TOOL_NAME, + type: 'tool_call', + args: { + question: 'How should I get the data?', + options: [{ label: 'x'.repeat(161), value: 'public-data' }], + }, + }), + ).rejects.toThrow( + 'Option labels must be 120 characters or fewer. Shorten the label and retry.', + ); + expect(validationErrors).toEqual( + new Map([['tool-1', { fieldPath: 'options[0].label', isLengthLimit: true }]]), + ); + }); + test('accepts multiSelect as an optional boolean and rejects other types', () => { const input = { question: 'Which apply?', @@ -120,6 +140,22 @@ describe('ask_user_question tool contract', () => { 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(120); + 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', () => { @@ -127,6 +163,10 @@ describe('ask_user_question tool contract', () => { expect(instance.description).toBe(AskUserQuestionToolDefinition.description); expect(instance.description).toContain('exactly ONE question per turn'); expect(instance.description).toContain('NEVER call this tool in parallel'); + expect(instance.description).toContain('option label within 120 characters'); + expect( + AskUserQuestionToolDefinition.schema.properties.options.items.properties.label.description, + ).toContain('Maximum 120 characters'); }); }); }); diff --git a/packages/api/src/agents/hitl/askUserQuestionTool.ts b/packages/api/src/agents/hitl/askUserQuestionTool.ts index acf198b965..10f5eae071 100644 --- a/packages/api/src/agents/hitl/askUserQuestionTool.ts +++ b/packages/api/src/agents/hitl/askUserQuestionTool.ts @@ -2,6 +2,8 @@ import { z } from 'zod'; import { askUserQuestion } from '@librechat/agents'; import { tool } from '@librechat/agents/langchain/tools'; import type { DynamicStructuredTool } from '@librechat/agents/langchain/tools'; +import type { ToolInputValidationError } from '../toolValidation'; +import { recordToolInputValidationError } from '../toolValidation'; /** * Tool name. Deliberately identical to the SDK's interrupt discriminator @@ -22,6 +24,13 @@ const OPTION_LABEL_MAX = 120; const OPTION_VALUE_MAX = 500; const OPTIONS_MAX = 12; +const OPTION_LABEL_DESCRIPTION = + `Short choice shown to the user. Maximum ${OPTION_LABEL_MAX} characters; ` + + 'put supporting context in the question description.'; +const OPTION_LABEL_MAX_ERROR = + `Option labels must be ${OPTION_LABEL_MAX} characters or fewer. ` + + '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", @@ -29,7 +38,9 @@ const ASK_USER_QUESTION_DESCRIPTION = [ 'turn, and NEVER call this tool in parallel with any other tool call. 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 ", "). The user can always type a free-form answer instead — so do NOT include a', + `by ", "). Keep every option label within ${OPTION_LABEL_MAX} characters and put supporting`, + 'context in the question description. The user can always type a free-form answer instead — so', + 'do NOT include a', "catch-all option like 'Other' or 'Something else': the answer UI always offers free-form", 'input on its own.', ].join(' '); @@ -66,8 +77,8 @@ export const askUserQuestionToolSchema: z.ZodObject< label: z .string() .min(1) - .max(OPTION_LABEL_MAX) - .describe('Human-readable choice shown to the user.'), + .max(OPTION_LABEL_MAX, OPTION_LABEL_MAX_ERROR) + .describe(OPTION_LABEL_DESCRIPTION), value: z .string() .min(1) @@ -150,7 +161,7 @@ export const AskUserQuestionToolDefinition: AskUserQuestionToolDefinitionShape = type: 'string', minLength: 1, maxLength: OPTION_LABEL_MAX, - description: 'Human-readable choice shown to the user.', + description: OPTION_LABEL_DESCRIPTION, }, value: { type: 'string', @@ -190,10 +201,10 @@ 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. */ -export function createAskUserQuestionTool(): DynamicStructuredTool< - typeof askUserQuestionToolSchema -> { - return tool( +export function createAskUserQuestionTool( + validationErrorsByToolCallId?: Map, +): DynamicStructuredTool { + const askTool = tool( async (input: AskUserQuestionToolInput) => { const { answer } = askUserQuestion(input); return answer; @@ -204,4 +215,21 @@ export function createAskUserQuestionTool(): DynamicStructuredTool< schema: askUserQuestionToolSchema, }, ); + + /** LangChain validates the schema before starting tool callbacks. Catch at + * 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) => { + try { + return await invoke(input, config); + } catch (error) { + const toolCallId = + typeof input === 'object' && input != null && 'id' in input ? input.id : undefined; + recordToolInputValidationError(validationErrorsByToolCallId, error, toolCallId); + throw error; + } + }; + + return askTool; } diff --git a/packages/api/src/agents/index.ts b/packages/api/src/agents/index.ts index cdd28d2218..66e9ab429a 100644 --- a/packages/api/src/agents/index.ts +++ b/packages/api/src/agents/index.ts @@ -33,3 +33,4 @@ export * from './added'; export * from './load'; export * from './hitl'; export * from './steering'; +export * from './toolValidation'; diff --git a/packages/api/src/agents/run.ts b/packages/api/src/agents/run.ts index e55cc4845d..f063fecdf0 100644 --- a/packages/api/src/agents/run.ts +++ b/packages/api/src/agents/run.ts @@ -34,6 +34,7 @@ import type { } from 'librechat-data-provider'; import type { BaseMessage } from '@librechat/agents/langchain/messages'; import type { AppConfig, IUser } from '@librechat/data-schemas'; +import type { ToolInputValidationError } from '~/agents/toolValidation'; import type { SubagentUsageEvent } from '~/agents/usage'; import type * as t from '~/types'; import { @@ -1034,6 +1035,7 @@ export async function createRun({ subagentUsageSink, steering, hitlCapable = false, + toolInputValidationErrors, streaming = true, streamUsage = true, }: { @@ -1099,6 +1101,8 @@ export async function createRun({ * final response / `[DONE]` with the tool call left unresolved). */ hitlCapable?: boolean; + /** Request-scoped tool input failures consumed by the completion handler. */ + toolInputValidationErrors?: Map; } & Pick< RunConfig, 'tokenCounter' | 'customHandlers' | 'indexTokenCountMap' | 'initialSessions' @@ -1282,7 +1286,9 @@ export async function createRun({ toolRegistry.delete(ASK_USER_QUESTION_TOOL_NAME); } if (hitlCapable && !isSubagent && !askToolAdminDisabled) { - askGraphTools = [createAskUserQuestionTool() as unknown as GenericTool]; + askGraphTools = [ + createAskUserQuestionTool(toolInputValidationErrors) as unknown as GenericTool, + ]; } } diff --git a/packages/api/src/agents/toolValidation.spec.ts b/packages/api/src/agents/toolValidation.spec.ts new file mode 100644 index 0000000000..c079da54cd --- /dev/null +++ b/packages/api/src/agents/toolValidation.spec.ts @@ -0,0 +1,76 @@ +import { + getToolInputValidationDetails, + parseToolInputValidationError, + recordToolInputValidationError, +} from './toolValidation'; + +describe('getToolInputValidationDetails', () => { + test('classifies an overlong ask_user_question option label without returning raw content', () => { + const validationError = parseToolInputValidationError( + new Error( + 'Received tool input did not match expected schema\n' + + '✖ Option labels must be 120 characters or fewer. Shorten the label and retry.\n' + + ' → at options[0].label', + ), + ); + const details = getToolInputValidationDetails( + { + tool_call: { + name: 'ask_user_question', + }, + }, + validationError, + ); + + expect(details).toEqual({ + toolName: 'ask_user_question', + reason: 'option_label_too_long', + fieldPath: 'options[0].label', + }); + expect(JSON.stringify(details)).not.toContain('Shorten the label'); + }); + + test('classifies other schema failures without requiring a field path', () => { + expect( + getToolInputValidationDetails( + { + tool_call: { + name: 'search', + }, + }, + parseToolInputValidationError( + new Error('Received tool input did not match expected schema'), + ), + ), + ).toEqual({ toolName: 'search', reason: 'invalid_tool_input' }); + }); + + test('ignores matching successful tool output without an error signal', () => { + expect( + getToolInputValidationDetails( + { + tool_call: { + name: 'ask_user_question', + output: 'Received tool input did not match expected schema → at options[0].label', + }, + }, + null, + ), + ).toBeNull(); + }); + + test('records validation failures by tool call id only from thrown errors', () => { + const errorsByToolCallId = new Map(); + + recordToolInputValidationError( + errorsByToolCallId, + new Error('Received tool input did not match expected schema → at question'), + 'tool-1', + ); + recordToolInputValidationError(errorsByToolCallId, 'successful user response', 'tool-2'); + + expect(errorsByToolCallId).toEqual( + new Map([['tool-1', { fieldPath: 'question', isLengthLimit: false }]]), + ); + }); +}); diff --git a/packages/api/src/agents/toolValidation.ts b/packages/api/src/agents/toolValidation.ts new file mode 100644 index 0000000000..7445cc0fec --- /dev/null +++ b/packages/api/src/agents/toolValidation.ts @@ -0,0 +1,91 @@ +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 OPTION_LABEL_LIMIT_PATTERN = /(?:at most \d+|\d+ characters or fewer)/i; + +interface CompletedToolCall { + tool_call?: { + name?: unknown; + output?: unknown; + }; +} + +export type ToolInputValidationReason = 'invalid_tool_input' | 'option_label_too_long'; + +export interface ToolInputValidationError { + fieldPath?: string; + isLengthLimit: boolean; +} + +export interface ToolInputValidationDetails { + toolName: string; + reason: ToolInputValidationReason; + fieldPath?: string; +} + +function getErrorMessage(error: unknown): string | null { + if (error instanceof Error) { + return error.message; + } + return typeof error === 'string' ? error : null; +} + +/** + * Parse a schema-validation exception at the tool error boundary. Calling this + * with the thrown error, rather than completed tool output, prevents successful + * user-authored text from being mistaken for an execution failure. + */ +export function parseToolInputValidationError(error: unknown): ToolInputValidationError | null { + const message = getErrorMessage(error); + if (message == null || !message.includes(TOOL_INPUT_SCHEMA_ERROR)) { + return null; + } + + const fieldPath = message.match(SCHEMA_ERROR_PATH_PATTERN)?.[1]; + return { + isLengthLimit: OPTION_LABEL_LIMIT_PATTERN.test(message), + ...(fieldPath != null ? { fieldPath } : {}), + }; +} + +export function recordToolInputValidationError( + errorsByToolCallId: Map | null | undefined, + error: unknown, + toolCallId: unknown, +): void { + if (typeof toolCallId !== 'string' || toolCallId.length === 0) { + return; + } + const validationError = parseToolInputValidationError(error); + if (validationError != null) { + errorsByToolCallId?.set(toolCallId, validationError); + } +} + +/** + * Reduce a tool input validation failure to privacy-safe structured fields for + * observability. Tool arguments and the raw validation message can contain + * user/model content, so callers should log only the returned details. + */ +export function getToolInputValidationDetails( + result: CompletedToolCall | null | undefined, + validationError: ToolInputValidationError | null | undefined, +): ToolInputValidationDetails | null { + const toolName = result?.tool_call?.name; + if (typeof toolName !== 'string' || validationError == null) { + return null; + } + + const { fieldPath } = validationError; + const optionLabelTooLong = + toolName === 'ask_user_question' && + fieldPath != null && + ASK_OPTION_LABEL_PATH_PATTERN.test(fieldPath) && + validationError.isLengthLimit; + + return { + toolName, + reason: optionLabelTooLong ? 'option_label_too_long' : 'invalid_tool_input', + ...(fieldPath != null ? { fieldPath } : {}), + }; +} diff --git a/packages/data-provider/src/types/agents.ts b/packages/data-provider/src/types/agents.ts index 97d51b9c7c..f14d754b75 100644 --- a/packages/data-provider/src/types/agents.ts +++ b/packages/data-provider/src/types/agents.ts @@ -80,6 +80,8 @@ export namespace Agents { id?: string; /** If provided, the output of the tool call */ output?: string; + /** The tool call was rejected before execution because its input failed schema validation. */ + inputValidationError?: true; /** Auth URL */ auth?: string; /** Expiration time */