From 74c361ce014fa83a19a4bb295de26a1545a6b76d Mon Sep 17 00:00:00 2001 From: L-1ngg <2157878170@qq.com> Date: Fri, 2 Oct 2026 16:07:09 +0800 Subject: [PATCH] fix(openai-base): preserve malformed tool arguments for error recovery --- ...lformed-chat-completions-tool-arguments.md | 5 + docs/tools/server-tools.md | 2 +- .../src/adapters/chat-completions-text.ts | 18 +-- .../tests/chat-completions-text.test.ts | 113 ++++++++++-------- testing/e2e/src/routeTree.gen.ts | 22 ++++ .../api.openai-malformed-tool-arguments.ts | 99 +++++++++++++++ .../openai-malformed-tool-arguments.spec.ts | 25 ++++ 7 files changed, 220 insertions(+), 64 deletions(-) create mode 100644 .changeset/fix-malformed-chat-completions-tool-arguments.md create mode 100644 testing/e2e/src/routes/api.openai-malformed-tool-arguments.ts create mode 100644 testing/e2e/tests/openai-malformed-tool-arguments.spec.ts diff --git a/.changeset/fix-malformed-chat-completions-tool-arguments.md b/.changeset/fix-malformed-chat-completions-tool-arguments.md new file mode 100644 index 0000000000..8177744062 --- /dev/null +++ b/.changeset/fix-malformed-chat-completions-tool-arguments.md @@ -0,0 +1,5 @@ +--- +'@tanstack/openai-base': patch +--- + +Preserve malformed Chat Completions tool arguments so they return a tool error without executing the tool with an empty object. diff --git a/docs/tools/server-tools.md b/docs/tools/server-tools.md index 3835cc1fca..fefa1e5ec2 100644 --- a/docs/tools/server-tools.md +++ b/docs/tools/server-tools.md @@ -331,7 +331,7 @@ const getUserData = getUserDataDef.server(async ({ userId }) => { **Throwing vs. returning an error:** if your `.server()` function throws, the SDK catches it and surfaces it as a tool-result *error* (the model sees the failure but you lose control over the message). Returning a structured `{ error }` shape keeps the model in control of how to recover and is usually preferable. Either way, when an `outputSchema` is defined the returned value is validated against it (Zod) before being added to the conversation — so include the `error` field in your `outputSchema` if you return it. -Malformed JSON arguments and Standard Schema input validation failures are also surfaced as tool-result errors. The tool implementation is not called, and the agent loop can return the error to the model so it can repair the tool call. +Malformed JSON arguments and Standard Schema input validation failures are also surfaced as tool-result errors. The tool implementation is not called, and the agent loop can return the error to the model so it can repair the tool call. This also applies to Chat Completions tools with no required input fields. ## Using JSON Schema diff --git a/packages/openai-base/src/adapters/chat-completions-text.ts b/packages/openai-base/src/adapters/chat-completions-text.ts index ab6c48093a..14b7c58aee 100644 --- a/packages/openai-base/src/adapters/chat-completions-text.ts +++ b/packages/openai-base/src/adapters/chat-completions-text.ts @@ -1023,12 +1023,9 @@ export abstract class OpenAIBaseChatCompletionsTextAdapter< // upstream never sent both id and name. if (!toolCall.started) continue - // Parse arguments for TOOL_CALL_END. Surface parse failures via - // the logger so a model emitting malformed JSON for tool args - // is debuggable instead of silently invoking the tool with {}. - // Non-object JSON (e.g. a bare string or number) is also coerced - // to {} so downstream tool execution doesn't receive a primitive - // input, mirroring the Responses adapter's guard. + // Leave malformed arguments for the core's tool-error handling. + // An undefined input preserves the accumulated argument string. + // Non-object JSON still normalizes to {}. let parsedInput: unknown = {} if (toolCall.arguments) { try { @@ -1051,7 +1048,7 @@ export abstract class OpenAIBaseChatCompletionsTextAdapter< rawArguments: toolCall.arguments, }, ) - parsedInput = {} + parsedInput = undefined } } @@ -1112,10 +1109,7 @@ export abstract class OpenAIBaseChatCompletionsTextAdapter< parsed && typeof parsed === 'object' ? parsed : {}, ) } catch (parseError) { - // Mirror the finish_reason path's logger call — a truncated - // stream emitting malformed tool-call JSON would otherwise - // silently invoke the tool with `{}`, the exact failure the - // finish_reason logger was added to prevent. + // Preserve malformed arguments for the core, as on finish_reason. options.logger.errors( `${this.name}.processStreamChunks tool-args JSON parse failed (drain)`, { @@ -1129,7 +1123,7 @@ export abstract class OpenAIBaseChatCompletionsTextAdapter< rawArguments: toolCall.arguments, }, ) - parsedInput = {} + parsedInput = undefined } } yield { diff --git a/packages/openai-base/tests/chat-completions-text.test.ts b/packages/openai-base/tests/chat-completions-text.test.ts index 567e445c93..1fe050309a 100644 --- a/packages/openai-base/tests/chat-completions-text.test.ts +++ b/packages/openai-base/tests/chat-completions-text.test.ts @@ -1194,62 +1194,73 @@ describe('OpenAIBaseChatCompletionsTextAdapter', () => { }) }) - describe('drain-path tool args error handling', () => { - it('logs malformed JSON tool args via the logger when the stream ends without finish_reason', async () => { - // Simulates a truncated stream: tool call starts and accumulates - // malformed JSON, but no finish_reason chunk ever arrives. The drain - // block must still surface the parse failure rather than swallowing it. - const streamChunks = [ - { - id: 'chatcmpl-drain', - model: 'test-model', - choices: [ - { - delta: { - tool_calls: [ - { - index: 0, - id: 'call_drain', - type: 'function', - function: { - name: 'lookup_weather', - arguments: '{"location":', // truncated — invalid JSON + describe('malformed tool arguments', () => { + it.each([true, false])( + 'preserves malformed arguments for tool-error handling (finish_reason=%s)', + async (withFinishReason) => { + const streamChunks = [ + { + id: 'chatcmpl-drain', + model: 'test-model', + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + id: 'call_drain', + type: 'function', + function: { + name: 'lookup_weather', + arguments: '{"location":', // truncated — invalid JSON + }, }, - }, - ], + ], + }, + finish_reason: withFinishReason ? 'tool_calls' : null, }, - finish_reason: null, - }, - ], - }, - ] + ], + }, + ] - setupMockSdkClient(streamChunks) - const errorsSpy = vi.spyOn(testLogger, 'errors') - const adapter = new TestChatCompletionsAdapter(testConfig, 'test-model') + setupMockSdkClient(streamChunks) + const errorsSpy = vi.spyOn(testLogger, 'errors') + const adapter = new TestChatCompletionsAdapter(testConfig, 'test-model') + const chunks: Array = [] - try { - for await (const _ of adapter.chatStream({ - logger: testLogger, - model: 'test-model', - messages: [{ role: 'user', content: 'Weather?' }], - tools: [weatherTool], - })) { - // consume - } + try { + for await (const chunk of adapter.chatStream({ + logger: testLogger, + model: 'test-model', + messages: [{ role: 'user', content: 'Weather?' }], + tools: [weatherTool], + })) { + chunks.push(chunk) + } - const drainCall = errorsSpy.mock.calls.find((c) => - String(c[0]).includes('(drain)'), - ) - expect(drainCall).toBeDefined() - const ctx = drainCall![1] as Record - expect(ctx['toolCallId']).toBe('call_drain') - expect(ctx['toolName']).toBe('lookup_weather') - expect(ctx['rawArguments']).toBe('{"location":') - } finally { - errorsSpy.mockRestore() - } - }) + const toolEnd = chunks.find((chunk) => chunk.type === 'TOOL_CALL_END') + expect(toolEnd).toBeDefined() + expect(toolEnd?.input).toBeUndefined() + expect( + chunks.find((chunk) => chunk.type === 'TOOL_CALL_ARGS'), + ).toMatchObject({ + toolCallId: 'call_drain', + delta: '{"location":', + }) + + const parseError = errorsSpy.mock.calls.find((c) => + String(c[0]).includes('tool-args JSON parse failed'), + ) + expect(parseError).toBeDefined() + const ctx = parseError![1] as Record + expect(ctx['toolCallId']).toBe('call_drain') + expect(ctx['toolName']).toBe('lookup_weather') + expect(ctx['rawArguments']).toBe('{"location":') + } finally { + errorsSpy.mockRestore() + } + }, + ) }) describe('subclassing', () => { diff --git a/testing/e2e/src/routeTree.gen.ts b/testing/e2e/src/routeTree.gen.ts index 439b88d192..c4dc407798 100644 --- a/testing/e2e/src/routeTree.gen.ts +++ b/testing/e2e/src/routeTree.gen.ts @@ -74,6 +74,7 @@ import { Route as ApiOpenrouterCostRouteImport } from './routes/api.openrouter-c import { Route as ApiOpenaiUsageDetailsRouteImport } from './routes/api.openai-usage-details' import { Route as ApiOpenaiStrictToolNullWireRouteImport } from './routes/api.openai-strict-tool-null-wire' import { Route as ApiOpenaiShellSkillsWireRouteImport } from './routes/api.openai-shell-skills-wire' +import { Route as ApiOpenaiMalformedToolArgumentsRouteImport } from './routes/api.openai-malformed-tool-arguments' import { Route as ApiOpenaiImage25ModelsRouteImport } from './routes/api.openai-image-2-5-models' import { Route as ApiOpenaiCompletedResponseTextRouteImport } from './routes/api.openai-completed-response-text' import { Route as ApiNonStreamingRunErrorRouteImport } from './routes/api.non-streaming-run-error' @@ -474,6 +475,12 @@ const ApiOpenaiShellSkillsWireRoute = path: '/api/openai-shell-skills-wire', getParentRoute: () => rootRouteImport, } as any) +const ApiOpenaiMalformedToolArgumentsRoute = + ApiOpenaiMalformedToolArgumentsRouteImport.update({ + id: '/api/openai-malformed-tool-arguments', + path: '/api/openai-malformed-tool-arguments', + getParentRoute: () => rootRouteImport, + } as any) const ApiOpenaiImage25ModelsRoute = ApiOpenaiImage25ModelsRouteImport.update({ id: '/api/openai-image-2-5-models', path: '/api/openai-image-2-5-models', @@ -868,6 +875,7 @@ export interface FileRoutesByFullPath { '/api/non-streaming-run-error': typeof ApiNonStreamingRunErrorRoute '/api/openai-completed-response-text': typeof ApiOpenaiCompletedResponseTextRoute '/api/openai-image-2-5-models': typeof ApiOpenaiImage25ModelsRoute + '/api/openai-malformed-tool-arguments': typeof ApiOpenaiMalformedToolArgumentsRoute '/api/openai-shell-skills-wire': typeof ApiOpenaiShellSkillsWireRoute '/api/openai-strict-tool-null-wire': typeof ApiOpenaiStrictToolNullWireRoute '/api/openai-usage-details': typeof ApiOpenaiUsageDetailsRoute @@ -994,6 +1002,7 @@ export interface FileRoutesByTo { '/api/non-streaming-run-error': typeof ApiNonStreamingRunErrorRoute '/api/openai-completed-response-text': typeof ApiOpenaiCompletedResponseTextRoute '/api/openai-image-2-5-models': typeof ApiOpenaiImage25ModelsRoute + '/api/openai-malformed-tool-arguments': typeof ApiOpenaiMalformedToolArgumentsRoute '/api/openai-shell-skills-wire': typeof ApiOpenaiShellSkillsWireRoute '/api/openai-strict-tool-null-wire': typeof ApiOpenaiStrictToolNullWireRoute '/api/openai-usage-details': typeof ApiOpenaiUsageDetailsRoute @@ -1121,6 +1130,7 @@ export interface FileRoutesById { '/api/non-streaming-run-error': typeof ApiNonStreamingRunErrorRoute '/api/openai-completed-response-text': typeof ApiOpenaiCompletedResponseTextRoute '/api/openai-image-2-5-models': typeof ApiOpenaiImage25ModelsRoute + '/api/openai-malformed-tool-arguments': typeof ApiOpenaiMalformedToolArgumentsRoute '/api/openai-shell-skills-wire': typeof ApiOpenaiShellSkillsWireRoute '/api/openai-strict-tool-null-wire': typeof ApiOpenaiStrictToolNullWireRoute '/api/openai-usage-details': typeof ApiOpenaiUsageDetailsRoute @@ -1249,6 +1259,7 @@ export interface FileRouteTypes { | '/api/non-streaming-run-error' | '/api/openai-completed-response-text' | '/api/openai-image-2-5-models' + | '/api/openai-malformed-tool-arguments' | '/api/openai-shell-skills-wire' | '/api/openai-strict-tool-null-wire' | '/api/openai-usage-details' @@ -1375,6 +1386,7 @@ export interface FileRouteTypes { | '/api/non-streaming-run-error' | '/api/openai-completed-response-text' | '/api/openai-image-2-5-models' + | '/api/openai-malformed-tool-arguments' | '/api/openai-shell-skills-wire' | '/api/openai-strict-tool-null-wire' | '/api/openai-usage-details' @@ -1501,6 +1513,7 @@ export interface FileRouteTypes { | '/api/non-streaming-run-error' | '/api/openai-completed-response-text' | '/api/openai-image-2-5-models' + | '/api/openai-malformed-tool-arguments' | '/api/openai-shell-skills-wire' | '/api/openai-strict-tool-null-wire' | '/api/openai-usage-details' @@ -1628,6 +1641,7 @@ export interface RootRouteChildren { ApiNonStreamingRunErrorRoute: typeof ApiNonStreamingRunErrorRoute ApiOpenaiCompletedResponseTextRoute: typeof ApiOpenaiCompletedResponseTextRoute ApiOpenaiImage25ModelsRoute: typeof ApiOpenaiImage25ModelsRoute + ApiOpenaiMalformedToolArgumentsRoute: typeof ApiOpenaiMalformedToolArgumentsRoute ApiOpenaiShellSkillsWireRoute: typeof ApiOpenaiShellSkillsWireRoute ApiOpenaiStrictToolNullWireRoute: typeof ApiOpenaiStrictToolNullWireRoute ApiOpenaiUsageDetailsRoute: typeof ApiOpenaiUsageDetailsRoute @@ -2121,6 +2135,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof ApiOpenaiShellSkillsWireRouteImport parentRoute: typeof rootRouteImport } + '/api/openai-malformed-tool-arguments': { + id: '/api/openai-malformed-tool-arguments' + path: '/api/openai-malformed-tool-arguments' + fullPath: '/api/openai-malformed-tool-arguments' + preLoaderRoute: typeof ApiOpenaiMalformedToolArgumentsRouteImport + parentRoute: typeof rootRouteImport + } '/api/openai-image-2-5-models': { id: '/api/openai-image-2-5-models' path: '/api/openai-image-2-5-models' @@ -2682,6 +2703,7 @@ const rootRouteChildren: RootRouteChildren = { ApiNonStreamingRunErrorRoute: ApiNonStreamingRunErrorRoute, ApiOpenaiCompletedResponseTextRoute: ApiOpenaiCompletedResponseTextRoute, ApiOpenaiImage25ModelsRoute: ApiOpenaiImage25ModelsRoute, + ApiOpenaiMalformedToolArgumentsRoute: ApiOpenaiMalformedToolArgumentsRoute, ApiOpenaiShellSkillsWireRoute: ApiOpenaiShellSkillsWireRoute, ApiOpenaiStrictToolNullWireRoute: ApiOpenaiStrictToolNullWireRoute, ApiOpenaiUsageDetailsRoute: ApiOpenaiUsageDetailsRoute, diff --git a/testing/e2e/src/routes/api.openai-malformed-tool-arguments.ts b/testing/e2e/src/routes/api.openai-malformed-tool-arguments.ts new file mode 100644 index 0000000000..c49966c7aa --- /dev/null +++ b/testing/e2e/src/routes/api.openai-malformed-tool-arguments.ts @@ -0,0 +1,99 @@ +import { createFileRoute } from '@tanstack/react-router' +import { chat, maxIterations, toolDefinition } from '@tanstack/ai' +import { createOpenaiChatCompletions } from '@tanstack/ai-openai' +import { z } from 'zod' + +export const Route = createFileRoute('/api/openai-malformed-tool-arguments')({ + server: { + handlers: { + POST: async ({ request }) => { + const withFinishReason = + new URL(request.url).searchParams.get('terminal') !== 'false' + const requests: Array = [] + const executedInputs: Array = [] + const text: Array = [] + const runErrors: Array = [] + const tool = toolDefinition({ + name: 'local_action', + description: 'Record input without side effects', + inputSchema: z.object({ path: z.string().optional() }), + }).server((input) => { + executedInputs.push(input) + return 'tool ran' + }) + const adapter = createOpenaiChatCompletions( + 'gpt-4o', + 'sk-e2e-dummy-key', + { + maxRetries: 0, + fetch: async (input, init) => { + const providerRequest = + input instanceof Request ? input : new Request(input, init) + requests.push(await providerRequest.json()) + const chunk = ( + delta: Record, + finish_reason: 'tool_calls' | 'stop' | null = null, + ) => ({ + id: 'completion-malformed-tool', + object: 'chat.completion.chunk', + created: 1, + model: 'gpt-4o', + choices: [{ index: 0, delta, finish_reason }], + }) + const events = + requests.length === 1 + ? [ + chunk({ + tool_calls: [ + { + index: 0, + id: 'call-malformed', + type: 'function', + function: { + name: 'local_action', + arguments: '{"path":', + }, + }, + ], + }), + ] + : [ + chunk( + { content: 'Recovered from malformed arguments.' }, + 'stop', + ), + ] + if (requests.length === 1 && withFinishReason) { + events.push(chunk({}, 'tool_calls')) + } + return new Response( + events + .map((event) => `data: ${JSON.stringify(event)}\n\n`) + .join(''), + { headers: { 'Content-Type': 'text/event-stream' } }, + ) + }, + }, + ) + + for await (const chunk of chat({ + adapter, + messages: [{ role: 'user', content: 'Run the local action' }], + tools: [tool], + agentLoopStrategy: maxIterations(2), + debug: false, + })) { + if (chunk.type === 'TEXT_MESSAGE_CONTENT') text.push(chunk.delta) + if (chunk.type === 'RUN_ERROR') runErrors.push(chunk.message) + } + + return Response.json({ + requests, + executedInputs, + text: text.join(''), + runErrors, + }) + }, + }, + }, +}) diff --git a/testing/e2e/tests/openai-malformed-tool-arguments.spec.ts b/testing/e2e/tests/openai-malformed-tool-arguments.spec.ts new file mode 100644 index 0000000000..ee3bb28286 --- /dev/null +++ b/testing/e2e/tests/openai-malformed-tool-arguments.spec.ts @@ -0,0 +1,25 @@ +import { expect, test } from './fixtures' + +for (const withFinishReason of [true, false]) { + test(`Chat Completions rejects malformed tool arguments and continues (finish_reason=${withFinishReason})`, async ({ + request, + }) => { + const response = await request.post( + `/api/openai-malformed-tool-arguments?terminal=${withFinishReason}`, + ) + expect(response.ok()).toBe(true) + const result = await response.json() + + expect(result.executedInputs).toEqual([]) + expect(result.requests).toHaveLength(2) + expect(result.requests[1].messages).toContainEqual({ + role: 'tool', + tool_call_id: 'call-malformed', + content: JSON.stringify({ + error: 'Failed to parse tool arguments as JSON: {"path":', + }), + }) + expect(result.runErrors).toEqual([]) + expect(result.text).toBe('Recovered from malformed arguments.') + }) +}