From aaf43170f246fa5611166a1da263b7b673a833ac Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 2 Dec 2025 16:54:26 -0500 Subject: [PATCH 1/6] fix: handle malformed native tool calls to prevent hanging When a model sends a malformed tool call (e.g., apply_diff with only {"path": "file.go"} and missing required parameters), the NativeToolCallParser.finalizeStreamingToolCall() returns null because the JSON is incomplete or can't be parsed properly. Previously, when finalizeStreamingToolCall returned null, the code did nothing - leaving the partial tool use in assistantMessageContent with partial: true. This caused the tool to never be executed and no tool_result was sent back to the API, causing the system to hang. This fix ensures that when finalizeStreamingToolCall returns null, we still mark the existing partial tool use as complete (partial: false) so it gets executed. The tool's validation will catch missing required parameters and return a proper error message to the model. Fixes #9542 --- .../__tests__/NativeToolCallParser.spec.ts | 78 +++++++++++++++++++ src/core/task/Task.ts | 46 ++++++++++- 2 files changed, 122 insertions(+), 2 deletions(-) diff --git a/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts b/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts index 0e81671cc15..8da513df4d9 100644 --- a/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts +++ b/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts @@ -237,5 +237,83 @@ describe("NativeToolCallParser", () => { } }) }) + + describe("malformed tool calls", () => { + it("should return null when JSON arguments are malformed (missing closing brace)", () => { + const id = "toolu_malformed_1" + NativeToolCallParser.startStreamingToolCall(id, "apply_diff") + + // Simulate malformed JSON - missing closing brace (model error scenario from PR #9542) + NativeToolCallParser.processStreamingChunk(id, '{"path": "alphametics.go"') + + const result = NativeToolCallParser.finalizeStreamingToolCall(id) + + // Should return null because JSON.parse will fail + expect(result).toBeNull() + }) + + it("should return null when JSON is completely empty", () => { + const id = "toolu_malformed_2" + NativeToolCallParser.startStreamingToolCall(id, "apply_diff") + + // Process empty/no arguments + NativeToolCallParser.processStreamingChunk(id, "") + + const result = NativeToolCallParser.finalizeStreamingToolCall(id) + + expect(result).toBeNull() + }) + + it("should return null when JSON is only whitespace", () => { + const id = "toolu_malformed_3" + NativeToolCallParser.startStreamingToolCall(id, "apply_diff") + + NativeToolCallParser.processStreamingChunk(id, " ") + + const result = NativeToolCallParser.finalizeStreamingToolCall(id) + + expect(result).toBeNull() + }) + + it("should return null when tool call was never started", () => { + const result = NativeToolCallParser.finalizeStreamingToolCall("nonexistent_id") + + expect(result).toBeNull() + }) + + it("should return null for incomplete JSON with only opening bracket", () => { + const id = "toolu_malformed_4" + NativeToolCallParser.startStreamingToolCall(id, "read_file") + + NativeToolCallParser.processStreamingChunk(id, "{") + + const result = NativeToolCallParser.finalizeStreamingToolCall(id) + + expect(result).toBeNull() + }) + + it("should return null for JSON with syntax error (trailing comma)", () => { + const id = "toolu_malformed_5" + NativeToolCallParser.startStreamingToolCall(id, "read_file") + + NativeToolCallParser.processStreamingChunk(id, '{"files": [],}') + + const result = NativeToolCallParser.finalizeStreamingToolCall(id) + + expect(result).toBeNull() + }) + + it("should handle tool call with valid JSON but parseToolCall returns null", () => { + const id = "toolu_parse_fail_1" + NativeToolCallParser.startStreamingToolCall(id, "unknown_tool" as any) + + NativeToolCallParser.processStreamingChunk(id, '{"some": "data"}') + + const result = NativeToolCallParser.finalizeStreamingToolCall(id) + + // parseToolCall returns null for unknown tools, so this should be null + expect(result).toBeNull() + }) + }) }) }) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 12a4df0639e..5df2e025fe1 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2473,12 +2473,14 @@ export class Task extends EventEmitter implements TaskLike { // Finalize the streaming tool call const finalToolUse = NativeToolCallParser.finalizeStreamingToolCall(event.id) + // Get the index for this tool call + const toolUseIndex = this.streamingToolCallIndices.get(event.id) + if (finalToolUse) { // Store the tool call ID ;(finalToolUse as any).id = event.id // Get the index and replace partial with final - const toolUseIndex = this.streamingToolCallIndices.get(event.id) if (toolUseIndex !== undefined) { this.assistantMessageContent[toolUseIndex] = finalToolUse } @@ -2491,6 +2493,25 @@ export class Task extends EventEmitter implements TaskLike { // Present the finalized tool call presentAssistantMessage(this) + } else if (toolUseIndex !== undefined) { + // finalizeStreamingToolCall returned null (malformed JSON or missing args) + // We still need to mark the tool as non-partial so it gets executed + // The tool's validation will catch any missing required parameters + const existingToolUse = this.assistantMessageContent[toolUseIndex] + if (existingToolUse && existingToolUse.type === "tool_use") { + existingToolUse.partial = false + // Ensure it has the ID for native protocol + ;(existingToolUse as any).id = event.id + } + + // Clean up tracking + this.streamingToolCallIndices.delete(event.id) + + // Mark that we have new content to process + this.userMessageContentReady = false + + // Present the tool call - validation will handle missing params + presentAssistantMessage(this) } } } @@ -2611,12 +2632,14 @@ export class Task extends EventEmitter implements TaskLike { // Finalize the streaming tool call const finalToolUse = NativeToolCallParser.finalizeStreamingToolCall(event.id) + // Get the index for this tool call + const toolUseIndex = this.streamingToolCallIndices.get(event.id) + if (finalToolUse) { // Store the tool call ID ;(finalToolUse as any).id = event.id // Get the index and replace partial with final - const toolUseIndex = this.streamingToolCallIndices.get(event.id) if (toolUseIndex !== undefined) { this.assistantMessageContent[toolUseIndex] = finalToolUse } @@ -2629,6 +2652,25 @@ export class Task extends EventEmitter implements TaskLike { // Present the finalized tool call presentAssistantMessage(this) + } else if (toolUseIndex !== undefined) { + // finalizeStreamingToolCall returned null (malformed JSON or missing args) + // We still need to mark the tool as non-partial so it gets executed + // The tool's validation will catch any missing required parameters + const existingToolUse = this.assistantMessageContent[toolUseIndex] + if (existingToolUse && existingToolUse.type === "tool_use") { + existingToolUse.partial = false + // Ensure it has the ID for native protocol + ;(existingToolUse as any).id = event.id + } + + // Clean up tracking + this.streamingToolCallIndices.delete(event.id) + + // Mark that we have new content to process + this.userMessageContentReady = false + + // Present the tool call - validation will handle missing params + presentAssistantMessage(this) } } } From 24bec99555afd4c3103a1d96fa9ff80144108d02 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 2 Dec 2025 17:48:05 -0500 Subject: [PATCH 2/6] fix: restore presentAssistantMessage for all partial blocks PR #9542 added a condition that only called presentAssistantMessage() if there was at least one non-tool_use partial block. This broke XML protocol because XML tools are parsed from the text stream (not via native events), and when only a malformed tool_use was the partial block, presentAssistantMessage was never called, causing a hang. This commit restores the original behavior that works for both protocols. --- src/core/task/Task.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 5df2e025fe1..bb5df684dc4 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2940,9 +2940,11 @@ export class Task extends EventEmitter implements TaskLike { this.assistantMessageContent = parsedBlocks } - // Only present partial blocks that were just completed (from XML parsing) - // Native tool blocks were already presented during streaming, so don't re-present them - if (partialBlocks.length > 0 && partialBlocks.some((block) => block.type !== "tool_use")) { + // Present any partial blocks that were just completed + // For XML protocol: includes both text and tool_use blocks parsed from the text stream + // For native protocol: tool_use blocks were already presented during streaming via + // tool_call_partial events, but we still need to present them if they exist (e.g., malformed) + if (partialBlocks.length > 0) { // If there is content to update then it will complete and // update `this.userMessageContentReady` to true, which we // `pWaitFor` before making the next request. From d9420141a9bd321e181b0a727c8534199e86f218 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 2 Dec 2025 17:54:15 -0500 Subject: [PATCH 3/6] test: add regression tests for PR #9542 tool_use partial blocks bug These tests verify that presentAssistantMessage is called at stream end even when only tool_use blocks are partial. The tests explicitly show the difference between the fixed condition and the broken condition that was introduced in PR #9542. --- src/core/task/__tests__/Task.spec.ts | 101 +++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 4bae9c49d09..c8a13ded5a1 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -1845,6 +1845,107 @@ describe("Cline", () => { }) }) }) + + describe("Partial tool_use blocks at stream end", () => { + /** + * These tests verify that presentAssistantMessage is called at stream end + * even when only tool_use blocks are partial. This is critical because: + * - For XML protocol: tool_use blocks come from text parsing, not native events + * - If presentAssistantMessage isn't called, tools never execute and the task hangs + * + * A previous regression was caused by adding the condition: + * `partialBlocks.some((block) => block.type !== "tool_use")` + * which prevented presentAssistantMessage from being called when only + * tool_use blocks were partial. + */ + it("should call presentAssistantMessage when only tool_use blocks are partial", () => { + const cline = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Simulate the state at stream end with only a tool_use block that's partial + cline.assistantMessageContent = [ + { + type: "tool_use" as const, + name: "read_file" as any, + params: { path: "test.txt" }, + partial: true, + }, + ] + + // Get partial blocks (simulating what happens at stream end) + const partialBlocks = cline.assistantMessageContent.filter((block) => block.partial) + + // This is what the FIXED condition should evaluate to + const shouldPresentMessage = partialBlocks.length > 0 + expect(shouldPresentMessage).toBe(true) + + // This is what the BROKEN condition (PR #9542) would evaluate to + const brokenCondition = partialBlocks.length > 0 && partialBlocks.some((block) => block.type !== "tool_use") + expect(brokenCondition).toBe(false) // This was the bug - it evaluated to false! + }) + + it("should call presentAssistantMessage when text and tool_use blocks are partial", () => { + const cline = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Simulate the state at stream end with both text and tool_use blocks partial + cline.assistantMessageContent = [ + { + type: "text" as const, + content: "Some text", + partial: true, + }, + { + type: "tool_use" as const, + name: "read_file" as any, + params: { path: "test.txt" }, + partial: true, + }, + ] + + const partialBlocks = cline.assistantMessageContent.filter((block) => block.partial) + + // Both conditions should be true in this case + const shouldPresentMessage = partialBlocks.length > 0 + expect(shouldPresentMessage).toBe(true) + + const brokenCondition = partialBlocks.length > 0 && partialBlocks.some((block) => block.type !== "tool_use") + expect(brokenCondition).toBe(true) // The old condition worked when text was also partial + }) + + it("should not call presentAssistantMessage when no blocks are partial", () => { + const cline = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Simulate completed blocks (no partial blocks) + cline.assistantMessageContent = [ + { + type: "tool_use" as const, + name: "read_file" as any, + params: { path: "test.txt" }, + partial: false, + }, + ] + + const partialBlocks = cline.assistantMessageContent.filter((block) => block.partial) + + // Neither condition should trigger presentAssistantMessage + const shouldPresentMessage = partialBlocks.length > 0 + expect(shouldPresentMessage).toBe(false) + }) + }) }) describe("Queued message processing after condense", () => { From cdfb8f477c192cfe98035968bfa8335ae10a72be Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 2 Dec 2025 18:28:35 -0500 Subject: [PATCH 4/6] test: add regression test for partial tool_use blocks condition --- src/core/task/__tests__/Task.spec.ts | 43 +++++++++++++++++----------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index c8a13ded5a1..d5bb0bd9f4a 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -1857,8 +1857,23 @@ describe("Cline", () => { * `partialBlocks.some((block) => block.type !== "tool_use")` * which prevented presentAssistantMessage from being called when only * tool_use blocks were partial. + * + * This test uses a helper that replicates the exact condition from Task.ts + * to verify the behavior is correct. */ - it("should call presentAssistantMessage when only tool_use blocks are partial", () => { + + // This helper function MUST match the condition in Task.ts around line ~2905 + // If Task.ts changes, this must be updated to match + // THE KEY: This is what we're testing - if someone introduces the broken condition, + // this function should be updated to match Task.ts, and then the test will fail + function shouldCallPresentAssistantMessage(partialBlocks: Array<{ type: string; partial: boolean }>): boolean { + // This condition MUST match what's in Task.ts + // CORRECT: partialBlocks.length > 0 + // BROKEN (causes hang): partialBlocks.length > 0 && partialBlocks.some((block) => block.type !== "tool_use") + return partialBlocks.length > 0 && partialBlocks.some((block) => block.type !== "tool_use") + } + + it("should call presentAssistantMessage when only tool_use blocks are partial", async () => { const cline = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, @@ -1879,13 +1894,12 @@ describe("Cline", () => { // Get partial blocks (simulating what happens at stream end) const partialBlocks = cline.assistantMessageContent.filter((block) => block.partial) - // This is what the FIXED condition should evaluate to - const shouldPresentMessage = partialBlocks.length > 0 - expect(shouldPresentMessage).toBe(true) - - // This is what the BROKEN condition (PR #9542) would evaluate to - const brokenCondition = partialBlocks.length > 0 && partialBlocks.some((block) => block.type !== "tool_use") - expect(brokenCondition).toBe(false) // This was the bug - it evaluated to false! + // THE KEY TEST: When only tool_use blocks are partial, presentAssistantMessage + // MUST be called (return true). If the condition returns false, tools won't execute + // and the task will hang indefinitely. + // + // This test will FAIL if Task.ts uses the broken condition that filters out tool_use blocks. + expect(shouldCallPresentAssistantMessage(partialBlocks)).toBe(true) }) it("should call presentAssistantMessage when text and tool_use blocks are partial", () => { @@ -1913,12 +1927,8 @@ describe("Cline", () => { const partialBlocks = cline.assistantMessageContent.filter((block) => block.partial) - // Both conditions should be true in this case - const shouldPresentMessage = partialBlocks.length > 0 - expect(shouldPresentMessage).toBe(true) - - const brokenCondition = partialBlocks.length > 0 && partialBlocks.some((block) => block.type !== "tool_use") - expect(brokenCondition).toBe(true) // The old condition worked when text was also partial + // When both text and tool_use blocks are partial, presentAssistantMessage should be called + expect(shouldCallPresentAssistantMessage(partialBlocks)).toBe(true) }) it("should not call presentAssistantMessage when no blocks are partial", () => { @@ -1941,9 +1951,8 @@ describe("Cline", () => { const partialBlocks = cline.assistantMessageContent.filter((block) => block.partial) - // Neither condition should trigger presentAssistantMessage - const shouldPresentMessage = partialBlocks.length > 0 - expect(shouldPresentMessage).toBe(false) + // When no blocks are partial, presentAssistantMessage should NOT be called + expect(shouldCallPresentAssistantMessage(partialBlocks)).toBe(false) }) }) }) From 5b39b2afbae36dd76bc04aa256f90ff12af5eb90 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 2 Dec 2025 18:52:49 -0500 Subject: [PATCH 5/6] fix(tests): ensure presentAssistantMessage is called for partial tool_use blocks to prevent hangs --- src/core/task/__tests__/Task.spec.ts | 130 +++++++++++++-------------- 1 file changed, 64 insertions(+), 66 deletions(-) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index d5bb0bd9f4a..5aed6800d37 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -17,6 +17,7 @@ import { processUserContentMentions } from "../../mentions/processUserContentMen import { MultiSearchReplaceDiffStrategy } from "../../diff/strategies/multi-search-replace" import { MultiFileSearchReplaceDiffStrategy } from "../../diff/strategies/multi-file-search-replace" import { EXPERIMENT_IDS } from "../../../shared/experiments" +import * as assistantMessage from "../../assistant-message" // Mock delay before any imports that might use it vi.mock("delay", () => ({ @@ -1848,41 +1849,29 @@ describe("Cline", () => { describe("Partial tool_use blocks at stream end", () => { /** - * These tests verify that presentAssistantMessage is called at stream end - * even when only tool_use blocks are partial. This is critical because: - * - For XML protocol: tool_use blocks come from text parsing, not native events - * - If presentAssistantMessage isn't called, tools never execute and the task hangs + * This test verifies that when the stream ends with partial tool_use blocks, + * they get finalized (partial=false) and presentAssistantMessage is called. * - * A previous regression was caused by adding the condition: - * `partialBlocks.some((block) => block.type !== "tool_use")` - * which prevented presentAssistantMessage from being called when only - * tool_use blocks were partial. + * The bug: malformed native tool calls leave partial=true, causing hang. * - * This test uses a helper that replicates the exact condition from Task.ts - * to verify the behavior is correct. + * The fix at Task.ts line ~2947 must be: + * if (partialBlocks.length > 0) { presentAssistantMessage(this) } + * + * THE BROKEN CONDITION (causes infinite hang): + * if (partialBlocks.length > 0 && partialBlocks.some(block => block.type !== "tool_use")) */ - // This helper function MUST match the condition in Task.ts around line ~2905 - // If Task.ts changes, this must be updated to match - // THE KEY: This is what we're testing - if someone introduces the broken condition, - // this function should be updated to match Task.ts, and then the test will fail - function shouldCallPresentAssistantMessage(partialBlocks: Array<{ type: string; partial: boolean }>): boolean { - // This condition MUST match what's in Task.ts - // CORRECT: partialBlocks.length > 0 - // BROKEN (causes hang): partialBlocks.length > 0 && partialBlocks.some((block) => block.type !== "tool_use") - return partialBlocks.length > 0 && partialBlocks.some((block) => block.type !== "tool_use") - } - - it("should call presentAssistantMessage when only tool_use blocks are partial", async () => { - const cline = new Task({ + it("should finalize all partial blocks at stream end regardless of type", () => { + const task = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, task: "test task", startTask: false, }) - // Simulate the state at stream end with only a tool_use block that's partial - cline.assistantMessageContent = [ + // Set up the state when stream ends with ONLY partial tool_use blocks + // This is the critical scenario for malformed native tool calls + task.assistantMessageContent = [ { type: "tool_use" as const, name: "read_file" as any, @@ -1891,32 +1880,31 @@ describe("Cline", () => { }, ] - // Get partial blocks (simulating what happens at stream end) - const partialBlocks = cline.assistantMessageContent.filter((block) => block.partial) + // This is the stream end logic from Task.ts around line 2928 + const partialBlocks = task.assistantMessageContent.filter((block) => block.partial) + partialBlocks.forEach((block) => (block.partial = false)) + + // VERIFY: All blocks have partial=false after finalization + expect(task.assistantMessageContent[0].partial).toBe(false) - // THE KEY TEST: When only tool_use blocks are partial, presentAssistantMessage - // MUST be called (return true). If the condition returns false, tools won't execute - // and the task will hang indefinitely. - // - // This test will FAIL if Task.ts uses the broken condition that filters out tool_use blocks. - expect(shouldCallPresentAssistantMessage(partialBlocks)).toBe(true) + // VERIFY: The condition to call presentAssistantMessage MUST be TRUE + // If this fails, the task will hang forever + // The CORRECT condition is: partialBlocks.length > 0 + // The BROKEN condition is: partialBlocks.length > 0 && partialBlocks.some(block => block.type !== "tool_use") + const shouldCallPresentAssistantMessage = partialBlocks.length > 0 + expect(shouldCallPresentAssistantMessage).toBe(true) }) - it("should call presentAssistantMessage when text and tool_use blocks are partial", () => { - const cline = new Task({ + it("REGRESSION TEST: broken condition causes hang with tool_use-only partial blocks", () => { + const task = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, task: "test task", startTask: false, }) - // Simulate the state at stream end with both text and tool_use blocks partial - cline.assistantMessageContent = [ - { - type: "text" as const, - content: "Some text", - partial: true, - }, + // Simulate the exact bug scenario: stream ends with ONLY partial tool_use blocks + task.assistantMessageContent = [ { type: "tool_use" as const, name: "read_file" as any, @@ -1925,34 +1913,44 @@ describe("Cline", () => { }, ] - const partialBlocks = cline.assistantMessageContent.filter((block) => block.partial) - - // When both text and tool_use blocks are partial, presentAssistantMessage should be called - expect(shouldCallPresentAssistantMessage(partialBlocks)).toBe(true) - }) + const partialBlocks = task.assistantMessageContent.filter((block) => block.partial) - it("should not call presentAssistantMessage when no blocks are partial", () => { - const cline = new Task({ - provider: mockProvider, - apiConfiguration: mockApiConfig, - task: "test task", - startTask: false, - }) + // THE CORRECT CONDITION (Task.ts line ~2947 MUST use this): + const correctCondition = partialBlocks.length > 0 + expect(correctCondition).toBe(true) - // Simulate completed blocks (no partial blocks) - cline.assistantMessageContent = [ - { - type: "tool_use" as const, - name: "read_file" as any, - params: { path: "test.txt" }, - partial: false, - }, - ] + // THE BROKEN CONDITION that caused the infinite hang bug: + const brokenCondition = partialBlocks.length > 0 && partialBlocks.some((block) => block.type !== "tool_use") + expect(brokenCondition).toBe(false) - const partialBlocks = cline.assistantMessageContent.filter((block) => block.partial) + // This explicitly documents the bug: + // - correctCondition returns TRUE, so presentAssistantMessage gets called + // - brokenCondition returns FALSE, so presentAssistantMessage DOESN'T get called + // - If brokenCondition is used, the task hangs forever waiting for partial blocks to complete + expect(correctCondition).not.toBe(brokenCondition) + }) - // When no blocks are partial, presentAssistantMessage should NOT be called - expect(shouldCallPresentAssistantMessage(partialBlocks)).toBe(false) + it("ACTUAL CODE TEST: Task.ts must NOT filter out tool_use blocks when calling presentAssistantMessage", async () => { + // Read the actual Task.ts source code and verify the fix is correct + const fs = await import("fs") + const path = await import("path") + + // Read Task.ts source + const taskTsPath = path.join(__dirname, "..", "Task.ts") + const taskSource = fs.readFileSync(taskTsPath, "utf-8") + + // Find the line around "Present any partial blocks that were just completed" + // The CORRECT pattern: if (partialBlocks.length > 0) { + // The BROKEN pattern: if (partialBlocks.length > 0 && partialBlocks.some(block => block.type !== "tool_use")) + const hasCorrectCondition = taskSource.includes( + "if (partialBlocks.length > 0) {\n\t\t\t\t\t// If there is content to update", + ) + const hasBrokenCondition = taskSource.includes('partialBlocks.some((block) => block.type !== "tool_use")') + + // The code MUST use the correct condition (simple length check) + // and MUST NOT use the broken condition (filtering out tool_use) + expect(hasBrokenCondition).toBe(false) + expect(hasCorrectCondition).toBe(true) }) }) }) From 52b890e22fc8b795c1660f30f9729560b313bdf2 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Tue, 2 Dec 2025 19:35:00 -0500 Subject: [PATCH 6/6] Revert tests for now --- .../__tests__/NativeToolCallParser.spec.ts | 78 ------------- src/core/task/__tests__/Task.spec.ts | 108 ------------------ 2 files changed, 186 deletions(-) diff --git a/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts b/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts index 8da513df4d9..0e81671cc15 100644 --- a/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts +++ b/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts @@ -237,83 +237,5 @@ describe("NativeToolCallParser", () => { } }) }) - - describe("malformed tool calls", () => { - it("should return null when JSON arguments are malformed (missing closing brace)", () => { - const id = "toolu_malformed_1" - NativeToolCallParser.startStreamingToolCall(id, "apply_diff") - - // Simulate malformed JSON - missing closing brace (model error scenario from PR #9542) - NativeToolCallParser.processStreamingChunk(id, '{"path": "alphametics.go"') - - const result = NativeToolCallParser.finalizeStreamingToolCall(id) - - // Should return null because JSON.parse will fail - expect(result).toBeNull() - }) - - it("should return null when JSON is completely empty", () => { - const id = "toolu_malformed_2" - NativeToolCallParser.startStreamingToolCall(id, "apply_diff") - - // Process empty/no arguments - NativeToolCallParser.processStreamingChunk(id, "") - - const result = NativeToolCallParser.finalizeStreamingToolCall(id) - - expect(result).toBeNull() - }) - - it("should return null when JSON is only whitespace", () => { - const id = "toolu_malformed_3" - NativeToolCallParser.startStreamingToolCall(id, "apply_diff") - - NativeToolCallParser.processStreamingChunk(id, " ") - - const result = NativeToolCallParser.finalizeStreamingToolCall(id) - - expect(result).toBeNull() - }) - - it("should return null when tool call was never started", () => { - const result = NativeToolCallParser.finalizeStreamingToolCall("nonexistent_id") - - expect(result).toBeNull() - }) - - it("should return null for incomplete JSON with only opening bracket", () => { - const id = "toolu_malformed_4" - NativeToolCallParser.startStreamingToolCall(id, "read_file") - - NativeToolCallParser.processStreamingChunk(id, "{") - - const result = NativeToolCallParser.finalizeStreamingToolCall(id) - - expect(result).toBeNull() - }) - - it("should return null for JSON with syntax error (trailing comma)", () => { - const id = "toolu_malformed_5" - NativeToolCallParser.startStreamingToolCall(id, "read_file") - - NativeToolCallParser.processStreamingChunk(id, '{"files": [],}') - - const result = NativeToolCallParser.finalizeStreamingToolCall(id) - - expect(result).toBeNull() - }) - - it("should handle tool call with valid JSON but parseToolCall returns null", () => { - const id = "toolu_parse_fail_1" - NativeToolCallParser.startStreamingToolCall(id, "unknown_tool" as any) - - NativeToolCallParser.processStreamingChunk(id, '{"some": "data"}') - - const result = NativeToolCallParser.finalizeStreamingToolCall(id) - - // parseToolCall returns null for unknown tools, so this should be null - expect(result).toBeNull() - }) - }) }) }) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 5aed6800d37..4bae9c49d09 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -17,7 +17,6 @@ import { processUserContentMentions } from "../../mentions/processUserContentMen import { MultiSearchReplaceDiffStrategy } from "../../diff/strategies/multi-search-replace" import { MultiFileSearchReplaceDiffStrategy } from "../../diff/strategies/multi-file-search-replace" import { EXPERIMENT_IDS } from "../../../shared/experiments" -import * as assistantMessage from "../../assistant-message" // Mock delay before any imports that might use it vi.mock("delay", () => ({ @@ -1846,113 +1845,6 @@ describe("Cline", () => { }) }) }) - - describe("Partial tool_use blocks at stream end", () => { - /** - * This test verifies that when the stream ends with partial tool_use blocks, - * they get finalized (partial=false) and presentAssistantMessage is called. - * - * The bug: malformed native tool calls leave partial=true, causing hang. - * - * The fix at Task.ts line ~2947 must be: - * if (partialBlocks.length > 0) { presentAssistantMessage(this) } - * - * THE BROKEN CONDITION (causes infinite hang): - * if (partialBlocks.length > 0 && partialBlocks.some(block => block.type !== "tool_use")) - */ - - it("should finalize all partial blocks at stream end regardless of type", () => { - const task = new Task({ - provider: mockProvider, - apiConfiguration: mockApiConfig, - task: "test task", - startTask: false, - }) - - // Set up the state when stream ends with ONLY partial tool_use blocks - // This is the critical scenario for malformed native tool calls - task.assistantMessageContent = [ - { - type: "tool_use" as const, - name: "read_file" as any, - params: { path: "test.txt" }, - partial: true, - }, - ] - - // This is the stream end logic from Task.ts around line 2928 - const partialBlocks = task.assistantMessageContent.filter((block) => block.partial) - partialBlocks.forEach((block) => (block.partial = false)) - - // VERIFY: All blocks have partial=false after finalization - expect(task.assistantMessageContent[0].partial).toBe(false) - - // VERIFY: The condition to call presentAssistantMessage MUST be TRUE - // If this fails, the task will hang forever - // The CORRECT condition is: partialBlocks.length > 0 - // The BROKEN condition is: partialBlocks.length > 0 && partialBlocks.some(block => block.type !== "tool_use") - const shouldCallPresentAssistantMessage = partialBlocks.length > 0 - expect(shouldCallPresentAssistantMessage).toBe(true) - }) - - it("REGRESSION TEST: broken condition causes hang with tool_use-only partial blocks", () => { - const task = new Task({ - provider: mockProvider, - apiConfiguration: mockApiConfig, - task: "test task", - startTask: false, - }) - - // Simulate the exact bug scenario: stream ends with ONLY partial tool_use blocks - task.assistantMessageContent = [ - { - type: "tool_use" as const, - name: "read_file" as any, - params: { path: "test.txt" }, - partial: true, - }, - ] - - const partialBlocks = task.assistantMessageContent.filter((block) => block.partial) - - // THE CORRECT CONDITION (Task.ts line ~2947 MUST use this): - const correctCondition = partialBlocks.length > 0 - expect(correctCondition).toBe(true) - - // THE BROKEN CONDITION that caused the infinite hang bug: - const brokenCondition = partialBlocks.length > 0 && partialBlocks.some((block) => block.type !== "tool_use") - expect(brokenCondition).toBe(false) - - // This explicitly documents the bug: - // - correctCondition returns TRUE, so presentAssistantMessage gets called - // - brokenCondition returns FALSE, so presentAssistantMessage DOESN'T get called - // - If brokenCondition is used, the task hangs forever waiting for partial blocks to complete - expect(correctCondition).not.toBe(brokenCondition) - }) - - it("ACTUAL CODE TEST: Task.ts must NOT filter out tool_use blocks when calling presentAssistantMessage", async () => { - // Read the actual Task.ts source code and verify the fix is correct - const fs = await import("fs") - const path = await import("path") - - // Read Task.ts source - const taskTsPath = path.join(__dirname, "..", "Task.ts") - const taskSource = fs.readFileSync(taskTsPath, "utf-8") - - // Find the line around "Present any partial blocks that were just completed" - // The CORRECT pattern: if (partialBlocks.length > 0) { - // The BROKEN pattern: if (partialBlocks.length > 0 && partialBlocks.some(block => block.type !== "tool_use")) - const hasCorrectCondition = taskSource.includes( - "if (partialBlocks.length > 0) {\n\t\t\t\t\t// If there is content to update", - ) - const hasBrokenCondition = taskSource.includes('partialBlocks.some((block) => block.type !== "tool_use")') - - // The code MUST use the correct condition (simple length check) - // and MUST NOT use the broken condition (filtering out tool_use) - expect(hasBrokenCondition).toBe(false) - expect(hasCorrectCondition).toBe(true) - }) - }) }) describe("Queued message processing after condense", () => {