diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 2f1222f8ee..242d2a8612 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -106,11 +106,14 @@ export interface ExtensionMessage { | "skills" | "rules" | "fileContent" + | "originalContent" | "rooHistoryImportProgress" | "themeFixtureProbeRequest" text?: string /** For fileContent: { path, content, error? } */ fileContent?: { path: string; content: string | null; error?: string } + /** For originalContent: the pre-edit file content of the requested tool message (null when unavailable) */ + originalContentInfo?: { ts: number; messageId?: string; taskId?: string; content: string | null } payload?: any // eslint-disable-line @typescript-eslint/no-explicit-any checkpointWarning?: { type: "WAIT_TIMEOUT" | "INIT_TIMEOUT" @@ -499,6 +502,7 @@ export interface WebviewMessage { | "saveImage" | "openFile" | "readFileContent" + | "readOriginalContent" | "openMention" | "cancelTask" | "cancelAutoApproval" @@ -697,6 +701,7 @@ export interface WebviewMessage { ids?: string[] terminalOperation?: "continue" | "abort" messageTs?: number + messageId?: string restoreCheckpoint?: boolean historyPreviewCollapsed?: boolean filters?: { type?: string; search?: string; tags?: string[] } @@ -859,6 +864,8 @@ export interface ClineSayTool { content?: string // Original file content before first edit (for merged diff display in FileChangesPanel) originalContent?: string + // Length of the originalContent the extension left out of the webview state (request it with readOriginalContent) + originalContentLength?: number // Unified diff statistics computed by the extension diffStats?: { added: number; removed: number } regex?: string diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 874e2f8f08..65c22fef94 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -131,6 +131,7 @@ import { import { readTaskMessages } from "../task-persistence/taskMessages" import { getNonce } from "./getNonce" import { getUri } from "./getUri" +import { omitOriginalContentFromExtensionMessage } from "./stripOriginalContent" import { REQUESTY_BASE_URL } from "../../shared/utils/requesty" import { validateAndFixToolResultIds } from "../task/validateToolResultIds" import { PendingEditOperationStore, type PendingEditOperationInput } from "./PendingEditOperationStore" @@ -1466,7 +1467,7 @@ export class ClineProvider } try { - await this.view?.webview.postMessage(message) + await this.view?.webview.postMessage(omitOriginalContentFromExtensionMessage(message)) } catch { // View disposed, drop message silently } diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 57fe347acf..e2e44be500 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -841,6 +841,60 @@ describe("ClineProvider", () => { await expect(provider.postMessageToWebview(message)).resolves.toBeUndefined() }) + test("postMessageToWebview leaves originalContent of file-edit tool messages out of the state it posts", async () => { + await provider.resolveWebviewView(mockWebviewView) + + const originalFile = "line of the original file\n".repeat(500) + const toolText = JSON.stringify({ + tool: "appliedDiff", + path: "a.ts", + diff: "@@ d", + originalContent: originalFile, + }) + // Only the field under test is populated; the rest of ExtensionState is irrelevant here. + const message = { + type: "state", + state: { clineMessages: [{ ts: 1, type: "ask", ask: "tool", text: toolText }] }, + } as unknown as ExtensionMessage + + await provider.postMessageToWebview(message) + + const posted = mockPostMessage.mock.calls.at(-1)![0] as ExtensionMessage + const postedText = posted.state!.clineMessages![0]!.text! + + expect(JSON.parse(postedText)).toEqual({ + tool: "appliedDiff", + path: "a.ts", + diff: "@@ d", + originalContentLength: originalFile.length, + }) + // the extension's own message (and so the persisted task) keeps the full content + expect(message.state!.clineMessages![0]!.text).toBe(toolText) + }) + + test("postMessageToWebview leaves originalContent out of messageUpdated", async () => { + await provider.resolveWebviewView(mockWebviewView) + + const toolText = JSON.stringify({ + tool: "appliedDiff", + path: "a.ts", + originalContent: "original file\n".repeat(100), + }) + + await provider.postMessageToWebview({ + type: "messageUpdated", + clineMessage: { ts: 2, type: "ask", ask: "tool", text: toolText }, + }) + + const posted = mockPostMessage.mock.calls.at(-1)![0] as ExtensionMessage + + expect(JSON.parse(posted.clineMessage!.text!)).toEqual({ + tool: "appliedDiff", + path: "a.ts", + originalContentLength: "original file\n".length * 100, + }) + }) + describe("theme fixture probes", () => { const fixture = { themeId: "Default Dark Modern", diff --git a/src/core/webview/__tests__/stripOriginalContent.spec.ts b/src/core/webview/__tests__/stripOriginalContent.spec.ts new file mode 100644 index 0000000000..30c9335e9c --- /dev/null +++ b/src/core/webview/__tests__/stripOriginalContent.spec.ts @@ -0,0 +1,290 @@ +import type { ClineMessage, ExtensionMessage } from "@roo-code/types" + +import { + findOriginalContent, + omitOriginalContent, + omitOriginalContentFromExtensionMessage, +} from "../stripOriginalContent" + +let ts = 0 +const toolAsk = (payload: unknown, extra: Partial = {}): ClineMessage => ({ + ts: ++ts, + type: "ask", + ask: "tool", + text: typeof payload === "string" ? payload : JSON.stringify(payload), + ...extra, +}) + +const approvedAsk = (payload: unknown, extra: Partial = {}): ClineMessage => + toolAsk(payload, { isAnswered: true, ...extra }) + +const bigOriginal = "line of the original file\n".repeat(2000) + +describe("omitOriginalContent", () => { + it("removes a non-empty originalContent and records its length", () => { + const message = toolAsk({ + tool: "appliedDiff", + path: "a.ts", + diff: "@@ d", + content: "patch", + originalContent: bigOriginal, + }) + + const result = omitOriginalContent(message) + const payload = JSON.parse(result.text!) + + expect(payload).toEqual({ + tool: "appliedDiff", + path: "a.ts", + diff: "@@ d", + content: "patch", + originalContentLength: bigOriginal.length, + }) + expect(result.text!.length).toBeLessThan(message.text!.length / 10) + expect(result).not.toBe(message) + expect(result.ts).toBe(message.ts) + }) + + it("does not modify the original message object", () => { + const message = toolAsk({ tool: "appliedDiff", path: "a.ts", originalContent: bigOriginal }) + const before = message.text + + omitOriginalContent(message) + + expect(message.text).toBe(before) + }) + + it("keeps an empty originalContent (a new file) inline", () => { + const message = toolAsk({ tool: "newFileCreated", path: "new.ts", content: "x", originalContent: "" }) + + expect(omitOriginalContent(message)).toBe(message) + }) + + it("works for say tool messages", () => { + const message: ClineMessage = { + ts: ++ts, + type: "say", + say: "tool", + text: JSON.stringify({ tool: "editedExistingFile", path: "a.ts", originalContent: bigOriginal }), + } + + expect(JSON.parse(omitOriginalContent(message).text!).originalContentLength).toBe(bigOriginal.length) + }) + + it("leaves messages without originalContent untouched", () => { + const messages = [ + toolAsk({ tool: "readFile", path: "a.ts" }), + toolAsk({ tool: "appliedDiff", path: "a.ts", diff: "d" }), + { ts: ++ts, type: "say", say: "text", text: "originalContent is only a word here" } as ClineMessage, + { ts: ++ts, type: "ask", ask: "command", text: '{"originalContent":"not a tool message"}' } as ClineMessage, + { ts: ++ts, type: "ask", ask: "tool" } as ClineMessage, + ] + + for (const message of messages) { + expect(omitOriginalContent(message)).toBe(message) + } + }) + + it("leaves unparsable text untouched", () => { + const message = toolAsk('{"tool":"appliedDiff","originalContent":"cut off') + + expect(omitOriginalContent(message)).toBe(message) + }) + + it("leaves a non-string originalContent untouched", () => { + const message = toolAsk({ tool: "appliedDiff", originalContent: 42 }) + + expect(omitOriginalContent(message)).toBe(message) + }) + + it("reuses the result while the text is unchanged and recomputes after an in-place update", () => { + const parse = vi.spyOn(JSON, "parse") + const message = toolAsk({ tool: "appliedDiff", path: "a.ts", originalContent: bigOriginal }) + + const first = omitOriginalContent(message) + const second = omitOriginalContent(message) + + expect(second).toEqual(first) + expect(parse).toHaveBeenCalledTimes(1) + parse.mockRestore() + + // partial messages are updated in place by the task, so a new text must not return a stale result + message.text = JSON.stringify({ tool: "appliedDiff", path: "b.ts", originalContent: bigOriginal + "more" }) + const third = omitOriginalContent(message) + + expect(JSON.parse(third.text!)).toMatchObject({ path: "b.ts", originalContentLength: bigOriginal.length + 4 }) + }) + + it("takes metadata from the current message when the cached text is reused", () => { + const message = toolAsk( + { tool: "appliedDiff", path: "a.ts", originalContent: bigOriginal }, + { partial: false, isAnswered: false }, + ) + + expect(omitOriginalContent(message).isAnswered).toBe(false) + + // approval only flips metadata; the text is unchanged + message.isAnswered = true + message.partial = true + const result = omitOriginalContent(message) + + expect(result).toMatchObject({ isAnswered: true, partial: true }) + expect(JSON.parse(result.text!)).toMatchObject({ originalContentLength: bigOriginal.length }) + expect(result.text).not.toContain(bigOriginal.slice(0, 50)) + }) + + it("is idempotent", () => { + const once = omitOriginalContent(toolAsk({ tool: "appliedDiff", path: "a.ts", originalContent: bigOriginal })) + + expect(omitOriginalContent(once)).toBe(once) + }) +}) + +describe("findOriginalContent", () => { + it("returns the originalContent of the tool message with that ts", () => { + const messages = [ + approvedAsk({ tool: "appliedDiff", path: "a.ts", originalContent: bigOriginal }), + approvedAsk({ tool: "appliedDiff", path: "b.ts", originalContent: "other" }), + ] + + expect(findOriginalContent(messages, { ts: messages[0]!.ts })).toBe(bigOriginal) + expect(findOriginalContent(messages, { ts: messages[1]!.ts })).toBe("other") + }) + + it("tells messages created in the same millisecond apart by messageId", () => { + const first = approvedAsk( + { tool: "appliedDiff", path: "a.ts", originalContent: "first" }, + { messageId: "id-1" }, + ) + const second = approvedAsk( + { tool: "appliedDiff", path: "b.ts", originalContent: "second" }, + { messageId: "id-2", ts: first.ts }, + ) + const messages = [first, second] + + expect(findOriginalContent(messages, { messageId: "id-2", ts: first.ts })).toBe("second") + expect(findOriginalContent(messages, { messageId: "id-1", ts: first.ts })).toBe("first") + expect(findOriginalContent(messages, { messageId: "missing", ts: first.ts })).toBeNull() + // messages persisted without an id are still found by ts + expect(findOriginalContent(messages, { ts: first.ts })).toBe("first") + }) + + it("finds say tool messages too", () => { + const message: ClineMessage = { + ts: ++ts, + type: "say", + say: "tool", + text: JSON.stringify({ tool: "editedExistingFile", originalContent: bigOriginal }), + } + + expect(findOriginalContent([message], { ts: message.ts })).toBe(bigOriginal) + }) + + it("returns null when there is nothing to return", () => { + const noOriginal = toolAsk({ tool: "readFile", path: "a.ts" }) + const notTool: ClineMessage = { + ts: ++ts, + type: "say", + say: "text", + text: JSON.stringify({ originalContent: "x" }), + } + const editPayload = JSON.stringify({ tool: "appliedDiff", originalContent: "x" }) + const sayText: ClineMessage = { ts: ++ts, type: "say", say: "text", text: editPayload } + const askCommand: ClineMessage = { + ts: ++ts, + type: "ask", + ask: "command", + isAnswered: true, + text: editPayload, + } + const unparsable = toolAsk('{"originalContent":"cut off') + const notAString = toolAsk({ tool: "appliedDiff", originalContent: 42 }) + const noText: ClineMessage = { ts: ++ts, type: "ask", ask: "tool" } + const all = [noOriginal, notTool, sayText, askCommand, unparsable, notAString, noText] + + for (const message of all) { + expect(findOriginalContent(all, { ts: message.ts })).toBeNull() + } + + expect(findOriginalContent(all, { ts: -1 })).toBeNull() + expect(findOriginalContent(undefined, { ts: 1 })).toBeNull() + }) + + it("does not disclose the original of a pending or denied approval", () => { + const payload = { tool: "appliedDiff", path: "a.ts", originalContent: bigOriginal } + const pending = toolAsk(payload) + const denied = toolAsk(payload, { isAnswered: false }) + const all = [pending, denied] + + for (const message of all) { + expect(findOriginalContent(all, { ts: message.ts })).toBeNull() + } + }) + + it("does not disclose the original of a partial message", () => { + const message = approvedAsk( + { tool: "appliedDiff", path: "a.ts", originalContent: bigOriginal }, + { partial: true }, + ) + + expect(findOriginalContent([message], { ts: message.ts })).toBeNull() + }) + + it("only discloses the original of file-edit tools", () => { + const message = approvedAsk({ tool: "readFile", path: ".env", originalContent: "SECRET=1" }) + const missingTool = approvedAsk({ path: "a.ts", originalContent: "x" }) + const all = [message, missingTool] + + for (const m of all) { + expect(findOriginalContent(all, { ts: m.ts })).toBeNull() + } + }) + + it("returns an empty original as an empty string", () => { + const message = approvedAsk({ tool: "newFileCreated", content: "x", originalContent: "" }) + + expect(findOriginalContent([message], { ts: message.ts })).toBe("") + }) +}) + +describe("omitOriginalContentFromExtensionMessage", () => { + it("applies to the chat messages inside a state message without touching other state", () => { + const message = { + type: "state", + state: { + version: "1", + mode: "code", + clineMessages: [toolAsk({ tool: "appliedDiff", path: "a.ts", originalContent: bigOriginal })], + }, + } as unknown as ExtensionMessage + + const result = omitOriginalContentFromExtensionMessage(message) + + expect(result.state).toMatchObject({ version: "1", mode: "code" }) + expect(JSON.parse(result.state!.clineMessages![0]!.text!).originalContentLength).toBe(bigOriginal.length) + }) + + it("applies to messageUpdated", () => { + const message = { + type: "messageUpdated", + clineMessage: toolAsk({ tool: "appliedDiff", path: "a.ts", originalContent: bigOriginal }), + } as ExtensionMessage + + const result = omitOriginalContentFromExtensionMessage(message) + + expect(JSON.parse(result.clineMessage!.text!).originalContentLength).toBe(bigOriginal.length) + }) + + it("passes every other message through unchanged", () => { + const others = [ + { type: "action", action: "didBecomeVisible" }, + { type: "state", state: { clineMessages: [] } }, + { type: "state" }, + { type: "fileContent", fileContent: { path: "a", content: bigOriginal } }, + ] as unknown as ExtensionMessage[] + + for (const message of others) { + expect(omitOriginalContentFromExtensionMessage(message)).toBe(message) + } + }) +}) diff --git a/src/core/webview/__tests__/webviewMessageHandler.readOriginalContent.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.readOriginalContent.spec.ts new file mode 100644 index 0000000000..e71a964784 --- /dev/null +++ b/src/core/webview/__tests__/webviewMessageHandler.readOriginalContent.spec.ts @@ -0,0 +1,189 @@ +// npx vitest core/webview/__tests__/webviewMessageHandler.readOriginalContent.spec.ts + +import { describe, it, expect, vi, beforeEach } from "vitest" + +vi.mock("../../../api/providers/fetchers/modelCache") + +vi.mock("vscode", () => ({ + window: { + showInformationMessage: vi.fn(), + showErrorMessage: vi.fn(), + showTextDocument: vi.fn(), + }, + workspace: { + workspaceFolders: [{ uri: { fsPath: "/mock/workspace" } }], + openTextDocument: vi.fn().mockResolvedValue({}), + }, +})) + +vi.mock("../../../i18n", () => ({ + t: vi.fn((key: string) => key), +})) + +vi.mock("fs/promises", () => { + const readFile = vi.fn().mockResolvedValue("file content here") + return { + default: { + rm: vi.fn(), + mkdir: vi.fn(), + readFile, + writeFile: vi.fn(), + }, + rm: vi.fn(), + mkdir: vi.fn(), + readFile, + writeFile: vi.fn(), + } +}) + +vi.mock("../../../utils/fs") +vi.mock("../../../utils/path") +vi.mock("../../../utils/globalContext") + +vi.mock("../../../utils/pathUtils", () => ({ + isPathOutsideWorkspace: vi.fn((filePath: string) => { + const nodePath = require("path") + const normalized = nodePath.resolve(filePath) + const workspaceRoot = nodePath.resolve("/mock/workspace") + // Path is inside workspace if it equals or is under workspace root + if (normalized === workspaceRoot) return false + if (normalized.startsWith(workspaceRoot + nodePath.sep)) return false + return true + }), +})) + +vi.mock("../../mentions/resolveImageMentions", () => ({ + resolveImageMentions: vi.fn(async ({ text, images }: { text: string; images?: string[] }) => ({ + text, + images: [...(images ?? [])], + })), +})) + +import { webviewMessageHandler } from "../webviewMessageHandler" +import type { ClineProvider } from "../ClineProvider" +import type { ClineMessage } from "@roo-code/types" + +const originalFile = "const a = 1\nconst b = 2\n" + +const toolMessage = ( + ts: number, + payload: unknown, + type: "ask" | "say" = "ask", + messageId?: string, + isAnswered = true, +): ClineMessage => + type === "ask" + ? { ts, type: "ask", ask: "tool", text: JSON.stringify(payload), isAnswered, ...(messageId && { messageId }) } + : { ts, type: "say", say: "tool", text: JSON.stringify(payload), ...(messageId && { messageId }) } + +function createProvider(clineMessages: ClineMessage[] | undefined) { + const postMessageToWebview = vi.fn() + // Only the members the handler touches for this message type. + const provider = { + postMessageToWebview, + getCurrentTask: vi.fn().mockReturnValue(clineMessages ? { taskId: "task-1", clineMessages } : undefined), + } as unknown as ClineProvider + + return { provider, postMessageToWebview } +} + +describe("webviewMessageHandler - readOriginalContent", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("answers with the originalContent of the requested tool message", async () => { + const { provider, postMessageToWebview } = createProvider([ + toolMessage(10, { tool: "appliedDiff", path: "a.ts", diff: "d", originalContent: originalFile }), + toolMessage(11, { tool: "appliedDiff", path: "b.ts", diff: "d", originalContent: "other" }), + ]) + + await webviewMessageHandler(provider, { type: "readOriginalContent", messageTs: 10 }) + + expect(postMessageToWebview).toHaveBeenCalledWith({ + type: "originalContent", + originalContentInfo: { ts: 10, messageId: undefined, taskId: undefined, content: originalFile }, + }) + }) + + it("answers with null content for an unknown ts or when there is no current task", async () => { + const unknown = createProvider([toolMessage(40, { tool: "appliedDiff", originalContent: "x" })]) + await webviewMessageHandler(unknown.provider, { type: "readOriginalContent", messageTs: 999 }) + expect(unknown.postMessageToWebview).toHaveBeenCalledWith({ + type: "originalContent", + originalContentInfo: { ts: 999, messageId: undefined, taskId: undefined, content: null }, + }) + + const noTask = createProvider(undefined) + await webviewMessageHandler(noTask.provider, { type: "readOriginalContent", messageTs: 10 }) + expect(noTask.postMessageToWebview).toHaveBeenCalledWith({ + type: "originalContent", + originalContentInfo: { ts: 10, messageId: undefined, taskId: undefined, content: null }, + }) + }) + + it("picks the message by messageId when two messages share a ts", async () => { + const { provider, postMessageToWebview } = createProvider([ + toolMessage(10, { tool: "appliedDiff", path: "a.ts", originalContent: "first" }, "ask", "id-1"), + toolMessage(10, { tool: "appliedDiff", path: "b.ts", originalContent: "second" }, "ask", "id-2"), + ]) + + await webviewMessageHandler(provider, { type: "readOriginalContent", messageTs: 10, messageId: "id-2" }) + + expect(postMessageToWebview).toHaveBeenCalledWith({ + type: "originalContent", + originalContentInfo: { ts: 10, messageId: "id-2", taskId: undefined, content: "second" }, + }) + }) + + it("echoes the task id and answers null for a request made for another task", async () => { + const { provider, postMessageToWebview } = createProvider([ + toolMessage(10, { tool: "appliedDiff", originalContent: originalFile }, "ask", "id-1"), + ]) + + await webviewMessageHandler(provider, { + type: "readOriginalContent", + messageTs: 10, + messageId: "id-1", + taskId: "task-1", + }) + await webviewMessageHandler(provider, { + type: "readOriginalContent", + messageTs: 10, + messageId: "id-1", + taskId: "task-2", + }) + + expect(postMessageToWebview).toHaveBeenNthCalledWith(1, { + type: "originalContent", + originalContentInfo: { ts: 10, messageId: "id-1", taskId: "task-1", content: originalFile }, + }) + expect(postMessageToWebview).toHaveBeenNthCalledWith(2, { + type: "originalContent", + originalContentInfo: { ts: 10, messageId: "id-1", taskId: "task-2", content: null }, + }) + }) + + it("answers null for an unanswered or denied edit approval", async () => { + const { provider, postMessageToWebview } = createProvider([ + toolMessage(10, { tool: "appliedDiff", originalContent: originalFile }, "ask", "id-1", false), + ]) + + await webviewMessageHandler(provider, { type: "readOriginalContent", messageTs: 10, messageId: "id-1" }) + + expect(postMessageToWebview).toHaveBeenCalledWith({ + type: "originalContent", + originalContentInfo: { ts: 10, messageId: "id-1", taskId: undefined, content: null }, + }) + }) + + it("ignores a request without a ts", async () => { + const { provider, postMessageToWebview } = createProvider([ + toolMessage(10, { tool: "appliedDiff", originalContent: "x" }), + ]) + + await webviewMessageHandler(provider, { type: "readOriginalContent" }) + + expect(postMessageToWebview).not.toHaveBeenCalled() + }) +}) diff --git a/src/core/webview/stripOriginalContent.ts b/src/core/webview/stripOriginalContent.ts new file mode 100644 index 0000000000..4358f4d665 --- /dev/null +++ b/src/core/webview/stripOriginalContent.ts @@ -0,0 +1,85 @@ +import type { ClineMessage, ExtensionMessage } from "@roo-code/types" + +// Keyed by message object; only the transformed text is cached (valid while the source text is unchanged), so +// metadata such as `isAnswered` and `partial` is always taken from the current message. +const cache = new WeakMap() + +const FILE_EDIT_TOOLS = new Set(["editedExistingFile", "appliedDiff", "newFileCreated"]) + +function isToolMessage(message: ClineMessage): boolean { + return (message.type === "ask" && message.ask === "tool") || (message.type === "say" && message.say === "tool") +} + +/** Replaces a file-edit tool message's `originalContent` (the whole pre-edit file) with its length. */ +export function omitOriginalContent(message: ClineMessage): ClineMessage { + const text = message.text + + if (typeof text !== "string" || !isToolMessage(message) || !text.includes('"originalContent"')) { + return message + } + + let entry = cache.get(message) + + if (!entry || entry.source !== text) { + entry = { source: text, strippedText: stripOriginalContentFromText(text) } + cache.set(message, entry) + } + + return entry.strippedText === undefined ? message : { ...message, text: entry.strippedText } +} + +function stripOriginalContentFromText(text: string): string | undefined { + try { + const { originalContent, ...rest } = JSON.parse(text) as Record + + // An empty original (new file) is free and still means "has an original" to the webview, so it stays. + if (typeof originalContent === "string" && originalContent.length > 0) { + return JSON.stringify({ ...rest, originalContentLength: originalContent.length }) + } + } catch { + // Not valid JSON (e.g. a truncated partial message): leave it untouched. + } + + return undefined +} + +/** + * The `originalContent` of an approved file-edit tool message, or null when there is none. A pending, denied or + * partial approval never discloses it, since the file has not been authorized for the webview yet. `ts` is not unique (two messages can be + * created in the same millisecond), so `messageId` is preferred; `ts` only serves messages persisted without one. + */ +export function findOriginalContent( + messages: ClineMessage[] | undefined, + id: { messageId?: string; ts: number }, +): string | null { + const message = messages?.find( + (m) => isToolMessage(m) && (id.messageId !== undefined ? m.messageId === id.messageId : m.ts === id.ts), + ) + + if (!message?.text || message.partial || (message.type === "ask" && message.isAnswered !== true)) { + return null + } + + try { + const { tool, originalContent } = JSON.parse(message.text) as { tool?: unknown; originalContent?: unknown } + return FILE_EDIT_TOOLS.has(tool) && typeof originalContent === "string" ? originalContent : null + } catch { + return null + } +} + +/** The webview gets `originalContent` on demand (`readOriginalContent`) instead of inside every chat message. */ +export function omitOriginalContentFromExtensionMessage(message: ExtensionMessage): ExtensionMessage { + if (message.type === "state" && message.state?.clineMessages?.length) { + return { + ...message, + state: { ...message.state, clineMessages: message.state.clineMessages.map(omitOriginalContent) }, + } + } + + if (message.type === "messageUpdated" && message.clineMessage) { + return { ...message, clineMessage: omitOriginalContent(message.clineMessage) } + } + + return message +} diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 4ba94d454c..193540455b 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -40,6 +40,7 @@ import { saveTaskMessages } from "../task-persistence" import { importRooTaskHistory } from "../task-persistence/importRooTaskHistory" import { ClineProvider } from "./ClineProvider" +import { findOriginalContent } from "./stripOriginalContent" import { handleCheckpointRestoreOperation } from "./checkpointRestoreHandler" import { generateErrorDiagnostics } from "./diagnosticsHandler" import { @@ -1617,6 +1618,29 @@ export const webviewMessageHandler = async ( } break } + case "readOriginalContent": { + const ts = message.messageTs + + if (typeof ts !== "number") { + break + } + + const task = provider.getCurrentTask() + // A request made for another task must not be answered from the current one. + const isRequestedTask = message.taskId === undefined || message.taskId === task?.taskId + const id = { messageId: message.messageId, ts } + + await provider.postMessageToWebview({ + type: "originalContent", + originalContentInfo: { + ts, + messageId: message.messageId, + taskId: message.taskId, + content: isRequestedTask ? findOriginalContent(task?.clineMessages, id) : null, + }, + }) + break + } case "openMention": await openMention(getCurrentCwd(), message.text) break diff --git a/webview-ui/src/__tests__/FileChangesPanel.spec.tsx b/webview-ui/src/__tests__/FileChangesPanel.spec.tsx index 2208bc127d..7b74a12bc2 100644 --- a/webview-ui/src/__tests__/FileChangesPanel.spec.tsx +++ b/webview-ui/src/__tests__/FileChangesPanel.spec.tsx @@ -1,4 +1,5 @@ import React from "react" +import { act } from "@testing-library/react" import { fireEvent, render, screen } from "@/utils/test-utils" import type { ClineMessage } from "@roo-code/types" import { TranslationProvider } from "@/i18n/__mocks__/TranslationContext" @@ -28,15 +29,18 @@ vi.mock("react-i18next", () => ({ vi.mock("@src/components/common/CodeAccordion", () => ({ default: ({ path, + code, isExpanded, onToggleExpand, }: { path?: string + code?: string isExpanded: boolean onToggleExpand: () => void }) => (
{path} +
{code}
@@ -64,10 +68,10 @@ function createFileEditMessage( } } -function renderPanel(messages: ClineMessage[] | undefined) { +function renderPanel(messages: ClineMessage[] | undefined, taskId?: string) { return render( - + , ) } @@ -196,4 +200,256 @@ describe("FileChangesPanel", () => { expect(screen.getByTestId("total-added")).toHaveTextContent("+5") expect(screen.getByTestId("total-removed")).toHaveTextContent("-6") }) + describe("original content omitted by the extension", () => { + const TS = 1234 + + function createEditWithOriginal(payload: Record): ClineMessage { + return { + type: "ask", + ask: "tool", + ts: TS, + partial: false, + isAnswered: true, + text: JSON.stringify({ + tool: "appliedDiff", + path: "src/foo.ts", + diff: "the recorded diff", + ...payload, + }), + } + } + + function expandRow() { + fireEvent.click(screen.getByText("1 file(s) changed in this conversation").closest("button")!) + fireEvent.click(screen.getByTestId("accordian-toggle")) + } + + function respond(message: Record) { + act(() => { + window.dispatchEvent(new MessageEvent("message", { data: message })) + }) + } + + const requestsOfType = (type: string) => + mockPostMessage.mock.calls.map(([m]) => m).filter((m: { type: string }) => m.type === type) + + it("requests nothing until a row is expanded, then asks for the final and the original content", () => { + renderPanel([createEditWithOriginal({ originalContentLength: 5000 })]) + fireEvent.click(screen.getByText("1 file(s) changed in this conversation").closest("button")!) + + expect(mockPostMessage).not.toHaveBeenCalled() + + fireEvent.click(screen.getByTestId("accordian-toggle")) + + expect(requestsOfType("readFileContent")).toEqual([{ type: "readFileContent", text: "src/foo.ts" }]) + expect(requestsOfType("readOriginalContent")).toEqual([ + { type: "readOriginalContent", messageTs: TS, messageId: undefined, taskId: undefined }, + ]) + }) + + it("keeps an empty inline original (new file) without requesting it", () => { + renderPanel([createEditWithOriginal({ tool: "newFileCreated", originalContent: "" })]) + expandRow() + + respond({ type: "fileContent", fileContent: { path: "src/foo.ts", content: "new line\n" } }) + + expect(requestsOfType("readOriginalContent")).toEqual([]) + expect(screen.getByTestId("accordian-code")).toHaveTextContent("+new line") + expect(screen.getByTestId("accordian-code")).not.toHaveTextContent("the recorded diff") + }) + + it("shows the merged diff once both the original and the final content arrive", () => { + renderPanel([createEditWithOriginal({ originalContentLength: 5000 })]) + expandRow() + + expect(screen.getByTestId("accordian-code")).toHaveTextContent("the recorded diff") + + respond({ type: "fileContent", fileContent: { path: "src/foo.ts", content: "new line\n" } }) + expect(screen.getByTestId("accordian-code")).toHaveTextContent("the recorded diff") + + respond({ type: "originalContent", originalContentInfo: { ts: TS, content: "old line\n" } }) + expect(screen.getByTestId("accordian-code")).toHaveTextContent("-old line") + expect(screen.getByTestId("accordian-code")).toHaveTextContent("+new line") + }) + + it("keeps the recorded diff when the original cannot be loaded", () => { + renderPanel([createEditWithOriginal({ originalContentLength: 5000 })]) + expandRow() + + respond({ type: "fileContent", fileContent: { path: "src/foo.ts", content: "new line\n" } }) + respond({ type: "originalContent", originalContentInfo: { ts: TS, content: null } }) + + expect(screen.getByTestId("accordian-code")).toHaveTextContent("the recorded diff") + expect(requestsOfType("readOriginalContent")).toHaveLength(1) + }) + + it("does not request the original when it is already inline", () => { + renderPanel([createEditWithOriginal({ originalContent: "old line\n" })]) + expandRow() + + expect(requestsOfType("readOriginalContent")).toHaveLength(0) + + respond({ type: "fileContent", fileContent: { path: "src/foo.ts", content: "new line\n" } }) + expect(screen.getByTestId("accordian-code")).toHaveTextContent("-old line") + expect(screen.getByTestId("accordian-code")).toHaveTextContent("+new line") + }) + + it("requests nothing for an edit that has no original", () => { + renderPanel([createEditWithOriginal({})]) + expandRow() + + expect(mockPostMessage).not.toHaveBeenCalled() + }) + + it("requests and matches originals by messageId when messages share a ts", () => { + const edit = (path: string, messageId: string): ClineMessage => ({ + ...createEditWithOriginal({ path, originalContentLength: 5000 }), + messageId, + }) + renderPanel([edit("src/a.ts", "id-a"), edit("src/b.ts", "id-b")], "task-1") + fireEvent.click(screen.getByText("2 file(s) changed in this conversation").closest("button")!) + screen.getAllByTestId("accordian-toggle").forEach((toggle) => fireEvent.click(toggle)) + + expect(requestsOfType("readOriginalContent")).toEqual([ + { type: "readOriginalContent", messageTs: TS, messageId: "id-a", taskId: "task-1" }, + { type: "readOriginalContent", messageTs: TS, messageId: "id-b", taskId: "task-1" }, + ]) + + respond({ type: "fileContent", fileContent: { path: "src/a.ts", content: "new a\n" } }) + respond({ type: "fileContent", fileContent: { path: "src/b.ts", content: "new b\n" } }) + respond({ + type: "originalContent", + originalContentInfo: { ts: TS, messageId: "id-b", taskId: "task-1", content: "old b\n" }, + }) + + const [a, b] = screen.getAllByTestId("accordian-code") + expect(a).toHaveTextContent("the recorded diff") + expect(b).toHaveTextContent("-old b") + }) + + it("requests the original again when the task id arrives after a request is already pending", () => { + const messages = [createEditWithOriginal({ originalContentLength: 5000 })] + const { rerender } = renderPanel(messages) + expandRow() + expect(requestsOfType("readOriginalContent")).toHaveLength(1) + + rerender( + + + , + ) + fireEvent.click(screen.getByTestId("accordian-toggle")) + + const requests = requestsOfType("readOriginalContent") + expect(requests).toHaveLength(2) + expect(requests[1]).toMatchObject({ taskId: "task-1" }) + }) + + it("does not send a duplicate request when switching A -> B -> A before the first response arrives", () => { + const messages = [createEditWithOriginal({ originalContentLength: 5000 })] + const panel = (taskId: string) => ( + + + + ) + const { rerender } = renderPanel(messages, "task-A") + expandRow() + expect(requestsOfType("readOriginalContent")).toHaveLength(1) + + rerender(panel("task-B")) + rerender(panel("task-A")) + fireEvent.click(screen.getByTestId("accordian-toggle")) + + expect(requestsOfType("readOriginalContent").filter((m) => m.taskId === "task-A")).toHaveLength(1) + + respond({ type: "fileContent", fileContent: { path: "src/foo.ts", content: "new line\n" } }) + respond({ + type: "originalContent", + originalContentInfo: { ts: TS, taskId: "task-A", content: "old line\n" }, + }) + expect(screen.getByTestId("accordian-code")).toHaveTextContent("-old line") + }) + + it("requests again after a response that arrived for a task that is no longer current", () => { + const messages = [createEditWithOriginal({ originalContentLength: 5000 })] + const panel = (taskId: string) => ( + + + + ) + const { rerender } = renderPanel(messages, "task-A") + expandRow() + + rerender(panel("task-B")) + respond({ + type: "originalContent", + originalContentInfo: { ts: TS, taskId: "task-A", content: "old line\n" }, + }) + rerender(panel("task-A")) + fireEvent.click(screen.getByTestId("accordian-toggle")) + + expect(requestsOfType("readOriginalContent").filter((m) => m.taskId === "task-A")).toHaveLength(2) + }) + + it("does not request for rows expanded under the previous task when the task id changes", () => { + const messages = [createEditWithOriginal({ originalContentLength: 5000 })] + const { rerender } = renderPanel(messages, "task-A") + expandRow() + mockPostMessage.mockClear() + + rerender( + + + , + ) + + expect(mockPostMessage).not.toHaveBeenCalled() + }) + + it("keeps a loaded original when the messages are replaced by an update of the same task", () => { + const first = [createEditWithOriginal({ originalContentLength: 5000 })] + const { rerender } = renderPanel(first, "task-A") + expandRow() + respond({ type: "fileContent", fileContent: { path: "src/foo.ts", content: "new line\n" } }) + respond({ + type: "originalContent", + originalContentInfo: { ts: TS, taskId: "task-A", content: "old line\n" }, + }) + expect(requestsOfType("readOriginalContent")).toHaveLength(1) + + rerender( + + + , + ) + fireEvent.click(screen.getByTestId("accordian-toggle")) + respond({ type: "fileContent", fileContent: { path: "src/foo.ts", content: "new line\n" } }) + + expect(requestsOfType("readOriginalContent")).toHaveLength(1) + expect(screen.getByTestId("accordian-code")).toHaveTextContent("-old line") + }) + + it("ignores an original answered for a different task", () => { + renderPanel([createEditWithOriginal({ originalContentLength: 5000 })], "task-1") + expandRow() + + respond({ type: "fileContent", fileContent: { path: "src/foo.ts", content: "new line\n" } }) + respond({ + type: "originalContent", + originalContentInfo: { ts: TS, taskId: "task-2", content: "old line\n" }, + }) + + expect(screen.getByTestId("accordian-code")).toHaveTextContent("the recorded diff") + }) + + it("ignores an original for a different message", () => { + renderPanel([createEditWithOriginal({ originalContentLength: 5000 })]) + expandRow() + + respond({ type: "fileContent", fileContent: { path: "src/foo.ts", content: "new line\n" } }) + respond({ type: "originalContent", originalContentInfo: { ts: TS + 1, content: "old line\n" } }) + + expect(screen.getByTestId("accordian-code")).toHaveTextContent("the recorded diff") + }) + }) }) diff --git a/webview-ui/src/__tests__/fileChangesFromMessages.spec.ts b/webview-ui/src/__tests__/fileChangesFromMessages.spec.ts index 8fab8b14d5..b09f4978c4 100644 --- a/webview-ui/src/__tests__/fileChangesFromMessages.spec.ts +++ b/webview-ui/src/__tests__/fileChangesFromMessages.spec.ts @@ -105,6 +105,8 @@ describe("fileChangesFromMessages", () => { path: "src/foo.ts", diff: "@@ -1 +1 @@\n+line", diffStats: { added: 1, removed: 0 }, + ts: messages[0].ts, + hasOriginalContent: false, }) }) @@ -190,7 +192,7 @@ describe("fileChangesFromMessages", () => { ] const result = fileChangesFromMessages(messages) expect(result).toHaveLength(2) - expect(result[0]).toEqual({ path: "a.ts", diff: "content a" }) + expect(result[0]).toEqual({ path: "a.ts", diff: "content a", ts: messages[0].ts, hasOriginalContent: false }) expect(result[1].path).toBe("b.ts") expect(result[1].diff).toBe("content b") }) @@ -277,4 +279,44 @@ describe("fileChangesFromMessages", () => { ] expect(fileChangesFromMessages(messages)).toEqual([]) }) + describe("original content (omitted by the extension to keep the webview small)", () => { + const editMessage = (payload: Record) => + msg({ + type: "ask", + ask: "tool", + isAnswered: true, + text: JSON.stringify({ tool: "appliedDiff", path: "src/foo.ts", diff: "d", ...payload }), + }) + + it("keeps an inline originalContent and marks the entry as having one", () => { + const message = editMessage({ originalContent: "old file" }) + const [entry] = fileChangesFromMessages([message]) + + expect(entry.originalContent).toBe("old file") + expect(entry.hasOriginalContent).toBe(true) + expect(entry.ts).toBe(message.ts) + }) + + it("marks an entry whose originalContent was omitted (originalContentLength) as requestable", () => { + const message = editMessage({ originalContentLength: 4096 }) + const [entry] = fileChangesFromMessages([message]) + + expect(entry.originalContent).toBeUndefined() + expect(entry.hasOriginalContent).toBe(true) + expect(entry.ts).toBe(message.ts) + }) + + it("treats an empty inline originalContent (a new file) as an original", () => { + const [entry] = fileChangesFromMessages([editMessage({ originalContent: "" })]) + + expect(entry.originalContent).toBe("") + expect(entry.hasOriginalContent).toBe(true) + }) + + it("marks an entry without any original as not having one", () => { + const [entry] = fileChangesFromMessages([editMessage({})]) + + expect(entry.hasOriginalContent).toBe(false) + }) + }) }) diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 4694eeabf7..c4c4369ad1 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -1738,7 +1738,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction
- + {areButtonsVisible && (
{ +// `ts` is not unique, so the message's own id identifies it; `ts` only covers messages persisted without one. +const originalKey = (entry: { messageId?: string; ts: number }) => entry.messageId ?? `ts:${entry.ts}` + +const FileChangesPanel = memo(({ clineMessages, taskId, className }: FileChangesPanelProps) => { const { t } = useTranslation() const [panelExpanded, setPanelExpanded] = useState(false) const [expandedPaths, setExpandedPaths] = useState>(new Set()) const [finalContentByPath, setFinalContentByPath] = useState>({}) const pendingPathsRef = useRef>(new Set()) + // The extension omits `originalContent` from the messages it posts; it is requested when a row is expanded. + const [originalContentByKey, setOriginalContentByKey] = useState>({}) + // In-flight requests, keyed by task and message. Deliberately not reset with the caches below: a request cannot + // be cancelled, so a reset would let a task switch (A -> B -> A) send a duplicate while the first is still open. + const pendingOriginalRequestsRef = useRef>(new Set()) + + // Task the expanded rows belong to; the reset below only lands after the render in which `taskId` changed, so + // the request effect must not act on rows expanded under the previous task. + const expandedTaskIdRef = useRef(taskId) - // Reset expanded file rows and final content cache when switching to a different task + // Reset expanded file rows and final content cache when the messages change useEffect(() => { setExpandedPaths(new Set()) setFinalContentByPath({}) pendingPathsRef.current = new Set() - }, [clineMessages]) + }, [clineMessages, taskId]) + + // Originals are keyed by message id, which is stable across message updates, so they only reset per task + useEffect(() => { + setOriginalContentByKey({}) + }, [taskId]) const fileChanges = useMemo(() => fileChangesFromMessages(clineMessages), [clineMessages]) @@ -56,34 +74,54 @@ const FileChangesPanel = memo(({ clineMessages, className }: FileChangesPanelPro ) }, [fileChanges]) - const togglePath = useCallback((path: string) => { - setExpandedPaths((prev) => { - const next = new Set(prev) - if (next.has(path)) next.delete(path) - else next.add(path) - return next - }) - }, []) + const togglePath = useCallback( + (path: string) => { + expandedTaskIdRef.current = taskId + setExpandedPaths((prev) => { + const next = new Set(prev) + if (next.has(path)) next.delete(path) + else next.add(path) + return next + }) + }, + [taskId], + ) - // Request final file content when a row is expanded and we have originalContent + // Request the final file content (and the omitted original content) when a row is expanded and the edit has an original useEffect(() => { + if (expandedTaskIdRef.current !== taskId) return for (const path of expandedPaths) { const entries = byPath.get(path) if (!entries?.length) continue - const originalContent = entries[0].originalContent + const first = entries[0] const lookupPath = path.startsWith("./") ? path.slice(2) : path if ( - originalContent !== undefined && + first.hasOriginalContent && !(lookupPath in finalContentByPath) && !pendingPathsRef.current.has(lookupPath) ) { pendingPathsRef.current.add(lookupPath) vscode.postMessage({ type: "readFileContent", text: lookupPath }) } + const requestKey = `${taskId ?? ""}|${originalKey(first)}` + if ( + first.hasOriginalContent && + first.originalContent === undefined && + !(originalKey(first) in originalContentByKey) && + !pendingOriginalRequestsRef.current.has(requestKey) + ) { + pendingOriginalRequestsRef.current.add(requestKey) + vscode.postMessage({ + type: "readOriginalContent", + messageTs: first.ts, + messageId: first.messageId, + taskId, + }) + } } - }, [expandedPaths, byPath, finalContentByPath]) + }, [expandedPaths, byPath, finalContentByPath, originalContentByKey, taskId]) - // Listen for fileContent responses + // Listen for fileContent and originalContent responses useEffect(() => { const handler = (event: MessageEvent) => { const message: ExtensionMessage = event.data @@ -91,11 +129,18 @@ const FileChangesPanel = memo(({ clineMessages, className }: FileChangesPanelPro const fc = message.fileContent pendingPathsRef.current.delete(fc.path) setFinalContentByPath((prev) => ({ ...prev, [fc.path]: fc.content ?? null })) + } else if (message.type === "originalContent" && message.originalContentInfo) { + const { taskId: responseTaskId, content, ...id } = message.originalContentInfo + const key = originalKey(id) + pendingOriginalRequestsRef.current.delete(`${responseTaskId ?? ""}|${key}`) + // A late response for another task must not populate this task's cache. + if (responseTaskId !== taskId) return + setOriginalContentByKey((prev) => ({ ...prev, [key]: content })) } } window.addEventListener("message", handler) return () => window.removeEventListener("message", handler) - }, []) + }, [taskId]) if (fileChanges.length === 0) return null @@ -133,7 +178,8 @@ const FileChangesPanel = memo(({ clineMessages, className }: FileChangesPanelPro
{Array.from(byPath.entries()).map(([path, entries]) => { - const originalContent = entries[0].originalContent + const originalContent = + entries[0].originalContent ?? originalContentByKey[originalKey(entries[0])] ?? undefined const lookupPath = path.startsWith("./") ? path.slice(2) : path const finalContent = finalContentByPath[lookupPath] const hasMergedDiff = diff --git a/webview-ui/src/components/chat/utils/fileChangesFromMessages.ts b/webview-ui/src/components/chat/utils/fileChangesFromMessages.ts index 738305ad15..35c55e496d 100644 --- a/webview-ui/src/components/chat/utils/fileChangesFromMessages.ts +++ b/webview-ui/src/components/chat/utils/fileChangesFromMessages.ts @@ -10,6 +10,11 @@ export interface FileChangeEntry { diffStats?: { added: number; removed: number } /** Original file content before first edit (for merged diff display) */ originalContent?: string + /** Identity of the message this entry comes from; used to request `originalContent` when the extension omitted it */ + ts: number + messageId?: string + /** True when the edit has an original (inline, or omitted by the extension and requestable by `ts`) */ + hasOriginalContent: boolean } /** @@ -44,6 +49,9 @@ export function fileChangesFromMessages(messages: ClineMessage[] | undefined): F path: file.path, diff: content, diffStats: file.diffStats, + ts: msg.ts, + messageId: msg.messageId, + hasOriginalContent: false, }) } } @@ -59,6 +67,9 @@ export function fileChangesFromMessages(messages: ClineMessage[] | undefined): F diff, diffStats: tool.diffStats, originalContent: tool.originalContent, + ts: msg.ts, + messageId: msg.messageId, + hasOriginalContent: tool.originalContent !== undefined || (tool.originalContentLength ?? 0) > 0, }) } }