diff --git a/.changeset/fix-tool-name-undefined-after-approval.md b/.changeset/fix-tool-name-undefined-after-approval.md new file mode 100644 index 000000000..6c19e7ec5 --- /dev/null +++ b/.changeset/fix-tool-name-undefined-after-approval.md @@ -0,0 +1,9 @@ +--- +'@tanstack/ai': patch +--- + +Fix `tool_use.name: String should have at least 1 character` 400 from Anthropic when sending a follow-up message after approving a tool that needs approval (issue #532). + +The agent loop's continuation re-emit of `TOOL_CALL_START` after a server-side post-approval execution now includes the AG-UI spec field `toolCallName` alongside the deprecated `toolName` alias, so the client's `StreamProcessor` records a tool-call part with a defined `name` instead of `undefined`. As a defensive measure, `StreamProcessor` also accepts the deprecated `toolName` field as a fallback when `toolCallName` is missing. + +The post-approval execution also now replaces the `pendingExecution: true` placeholder tool message in the agent loop's message history with the real tool result, instead of appending a duplicate. This prevents the Anthropic adapter's `tool_result` de-dup (which keeps the first match) from discarding the real result, so the model sees the actual tool output during the post-approval streaming response. diff --git a/packages/typescript/ai/src/activities/chat/index.ts b/packages/typescript/ai/src/activities/chat/index.ts index 27564d49e..8ce192a4f 100644 --- a/packages/typescript/ai/src/activities/chat/index.ts +++ b/packages/typescript/ai/src/activities/chat/index.ts @@ -1250,6 +1250,7 @@ class TextEngine< timestamp: Date.now(), model: finishEvent.model, toolCallId: result.toolCallId, + toolCallName: result.toolName, toolName: result.toolName, } as StreamChunk) @@ -1285,14 +1286,39 @@ class TextEngine< role: 'tool', } as StreamChunk) - this.messages = [ - ...this.messages, - { - role: 'tool', - content, - toolCallId: result.toolCallId, - }, - ] + // If a placeholder tool message exists for this toolCallId (created by + // uiMessageToModelMessages for an approval-responded part with no + // output yet), replace it with the real result. Otherwise the LLM sees + // both messages — and since the Anthropic adapter dedupes tool_result + // blocks by tool_use_id keeping the first match, the placeholder wins + // and the real result is dropped (see issue #532). + const placeholderIdx = this.messages.findIndex((m) => { + if (m.role !== 'tool' || m.toolCallId !== result.toolCallId) { + return false + } + if (typeof m.content !== 'string') return false + try { + return JSON.parse(m.content)?.pendingExecution === true + } catch { + return false + } + }) + + const newToolMessage: ModelMessage = { + role: 'tool', + content, + toolCallId: result.toolCallId, + } + + if (placeholderIdx >= 0) { + this.messages = [ + ...this.messages.slice(0, placeholderIdx), + newToolMessage, + ...this.messages.slice(placeholderIdx + 1), + ] + } else { + this.messages = [...this.messages, newToolMessage] + } } return chunks diff --git a/packages/typescript/ai/src/activities/chat/stream/processor.ts b/packages/typescript/ai/src/activities/chat/stream/processor.ts index 34daa549d..335d457bc 100644 --- a/packages/typescript/ai/src/activities/chat/stream/processor.ts +++ b/packages/typescript/ai/src/activities/chat/stream/processor.ts @@ -928,7 +928,13 @@ export class StreamProcessor { // New tool call starting const initialState: ToolCallState = 'awaiting-input' - const toolName = chunk.toolCallName + // `toolName` is a deprecated alias for `toolCallName` (see ToolCallStartEvent + // in types.ts). Accept either so chunks from older code paths or any + // adapter that only sets the deprecated field still produce a named part. + // The type marks both as required strings, but in practice some emitters + // only set one — fall back via the runtime value rather than the type. + const toolName = + (chunk as { toolCallName?: string }).toolCallName ?? chunk.toolName const newToolCall: InternalToolCallState = { id: chunk.toolCallId, diff --git a/packages/typescript/ai/tests/chat.test.ts b/packages/typescript/ai/tests/chat.test.ts index 5890c4d89..f69234c54 100644 --- a/packages/typescript/ai/tests/chat.test.ts +++ b/packages/typescript/ai/tests/chat.test.ts @@ -707,6 +707,9 @@ describe('chat()', () => { c.type === 'TOOL_CALL_START' && (c as any).toolCallId === 'call_1', ) expect(toolStartChunks).toHaveLength(1) + // Both AG-UI spec field `toolCallName` and deprecated alias `toolName` + // must be set so consumers reading either get a valid name (issue #532). + expect((toolStartChunks[0] as any).toolCallName).toBe('getWeather') expect((toolStartChunks[0] as any).toolName).toBe('getWeather') const toolArgsChunks = chunks.filter( @@ -789,6 +792,7 @@ describe('chat()', () => { (c) => c.type === 'TOOL_CALL_START' && (c as any).toolCallId === id, ) expect(starts).toHaveLength(1) + expect((starts[0] as any).toolCallName).toBe(name) expect((starts[0] as any).toolName).toBe(name) const argChunks = chunks.filter( @@ -859,6 +863,7 @@ describe('chat()', () => { (c as any).toolCallId === 'call_server', ) expect(starts).toHaveLength(1) + expect((starts[0] as any).toolCallName).toBe('getWeather') expect((starts[0] as any).toolName).toBe('getWeather') const argChunks = chunks.filter( @@ -882,6 +887,94 @@ describe('chat()', () => { expect(startIdx).toBeLessThan(argsIdx) expect(argsIdx).toBeLessThan(endIdx) }) + + it('should replace pendingExecution placeholder with the real tool result and supply both toolCallName/toolName (issue #532)', async () => { + const executeSpy = vi.fn().mockReturnValue({ status: 'ok' }) + + const { adapter, calls } = createMockAdapter({ + iterations: [ + [ + ev.runStarted(), + ev.textStart(), + ev.textContent('Done.'), + ev.textEnd(), + ev.runFinished('stop'), + ], + ], + }) + + // Simulate the UIMessages a client sends back after approving a tool. + // The chat activity extracts the approval decision from the + // `approval-responded` part and converts the rest into ModelMessages, + // which includes a placeholder `tool` message marked pendingExecution. + const stream = chat({ + adapter, + messages: [ + { + id: 'm-user', + role: 'user', + parts: [{ type: 'text', content: 'Run it' }], + }, + { + id: 'm-assistant', + role: 'assistant', + parts: [ + { + type: 'tool-call', + id: 'call_approval', + name: 'approvedTool', + arguments: '{"x":1}', + state: 'approval-responded', + approval: { + id: 'approval_call_approval', + needsApproval: true, + approved: true, + }, + }, + ], + }, + ] as any, + tools: [ + { ...serverTool('approvedTool', executeSpy), needsApproval: true }, + ], + }) + + const chunks = await collectChunks(stream as AsyncIterable) + + // The tool must have actually executed because the placeholder marks + // it as pendingExecution. + expect(executeSpy).toHaveBeenCalledTimes(1) + + // Synthesized TOOL_CALL_START must include both `toolCallName` (AG-UI + // spec) and `toolName` (deprecated alias). Without `toolCallName` the + // chat-client's StreamProcessor would create a tool-call part with + // name=undefined and the next outbound request would fail at Anthropic + // with `tool_use.name: String should have at least 1 character`. + const toolStart = chunks.find( + (c) => + c.type === 'TOOL_CALL_START' && + (c as any).toolCallId === 'call_approval', + ) + expect(toolStart).toBeDefined() + expect((toolStart as any).toolCallName).toBe('approvedTool') + expect((toolStart as any).toolName).toBe('approvedTool') + + // The follow-up adapter call (after the tool ran) must see the real + // tool result, not the placeholder. With the placeholder still in the + // messages array, the Anthropic adapter's tool_result de-dup would + // keep the placeholder and drop the real result. + expect(calls).toHaveLength(1) + const adapterMessages = calls[0]!.messages as Array<{ + role: string + content: unknown + toolCallId?: string + }> + const toolMessages = adapterMessages.filter( + (m) => m.role === 'tool' && m.toolCallId === 'call_approval', + ) + expect(toolMessages).toHaveLength(1) + expect(toolMessages[0]!.content).toBe(JSON.stringify({ status: 'ok' })) + }) }) // ========================================================================== diff --git a/packages/typescript/ai/tests/stream-processor.test.ts b/packages/typescript/ai/tests/stream-processor.test.ts index 85136ef70..bce85813a 100644 --- a/packages/typescript/ai/tests/stream-processor.test.ts +++ b/packages/typescript/ai/tests/stream-processor.test.ts @@ -1526,6 +1526,32 @@ describe('StreamProcessor', () => { // Edge cases // ========================================================================== describe('edge cases', () => { + it('TOOL_CALL_START with only the deprecated `toolName` field should still produce a named tool-call part (issue #532)', () => { + const processor = new StreamProcessor() + processor.prepareAssistantMessage() + + // Some chunk producers (notably the agent loop's continuation re-emit + // before the fix) only set the deprecated alias `toolName`. The + // processor must fall back to it so the resulting tool-call part has + // a defined `name` — otherwise the next outbound request fails with + // `tool_use.name: String should have at least 1 character` at Anthropic. + processor.processChunk({ + type: 'TOOL_CALL_START', + timestamp: Date.now(), + toolCallId: 'tc-1', + toolName: 'legacyTool', + // toolCallName intentionally omitted + } as any) + + const state = processor.getState() + expect(state.toolCalls.get('tc-1')?.name).toBe('legacyTool') + + const toolPart = processor + .getMessages()[0]! + .parts.find((p) => p.type === 'tool-call') + expect(toolPart && (toolPart as any).name).toBe('legacyTool') + }) + it('duplicate TOOL_CALL_START with same toolCallId should be a no-op', () => { const processor = new StreamProcessor() processor.prepareAssistantMessage() diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a0423a2f2..f05b781d9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -589,7 +589,7 @@ importers: version: 0.561.0(react@19.2.3) nitro: specifier: latest - version: 3.0.260415-beta(chokidar@5.0.0)(dotenv@17.2.3)(giget@2.0.0)(jiti@2.6.1)(miniflare@4.20260504.0)(rollup@4.60.1)(vite@7.3.1(@types/node@24.10.3)(jiti@2.6.1)(lightningcss@1.30.2)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2)) + version: 3.0.260429-beta(chokidar@5.0.0)(dotenv@17.2.3)(giget@2.0.0)(jiti@2.6.1)(miniflare@4.20260504.0)(rollup@4.60.1)(vite@7.3.1(@types/node@24.10.3)(jiti@2.6.1)(lightningcss@1.30.2)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2)) radix-ui: specifier: ^1.4.3 version: 1.4.3(@types/react-dom@19.2.3(@types/react@19.2.7))(@types/react@19.2.7)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -1672,6 +1672,9 @@ importers: '@copilotkit/aimock': specifier: ^1.18.0 version: 1.18.0(vitest@4.1.4(@opentelemetry/api@1.9.1)(@types/node@24.10.3)(happy-dom@20.0.11)(jsdom@27.3.0(postcss@8.5.9))(vite@7.3.1(@types/node@24.10.3)(jiti@2.6.1)(lightningcss@1.30.2)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2))) + '@openrouter/sdk': + specifier: 0.12.14 + version: 0.12.14 '@opentelemetry/api': specifier: ^1.9.0 version: 1.9.1 @@ -9513,16 +9516,16 @@ packages: xml2js: optional: true - nitro@3.0.260415-beta: - resolution: {integrity: sha512-J0ntJERWtIdvweZdmkCiF8eOFvP9fIAJR2gpeIDrHbAlYavK41WQfADo/YoZ/LF7RMTZBiPaH/pt2s/nPru9Iw==} + nitro@3.0.260429-beta: + resolution: {integrity: sha512-KweLVCUN5X9v9g+4yxAyRcz3FcOlnjmt9FyrAIWDxJETJmNT7I0JV0clgsONjo2nI0U5gwedXYA3RaNtF5XWzg==} engines: {node: ^20.19.0 || >=22.12.0} hasBin: true peerDependencies: - '@vercel/queue': ^0.1.4 + '@vercel/queue': ^0.1.6 dotenv: '*' giget: '*' jiti: ^2.6.1 - rollup: ^4.60.1 + rollup: ^4.60.2 vite: ^7 || ^8 xml2js: ^0.6.2 zephyr-agent: ^0.2.0 @@ -20737,7 +20740,7 @@ snapshots: - sqlite3 - uploadthing - nitro@3.0.260415-beta(chokidar@5.0.0)(dotenv@17.2.3)(giget@2.0.0)(jiti@2.6.1)(miniflare@4.20260504.0)(rollup@4.60.1)(vite@7.3.1(@types/node@24.10.3)(jiti@2.6.1)(lightningcss@1.30.2)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2)): + nitro@3.0.260429-beta(chokidar@5.0.0)(dotenv@17.2.3)(giget@2.0.0)(jiti@2.6.1)(miniflare@4.20260504.0)(rollup@4.60.1)(vite@7.3.1(@types/node@24.10.3)(jiti@2.6.1)(lightningcss@1.30.2)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2)): dependencies: consola: 3.4.2 crossws: 0.4.5(srvx@0.11.15) diff --git a/testing/e2e/fixtures/tool-approval/approval.json b/testing/e2e/fixtures/tool-approval/approval.json index 57ff40617..14ed7d29e 100644 --- a/testing/e2e/fixtures/tool-approval/approval.json +++ b/testing/e2e/fixtures/tool-approval/approval.json @@ -45,6 +45,15 @@ "response": { "content": "Understood, I won't add the guitar to your cart. Let me know if you change your mind." } + }, + { + "match": { + "userMessage": "[approval] follow-up: anything else?", + "sequenceIndex": 0 + }, + "response": { + "content": "Here is the follow-up reply: nothing else needed for now." + } } ] } diff --git a/testing/e2e/package.json b/testing/e2e/package.json index a33dbaae4..05ebe10f0 100644 --- a/testing/e2e/package.json +++ b/testing/e2e/package.json @@ -12,6 +12,7 @@ }, "dependencies": { "@copilotkit/aimock": "^1.18.0", + "@openrouter/sdk": "0.12.14", "@opentelemetry/api": "^1.9.0", "@tailwindcss/vite": "^4.1.18", "@tanstack/ai": "workspace:*", diff --git a/testing/e2e/src/lib/providers.ts b/testing/e2e/src/lib/providers.ts index d2051ef25..ca2d00c4d 100644 --- a/testing/e2e/src/lib/providers.ts +++ b/testing/e2e/src/lib/providers.ts @@ -7,6 +7,7 @@ import { createOllamaChat } from '@tanstack/ai-ollama' import { createGroqText } from '@tanstack/ai-groq' import { createGrokText } from '@tanstack/ai-grok' import { createOpenRouterText } from '@tanstack/ai-openrouter' +import { HTTPClient } from '@openrouter/sdk' import type { Provider } from '@/lib/types' const LLMOCK_DEFAULT_BASE = process.env.LLMOCK_URL || 'http://127.0.0.1:4010' @@ -87,15 +88,28 @@ export function createTextAdapter( defaultHeaders: testHeaders, }), }), - openrouter: () => - createChatOptions({ + openrouter: () => { + // OpenRouter SDK exposes an HTTPClient with beforeRequest hooks. Use + // that to inject X-Test-Id, since `defaultHeaders` isn't supported and + // the SDK strips query params off `serverURL` when building per-request + // URLs (it does `new URL(path, baseURL)` which drops the search), so + // the previous `?testId=...` trick never actually reached aimock and + // multiple openrouter tests collided on the `__default__` test bucket. + const httpClient = new HTTPClient() + if (testId) { + httpClient.addHook('beforeRequest', (req) => { + const next = new Request(req) + next.headers.set('X-Test-Id', testId) + return next + }) + } + return createChatOptions({ adapter: createOpenRouterText(model as 'openai/gpt-4o', DUMMY_KEY, { - // OpenRouter SDK doesn't support defaultHeaders, so pass testId via query param - serverURL: testId - ? `${openaiUrl}?testId=${encodeURIComponent(testId)}` - : openaiUrl, + serverURL: openaiUrl, + httpClient, }), - }), + }) + }, elevenlabs: () => { throw new Error( 'ElevenLabs has no text/chat adapter — use createTTSAdapter or createTranscriptionAdapter.', diff --git a/testing/e2e/tests/tool-approval.spec.ts b/testing/e2e/tests/tool-approval.spec.ts index 5b6cc1b6b..4af029661 100644 --- a/testing/e2e/tests/tool-approval.spec.ts +++ b/testing/e2e/tests/tool-approval.spec.ts @@ -29,6 +29,29 @@ for (const provider of providersFor('tool-approval')) { await waitForAssistantText(page, 'added') }) + test('follow-up message after approval does not produce empty tool_use.name (issue #532)', async ({ + page, + testId, + aimockPort, + }) => { + await page.goto(featureUrl(provider, 'tool-approval', testId, aimockPort)) + + await sendMessage(page, '[approval] add the stratocaster to my cart') + + await expect(page.getByTestId('approval-prompt-addToCart')).toBeVisible({ + timeout: 20000, + }) + await approveToolCall(page, 'addToCart') + await waitForAssistantText(page, 'added') + + // The approved tool call now lives in message history. Sending a new + // message previously produced a 400 from Anthropic because the + // tool_use block had an empty `name` (issue #532). The follow-up must + // round-trip without error. + await sendMessage(page, '[approval] follow-up: anything else?') + await waitForAssistantText(page, 'follow-up') + }) + test('handles denial', async ({ page, testId, aimockPort }) => { await page.goto(featureUrl(provider, 'tool-approval', testId, aimockPort))