From 24e8d92689c84d97025923edacaa90c2e5a19714 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Fri, 30 Jan 2026 00:41:41 +0000 Subject: [PATCH] fix: prevent tool_call_end events for unstarted tool calls (#11071) This fixes an issue where GLM4.5 and other models get stuck in infinite tool call retry loops when using LM Studio or OpenAI-compatible endpoints. Root cause: processFinishReason() was emitting tool_call_end events for ALL tracked tool calls, even those that never had a tool_call_start emitted (due to missing tool name). This caused finalizeStreamingToolCall to fail silently, resulting in no tool_result being sent to the model. The fix ensures processFinishReason() only emits tool_call_end for tool calls where hasStarted=true, keeping the rawChunkTracker and streamingToolCalls maps synchronized. Also adds diagnostic logging when tool calls are tracked but never started, helping debug malformed tool call patterns from various models. --- .../assistant-message/NativeToolCallParser.ts | 27 ++++- .../__tests__/NativeToolCallParser.spec.ts | 103 ++++++++++++++++++ 2 files changed, 126 insertions(+), 4 deletions(-) diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index 72c34f94a0..02d24c6b3b 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -166,16 +166,35 @@ export class NativeToolCallParser { /** * Process stream finish reason. * Emits end events when finish_reason is 'tool_calls'. + * + * IMPORTANT: Only emits tool_call_end for tool calls that have actually started + * (i.e., where tool_call_start was emitted). This prevents finalizeStreamingToolCall + * from receiving IDs that were never registered via startStreamingToolCall, which + * would cause tool results to be silently dropped and trigger infinite retry loops. */ public static processFinishReason(finishReason: string | null | undefined): ToolCallStreamEvent[] { const events: ToolCallStreamEvent[] = [] if (finishReason === "tool_calls" && this.rawChunkTracker.size > 0) { for (const [, tracked] of this.rawChunkTracker.entries()) { - events.push({ - type: "tool_call_end", - id: tracked.id, - }) + // Only emit tool_call_end for tool calls that have actually started. + // Tool calls without hasStarted=true never had a tool_call_start emitted + // (likely due to missing tool name), so they were never registered in + // streamingToolCalls. Emitting tool_call_end for these would cause + // finalizeStreamingToolCall to fail, resulting in no tool_result being + // sent to the model and triggering infinite retry loops. + if (tracked.hasStarted) { + events.push({ + type: "tool_call_end", + id: tracked.id, + }) + } else { + // Log a warning for tool calls that were tracked but never started. + // This helps diagnose issues with models that send malformed tool calls. + console.warn( + `[NativeToolCallParser] Skipping tool_call_end for unstarted tool call: ${tracked.id} (no name received)`, + ) + } } } diff --git a/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts b/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts index db0dc00de4..1b4b1a7e17 100644 --- a/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts +++ b/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts @@ -343,4 +343,107 @@ describe("NativeToolCallParser", () => { }) }) }) + + describe("processFinishReason", () => { + describe("tool call tracking synchronization", () => { + it("should emit tool_call_end only for tool calls that have started", () => { + // Simulate a tool call with both ID and name (will start) + NativeToolCallParser.processRawChunk({ + index: 0, + id: "call_started_123", + name: "read_file", + }) + + const events = NativeToolCallParser.processFinishReason("tool_calls") + + expect(events).toHaveLength(1) + expect(events[0]).toEqual({ + type: "tool_call_end", + id: "call_started_123", + }) + }) + + it("should NOT emit tool_call_end for tool calls without a name (never started)", () => { + // Simulate a tool call with ID but NO name - this happens when models + // send malformed tool calls or split ID/name across chunks incorrectly + NativeToolCallParser.processRawChunk({ + index: 0, + id: "call_no_name_456", + // No name provided - tool_call_start will not be emitted + }) + + // Capture console.warn to verify warning is logged + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + + const events = NativeToolCallParser.processFinishReason("tool_calls") + + // Should NOT emit tool_call_end since tool was never started + expect(events).toHaveLength(0) + + // Should log a warning about the unstarted tool call + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("Skipping tool_call_end for unstarted tool call"), + ) + + warnSpy.mockRestore() + }) + + it("should handle mixed started and unstarted tool calls correctly", () => { + // Tool call with ID and name (will start) + NativeToolCallParser.processRawChunk({ + index: 0, + id: "call_with_name", + name: "read_file", + }) + + // Tool call with only ID (will not start) + NativeToolCallParser.processRawChunk({ + index: 1, + id: "call_without_name", + // No name + }) + + // Another tool call with ID and name (will start) + NativeToolCallParser.processRawChunk({ + index: 2, + id: "call_also_with_name", + name: "write_to_file", + }) + + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + + const events = NativeToolCallParser.processFinishReason("tool_calls") + + // Should only emit tool_call_end for the two started tool calls + expect(events).toHaveLength(2) + expect(events.map((e) => e.id)).toContain("call_with_name") + expect(events.map((e) => e.id)).toContain("call_also_with_name") + expect(events.map((e) => e.id)).not.toContain("call_without_name") + + // Should log warning for the unstarted tool call + expect(warnSpy).toHaveBeenCalledTimes(1) + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("call_without_name")) + + warnSpy.mockRestore() + }) + + it("should return empty array when finish_reason is not tool_calls", () => { + NativeToolCallParser.processRawChunk({ + index: 0, + id: "call_123", + name: "read_file", + }) + + const events = NativeToolCallParser.processFinishReason("stop") + + expect(events).toHaveLength(0) + }) + + it("should return empty array when no tool calls are tracked", () => { + const events = NativeToolCallParser.processFinishReason("tool_calls") + + expect(events).toHaveLength(0) + }) + }) + }) })