From ed4173625ed3d6a4816845601aa54b106bc0448b Mon Sep 17 00:00:00 2001 From: Thomas Brugman Date: Tue, 13 May 2025 13:58:51 +0200 Subject: [PATCH 1/4] Fix: Allow write_to_file to handle newline-only and empty content --- src/core/tools/writeToFileTool.ts | 72 ++++++++++++++++++++----------- 1 file changed, 46 insertions(+), 26 deletions(-) diff --git a/src/core/tools/writeToFileTool.ts b/src/core/tools/writeToFileTool.ts index d4469e9099..0c9a7a896a 100644 --- a/src/core/tools/writeToFileTool.ts +++ b/src/core/tools/writeToFileTool.ts @@ -8,7 +8,7 @@ import { formatResponse } from "../prompts/responses" import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../shared/tools" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" import { fileExistsAtPath } from "../../utils/fs" -import { stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text" +import { addLineNumbers, stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text" import { getReadablePath } from "../../utils/path" import { isPathOutsideWorkspace } from "../../utils/pathUtils" import { detectCodeOmission } from "../../integrations/editor/detect-omission" @@ -26,28 +26,12 @@ export async function writeToFileTool( let newContent: string | undefined = block.params.content let predictedLineCount: number | undefined = parseInt(block.params.line_count ?? "0") - if (block.partial && (!relPath || newContent === undefined)) { + if (!relPath || newContent === undefined) { // checking for newContent ensure relPath is complete // wait so we can determine if it's a new file or editing an existing file return } - if (!relPath) { - cline.consecutiveMistakeCount++ - cline.recordToolError("write_to_file") - pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "path")) - await cline.diffViewProvider.reset() - return - } - - if (newContent === undefined) { - cline.consecutiveMistakeCount++ - cline.recordToolError("write_to_file") - pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "content")) - await cline.diffViewProvider.reset() - return - } - const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath) if (!accessAllowed) { @@ -73,11 +57,11 @@ export async function writeToFileTool( // pre-processing newContent for cases where weaker models might add artifacts like markdown codeblock markers (deepseek/llama) or extra escape characters (gemini) if (newContent.startsWith("```")) { // cline handles cases where it includes language specifiers like ```python ```js - newContent = newContent.split("\n").slice(1).join("\n").trim() + newContent = newContent.split("\n").slice(1).join("\n") } if (newContent.endsWith("```")) { - newContent = newContent.split("\n").slice(0, -1).join("\n").trim() + newContent = newContent.split("\n").slice(0, -1).join("\n") } if (!cline.api.getModel().id.includes("claude")) { @@ -116,7 +100,23 @@ export async function writeToFileTool( return } else { - if (predictedLineCount === undefined) { + if (!relPath) { + cline.consecutiveMistakeCount++ + cline.recordToolError("write_to_file") + pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "path")) + await cline.diffViewProvider.reset() + return + } + + if (newContent === undefined) { + cline.consecutiveMistakeCount++ + cline.recordToolError("write_to_file") + pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "content")) + await cline.diffViewProvider.reset() + return + } + + if (!predictedLineCount) { cline.consecutiveMistakeCount++ cline.recordToolError("write_to_file") @@ -212,8 +212,7 @@ export async function writeToFileTool( return } - // Call saveChanges to update the DiffViewProvider properties - await cline.diffViewProvider.saveChanges() + const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges() // Track file edit operation if (relPath) { @@ -222,10 +221,31 @@ export async function writeToFileTool( cline.didEditFile = true // used to determine if we should wait for busy terminal to update before sending api request - // Get the formatted response message - const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists) + if (userEdits) { + await cline.say( + "user_feedback_diff", + JSON.stringify({ + tool: fileExists ? "editedExistingFile" : "newFileCreated", + path: getReadablePath(cline.cwd, relPath), + diff: userEdits, + } satisfies ClineSayTool), + ) - pushToolResult(message) + pushToolResult( + `The user made the following updates to your content:\n\n${userEdits}\n\n` + + `The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file, including line numbers:\n\n` + + `\n${addLineNumbers( + finalContent || "", + )}\n\n\n` + + `Please note:\n` + + `1. You do not need to re-write the file with these changes, as they have already been applied.\n` + + `2. Proceed with the task using this updated file content as the new baseline.\n` + + `3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` + + `${newProblemsMessage}`, + ) + } else { + pushToolResult(`The content was successfully saved to ${relPath.toPosix()}.${newProblemsMessage}`) + } await cline.diffViewProvider.reset() From f7f9c24f31e7adaaba4d9cdd0fdee47d5abcea9d Mon Sep 17 00:00:00 2001 From: Daniel Riccio Date: Mon, 16 Jun 2025 19:01:05 -0500 Subject: [PATCH 2/4] fix: update writeToFileTool to return early without error on missing or empty parameters --- .../tools/__tests__/writeToFileTool.test.ts | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/core/tools/__tests__/writeToFileTool.test.ts b/src/core/tools/__tests__/writeToFileTool.test.ts index e0789f766c..0f8ff32ae6 100644 --- a/src/core/tools/__tests__/writeToFileTool.test.ts +++ b/src/core/tools/__tests__/writeToFileTool.test.ts @@ -401,31 +401,37 @@ describe("writeToFileTool", () => { }) describe("parameter validation", () => { - it("errors and resets on missing path parameter", async () => { + it("returns early without error on missing path parameter", async () => { await executeWriteFileTool({ path: undefined }) - expect(mockCline.consecutiveMistakeCount).toBe(1) - expect(mockCline.recordToolError).toHaveBeenCalledWith("write_to_file") - expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("write_to_file", "path") - expect(mockCline.diffViewProvider.reset).toHaveBeenCalled() + // With the new behavior, it should return early without errors + expect(mockCline.consecutiveMistakeCount).toBe(0) + expect(mockCline.recordToolError).not.toHaveBeenCalled() + expect(mockCline.sayAndCreateMissingParamError).not.toHaveBeenCalled() + expect(mockCline.diffViewProvider.reset).not.toHaveBeenCalled() + expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled() }) - it("errors and resets on empty path parameter", async () => { + it("returns early without error on empty path parameter", async () => { await executeWriteFileTool({ path: "" }) - expect(mockCline.consecutiveMistakeCount).toBe(1) - expect(mockCline.recordToolError).toHaveBeenCalledWith("write_to_file") - expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("write_to_file", "path") - expect(mockCline.diffViewProvider.reset).toHaveBeenCalled() + // Empty string is falsy in the context of !relPath check, so it returns early + expect(mockCline.consecutiveMistakeCount).toBe(0) + expect(mockCline.recordToolError).not.toHaveBeenCalled() + expect(mockCline.sayAndCreateMissingParamError).not.toHaveBeenCalled() + expect(mockCline.diffViewProvider.reset).not.toHaveBeenCalled() + expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled() }) - it("errors and resets on missing content parameter", async () => { + it("returns early without error on missing content parameter", async () => { await executeWriteFileTool({ content: undefined }) - expect(mockCline.consecutiveMistakeCount).toBe(1) - expect(mockCline.recordToolError).toHaveBeenCalledWith("write_to_file") - expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("write_to_file", "content") - expect(mockCline.diffViewProvider.reset).toHaveBeenCalled() + // With the new behavior, it should return early without errors + expect(mockCline.consecutiveMistakeCount).toBe(0) + expect(mockCline.recordToolError).not.toHaveBeenCalled() + expect(mockCline.sayAndCreateMissingParamError).not.toHaveBeenCalled() + expect(mockCline.diffViewProvider.reset).not.toHaveBeenCalled() + expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled() }) }) }) From d24d3b1af3ed0a7127da3af6ad16a66b12cdc11a Mon Sep 17 00:00:00 2001 From: Daniel Riccio Date: Mon, 16 Jun 2025 21:01:02 -0500 Subject: [PATCH 3/4] fix: preserve newlines in content parameters and update error handling in writeToFileTool tests --- .../parseAssistantMessage.ts | 14 ++-- .../parseAssistantMessageV2.ts | 25 +++---- .../tools/__tests__/writeToFileTool.test.ts | 36 ++++------ src/core/tools/writeToFileTool.ts | 68 +++++++------------ src/integrations/editor/DiffViewProvider.ts | 10 +-- 5 files changed, 64 insertions(+), 89 deletions(-) diff --git a/src/core/assistant-message/parseAssistantMessage.ts b/src/core/assistant-message/parseAssistantMessage.ts index ae848e0ae7..d3d2ae4419 100644 --- a/src/core/assistant-message/parseAssistantMessage.ts +++ b/src/core/assistant-message/parseAssistantMessage.ts @@ -24,7 +24,10 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag const paramClosingTag = `` if (currentParamValue.endsWith(paramClosingTag)) { // End of param value. - currentToolUse.params[currentParamName] = currentParamValue.slice(0, -paramClosingTag.length).trim() + // Don't trim content parameters to preserve newlines + const paramValue = currentParamValue.slice(0, -paramClosingTag.length) + currentToolUse.params[currentParamName] = + currentParamName === "content" ? paramValue : paramValue.trim() currentParamName = undefined continue } else { @@ -72,9 +75,8 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag const contentEndIndex = toolContent.lastIndexOf(contentEndTag) if (contentStartIndex !== -1 && contentEndIndex !== -1 && contentEndIndex > contentStartIndex) { - currentToolUse.params[contentParamName] = toolContent - .slice(contentStartIndex, contentEndIndex) - .trim() + // Don't trim content to preserve newlines + currentToolUse.params[contentParamName] = toolContent.slice(contentStartIndex, contentEndIndex) } } @@ -138,7 +140,9 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag // Stream did not complete tool call, add it as partial. if (currentParamName) { // Tool call has a parameter that was not completed. - currentToolUse.params[currentParamName] = accumulator.slice(currentParamValueStartIndex).trim() + // Don't trim content parameters to preserve newlines + const paramValue = accumulator.slice(currentParamValueStartIndex) + currentToolUse.params[currentParamName] = currentParamName === "content" ? paramValue : paramValue.trim() } contentBlocks.push(currentToolUse) diff --git a/src/core/assistant-message/parseAssistantMessageV2.ts b/src/core/assistant-message/parseAssistantMessageV2.ts index 6d3594cf60..4c58ff1c21 100644 --- a/src/core/assistant-message/parseAssistantMessageV2.ts +++ b/src/core/assistant-message/parseAssistantMessageV2.ts @@ -76,13 +76,12 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess ) ) { // Found the closing tag for the parameter. - const value = assistantMessage - .slice( - currentParamValueStart, // Start after the opening tag. - currentCharIndex - closeTag.length + 1, // End before the closing tag. - ) - .trim() - currentToolUse.params[currentParamName] = value + const value = assistantMessage.slice( + currentParamValueStart, // Start after the opening tag. + currentCharIndex - closeTag.length + 1, // End before the closing tag. + ) + // Don't trim content parameters to preserve newlines + currentToolUse.params[currentParamName] = currentParamName === "content" ? value : value.trim() currentParamName = undefined // Go back to parsing tool content. // We don't continue loop here, need to check for tool close or other params at index i. } else { @@ -146,10 +145,8 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess const contentEnd = toolContentSlice.lastIndexOf(contentEndTag) if (contentStart !== -1 && contentEnd !== -1 && contentEnd > contentStart) { - const contentValue = toolContentSlice - .slice(contentStart + contentStartTag.length, contentEnd) - .trim() - + // Don't trim content to preserve newlines + const contentValue = toolContentSlice.slice(contentStart + contentStartTag.length, contentEnd) currentToolUse.params[contentParamName] = contentValue } } @@ -251,9 +248,9 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess // Finalize any open parameter within an open tool use. if (currentToolUse && currentParamName) { - currentToolUse.params[currentParamName] = assistantMessage - .slice(currentParamValueStart) // From param start to end of string. - .trim() + const value = assistantMessage.slice(currentParamValueStart) // From param start to end of string. + // Don't trim content parameters to preserve newlines + currentToolUse.params[currentParamName] = currentParamName === "content" ? value : value.trim() // Tool use remains partial. } diff --git a/src/core/tools/__tests__/writeToFileTool.test.ts b/src/core/tools/__tests__/writeToFileTool.test.ts index 0f8ff32ae6..e0789f766c 100644 --- a/src/core/tools/__tests__/writeToFileTool.test.ts +++ b/src/core/tools/__tests__/writeToFileTool.test.ts @@ -401,37 +401,31 @@ describe("writeToFileTool", () => { }) describe("parameter validation", () => { - it("returns early without error on missing path parameter", async () => { + it("errors and resets on missing path parameter", async () => { await executeWriteFileTool({ path: undefined }) - // With the new behavior, it should return early without errors - expect(mockCline.consecutiveMistakeCount).toBe(0) - expect(mockCline.recordToolError).not.toHaveBeenCalled() - expect(mockCline.sayAndCreateMissingParamError).not.toHaveBeenCalled() - expect(mockCline.diffViewProvider.reset).not.toHaveBeenCalled() - expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled() + expect(mockCline.consecutiveMistakeCount).toBe(1) + expect(mockCline.recordToolError).toHaveBeenCalledWith("write_to_file") + expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("write_to_file", "path") + expect(mockCline.diffViewProvider.reset).toHaveBeenCalled() }) - it("returns early without error on empty path parameter", async () => { + it("errors and resets on empty path parameter", async () => { await executeWriteFileTool({ path: "" }) - // Empty string is falsy in the context of !relPath check, so it returns early - expect(mockCline.consecutiveMistakeCount).toBe(0) - expect(mockCline.recordToolError).not.toHaveBeenCalled() - expect(mockCline.sayAndCreateMissingParamError).not.toHaveBeenCalled() - expect(mockCline.diffViewProvider.reset).not.toHaveBeenCalled() - expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled() + expect(mockCline.consecutiveMistakeCount).toBe(1) + expect(mockCline.recordToolError).toHaveBeenCalledWith("write_to_file") + expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("write_to_file", "path") + expect(mockCline.diffViewProvider.reset).toHaveBeenCalled() }) - it("returns early without error on missing content parameter", async () => { + it("errors and resets on missing content parameter", async () => { await executeWriteFileTool({ content: undefined }) - // With the new behavior, it should return early without errors - expect(mockCline.consecutiveMistakeCount).toBe(0) - expect(mockCline.recordToolError).not.toHaveBeenCalled() - expect(mockCline.sayAndCreateMissingParamError).not.toHaveBeenCalled() - expect(mockCline.diffViewProvider.reset).not.toHaveBeenCalled() - expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled() + expect(mockCline.consecutiveMistakeCount).toBe(1) + expect(mockCline.recordToolError).toHaveBeenCalledWith("write_to_file") + expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("write_to_file", "content") + expect(mockCline.diffViewProvider.reset).toHaveBeenCalled() }) }) }) diff --git a/src/core/tools/writeToFileTool.ts b/src/core/tools/writeToFileTool.ts index 0c9a7a896a..84f8ef807e 100644 --- a/src/core/tools/writeToFileTool.ts +++ b/src/core/tools/writeToFileTool.ts @@ -8,7 +8,7 @@ import { formatResponse } from "../prompts/responses" import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../shared/tools" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" import { fileExistsAtPath } from "../../utils/fs" -import { addLineNumbers, stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text" +import { stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text" import { getReadablePath } from "../../utils/path" import { isPathOutsideWorkspace } from "../../utils/pathUtils" import { detectCodeOmission } from "../../integrations/editor/detect-omission" @@ -26,12 +26,28 @@ export async function writeToFileTool( let newContent: string | undefined = block.params.content let predictedLineCount: number | undefined = parseInt(block.params.line_count ?? "0") - if (!relPath || newContent === undefined) { + if (block.partial && (!relPath || newContent === undefined)) { // checking for newContent ensure relPath is complete // wait so we can determine if it's a new file or editing an existing file return } + if (!relPath) { + cline.consecutiveMistakeCount++ + cline.recordToolError("write_to_file") + pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "path")) + await cline.diffViewProvider.reset() + return + } + + if (newContent === undefined) { + cline.consecutiveMistakeCount++ + cline.recordToolError("write_to_file") + pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "content")) + await cline.diffViewProvider.reset() + return + } + const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath) if (!accessAllowed) { @@ -100,23 +116,7 @@ export async function writeToFileTool( return } else { - if (!relPath) { - cline.consecutiveMistakeCount++ - cline.recordToolError("write_to_file") - pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "path")) - await cline.diffViewProvider.reset() - return - } - - if (newContent === undefined) { - cline.consecutiveMistakeCount++ - cline.recordToolError("write_to_file") - pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "content")) - await cline.diffViewProvider.reset() - return - } - - if (!predictedLineCount) { + if (predictedLineCount === undefined) { cline.consecutiveMistakeCount++ cline.recordToolError("write_to_file") @@ -212,7 +212,8 @@ export async function writeToFileTool( return } - const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges() + // Call saveChanges to update the DiffViewProvider properties + await cline.diffViewProvider.saveChanges() // Track file edit operation if (relPath) { @@ -221,31 +222,10 @@ export async function writeToFileTool( cline.didEditFile = true // used to determine if we should wait for busy terminal to update before sending api request - if (userEdits) { - await cline.say( - "user_feedback_diff", - JSON.stringify({ - tool: fileExists ? "editedExistingFile" : "newFileCreated", - path: getReadablePath(cline.cwd, relPath), - diff: userEdits, - } satisfies ClineSayTool), - ) + // Get the formatted response message + const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists) - pushToolResult( - `The user made the following updates to your content:\n\n${userEdits}\n\n` + - `The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file, including line numbers:\n\n` + - `\n${addLineNumbers( - finalContent || "", - )}\n\n\n` + - `Please note:\n` + - `1. You do not need to re-write the file with these changes, as they have already been applied.\n` + - `2. Proceed with the task using this updated file content as the new baseline.\n` + - `3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` + - `${newProblemsMessage}`, - ) - } else { - pushToolResult(`The content was successfully saved to ${relPath.toPosix()}.${newProblemsMessage}`) - } + pushToolResult(message) await cline.diffViewProvider.reset() diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index b97886d32d..46423cfe14 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -129,7 +129,8 @@ export class DiffViewProvider { // Replace all content up to the current line with accumulated lines. const edit = new vscode.WorkspaceEdit() const rangeToReplace = new vscode.Range(0, 0, endLine, 0) - const contentToReplace = accumulatedLines.slice(0, endLine + 1).join("\n") + "\n" + const contentToReplace = + accumulatedLines.slice(0, endLine).join("\n") + (accumulatedLines.length > 0 ? "\n" : "") edit.replace(document.uri, rangeToReplace, this.stripAllBOMs(contentToReplace)) await vscode.workspace.applyEdit(edit) // Update decorations. @@ -229,12 +230,11 @@ export class DiffViewProvider { // show a diff with all the EOL differences. const newContentEOL = this.newContent.includes("\r\n") ? "\r\n" : "\n" - // `trimEnd` to fix issue where editor adds in extra new line - // automatically. - const normalizedEditedContent = editedContent.replace(/\r\n|\n/g, newContentEOL).trimEnd() + newContentEOL + // Normalize EOL characters without trimming content + const normalizedEditedContent = editedContent.replace(/\r\n|\n/g, newContentEOL) // Just in case the new content has a mix of varying EOL characters. - const normalizedNewContent = this.newContent.replace(/\r\n|\n/g, newContentEOL).trimEnd() + newContentEOL + const normalizedNewContent = this.newContent.replace(/\r\n|\n/g, newContentEOL) if (normalizedEditedContent !== normalizedNewContent) { // User made changes before approving edit. From fda09c0b7094833851034745970798b058b5fecb Mon Sep 17 00:00:00 2001 From: Daniel Riccio Date: Mon, 23 Jun 2025 17:35:51 -0500 Subject: [PATCH 4/4] fix: update parseAssistantMessage and parseAssistantMessageV2 to preserve newlines in content parameters while stripping leading and trailing newlines --- .../assistant-message/parseAssistantMessage.ts | 18 ++++++++++++------ .../parseAssistantMessageV2.ts | 17 +++++++++++------ 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/core/assistant-message/parseAssistantMessage.ts b/src/core/assistant-message/parseAssistantMessage.ts index d3d2ae4419..ebb8674c8f 100644 --- a/src/core/assistant-message/parseAssistantMessage.ts +++ b/src/core/assistant-message/parseAssistantMessage.ts @@ -24,10 +24,12 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag const paramClosingTag = `` if (currentParamValue.endsWith(paramClosingTag)) { // End of param value. - // Don't trim content parameters to preserve newlines + // Don't trim content parameters to preserve newlines, but strip first and last newline only const paramValue = currentParamValue.slice(0, -paramClosingTag.length) currentToolUse.params[currentParamName] = - currentParamName === "content" ? paramValue : paramValue.trim() + currentParamName === "content" + ? paramValue.replace(/^\n/, "").replace(/\n$/, "") + : paramValue.trim() currentParamName = undefined continue } else { @@ -75,8 +77,11 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag const contentEndIndex = toolContent.lastIndexOf(contentEndTag) if (contentStartIndex !== -1 && contentEndIndex !== -1 && contentEndIndex > contentStartIndex) { - // Don't trim content to preserve newlines - currentToolUse.params[contentParamName] = toolContent.slice(contentStartIndex, contentEndIndex) + // Don't trim content to preserve newlines, but strip first and last newline only + currentToolUse.params[contentParamName] = toolContent + .slice(contentStartIndex, contentEndIndex) + .replace(/^\n/, "") + .replace(/\n$/, "") } } @@ -140,9 +145,10 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag // Stream did not complete tool call, add it as partial. if (currentParamName) { // Tool call has a parameter that was not completed. - // Don't trim content parameters to preserve newlines + // Don't trim content parameters to preserve newlines, but strip first and last newline only const paramValue = accumulator.slice(currentParamValueStartIndex) - currentToolUse.params[currentParamName] = currentParamName === "content" ? paramValue : paramValue.trim() + currentToolUse.params[currentParamName] = + currentParamName === "content" ? paramValue.replace(/^\n/, "").replace(/\n$/, "") : paramValue.trim() } contentBlocks.push(currentToolUse) diff --git a/src/core/assistant-message/parseAssistantMessageV2.ts b/src/core/assistant-message/parseAssistantMessageV2.ts index 4c58ff1c21..7c7526cbdb 100644 --- a/src/core/assistant-message/parseAssistantMessageV2.ts +++ b/src/core/assistant-message/parseAssistantMessageV2.ts @@ -80,8 +80,9 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess currentParamValueStart, // Start after the opening tag. currentCharIndex - closeTag.length + 1, // End before the closing tag. ) - // Don't trim content parameters to preserve newlines - currentToolUse.params[currentParamName] = currentParamName === "content" ? value : value.trim() + // Don't trim content parameters to preserve newlines, but strip first and last newline only + currentToolUse.params[currentParamName] = + currentParamName === "content" ? value.replace(/^\n/, "").replace(/\n$/, "") : value.trim() currentParamName = undefined // Go back to parsing tool content. // We don't continue loop here, need to check for tool close or other params at index i. } else { @@ -145,8 +146,11 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess const contentEnd = toolContentSlice.lastIndexOf(contentEndTag) if (contentStart !== -1 && contentEnd !== -1 && contentEnd > contentStart) { - // Don't trim content to preserve newlines - const contentValue = toolContentSlice.slice(contentStart + contentStartTag.length, contentEnd) + // Don't trim content to preserve newlines, but strip first and last newline only + const contentValue = toolContentSlice + .slice(contentStart + contentStartTag.length, contentEnd) + .replace(/^\n/, "") + .replace(/\n$/, "") currentToolUse.params[contentParamName] = contentValue } } @@ -249,8 +253,9 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess // Finalize any open parameter within an open tool use. if (currentToolUse && currentParamName) { const value = assistantMessage.slice(currentParamValueStart) // From param start to end of string. - // Don't trim content parameters to preserve newlines - currentToolUse.params[currentParamName] = currentParamName === "content" ? value : value.trim() + // Don't trim content parameters to preserve newlines, but strip first and last newline only + currentToolUse.params[currentParamName] = + currentParamName === "content" ? value.replace(/^\n/, "").replace(/\n$/, "") : value.trim() // Tool use remains partial. }