fix: clear nativeArgs when tool-call finalize fails (#1221)

src/core/task/Task.ts

@@ -3826,12 +3826,22 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 							/* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */
 							this.presentAssistantMessageSafe()
 						} 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
+							// finalizeStreamingToolCall returned null (malformed JSON or missing args).
+							// existingToolUse is the same object the streaming phase was mutating in
+							// place, so it still carries nativeArgs AND params built from the incomplete
+							// partial parse (e.g. a truncated write_to_file `content` string) - both were
+							// only ever meant for live progress display, never for execution or for
+							// ending up in conversation history. Mark the tool as non-partial so it's
+							// presented as complete, and clear both so presentAssistantMessage's
+							// `!block.nativeArgs` guard short-circuits with a structured tool_result
+							// instead of executing the truncated value, and so the toolUse.nativeArgs ||
+							// toolUse.params fallback used when recording history doesn't fall through to
+							// the same truncated data under a different name.
 							const existingToolUse = this.assistantMessageContent[toolUseIndex]
 							if (existingToolUse && existingToolUse.type === "tool_use") {
 								existingToolUse.partial = false
+								existingToolUse.nativeArgs = undefined
+								existingToolUse.params = {}
 								// Ensure it has the ID for native protocol
 								;(existingToolUse as any).id = event.id
 							}

src/core/task/__tests__/Task.spec.ts

@@ -33,6 +33,7 @@ import type { ApiMessage } from "../../task-persistence"
 import { asyncStreamFrom } from "../../../test-utils/stream"
 import { McpHub } from "../../../services/mcp/McpHub"
 import { McpServerManager } from "../../../services/mcp/McpServerManager"
+import { writeToFileTool } from "../../tools/WriteToFileTool"
 
 type TaskTestAccess = {
 	getSystemPrompt: (requestState: ProviderState | undefined, requestModelInfo?: ModelInfo) => Promise<string>
@@ -750,6 +751,102 @@ describe("Cline", () => {
 				},
 			])
 		})
+
+		it("blocks a truncated write_to_file call instead of executing it (issue #1221)", async () => {
+			// Regression test for #1221: if the model's stream is cut off mid-way
+			// through a write_to_file tool call's `content` argument (e.g. it hits
+			// max_tokens), finalizeStreamingToolCall() can't parse the incomplete
+			// JSON and returns null. Task.ts must not let the truncated content
+			// reach writeToFileTool's execution path - it must clear nativeArgs so
+			// presentAssistantMessage's fail-closed guard emits a structured
+			// tool_result error instead.
+			//
+			// Unlike the simulation-based tests in truncated-native-tool-args.spec.ts,
+			// this drives the real streaming + presentAssistantMessage flow through
+			// Task, and spies on the actual tool handler to prove it is never invoked.
+			const task = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				task: "truncated tool call test",
+				startTask: false,
+			})
+
+			vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined)
+			vi.spyOn(getTaskTestAccess(task), "safeEnsureModelFetched").mockResolvedValue(stubModelInfo)
+			// presentAssistantMessageSafe is intentionally left un-mocked here (unlike
+			// the other tests in this block) - the whole point is to exercise the real
+			// dispatch/guard logic, not just tool_use finalization.
+
+			const writeToFileHandleSpy = vi.spyOn(writeToFileTool, "handle")
+			// Spy directly on the guard's own push, rather than inspecting
+			// userMessageContent/apiConversationHistory afterwards - the task
+			// recurses into a follow-up request once the tool_result is ready
+			// (see the second mocked stream below), which resets those arrays
+			// for the new turn before this function returns.
+			const pushToolResultSpy = vi.spyOn(task, "pushToolResultToUserContent")
+
+			vi.spyOn(task, "attemptApiRequest")
+				.mockImplementationOnce(() =>
+					asyncStreamFrom<ApiStreamChunk>([
+						{
+							type: "tool_call_partial",
+							index: 0,
+							id: "call_truncated",
+							name: "write_to_file",
+						},
+						{
+							type: "tool_call_partial",
+							index: 0,
+							// Cut off mid-string: no closing quote/brace, and the stream
+							// ends here with no explicit tool_call_end - exactly what
+							// happens when the model hits max_tokens mid-argument.
+							// .md path deliberately used so the only thing that can block
+							// execution is the nativeArgs guard under test - an arbitrary
+							// extension could also get caught by unrelated mode-based file
+							// restrictions (e.g. Architect mode's markdown-only rule),
+							// which would produce a false pass/fail unrelated to this bug.
+							arguments: '{"path":"docs/config.md","content":"sk-live-abc123',
+						},
+					]),
+				)
+				// The task recurses once the error tool_result makes the turn
+				// "ready" - this bounds that follow-up to a single harmless text
+				// reply instead of an unmocked second call.
+				.mockImplementationOnce(() => asyncStreamFrom<ApiStreamChunk>([{ type: "text", text: "" }]))
+
+			await task.recursivelyMakeClineRequests([{ type: "text", text: "truncated tool call test" }])
+
+			// handle() legitimately gets called with partial: true while the call is
+			// still streaming (BaseTool.handle short-circuits to a no-op preview hook
+			// in that case) - that's expected and safe. What must never happen is a
+			// call with partial: false, which is what actually reaches execute() and
+			// writes to disk.
+			const nonPartialCalls = writeToFileHandleSpy.mock.calls.filter(
+				([, block]) => (block as { partial?: boolean }).partial === false,
+			)
+			expect(nonPartialCalls).toHaveLength(0)
+
+			// Neither nativeArgs nor params should leak the truncated content into
+			// the recorded assistant turn - Task.ts builds that entry's `input` via
+			// `toolUse.nativeArgs || toolUse.params`, so clearing nativeArgs alone
+			// would just have shifted the leak to params instead of closing it.
+			const assistantEntry = task.apiConversationHistory.find(
+				(m) => m.role === "assistant" && Array.isArray(m.content) && m.content[0]?.type === "tool_use",
+			)
+			expect(JSON.stringify(assistantEntry)).not.toContain("sk-live-abc123")
+
+			// A structured, matching tool_result error must have been pushed for
+			// the truncated call's ID instead of letting it execute.
+			const truncatedCallResult = pushToolResultSpy.mock.calls.find(
+				([result]) => result.tool_use_id === "call_truncated",
+			)?.[0]
+			expect(truncatedCallResult).toMatchObject({
+				type: "tool_result",
+				tool_use_id: "call_truncated",
+				is_error: true,
+			})
+			expect(JSON.stringify(truncatedCallResult)).toContain("missing nativeArgs")
+		})
 	})
 
 	describe("constructor", () => {

src/core/task/__tests__/truncated-native-tool-args.spec.ts

@@ -0,0 +1,113 @@
+/**
+ * Regression test for issue #1221: truncated tool-call arguments can be silently
+ * written to disk.
+ *
+ * When a streamed native tool call's arguments are cut off mid-value (e.g. the
+ * model hits max_tokens while still writing write_to_file's `content` string),
+ * NativeToolCallParser.finalizeStreamingToolCall() returns null. Task.ts
+ * (~line 3748) reuses the same tool-use object the streaming phase was mutating
+ * in place and only sets `partial = false` - before the fix it left `nativeArgs`
+ * (built from the incomplete partial parse) untouched.
+ *
+ * presentAssistantMessage.ts (~line 443) is supposed to short-circuit exactly
+ * this case with a structured tool_result instead of executing the tool - but
+ * its guard is `isKnownTool && !block.nativeArgs && !customTool`. With
+ * nativeArgs still populated, the guard never fired and the truncated content
+ * would be passed straight to write_to_file's execution path.
+ *
+ * The fix clears `existingToolUse.nativeArgs` alongside `partial = false` at
+ * the finalize-null site, so the pre-existing guard actually does what its own
+ * comment already claimed.
+ */
+
+import { isValidToolName } from "../../tools/validateToolUse"
+import type { ToolUse, WriteToFileToolUse } from "../../../shared/tools"
+
+describe("Truncated native tool-call args on finalize failure (issue #1221)", () => {
+	/**
+	 * Simulates the finalize-null branch from Task.ts (~line 3748) as it exists
+	 * after the fix: on finalizeStreamingToolCall() returning null, mark the
+	 * tool non-partial and clear nativeArgs.
+	 */
+	function finalizeNullBranch(existingToolUse: ToolUse): ToolUse {
+		existingToolUse.partial = false
+		existingToolUse.nativeArgs = undefined
+		return existingToolUse
+	}
+
+	/**
+	 * Simulates the finalize-null branch as it existed *before* the fix, for a
+	 * companion test proving the old behavior really was the bug (not just an
+	 * assumption).
+	 */
+	function finalizeNullBranchBeforeFix(existingToolUse: ToolUse): ToolUse {
+		existingToolUse.partial = false
+		return existingToolUse
+	}
+
+	/**
+	 * Simulates the short-circuit guard from presentAssistantMessage.ts (~line
+	 * 443): `isKnownTool && !block.nativeArgs && !customTool`. Returns true when
+	 * the tool call would be blocked (a structured tool_result emitted, no
+	 * execution), false when it would proceed to execution.
+	 */
+	function wouldBeBlocked(block: ToolUse, customTool: unknown = undefined): boolean {
+		const isKnownTool = isValidToolName(String(block.name))
+		return Boolean(isKnownTool && !block.nativeArgs && !customTool)
+	}
+
+	it("clears nativeArgs so a truncated write_to_file call is blocked instead of executed", () => {
+		// A write_to_file call whose `content` was cut off mid-stream - exactly
+		// the scenario in #1221. The streaming phase already populated nativeArgs
+		// from the incomplete partial-json parse before finalize failed.
+		const truncated: WriteToFileToolUse = {
+			type: "tool_use",
+			name: "write_to_file",
+			params: {},
+			partial: true,
+			nativeArgs: { path: "src/config.json", content: '{"apiKey": "sk-live-abc123' /* cut off mid-string */ },
+		}
+
+		finalizeNullBranch(truncated)
+
+		expect(truncated.partial).toBe(false)
+		expect(truncated.nativeArgs).toBeUndefined()
+		expect(wouldBeBlocked(truncated)).toBe(true)
+	})
+
+	it("companion: without the fix, the same truncated call would NOT have been blocked", () => {
+		const truncated: WriteToFileToolUse = {
+			type: "tool_use",
+			name: "write_to_file",
+			params: {},
+			partial: true,
+			nativeArgs: { path: "src/config.json", content: '{"apiKey": "sk-live-abc123' },
+		}
+
+		finalizeNullBranchBeforeFix(truncated)
+
+		// This is the bug: partial is false (presented as "complete"), but
+		// nativeArgs still carries the truncated value, so the guard's
+		// `!block.nativeArgs` never becomes true and the call would proceed to
+		// execution with the truncated content.
+		expect(truncated.partial).toBe(false)
+		expect(truncated.nativeArgs).toEqual({ path: "src/config.json", content: '{"apiKey": "sk-live-abc123' })
+		expect(wouldBeBlocked(truncated)).toBe(false)
+	})
+
+	it("does not affect a normally-finalized (non-null) tool call", () => {
+		// When finalizeStreamingToolCall() succeeds, Task.ts replaces the block
+		// with the freshly-finalized one instead of taking this branch at all -
+		// this test just confirms a complete, valid nativeArgs is never touched
+		// by wouldBeBlocked's guard simulation.
+		const complete: WriteToFileToolUse = {
+			type: "tool_use",
+			name: "write_to_file",
+			params: {},
+			partial: false,
+			nativeArgs: { path: "src/config.json", content: '{"apiKey": "sk-live-abc123xyz"}' },
+		}
+
+		expect(wouldBeBlocked(complete)).toBe(false)
+	})
+})