mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
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.
This commit is contained in:
parent
cc86049f10
commit
24e8d92689
2 changed files with 126 additions and 4 deletions
|
|
@ -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)`,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue