From af01da5fb7cb501854dd2fa4487fc3f89c233598 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Tue, 10 Nov 2026 23:49:12 +0000 Subject: [PATCH 01/15] fix(mentions): restrict expansion to authorized user input --- .../processUserContentMentions.spec.ts | 380 +++--------------- .../mentions/processUserContentMentions.ts | 127 +----- src/core/task/Task.ts | 21 +- src/core/task/__tests__/Task.spec.ts | 13 +- src/core/tools/AskFollowupQuestionTool.ts | 2 + src/core/tools/AttemptCompletionTool.ts | 2 + .../__tests__/askFollowupQuestionTool.spec.ts | 1 + .../__tests__/attemptCompletionTool.spec.ts | 1 + src/core/webview/ClineProvider.ts | 13 +- .../webview/__tests__/ClineProvider.spec.ts | 7 +- 10 files changed, 110 insertions(+), 457 deletions(-) diff --git a/src/core/mentions/__tests__/processUserContentMentions.spec.ts b/src/core/mentions/__tests__/processUserContentMentions.spec.ts index 1995c36bc0..1a83df9361 100644 --- a/src/core/mentions/__tests__/processUserContentMentions.spec.ts +++ b/src/core/mentions/__tests__/processUserContentMentions.spec.ts @@ -1,345 +1,73 @@ -// npx vitest core/mentions/__tests__/processUserContentMentions.spec.ts - -import { processUserContentMentions } from "../processUserContentMentions" +import type Anthropic from "@anthropic-ai/sdk" +import { processUserContentMentions, userMentionContent } from "../processUserContentMentions" import { parseMentions } from "../index" import { FileContextTracker } from "../../context-tracking/FileContextTracker" -// Mock the parseMentions function -vi.mock("../index", () => ({ - parseMentions: vi.fn(), -})) - -describe("processUserContentMentions", () => { - let mockFileContextTracker: FileContextTracker +vi.mock("../index", () => ({ parseMentions: vi.fn() })) +describe("processUserContentMentions authorization", () => { beforeEach(() => { vi.clearAllMocks() - - mockFileContextTracker = {} as FileContextTracker - - // Default mock implementation - returns ParseMentionsResult object vi.mocked(parseMentions).mockImplementation(async (text) => ({ text: `parsed: ${text}`, - mode: undefined, - contentBlocks: [], + contentBlocks: [{ type: "file", path: "allowed", content: "file contents" }], + mode: "code", + slashCommandHelp: "command help", })) }) - describe("content processing", () => { - it("should process text blocks with tags", async () => { - const userContent = [ - { - type: "text" as const, - text: "Do something", - }, - ] - - const result = await processUserContentMentions({ - userContent, - cwd: "/test", - fileContextTracker: mockFileContextTracker, - }) - - expect(parseMentions).toHaveBeenCalled() - expect(result.content[0]).toEqual({ - type: "text", - text: "parsed: Do something", - }) - expect(result.mode).toBeUndefined() - }) - - it("should not process text blocks without user_message tags", async () => { - const userContent = [ - { - type: "text" as const, - text: "Regular text without special tags", - }, - ] - - const result = await processUserContentMentions({ - userContent, - cwd: "/test", - fileContextTracker: mockFileContextTracker, - }) - - expect(parseMentions).not.toHaveBeenCalled() - expect(result.content[0]).toEqual(userContent[0]) - expect(result.mode).toBeUndefined() - }) - - it("should process tool_result blocks with string content", async () => { - const userContent = [ - { - type: "tool_result" as const, - tool_use_id: "123", - content: "Tool feedback", - }, - ] - - const result = await processUserContentMentions({ - userContent, - cwd: "/test", - fileContextTracker: mockFileContextTracker, - }) - - expect(parseMentions).toHaveBeenCalled() - // String content is now converted to array format to support content blocks - expect(result.content[0]).toEqual({ - type: "tool_result", - tool_use_id: "123", - content: [ - { - type: "text", - text: "parsed: Tool feedback", - }, - ], - }) - expect(result.mode).toBeUndefined() - }) - - it("should process tool_result blocks with array content", async () => { - const userContent = [ - { - type: "tool_result" as const, - tool_use_id: "123", - content: [ - { - type: "text" as const, - text: "Array task", - }, - { - type: "text" as const, - text: "Regular text", - }, - ], - }, - ] - - const result = await processUserContentMentions({ - userContent, - cwd: "/test", - fileContextTracker: mockFileContextTracker, - }) - - expect(parseMentions).toHaveBeenCalledTimes(1) - expect(result.content[0]).toEqual({ - type: "tool_result", - tool_use_id: "123", - content: [ - { - type: "text", - text: "parsed: Array task", - }, - { - type: "text", - text: "Regular text", - }, - ], - }) - expect(result.mode).toBeUndefined() - }) - - it("should handle mixed content types (text + image)", async () => { - const userContent = [ - { - type: "text" as const, - text: "First task", - }, - { - type: "image" as const, - image: "base64data", - mediaType: "image/png", - }, - ] - - const result = await processUserContentMentions({ - userContent: userContent as any, - cwd: "/test", - fileContextTracker: mockFileContextTracker, - }) - - expect(parseMentions).toHaveBeenCalledTimes(1) - expect(result.content).toHaveLength(2) - expect(result.content[0]).toEqual({ - type: "text", - text: "parsed: First task", - }) - expect(result.content[1]).toEqual(userContent[1]) // Image block unchanged - expect(result.mode).toBeUndefined() - }) + const process = (userContent: Anthropic.Messages.ContentBlockParam[]) => + processUserContentMentions({ userContent, cwd: "/test", fileContextTracker: {} as FileContextTracker }) + + it.each([ + { type: "text" as const, text: "@/secret @terminal" }, + { type: "tool_result" as const, tool_use_id: "1", content: "@/secret @terminal" }, + { + type: "tool_result" as const, + tool_use_id: "1", + content: [{ type: "text" as const, text: "@/secret @terminal" }], + }, + ])("does not expand untrusted content: $type", async (block) => { + const result = await process([block]) + expect(parseMentions).not.toHaveBeenCalled() + expect(result).toEqual({ content: [block], mode: undefined }) }) - describe("showRooIgnoredFiles parameter", () => { - it("should default showRooIgnoredFiles to false", async () => { - const userContent = [ - { - type: "text" as const, - text: "Test default", - }, - ] - - await processUserContentMentions({ - userContent, - cwd: "/test", - fileContextTracker: mockFileContextTracker, - }) - - expect(parseMentions).toHaveBeenCalledWith( - "Test default", - "/test", - mockFileContextTracker, - undefined, - false, // showRooIgnoredFiles should default to false - true, // includeDiagnosticMessages - 50, // maxDiagnosticMessages - undefined, - "code", - ) - }) - - it("should respect showRooIgnoredFiles when explicitly set to false", async () => { - const userContent = [ - { - type: "text" as const, - text: "Test explicit false", - }, - ] - - await processUserContentMentions({ - userContent, - cwd: "/test", - fileContextTracker: mockFileContextTracker, - showRooIgnoredFiles: false, - }) - - expect(parseMentions).toHaveBeenCalledWith( - "Test explicit false", - "/test", - mockFileContextTracker, - undefined, - false, - true, // includeDiagnosticMessages - 50, // maxDiagnosticMessages - undefined, - "code", - ) + it("expands explicit user input without requiring a textual marker", async () => { + const result = await process([userMentionContent("@/allowed")]) + expect(parseMentions).toHaveBeenCalledWith( + "@/allowed", + "/test", + expect.anything(), + undefined, + false, + true, + 50, + undefined, + "code", + ) + expect(result).toEqual({ + content: [ + { type: "text", text: "parsed: @/allowed" }, + { type: "text", text: "file contents" }, + { type: "text", text: "command help" }, + ], + mode: "code", }) }) - describe("slash command content processing", () => { - it("should separate slash command content into a new block", async () => { - vi.mocked(parseMentions).mockResolvedValueOnce({ - text: "parsed text", - slashCommandHelp: "command help", - mode: undefined, - contentBlocks: [], - }) - - const userContent = [ - { - type: "text" as const, - text: "Run command", - }, - ] - - const result = await processUserContentMentions({ - userContent, - cwd: "/test", - fileContextTracker: mockFileContextTracker, - }) - - expect(result.content).toHaveLength(2) - expect(result.content[0]).toEqual({ - type: "text", - text: "parsed text", - }) - expect(result.content[1]).toEqual({ - type: "text", - text: "command help", - }) - }) - - it("should include slash command content in tool_result string content", async () => { - vi.mocked(parseMentions).mockResolvedValueOnce({ - text: "parsed tool output", - slashCommandHelp: "command help", - mode: undefined, - contentBlocks: [], - }) - - const userContent = [ - { - type: "tool_result" as const, - tool_use_id: "123", - content: "Tool output", - }, - ] - - const result = await processUserContentMentions({ - userContent, - cwd: "/test", - fileContextTracker: mockFileContextTracker, - }) - - expect(result.content).toHaveLength(1) - expect(result.content[0]).toEqual({ - type: "tool_result", - tool_use_id: "123", - content: [ - { - type: "text", - text: "parsed tool output", - }, - { - type: "text", - text: "command help", - }, - ], - }) - }) - - it("should include slash command content in tool_result array content", async () => { - vi.mocked(parseMentions).mockResolvedValueOnce({ - text: "parsed array item", - slashCommandHelp: "command help", - mode: undefined, - contentBlocks: [], - }) - - const userContent = [ - { - type: "tool_result" as const, - tool_use_id: "123", - content: [ - { - type: "text" as const, - text: "Array item", - }, - ], - }, - ] - - const result = await processUserContentMentions({ - userContent, - cwd: "/test", - fileContextTracker: mockFileContextTracker, - }) + it("expands only the user-authored portion of a support prompt", async () => { + const context = "@/secret @terminal" + const result = await process([userMentionContent(`@/allowed\n${context}`, "@/allowed")]) + expect(vi.mocked(parseMentions).mock.calls.map(([text]) => text)).toEqual(["@/allowed"]) + expect(result.content[0]).toEqual({ type: "text", text: `parsed: @/allowed\n${context}` }) + }) - expect(result.content).toHaveLength(1) - expect(result.content[0]).toEqual({ - type: "tool_result", - tool_use_id: "123", - content: [ - { - type: "text", - text: "parsed array item", - }, - { - type: "text", - text: "command help", - }, - ], - }) - }) + it("does not trust restored, copied, or modified blocks", async () => { + const block = userMentionContent("@/allowed") + const copy = { ...block } + block.text += " @/secret" + await process([copy, block, userMentionContent("context @/secret", "")]) + expect(parseMentions).not.toHaveBeenCalled() }) }) diff --git a/src/core/mentions/processUserContentMentions.ts b/src/core/mentions/processUserContentMentions.ts index 0a8ad40dfa..55071058c6 100644 --- a/src/core/mentions/processUserContentMentions.ts +++ b/src/core/mentions/processUserContentMentions.ts @@ -8,6 +8,16 @@ import type { SkillLookup } from "../../services/skills/skillInvocation" type TextPart = Anthropic.Messages.TextBlockParam type ImagePart = Anthropic.Messages.ImageBlockParam +// Authorization stays in memory and cannot be forged by message text or restored history. +const authorizedInputs = new WeakMap() + +/** Call only at a user-input boundary, before adding lower-trust context. */ +export function userMentionContent(text: string, input: string = text): TextPart { + const block: TextPart = { type: "text", text } + if (input && text.includes(input)) authorizedInputs.set(block, { text, input }) + return block +} + export interface ProcessUserContentMentionsResult { content: Anthropic.Messages.ContentBlockParam[] mode?: string // Mode from the first slash command that has one @@ -26,7 +36,7 @@ function contentBlocksToTextParts(contentBlocks: MentionContentBlock[]): TextPar } /** - * Process mentions in user content, specifically within task and feedback tags. + * Process mentions only in explicitly authorized user input. * * File/folder @ mentions are now returned as separate text blocks that * look like read_file tool results, making it clear to the model that @@ -57,16 +67,15 @@ export async function processUserContentMentions({ let commandMode: string | undefined // Process userContent array, which contains text and image parts. - // We need to apply parseMentions() to TextPart's text that contains "". + // Tool results and unmarked text never authorize resource access. const content = ( await Promise.all( userContent.map(async (block) => { - const shouldProcessMentions = (text: string) => text.includes("") - if (block.type === "text") { - if (shouldProcessMentions(block.text)) { + const authorized = authorizedInputs.get(block) + if (authorized && authorized.text === block.text) { const result = await parseMentions( - block.text, + authorized.input, cwd, fileContextTracker, rooIgnoreController, @@ -88,7 +97,7 @@ export async function processUserContentMentions({ const blocks: Array = [ { ...block, - text: result.text, + text: block.text.replace(authorized.input, () => result.text), }, ] @@ -106,110 +115,6 @@ export async function processUserContentMentions({ return blocks } - return block - } else if (block.type === "tool_result") { - if (typeof block.content === "string") { - if (shouldProcessMentions(block.content)) { - const result = await parseMentions( - block.content, - cwd, - fileContextTracker, - rooIgnoreController, - showRooIgnoredFiles, - includeDiagnosticMessages, - maxDiagnosticMessages, - skillsManager, - currentMode, - ) - // Capture the first mode found - if (!commandMode && result.mode) { - commandMode = result.mode - } - - // Build content array with file blocks included - const contentParts: Array<{ type: "text"; text: string }> = [ - { - type: "text" as const, - text: result.text, - }, - ] - - // Add file/folder content blocks - for (const contentBlock of result.contentBlocks) { - contentParts.push({ - type: "text" as const, - text: contentBlock.content, - }) - } - - if (result.slashCommandHelp) { - contentParts.push({ - type: "text" as const, - text: result.slashCommandHelp, - }) - } - - return { - ...block, - content: contentParts, - } - } - - return block - } else if (Array.isArray(block.content)) { - const parsedContent = ( - await Promise.all( - block.content.map(async (contentBlock) => { - if (contentBlock.type === "text" && shouldProcessMentions(contentBlock.text)) { - const result = await parseMentions( - contentBlock.text, - cwd, - fileContextTracker, - rooIgnoreController, - showRooIgnoredFiles, - includeDiagnosticMessages, - maxDiagnosticMessages, - skillsManager, - currentMode, - ) - // Capture the first mode found - if (!commandMode && result.mode) { - commandMode = result.mode - } - - // Build blocks array with file content - const blocks: Array<{ type: "text"; text: string }> = [ - { - ...contentBlock, - text: result.text, - }, - ] - - // Add file/folder content blocks - for (const cb of result.contentBlocks) { - blocks.push({ - type: "text" as const, - text: cb.content, - }) - } - - if (result.slashCommandHelp) { - blocks.push({ - type: "text" as const, - text: result.slashCommandHelp, - }) - } - return blocks - } - - return contentBlock - }), - ) - ).flat() - - return { ...block, content: parsedContent } - } - return block } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index aa6026e56b..1a15e55a5a 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -134,7 +134,7 @@ import { checkpointRestore, checkpointDiff, } from "../checkpoints" -import { processUserContentMentions } from "../mentions/processUserContentMentions" +import { processUserContentMentions, userMentionContent } from "../mentions/processUserContentMentions" import { getMessagesSinceLastSummary, summarizeConversation, getEffectiveApiHistory } from "../condense" import { MessageQueueService } from "../message-queue/MessageQueueService" import { type AutoDenyDetail, AutoApprovalHandler, checkAutoApproval } from "../auto-approval" @@ -254,6 +254,8 @@ export interface TaskOptions extends CreateTaskOptions { checkpointTimeout?: number consecutiveMistakeLimit?: number task?: string + /** User-authored portion of a generated support prompt; empty disables expansion. */ + authorizedMentionText?: string images?: string[] historyItem?: HistoryItem experiments?: Record @@ -693,7 +695,10 @@ export class Task extends EventEmitter implements TaskLike { // MessageManager for high-level message operations (lazy initialized) private _messageManager?: MessageManager + private readonly authorizedMentionText?: string + constructor({ + authorizedMentionText, provider, apiConfiguration, enableCheckpoints = true, @@ -718,6 +723,8 @@ export class Task extends EventEmitter implements TaskLike { handoffExecutionContext, }: TaskOptions) { super() + this.authorizedMentionText = authorizedMentionText + this.resetAssistantMessagePersistence() if (startTask && !task && !images && !historyItem) { @@ -3158,10 +3165,7 @@ export class Task extends EventEmitter implements TaskLike { // Task starting await this.initiateTaskLoop([ - { - type: "text", - text: `\n${task}\n`, - }, + userMentionContent(`\n${task}\n`, this.authorizedMentionText ?? task), ...imageBlocks, ]).catch((error) => { // Swallow loop rejection when the task was intentionally abandoned/aborted @@ -3423,10 +3427,9 @@ export class Task extends EventEmitter implements TaskLike { const newUserContent: Anthropic.Messages.ContentBlockParam[] = [...modifiedOldUserContent] if (responseText) { - newUserContent.push({ - type: "text", - text: `\n${responseText}\n`, - }) + newUserContent.push( + userMentionContent(`\n${responseText}\n`, responseText), + ) } if (responseImages && responseImages.length > 0) { diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index c56d5291f6..db70600094 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -32,7 +32,7 @@ import { ClineProvider } from "../../webview/ClineProvider" import { ApiStreamChunk } from "../../../api/transform/stream" import { OutputTokenLimitError } from "../../../api/providers/utils/output-token-limit-error" import { ContextProxy } from "../../config/ContextProxy" -import { processUserContentMentions } from "../../mentions/processUserContentMentions" +import { processUserContentMentions, userMentionContent } from "../../mentions/processUserContentMentions" import { MultiSearchReplaceDiffStrategy } from "../../diff/strategies/multi-search-replace" import type { ApiMessage } from "../../task-persistence" import type { RequestPolicySnapshot } from "../../prompts/tools/effective-tool-policy" @@ -3523,10 +3523,9 @@ describe("Cline", () => { type: "text", text: "Regular text with 'some/path' (see below for file content)", } as const, - { - type: "text", - text: "Text with 'some/path' (see below for file content) in user_message tags", - } as const, + userMentionContent( + "Text with 'some/path' (see below for file content) in user_message tags", + ), { type: "tool_result", tool_use_id: "test-id", @@ -3566,10 +3565,10 @@ describe("Cline", () => { "Text with 'some/path' (see below for file content) in user_message tags", ) - // user_message tag content should be processed + // Tool result markers cannot authorize mention expansion const toolResult1 = processedContent[2] as Anthropic.ToolResultBlockParam const content1 = Array.isArray(toolResult1.content) ? toolResult1.content[0] : toolResult1.content - expect((content1 as Anthropic.TextBlockParam).text).toContain("processed:") + expect((content1 as Anthropic.TextBlockParam).text).not.toContain("processed:") expect((content1 as Anthropic.TextBlockParam).text).toContain( "Check 'some/path' (see below for file content)", ) diff --git a/src/core/tools/AskFollowupQuestionTool.ts b/src/core/tools/AskFollowupQuestionTool.ts index 3baea7ed52..24262fe4dc 100644 --- a/src/core/tools/AskFollowupQuestionTool.ts +++ b/src/core/tools/AskFollowupQuestionTool.ts @@ -1,3 +1,4 @@ +import { userMentionContent } from "../mentions/processUserContentMentions" import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import type { ToolUse } from "../../shared/tools" @@ -73,6 +74,7 @@ export class AskFollowupQuestionTool extends BaseTool<"ask_followup_question"> { const { text, images } = await task.ask("followup", JSON.stringify(follow_up_json), false) const safeText = text ?? "" await task.say("user_feedback", safeText, images) + task.userMessageContent.push(userMentionContent(`\n${safeText}\n`, safeText)) pushToolResult(formatResponse.toolResult(`\n${safeText}\n`, images)) } catch (error) { await handleError("asking question", error as Error) diff --git a/src/core/tools/AttemptCompletionTool.ts b/src/core/tools/AttemptCompletionTool.ts index 4fe03ce94c..4fea91e243 100644 --- a/src/core/tools/AttemptCompletionTool.ts +++ b/src/core/tools/AttemptCompletionTool.ts @@ -1,3 +1,4 @@ +import { userMentionContent } from "../mentions/processUserContentMentions" import * as vscode from "vscode" import { RooCodeEventName, type HistoryItem, type PendingTaskAction } from "@roo-code/types" @@ -245,6 +246,7 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { } const feedbackText = `\n${text}\n` + task.userMessageContent.push(userMentionContent(feedbackText, text ?? "")) pushToolResult(formatResponse.toolResult(feedbackText, images)) } catch (error) { await handleError("inspecting site", error as Error) diff --git a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts index 965cb23e19..3674f7f2e2 100644 --- a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts +++ b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts @@ -18,6 +18,7 @@ describe("AskFollowupQuestionTool", () => { tool = new AskFollowupQuestionTool() mockTask = { + userMessageContent: [], consecutiveMistakeCount: 0, recordToolError: vi.fn(), didToolFailInCurrentTurn: false, diff --git a/src/core/tools/__tests__/attemptCompletionTool.spec.ts b/src/core/tools/__tests__/attemptCompletionTool.spec.ts index 29580ef203..e5923995c5 100644 --- a/src/core/tools/__tests__/attemptCompletionTool.spec.ts +++ b/src/core/tools/__tests__/attemptCompletionTool.spec.ts @@ -60,6 +60,7 @@ describe("attemptCompletionTool", () => { vi.mocked(vscode.workspace.getConfiguration).mockImplementation(mockGetConfiguration) mockTask = { + userMessageContent: [], consecutiveMistakeCount: 0, recordToolError: vi.fn(), todoList: undefined, diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 6cec3bdbe7..371925421e 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1080,7 +1080,9 @@ export class ClineProvider return } - await this.createTask(prompt) + await this.createTask(prompt, undefined, undefined, { + authorizedMentionText: typeof params.userInput === "string" ? params.userInput : "", + }) } public static async handleTerminalAction( @@ -1110,7 +1112,9 @@ export class ClineProvider } try { - await visibleProvider.createTask(prompt) + await visibleProvider.createTask(prompt, undefined, undefined, { + authorizedMentionText: typeof params.userInput === "string" ? params.userInput : "", + }) } catch (error) { if (error instanceof OrganizationAllowListViolationError) { // Errors from terminal commands seem to get swallowed / ignored. @@ -4268,7 +4272,10 @@ export class ClineProvider text?: string, images?: string[], parentTask?: Task, - options: CreateTaskOptions & { handoffExecutionContext?: DelegatedChildContext } = {}, + options: CreateTaskOptions & { + handoffExecutionContext?: DelegatedChildContext + authorizedMentionText?: string + } = {}, configuration: RooCodeSettings = {}, ): Promise { if (configuration) { diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 464889418e..3ec0aaf5ae 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1017,7 +1017,12 @@ describe("ClineProvider", () => { const target = provider.webviewFocusTracker.getLastActiveProvider()! await target.handleCodeAction(command, promptType, { selectedText: "selected code" }) expect(sidebarTask).toHaveBeenCalledOnce() - expect(sidebarTask).toHaveBeenCalledWith(expect.stringContaining("selected code")) + expect(sidebarTask).toHaveBeenCalledWith( + expect.stringContaining("selected code"), + undefined, + undefined, + { authorizedMentionText: "" }, + ) expect(tabTask).not.toHaveBeenCalled() } finally { sidebarTask.mockRestore() From 6618866b0d8f44a88dc25d4da05ce4dfd65925cc Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sat, 14 Nov 2026 19:59:17 +0000 Subject: [PATCH 02/15] fix(mentions): fail closed on mention authorization --- .../presentAssistantMessage-images.spec.ts | 12 ++- .../processUserContentMentions.spec.ts | 4 +- .../mentions/processUserContentMentions.ts | 2 +- src/core/task/Task.ts | 7 +- src/core/task/__tests__/Task.spec.ts | 8 +- src/core/tools/AskFollowupQuestionTool.ts | 1 + src/core/tools/AttemptCompletionTool.ts | 1 + .../__tests__/askFollowupQuestionTool.spec.ts | 51 ++++++++++- .../__tests__/attemptCompletionTool.spec.ts | 88 +++++++++++++++++++ .../webview/__tests__/ClineProvider.spec.ts | 46 ++++++++++ src/eslint-suppressions.json | 5 -- 11 files changed, 209 insertions(+), 16 deletions(-) diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts index 74949a8742..c2c733d178 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts @@ -381,13 +381,15 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calling", () = await presentAssistantMessage(mockTask, baseSnapshot) - // Anthropic rejects user messages where image blocks sit between - // tool_result blocks, so images from both tools must be grouped - // after the last tool_result instead of interleaved. + // Images must come after all tool_result blocks (#1307). Feedback + // text blocks join the tail in push order. No image may appear + // before the second tool_result. expect(mockTask.userMessageContent.map((block: UserContentBlock) => block.type)).toEqual([ "tool_result", "tool_result", + "text", "image", + "text", "image", ]) expect( @@ -442,10 +444,14 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calling", () = await presentAssistantMessage(mockTask, baseSnapshot) + // The image must come after the last tool_result (#1307), never + // between tool_result blocks. Feedback text joins the tail. expect(mockTask.userMessageContent.map((block: UserContentBlock) => block.type)).toEqual([ "tool_result", "tool_result", + "text", "image", + "text", ]) expect( mockTask.userMessageContent diff --git a/src/core/mentions/__tests__/processUserContentMentions.spec.ts b/src/core/mentions/__tests__/processUserContentMentions.spec.ts index 1a83df9361..de45ee237f 100644 --- a/src/core/mentions/__tests__/processUserContentMentions.spec.ts +++ b/src/core/mentions/__tests__/processUserContentMentions.spec.ts @@ -34,7 +34,7 @@ describe("processUserContentMentions authorization", () => { }) it("expands explicit user input without requiring a textual marker", async () => { - const result = await process([userMentionContent("@/allowed")]) + const result = await process([userMentionContent("@/allowed", "@/allowed")]) expect(parseMentions).toHaveBeenCalledWith( "@/allowed", "/test", @@ -64,7 +64,7 @@ describe("processUserContentMentions authorization", () => { }) it("does not trust restored, copied, or modified blocks", async () => { - const block = userMentionContent("@/allowed") + const block = userMentionContent("@/allowed", "@/allowed") const copy = { ...block } block.text += " @/secret" await process([copy, block, userMentionContent("context @/secret", "")]) diff --git a/src/core/mentions/processUserContentMentions.ts b/src/core/mentions/processUserContentMentions.ts index 55071058c6..0803d4a3df 100644 --- a/src/core/mentions/processUserContentMentions.ts +++ b/src/core/mentions/processUserContentMentions.ts @@ -12,7 +12,7 @@ type ImagePart = Anthropic.Messages.ImageBlockParam const authorizedInputs = new WeakMap() /** Call only at a user-input boundary, before adding lower-trust context. */ -export function userMentionContent(text: string, input: string = text): TextPart { +export function userMentionContent(text: string, input: string): TextPart { const block: TextPart = { type: "text", text } if (input && text.includes(input)) authorizedInputs.set(block, { text, input }) return block diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 1a15e55a5a..473805fbb9 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -3165,7 +3165,12 @@ export class Task extends EventEmitter implements TaskLike { // Task starting await this.initiateTaskLoop([ - userMentionContent(`\n${task}\n`, this.authorizedMentionText ?? task), + // Child tasks get model-written text. Only an explicit + // authorizedMentionText may expand there. + userMentionContent( + `\n${task}\n`, + this.authorizedMentionText ?? (this.parentTask ? "" : (task ?? "")), + ), ...imageBlocks, ]).catch((error) => { // Swallow loop rejection when the task was intentionally abandoned/aborted diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index db70600094..91855fb876 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -3511,7 +3511,7 @@ describe("Cline", () => { }) describe("processUserContentMentions", () => { - it("should process mentions in user_message tags", async () => { + it("expands only the authorized user input and leaves tool results unexpanded", async () => { const [cline, task] = Task.create({ provider: mockProvider, apiConfiguration: mockApiConfig, @@ -3525,6 +3525,7 @@ describe("Cline", () => { } as const, userMentionContent( "Text with 'some/path' (see below for file content) in user_message tags", + "Text with 'some/path' (see below for file content) in user_message tags", ), { type: "tool_result", @@ -3559,10 +3560,11 @@ describe("Cline", () => { "Regular text with 'some/path' (see below for file content)", ) - // Text within user_message tags should be processed + // Only the authorized input is replaced; the surrounding + // wrapper text stays intact. expect((processedContent[1] as Anthropic.TextBlockParam).text).toContain("processed:") expect((processedContent[1] as Anthropic.TextBlockParam).text).toContain( - "Text with 'some/path' (see below for file content) in user_message tags", + "processed: Text with 'some/path' (see below for file content) in user_message tags", ) // Tool result markers cannot authorize mention expansion diff --git a/src/core/tools/AskFollowupQuestionTool.ts b/src/core/tools/AskFollowupQuestionTool.ts index 24262fe4dc..1099fc9363 100644 --- a/src/core/tools/AskFollowupQuestionTool.ts +++ b/src/core/tools/AskFollowupQuestionTool.ts @@ -74,6 +74,7 @@ export class AskFollowupQuestionTool extends BaseTool<"ask_followup_question"> { const { text, images } = await task.ask("followup", JSON.stringify(follow_up_json), false) const safeText = text ?? "" await task.say("user_feedback", safeText, images) + // Authorizes expansion. The tool result repeats the text by design. task.userMessageContent.push(userMentionContent(`\n${safeText}\n`, safeText)) pushToolResult(formatResponse.toolResult(`\n${safeText}\n`, images)) } catch (error) { diff --git a/src/core/tools/AttemptCompletionTool.ts b/src/core/tools/AttemptCompletionTool.ts index 4fea91e243..965e1d7d1d 100644 --- a/src/core/tools/AttemptCompletionTool.ts +++ b/src/core/tools/AttemptCompletionTool.ts @@ -246,6 +246,7 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { } const feedbackText = `\n${text}\n` + // Authorizes expansion. The tool result repeats the text by design. task.userMessageContent.push(userMentionContent(feedbackText, text ?? "")) pushToolResult(formatResponse.toolResult(feedbackText, images)) } catch (error) { diff --git a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts index 3674f7f2e2..0fa795bfcc 100644 --- a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts +++ b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts @@ -6,24 +6,36 @@ import type { ToolCallbacks } from "../BaseTool" import { AskFollowupQuestionTool, askFollowupQuestionTool } from "../AskFollowupQuestionTool" import { formatResponse } from "../../prompts/responses" import { NativeToolCallParser } from "../../assistant-message/NativeToolCallParser" +import { processUserContentMentions } from "../../mentions/processUserContentMentions" +import { parseMentions } from "../../mentions/index" +import { FileContextTracker } from "../../context-tracking/FileContextTracker" + +vi.mock("../../mentions/index", () => ({ parseMentions: vi.fn() })) describe("AskFollowupQuestionTool", () => { let tool: AskFollowupQuestionTool let mockTask: Task let mockCallbacks: ToolCallbacks + // Kept outside the Task double so tests can re-stub answers without casts. + let askMock: ReturnType beforeEach(() => { vi.clearAllMocks() + vi.mocked(parseMentions).mockImplementation(async (text) => ({ + text: `parsed: ${text}`, + contentBlocks: [], + })) tool = new AskFollowupQuestionTool() + askMock = vi.fn().mockResolvedValue({ text: "User answer", images: [] }) mockTask = { userMessageContent: [], consecutiveMistakeCount: 0, recordToolError: vi.fn(), didToolFailInCurrentTurn: false, sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"), - ask: vi.fn().mockResolvedValue({ text: "User answer", images: [] }), + ask: askMock, say: vi.fn().mockResolvedValue(undefined), } as unknown as Task @@ -198,6 +210,43 @@ describe("AskFollowupQuestionTool", () => { ) }) + it("pushes an authorized feedback block that mention processing expands", async () => { + const params = { question: "What?", follow_up: [{ text: "A" }] } + askMock.mockResolvedValue({ text: "@/allowed", images: [] }) + + await tool.execute(params, mockTask, mockCallbacks) + + // Deleting the push in AskFollowupQuestionTool fails this assertion. + expect(mockTask.userMessageContent).toHaveLength(1) + const block = mockTask.userMessageContent[0] + expect(block).toEqual({ type: "text", text: "\n@/allowed\n" }) + + // The pushed block authorizes expansion of its input. + const result = await processUserContentMentions({ + userContent: [block], + cwd: "/test", + fileContextTracker: {} as FileContextTracker, + }) + expect(vi.mocked(parseMentions).mock.calls[0]?.[0]).toBe("@/allowed") + expect(result.content).toEqual([{ type: "text", text: "\nparsed: @/allowed\n" }]) + }) + + it("does not authorize the feedback block when the user answer is null", async () => { + const params = { question: "What?", follow_up: [{ text: "A" }] } + askMock.mockResolvedValue({ text: null, images: [] }) + + await tool.execute(params, mockTask, mockCallbacks) + + expect(mockTask.userMessageContent).toHaveLength(1) + const result = await processUserContentMentions({ + userContent: [mockTask.userMessageContent[0]], + cwd: "/test", + fileContextTracker: {} as FileContextTracker, + }) + expect(parseMentions).not.toHaveBeenCalled() + expect(result.content).toEqual([{ type: "text", text: "\n\n" }]) + }) + it("should pass images from user response to toolResult", async () => { const params = { question: "Look at this?", diff --git a/src/core/tools/__tests__/attemptCompletionTool.spec.ts b/src/core/tools/__tests__/attemptCompletionTool.spec.ts index e5923995c5..b8ff862ab3 100644 --- a/src/core/tools/__tests__/attemptCompletionTool.spec.ts +++ b/src/core/tools/__tests__/attemptCompletionTool.spec.ts @@ -31,6 +31,11 @@ import { attemptCompletionTool, AttemptCompletionCallbacks } from "../AttemptCom import { Task } from "../../task/Task" import { AskApproval, HandleError, PushToolResult } from "../../../shared/tools" import * as vscode from "vscode" +import { processUserContentMentions } from "../../mentions/processUserContentMentions" +import { parseMentions } from "../../mentions/index" +import { FileContextTracker } from "../../context-tracking/FileContextTracker" + +vi.mock("../../mentions/index", () => ({ parseMentions: vi.fn() })) describe("attemptCompletionTool", () => { let mockTask: Partial @@ -1162,6 +1167,89 @@ describe("attemptCompletionTool", () => { expect(mockPushToolResult).toHaveBeenCalledTimes(1) }) + it("pushes an authorized feedback block that mention processing expands", async () => { + const block: ToolUse<"attempt_completion"> = { + type: "tool_use", + name: "attempt_completion", + params: { result: "Done" }, + nativeArgs: { result: "Done" }, + partial: false, + } + mockTask.ask = vi.fn().mockResolvedValue({ + response: "messageResponse", + text: "@/allowed", + images: [], + }) + vi.mocked(parseMentions).mockImplementation(async (text) => ({ + text: `parsed: ${text}`, + contentBlocks: [], + })) + vi.mocked(parseMentions).mockClear() + + await attemptCompletionTool.handle(mockTask as Task, block, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + toolCallId: "", + askFinishSubTaskApproval: mockAskFinishSubTaskApproval, + toolDescription: mockToolDescription, + }) + + // Deleting the push in AttemptCompletionTool fails this assertion. + expect(mockTask.userMessageContent).toHaveLength(1) + const block0 = mockTask.userMessageContent![0] + expect(block0).toEqual({ type: "text", text: "\n@/allowed\n" }) + + // The pushed block authorizes expansion of its input. + const result = await processUserContentMentions({ + userContent: [block0], + cwd: "/test", + fileContextTracker: {} as FileContextTracker, + }) + expect(vi.mocked(parseMentions).mock.calls[0]?.[0]).toBe("@/allowed") + expect(result.content).toEqual([ + { type: "text", text: "\nparsed: @/allowed\n" }, + ]) + }) + + it("does not authorize the feedback block when the feedback text is undefined", async () => { + const block: ToolUse<"attempt_completion"> = { + type: "tool_use", + name: "attempt_completion", + params: { result: "Done" }, + nativeArgs: { result: "Done" }, + partial: false, + } + mockTask.ask = vi.fn().mockResolvedValue({ + response: "messageResponse", + text: undefined, + images: [], + }) + vi.mocked(parseMentions).mockImplementation(async (text) => ({ + text: `parsed: ${text}`, + contentBlocks: [], + })) + vi.mocked(parseMentions).mockClear() + + await attemptCompletionTool.handle(mockTask as Task, block, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + toolCallId: "", + askFinishSubTaskApproval: mockAskFinishSubTaskApproval, + toolDescription: mockToolDescription, + }) + + expect(mockTask.userMessageContent).toHaveLength(1) + const result = await processUserContentMentions({ + userContent: [mockTask.userMessageContent![0]], + cwd: "/test", + fileContextTracker: {} as FileContextTracker, + }) + expect(parseMentions).not.toHaveBeenCalled() + expect(result.content).toEqual([{ type: "text", text: "\nundefined\n" }]) + }) + it("does not clear pending metadata when stale delegation continues without an action id", async () => { const block: ToolUse<"attempt_completion"> = { type: "tool_use", diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 3ec0aaf5ae..ceabbefaa2 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1041,6 +1041,52 @@ describe("ClineProvider", () => { sidebarTask.mockRestore() tabTask.mockRestore() }) + + test.each([ + ["explainCode", "EXPLAIN"], + ["fixCode", "FIX"], + ["improveCode", "IMPROVE"], + ] as const)( + "forwards user-authored code action input as authorizedMentionText for %s", + async (command, promptType) => { + const task = new Task(defaultTaskOptions) + const createTask = vi.spyOn(provider, "createTask").mockResolvedValue(task) + try { + await provider.handleCodeAction(command, promptType, { + selectedText: "selected code", + userInput: "@/allowed", + }) + // A regression that always sends "" would fail this assertion. + expect(createTask).toHaveBeenCalledWith(expect.any(String), undefined, undefined, { + authorizedMentionText: "@/allowed", + }) + } finally { + createTask.mockRestore() + } + }, + ) + + test("forwards user-authored terminal action input as authorizedMentionText", async () => { + const task = new Task(defaultTaskOptions) + const sidebarTask = vi.spyOn(provider, "createTask").mockResolvedValue(task) + const tabTask = vi.spyOn(tabProvider, "createTask").mockResolvedValue(task) + try { + // No chat focus fired, so the last registered visible provider + // (the tab) receives the task, as in the fallback test above. + await ClineProvider.handleTerminalAction("terminalFixCommand", "TERMINAL_FIX", { + userInput: "@/allowed", + }) + // A regression that always sends "" would fail this assertion. + expect(tabTask).toHaveBeenCalledOnce() + expect(tabTask).toHaveBeenCalledWith(expect.any(String), undefined, undefined, { + authorizedMentionText: "@/allowed", + }) + expect(sidebarTask).not.toHaveBeenCalled() + } finally { + sidebarTask.mockRestore() + tabTask.mockRestore() + } + }) }) test("loads full model details when preparing an LM Studio task", async () => { diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index ae3d504003..f06ec638eb 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -589,11 +589,6 @@ "count": 1 } }, - "core/mentions/__tests__/processUserContentMentions.spec.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 1 - } - }, "core/mentions/index.ts": { "@typescript-eslint/no-explicit-any": { "count": 1 From 2f7c01ead87dbf48e8f02a4928792f2b92ad0472 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sat, 14 Nov 2026 20:42:32 +0000 Subject: [PATCH 03/15] fix(mentions): carry add-to-context authorization through the chat box - Add authorizedMentionText to the setChatBoxMessage and newTask webview messages. The webview tracks the user-authored substring and clears it when the user edits the box. - webviewMessageHandler fails closed when the webview omits the field. - Task start no longer authorizes the full task text by default. The API start passes its own text as authorized. - Cover the handler forwarding, the task start boundary, and the chat box flow with tests. --- packages/types/src/vscode-extension-host.ts | 4 + src/core/task/Task.ts | 10 +- src/core/task/__tests__/Task.spec.ts | 37 ++++ src/core/webview/ClineProvider.ts | 6 + .../webview/__tests__/ClineProvider.spec.ts | 6 + .../webviewMessageHandler.newTask.spec.ts | 80 ++++++++ src/core/webview/webviewMessageHandler.ts | 8 +- src/extension/api.ts | 4 +- webview-ui/src/components/chat/ChatView.tsx | 41 +++- ...View.add-to-context-authorization.spec.tsx | 193 ++++++++++++++++++ 10 files changed, 377 insertions(+), 12 deletions(-) create mode 100644 src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts create mode 100644 webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 8a006c0639..e7a6d4b6d9 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -133,6 +133,8 @@ export interface ExtensionMessage { | "switchTab" | "toggleAutoApprove" invoke?: "newChat" | "sendMessage" | "primaryButtonClick" | "secondaryButtonClick" | "setChatBoxMessage" + /** With `invoke: "setChatBoxMessage"`: user-authored substring of `text` that may have mentions expanded. */ + authorizedMentionText?: string /** * Partial state updates are allowed to reduce message size (e.g. omit large fields like taskHistory). * The webview is responsible for merging. @@ -817,6 +819,8 @@ export interface WebviewMessage { updatedSettings?: RooCodeSettings /** Task configuration applied via `createTask()`. */ taskConfiguration?: RooCodeSettings + /** User-authored substring of `text` that may have mentions expanded. Empty disables expansion. */ + authorizedMentionText?: string // Worktree properties worktreePath?: string worktreeBranch?: string diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 473805fbb9..ae582445a7 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -3165,12 +3165,10 @@ export class Task extends EventEmitter implements TaskLike { // Task starting await this.initiateTaskLoop([ - // Child tasks get model-written text. Only an explicit - // authorizedMentionText may expand there. - userMentionContent( - `\n${task}\n`, - this.authorizedMentionText ?? (this.parentTask ? "" : (task ?? "")), - ), + // Without explicit authorization nothing expands. Task text + // can carry generated context, so callers must state the + // user-authored part. + userMentionContent(`\n${task}\n`, this.authorizedMentionText ?? ""), ...imageBlocks, ]).catch((error) => { // Swallow loop rejection when the task was intentionally abandoned/aborted diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 91855fb876..d06a1d4193 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -33,6 +33,7 @@ import { ApiStreamChunk } from "../../../api/transform/stream" import { OutputTokenLimitError } from "../../../api/providers/utils/output-token-limit-error" import { ContextProxy } from "../../config/ContextProxy" import { processUserContentMentions, userMentionContent } from "../../mentions/processUserContentMentions" +import { parseMentions } from "../../mentions" import { MultiSearchReplaceDiffStrategy } from "../../diff/strategies/multi-search-replace" import type { ApiMessage } from "../../task-persistence" import type { RequestPolicySnapshot } from "../../prompts/tools/effective-tool-policy" @@ -3511,6 +3512,10 @@ describe("Cline", () => { }) describe("processUserContentMentions", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + it("expands only the authorized user input and leaves tool results unexpanded", async () => { const [cline, task] = Task.create({ provider: mockProvider, @@ -3585,6 +3590,38 @@ describe("Cline", () => { await cline.abortTask(true) await task.catch(() => {}) }) + + it("does not expand task text when the start passes no authorization", async () => { + const [cline, task] = Task.create({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "read '@/secret' please", + authorizedMentionText: "", + }) + + // Wait until the loop reached mention processing, then stop it. + await vi.waitFor(() => expect(processUserContentMentions).toHaveBeenCalled()) + await cline.abortTask(true) + await task.catch(() => {}) + + expect(parseMentions).not.toHaveBeenCalled() + }) + + it("expands the task text when the start authorizes it", async () => { + const taskText = "read '@/secret' please" + const [cline, task] = Task.create({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: taskText, + authorizedMentionText: taskText, + }) + + await vi.waitFor(() => expect(parseMentions).toHaveBeenCalled()) + await cline.abortTask(true) + await task.catch(() => {}) + + expect(vi.mocked(parseMentions).mock.calls[0]?.[0]).toBe(taskText) + }) }) }) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 371925421e..7640afe666 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1075,6 +1075,9 @@ export class ClineProvider type: "invoke", invoke: "setChatBoxMessage", text: `${prompt}\n\n`, + // The generated prompt is context. Only the typed input may + // have mentions expanded. + authorizedMentionText: typeof params.userInput === "string" ? params.userInput : "", }) await this.postMessageToWebview({ type: "action", action: "focusInput" }) return @@ -1106,6 +1109,9 @@ export class ClineProvider type: "invoke", invoke: "setChatBoxMessage", text: `${prompt}\n\n`, + // The template interpolates the typed input, so only that + // part may have mentions expanded. + authorizedMentionText: typeof params.userInput === "string" ? params.userInput : "", }) await visibleProvider.postMessageToWebview({ type: "action", action: "focusInput" }) return diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index ceabbefaa2..5a8c3a8fb2 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -743,6 +743,8 @@ describe("ClineProvider", () => { type: "invoke", invoke: "setChatBoxMessage", text: expect.stringContaining("selected code"), + // Generated context is never the authorized substring. + authorizedMentionText: "", }) expect(sidebar.postMessage).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) expect(tab.postMessage).not.toHaveBeenCalled() @@ -933,6 +935,8 @@ describe("ClineProvider", () => { type: "invoke", invoke: "setChatBoxMessage", text: expect.stringContaining("selected code"), + // Generated context is never the authorized substring. + authorizedMentionText: "", }) expect(visible.postMessage).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) expect(vscode.commands.executeCommand).not.toHaveBeenCalled() @@ -961,6 +965,8 @@ describe("ClineProvider", () => { type: "invoke", invoke: "setChatBoxMessage", text: expect.stringContaining("selected code"), + // Generated context is never the authorized substring. + authorizedMentionText: "", }) expect(tab.postMessage).not.toHaveBeenCalled() expect(panel.reveal).not.toHaveBeenCalled() diff --git a/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts new file mode 100644 index 0000000000..bf1b074386 --- /dev/null +++ b/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts @@ -0,0 +1,80 @@ +import { describe, it, expect, beforeEach, vi } from "vitest" +import { webviewMessageHandler } from "../webviewMessageHandler" + +vi.mock("../../../i18n", () => ({ + t: vi.fn((key: string) => key), + changeLanguage: vi.fn(), +})) + +vi.mock("vscode", () => ({ + window: { + showErrorMessage: vi.fn(), + showWarningMessage: vi.fn(), + showInformationMessage: vi.fn(), + }, + workspace: { + workspaceFolders: undefined, + getConfiguration: vi.fn(() => ({ + get: vi.fn(), + update: vi.fn(), + })), + }, + ConfigurationTarget: { Global: 1, Workspace: 2, WorkspaceFolder: 3 }, + Uri: { + parse: vi.fn((str: string) => ({ toString: () => str })), + file: vi.fn((path: string) => ({ fsPath: path })), + }, + env: { openExternal: vi.fn() }, + commands: { executeCommand: vi.fn() }, +})) + +describe("webviewMessageHandler newTask authorization", () => { + let provider: Record> + + beforeEach(() => { + vi.clearAllMocks() + provider = { + createTask: vi.fn().mockResolvedValue({ taskId: "task-1" }), + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + getCurrentTask: vi.fn(() => undefined), + getState: vi.fn().mockResolvedValue({ maxImageFileSize: 5, maxTotalImageSize: 20 }), + } as unknown as Record> + }) + + it("forwards the authorized mention substring from the webview message", async () => { + await webviewMessageHandler( + provider as never, + { + type: "newTask", + text: "context @/secret\nuser line", + authorizedMentionText: "user line", + } as never, + ) + + expect(provider.createTask).toHaveBeenCalledWith( + "context @/secret\nuser line", + [], + undefined, + expect.objectContaining({ authorizedMentionText: "user line" }), + undefined, + ) + }) + + it("passes an empty authorized substring when the webview omits it", async () => { + await webviewMessageHandler( + provider as never, + { + type: "newTask", + text: "generated @/path from selected code", + } as never, + ) + + expect(provider.createTask).toHaveBeenCalledWith( + "generated @/path from selected code", + [], + undefined, + expect.objectContaining({ authorizedMentionText: "" }), + undefined, + ) + }) +}) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index c5abbc0a12..0f5019f986 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -716,7 +716,13 @@ export const webviewMessageHandler = async ( resolved.text, resolved.images, undefined, - { taskId: message.taskId }, + { + taskId: message.taskId, + // Fail closed when the webview omits the authorized + // substring, so generated context cannot expand. + authorizedMentionText: + typeof message.authorizedMentionText === "string" ? message.authorizedMentionText : "", + }, message.taskConfiguration, ) // Task created successfully - notify the UI to reset diff --git a/src/extension/api.ts b/src/extension/api.ts index 606bb7725d..e05513576a 100644 --- a/src/extension/api.ts +++ b/src/extension/api.ts @@ -250,9 +250,11 @@ export class API extends EventEmitter implements RooCodeAPI { // effectively unlimited. The soft repetition tier is disabled too: // without a reachable hard stop it would reject every identical call // with no escalation. - const options: CreateTaskOptions = { + const options: CreateTaskOptions & { authorizedMentionText?: string } = { consecutiveMistakeLimit: Number.MAX_SAFE_INTEGER, toolRepetitionSoftLimit: 0, + // The API caller authored the task text. + authorizedMentionText: text ?? "", } const task = await provider.createTask(text, images, undefined, options, configuration) diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 820e30849d..b9430bd722 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -202,6 +202,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction(null) const userRespondedRef = useRef(false) + // Null means the chat box value is fully user-authored. A string names + // the only substring that may have mentions expanded. + const authorizedMentionRef = useRef(null) const [currentFollowUpTs, setCurrentFollowUpTs] = useState(null) const [aggregatedCostsMap, setAggregatedCostsMap] = useState< Map< @@ -629,6 +632,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + (text: string, images: string[], authorizedMentionText?: string) => { // Avoid nested template literals by breaking down the logic let newValue = text @@ -734,6 +746,14 @@ const ChatViewComponent: React.ForwardRefRenderFunction string)) => { + authorizedMentionRef.current = null + setInputValue(value) + }, + [setInputValue], + ) + // Handle stop button click from textarea const handleStopTask = useCallback(() => { vscode.postMessage({ type: "cancelTask" }) @@ -940,7 +969,11 @@ const ChatViewComponent: React.ForwardRefRenderFunction ({ mockPostMessage: vi.fn() })) +vi.mock("@src/utils/vscode", () => ({ + vscode: { + postMessage: mockPostMessage, + }, +})) + +vi.mock("use-sound", () => ({ + default: vi.fn(() => [vi.fn()]), +})) + +vi.mock("../ChatRow", () => ({ + default: () => null, +})) + +vi.mock("../TaskHeader", () => ({ + default: () => null, +})) + +vi.mock("../AutoApproveMenu", () => ({ + default: () => null, +})) + +vi.mock("react-virtuoso", () => ({ + Virtuoso: () =>
, +})) + +vi.mock("../../common/VersionIndicator", () => ({ + default: () => null, +})) + +vi.mock("../Announcement", () => ({ + default: () => null, +})) + +vi.mock("../QueuedMessages", () => ({ + QueuedMessages: () => null, +})) + +vi.mock("@src/components/welcome/RooTips", () => ({ + default: () => null, +})) + +vi.mock("@src/components/welcome/RooHero", () => ({ + default: () => null, +})) + +vi.mock("../common/TelemetryBanner", () => ({ + default: () => null, +})) + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ t: (key: string) => key }), + initReactI18next: { type: "3rdParty", init: () => {} }, + Trans: ({ children }: { children?: React.ReactNode }) => <>{children}, +})) + +// ChatTextArea double: an input whose Enter triggers onSend and whose +// changes flow through the provided setInputValue. +vi.mock("../ChatTextArea", async () => { + const React = await import("react") + + const ChatTextAreaComponent = React.forwardRef(function MockChatTextArea(props: any, _ref: unknown) { + return ( +
+ ) => props.setInputValue(e.target.value)} + onKeyDown={(e: React.KeyboardEvent) => { + if (e.key === "Enter" && !e.shiftKey) { + e.preventDefault() + props.onSend() + } + }} + /> +
+ ) + }) + + return { default: ChatTextAreaComponent, ChatTextArea: ChatTextAreaComponent } +}) + +vi.mock("@vscode/webview-ui-toolkit/react", () => ({ + VSCodeButton: ({ children, onClick }: { children: React.ReactNode; onClick?: () => void }) => ( + + ), + VSCodeCheckbox: ({ children, checked, onChange }: any) => ( + + ), + VSCodeTextField: ({ value, onInput, placeholder }: any) => ( + onInput?.({ target: { value: e.target.value } })} + placeholder={placeholder} + /> + ), + VSCodeLink: ({ children, href }: { children: React.ReactNode; href?: string }) => {children}, + VSCodeTag: ({ children }: { children: React.ReactNode }) => {children}, +})) + +const defaultProps: ChatViewProps = { + isHidden: false, + showAnnouncement: false, + hideAnnouncement: () => {}, +} + +const renderChatView = () => renderWithExtensionState() + +const sendNewTask = async (getByTestId: (id: string) => HTMLElement) => { + await act(async () => { + fireEvent.keyDown(getByTestId("chat-textarea").querySelector("input")!, { key: "Enter" }) + }) + await waitFor(() => expect(mockPostMessage).toHaveBeenCalledWith(expect.objectContaining({ type: "newTask" }))) + return mockPostMessage.mock.calls.find(([m]) => m.type === "newTask")?.[0] +} + +const addGeneratedContext = async (payload: Record) => { + await act(async () => { + window.postMessage( + { type: "invoke", invoke: "setChatBoxMessage", text: "context with @/secret code", ...payload }, + "*", + ) + }) + await waitFor(() => { + expect(getChatBoxValue()).toBe("context with @/secret code") + }) +} + +const getChatBoxValue = () => { + const input = document.querySelector('[data-testid="chat-textarea"] input') as HTMLInputElement | null + return input?.value ?? "" +} + +describe("ChatView - add-to-context mention authorization", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("sends no authorized substring when add-to-context carries no user input", async () => { + const { getByTestId } = renderChatView() + + await addGeneratedContext({ authorizedMentionText: "" }) + const message = await sendNewTask(getByTestId) + + expect(message).toMatchObject({ text: "context with @/secret code", authorizedMentionText: "" }) + }) + + it("sends only the user-authored substring the extension authorized", async () => { + const { getByTestId } = renderChatView() + + await addGeneratedContext({ authorizedMentionText: "explain this" }) + const message = await sendNewTask(getByTestId) + + expect(message).toMatchObject({ authorizedMentionText: "explain this" }) + }) + + it("authorizes the whole value when no authorization was recorded", async () => { + const { getByTestId } = renderChatView() + + await addGeneratedContext({}) + const message = await sendNewTask(getByTestId) + + expect(message).toMatchObject({ authorizedMentionText: "context with @/secret code" }) + }) + + it("re-authorizes the whole value after a manual edit", async () => { + const { getByTestId } = renderChatView() + + await addGeneratedContext({ authorizedMentionText: "" }) + + const input = getByTestId("chat-textarea").querySelector("input")! + await act(async () => { + fireEvent.change(input, { target: { value: "my own task with @/ok" } }) + }) + + const message = await sendNewTask(getByTestId) + expect(message).toMatchObject({ authorizedMentionText: "my own task with @/ok" }) + }) +}) From 6f4b1904155988aff0c0572eba36090d2f00e47c Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sat, 14 Nov 2026 21:11:40 +0000 Subject: [PATCH 04/15] fix(mentions): scope ask response and queued message authorization - Carry authorizedMentionText through askResponse and queueMessage, the host handler, stored ask responses, and queued messages. - Follow-up answers, completion feedback, resume feedback, and drained queued responses authorize only that substring. Absent values fail closed to an empty string. - Treat an omitted setChatBoxMessage field as authorizing nothing from the inserted text. - Route enhanced prompt and insert updates through a programmatic setter that preserves the recorded authorization scope. - Type the new test doubles without any or never casts and strengthen the task start tests with a distinct substring case. --- packages/types/src/message.ts | 2 + src/core/message-queue/MessageQueueService.ts | 5 +- src/core/task/Task.ts | 27 ++++- src/core/task/__tests__/Task.spec.ts | 23 +++- src/core/tools/AskFollowupQuestionTool.ts | 13 ++- src/core/tools/AttemptCompletionTool.ts | 11 +- .../__tests__/askFollowupQuestionTool.spec.ts | 2 +- .../__tests__/attemptCompletionTool.spec.ts | 1 + .../webviewMessageHandler.newTask.spec.ts | 103 ++++++++++++++---- .../__tests__/webviewMessageHandler.spec.ts | 10 +- src/core/webview/webviewMessageHandler.ts | 18 ++- .../src/components/chat/ChatTextArea.tsx | 39 +++++-- webview-ui/src/components/chat/ChatView.tsx | 32 ++++-- ...View.add-to-context-authorization.spec.tsx | 69 ++++++++++-- .../chat/__tests__/ChatView.spec.tsx | 7 ++ 15 files changed, 297 insertions(+), 65 deletions(-) diff --git a/packages/types/src/message.ts b/packages/types/src/message.ts index 6f5162544d..8a9842b454 100644 --- a/packages/types/src/message.ts +++ b/packages/types/src/message.ts @@ -376,6 +376,8 @@ export const queuedMessageSchema = z.object({ id: z.string(), text: z.string(), images: z.array(z.string()).optional(), + /** User-authored substring of `text` that may have mentions expanded. */ + authorizedMentionText: z.string().optional(), }) export type QueuedMessage = z.infer diff --git a/src/core/message-queue/MessageQueueService.ts b/src/core/message-queue/MessageQueueService.ts index 340fad6087..57fb7781b0 100644 --- a/src/core/message-queue/MessageQueueService.ts +++ b/src/core/message-queue/MessageQueueService.ts @@ -28,7 +28,7 @@ export class MessageQueueService extends EventEmitter { return { index, message: this._messages[index] } } - public addMessage(text: string, images?: string[]): QueuedMessage | undefined { + public addMessage(text: string, images?: string[], authorizedMentionText?: string): QueuedMessage | undefined { if (!text && !images?.length) { return undefined } @@ -38,6 +38,7 @@ export class MessageQueueService extends EventEmitter { id: uuidv4(), text, images, + authorizedMentionText, } this._messages.push(message) @@ -69,6 +70,8 @@ export class MessageQueueService extends EventEmitter { message.timestamp = Date.now() message.text = text message.images = images + // The edited text is user-authored, so it replaces any recorded scope. + message.authorizedMentionText = text this.emit("stateChanged", this._messages) return true } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index ae582445a7..ab475d096e 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -452,6 +452,7 @@ export class Task extends EventEmitter implements TaskLike { private askResponse?: ClineAskResponse private askResponseText?: string private askResponseImages?: string[] + private askResponseAuthorizedMentionText?: string /** * Structured detail of the automatic denial that resolved the current ask, * set when `checkAutoApproval` denies via policy (denylist, blanket @@ -1258,7 +1259,12 @@ export class Task extends EventEmitter implements TaskLike { } private handleQueuedAskResponse(message: QueuedMessage, resolution: QueuedAskResolution): string | undefined { - this.handleWebviewAskResponse(resolution.response, message.text, message.images) + this.handleWebviewAskResponse( + resolution.response, + message.text, + message.images, + message.authorizedMentionText ?? "", + ) if (resolution.requiresDurableAck) { return message.id } @@ -1928,6 +1934,8 @@ export class Task extends EventEmitter implements TaskLike { response: ClineAskResponse text?: string images?: string[] + /** User-authored substring of `text` that may have mentions expanded. */ + authorizedMentionText?: string queuedMessageId?: string /** * Present when the ask was resolved by an automatic (policy) denial @@ -2143,7 +2151,7 @@ export class Task extends EventEmitter implements TaskLike { // Store the auto-approval timeout so it can be cancelled if user interacts this.autoApprovalTimeoutRef = setTimeout(() => { const { askResponse, text, images } = approval.fn() - this.handleWebviewAskResponse(askResponse, text, images) + this.handleWebviewAskResponse(askResponse, text, images, "") this.autoApprovalTimeoutRef = undefined }, approval.timeout) timeouts.push(this.autoApprovalTimeoutRef) @@ -2446,12 +2454,14 @@ export class Task extends EventEmitter implements TaskLike { response: this.askResponse!, text: this.askResponseText, images: this.askResponseImages, + authorizedMentionText: this.askResponseAuthorizedMentionText ?? "", queuedMessageId, autoDenyDetail: this.pendingAutoDenyDetail, } this.askResponse = undefined this.askResponseText = undefined this.askResponseImages = undefined + this.askResponseAuthorizedMentionText = undefined this.pendingAutoDenyDetail = undefined // Switch back to an active state. @@ -2474,13 +2484,19 @@ export class Task extends EventEmitter implements TaskLike { } } - handleWebviewAskResponse(askResponse: ClineAskResponse, text?: string, images?: string[]) { + handleWebviewAskResponse( + askResponse: ClineAskResponse, + text?: string, + images?: string[], + authorizedMentionText = "", + ) { // Clear any pending auto-approval timeout when user responds this.cancelAutoApprovalTimeout() this.askResponse = askResponse this.askResponseText = text this.askResponseImages = images + this.askResponseAuthorizedMentionText = authorizedMentionText // Create a checkpoint whenever the user sends a message. // Use allowEmpty=true to ensure a checkpoint is recorded even if there are no file changes. @@ -3308,7 +3324,7 @@ export class Task extends EventEmitter implements TaskLike { this.markInitialized() - const { response, text, images } = await this.ask(askType) // Calls `postStateToWebview`. + const { response, text, images, authorizedMentionText } = await this.ask(askType) // Calls `postStateToWebview`. let responseText: string | undefined let responseImages: string[] | undefined @@ -3430,8 +3446,9 @@ export class Task extends EventEmitter implements TaskLike { const newUserContent: Anthropic.Messages.ContentBlockParam[] = [...modifiedOldUserContent] if (responseText) { + // Only the chat-box substring the webview authorized expands. newUserContent.push( - userMentionContent(`\n${responseText}\n`, responseText), + userMentionContent(`\n${responseText}\n`, authorizedMentionText ?? ""), ) } diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index d06a1d4193..7f2c96e258 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -3591,12 +3591,15 @@ describe("Cline", () => { await task.catch(() => {}) }) - it("does not expand task text when the start passes no authorization", async () => { + it.each([ + { label: "empty string", authorizedMentionText: "" }, + { label: "omitted option", authorizedMentionText: undefined }, + ])("does not expand task text when authorization is $label", async ({ authorizedMentionText }) => { const [cline, task] = Task.create({ provider: mockProvider, apiConfiguration: mockApiConfig, task: "read '@/secret' please", - authorizedMentionText: "", + authorizedMentionText, }) // Wait until the loop reached mention processing, then stop it. @@ -3622,6 +3625,22 @@ describe("Cline", () => { expect(vi.mocked(parseMentions).mock.calls[0]?.[0]).toBe(taskText) }) + + it("expands only the authorized substring of generated task text", async () => { + const [cline, task] = Task.create({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "context @/context.txt\nchosen @/chosen.txt", + authorizedMentionText: "chosen @/chosen.txt", + }) + + await vi.waitFor(() => expect(parseMentions).toHaveBeenCalled()) + await cline.abortTask(true) + await task.catch(() => {}) + + const texts = vi.mocked(parseMentions).mock.calls.map(([text]) => text) + expect(texts).toEqual(["chosen @/chosen.txt"]) + }) }) }) diff --git a/src/core/tools/AskFollowupQuestionTool.ts b/src/core/tools/AskFollowupQuestionTool.ts index 1099fc9363..3504765747 100644 --- a/src/core/tools/AskFollowupQuestionTool.ts +++ b/src/core/tools/AskFollowupQuestionTool.ts @@ -71,11 +71,18 @@ export class AskFollowupQuestionTool extends BaseTool<"ask_followup_question"> { } task.consecutiveMistakeCount = 0 - const { text, images } = await task.ask("followup", JSON.stringify(follow_up_json), false) + const { text, images, authorizedMentionText } = await task.ask( + "followup", + JSON.stringify(follow_up_json), + false, + ) const safeText = text ?? "" await task.say("user_feedback", safeText, images) - // Authorizes expansion. The tool result repeats the text by design. - task.userMessageContent.push(userMentionContent(`\n${safeText}\n`, safeText)) + // Authorizes only the chat-box substring the webview named. The + // tool result repeats the text by design. + task.userMessageContent.push( + userMentionContent(`\n${safeText}\n`, authorizedMentionText ?? ""), + ) pushToolResult(formatResponse.toolResult(`\n${safeText}\n`, images)) } catch (error) { await handleError("asking question", error as Error) diff --git a/src/core/tools/AttemptCompletionTool.ts b/src/core/tools/AttemptCompletionTool.ts index 965e1d7d1d..446204c5fc 100644 --- a/src/core/tools/AttemptCompletionTool.ts +++ b/src/core/tools/AttemptCompletionTool.ts @@ -219,7 +219,11 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { task.flushTelemetryInstallment("attempt_completion") } - const { response, text, images, queuedMessageId } = await task.ask("completion_result", "", false) + const { response, text, images, queuedMessageId, authorizedMentionText } = await task.ask( + "completion_result", + "", + false, + ) if (response === "yesButtonClicked") { // A stale history replay reruns this handler on a fresh Task instance for a @@ -246,8 +250,9 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { } const feedbackText = `\n${text}\n` - // Authorizes expansion. The tool result repeats the text by design. - task.userMessageContent.push(userMentionContent(feedbackText, text ?? "")) + // Authorizes only the chat-box substring the webview named. The + // tool result repeats the text by design. + task.userMessageContent.push(userMentionContent(feedbackText, authorizedMentionText ?? "")) pushToolResult(formatResponse.toolResult(feedbackText, images)) } catch (error) { await handleError("inspecting site", error as Error) diff --git a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts index 0fa795bfcc..e0e884a6e8 100644 --- a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts +++ b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts @@ -212,7 +212,7 @@ describe("AskFollowupQuestionTool", () => { it("pushes an authorized feedback block that mention processing expands", async () => { const params = { question: "What?", follow_up: [{ text: "A" }] } - askMock.mockResolvedValue({ text: "@/allowed", images: [] }) + askMock.mockResolvedValue({ text: "@/allowed", images: [], authorizedMentionText: "@/allowed" }) await tool.execute(params, mockTask, mockCallbacks) diff --git a/src/core/tools/__tests__/attemptCompletionTool.spec.ts b/src/core/tools/__tests__/attemptCompletionTool.spec.ts index b8ff862ab3..a0a4224eba 100644 --- a/src/core/tools/__tests__/attemptCompletionTool.spec.ts +++ b/src/core/tools/__tests__/attemptCompletionTool.spec.ts @@ -1179,6 +1179,7 @@ describe("attemptCompletionTool", () => { response: "messageResponse", text: "@/allowed", images: [], + authorizedMentionText: "@/allowed", }) vi.mocked(parseMentions).mockImplementation(async (text) => ({ text: `parsed: ${text}`, diff --git a/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts index bf1b074386..c5de2d35e1 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts @@ -1,5 +1,7 @@ import { describe, it, expect, beforeEach, vi } from "vitest" import { webviewMessageHandler } from "../webviewMessageHandler" +import type { ClineProvider } from "../ClineProvider" +import type { WebviewMessage } from "../../../shared/WebviewMessage" vi.mock("../../../i18n", () => ({ t: vi.fn((key: string) => key), @@ -28,28 +30,43 @@ vi.mock("vscode", () => ({ commands: { executeCommand: vi.fn() }, })) -describe("webviewMessageHandler newTask authorization", () => { - let provider: Record> +describe("webviewMessageHandler mention authorization forwarding", () => { + // Narrow double: the handler under test only touches these members. + // The single cast at construction keeps every use site type-checked. + let provider: { + createTask: ReturnType + postMessageToWebview: ReturnType + getCurrentTask: ReturnType + getState: ReturnType + } + let providerAsClineProvider: ClineProvider + let currentTask: { + handleWebviewAskResponse: ReturnType + messageQueueService: { addMessage: ReturnType } + } beforeEach(() => { vi.clearAllMocks() + currentTask = { + handleWebviewAskResponse: vi.fn(), + messageQueueService: { addMessage: vi.fn() }, + } provider = { createTask: vi.fn().mockResolvedValue({ taskId: "task-1" }), postMessageToWebview: vi.fn().mockResolvedValue(undefined), - getCurrentTask: vi.fn(() => undefined), + getCurrentTask: vi.fn(() => currentTask), getState: vi.fn().mockResolvedValue({ maxImageFileSize: 5, maxTotalImageSize: 20 }), - } as unknown as Record> + } + providerAsClineProvider = provider as unknown as ClineProvider }) it("forwards the authorized mention substring from the webview message", async () => { - await webviewMessageHandler( - provider as never, - { - type: "newTask", - text: "context @/secret\nuser line", - authorizedMentionText: "user line", - } as never, - ) + const message: WebviewMessage = { + type: "newTask", + text: "context @/secret\nuser line", + authorizedMentionText: "user line", + } + await webviewMessageHandler(providerAsClineProvider, message) expect(provider.createTask).toHaveBeenCalledWith( "context @/secret\nuser line", @@ -61,13 +78,11 @@ describe("webviewMessageHandler newTask authorization", () => { }) it("passes an empty authorized substring when the webview omits it", async () => { - await webviewMessageHandler( - provider as never, - { - type: "newTask", - text: "generated @/path from selected code", - } as never, - ) + const message: WebviewMessage = { + type: "newTask", + text: "generated @/path from selected code", + } + await webviewMessageHandler(providerAsClineProvider, message) expect(provider.createTask).toHaveBeenCalledWith( "generated @/path from selected code", @@ -77,4 +92,54 @@ describe("webviewMessageHandler newTask authorization", () => { undefined, ) }) + + it("forwards the authorized substring with the ask response", async () => { + const message: WebviewMessage = { + type: "askResponse", + askResponse: "messageResponse", + text: "context @/secret\nuser line", + authorizedMentionText: "user line", + } + await webviewMessageHandler(providerAsClineProvider, message) + + expect(currentTask.handleWebviewAskResponse).toHaveBeenCalledWith( + "messageResponse", + "context @/secret\nuser line", + [], + "user line", + ) + }) + + it("treats an omitted authorized substring on an ask response as empty", async () => { + const message: WebviewMessage = { + type: "askResponse", + askResponse: "messageResponse", + text: "generated @/path", + } + await webviewMessageHandler(providerAsClineProvider, message) + + expect(currentTask.handleWebviewAskResponse).toHaveBeenCalledWith("messageResponse", "generated @/path", [], "") + }) + + it("passes an empty authorized substring when a queued message omits it", async () => { + const message: WebviewMessage = { type: "queueMessage", text: "generated @/path" } + await webviewMessageHandler(providerAsClineProvider, message) + + expect(currentTask.messageQueueService.addMessage).toHaveBeenCalledWith("generated @/path", [], "") + }) + + it("forwards the authorized substring with a queued message", async () => { + const message: WebviewMessage = { + type: "queueMessage", + text: "context @/secret\nuser line", + authorizedMentionText: "user line", + } + await webviewMessageHandler(providerAsClineProvider, message) + + expect(currentTask.messageQueueService.addMessage).toHaveBeenCalledWith( + "context @/secret\nuser line", + [], + "user line", + ) + }) }) diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index 98c306a4f3..82dd500c77 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -964,9 +964,13 @@ describe("webviewMessageHandler - image mentions", () => { }) expect(vi.mocked(resolveImageMentions)).toHaveBeenCalled() - expect(mockHandleWebviewAskResponse).toHaveBeenCalledWith("messageResponse", "See @/img.png", [ - "data:image/png;base64,from-mention", - ]) + // An omitted authorized substring fails closed. + expect(mockHandleWebviewAskResponse).toHaveBeenCalledWith( + "messageResponse", + "See @/img.png", + ["data:image/png;base64,from-mention"], + "", + ) }) }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 0f5019f986..f3790df67e 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -743,9 +743,14 @@ export const webviewMessageHandler = async ( case "askResponse": { const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) - provider - .getCurrentTask() - ?.handleWebviewAskResponse(message.askResponse!, resolved.text, resolved.images) + provider.getCurrentTask()?.handleWebviewAskResponse( + message.askResponse!, + resolved.text, + resolved.images, + // Fail closed when the webview omits the authorized + // substring. + typeof message.authorizedMentionText === "string" ? message.authorizedMentionText : "", + ) } break @@ -4033,7 +4038,12 @@ export const webviewMessageHandler = async ( case "queueMessage": { const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) - provider.getCurrentTask()?.messageQueueService.addMessage(resolved.text, resolved.images) + provider.getCurrentTask()?.messageQueueService.addMessage( + resolved.text, + resolved.images, + // Fail closed when the webview omits the authorized substring. + typeof message.authorizedMentionText === "string" ? message.authorizedMentionText : "", + ) break } case "removeQueuedMessage": { diff --git a/webview-ui/src/components/chat/ChatTextArea.tsx b/webview-ui/src/components/chat/ChatTextArea.tsx index 8ba0df7ab2..d0e285f740 100644 --- a/webview-ui/src/components/chat/ChatTextArea.tsx +++ b/webview-ui/src/components/chat/ChatTextArea.tsx @@ -59,6 +59,8 @@ const isFileDrag = (dataTransfer: DataTransfer): boolean => interface ChatTextAreaProps { inputValue: string setInputValue: (value: string) => void + /** Programmatic updates preserve the chat box authorization scope. */ + setInputValuePreservingAuthorization?: (value: string) => void sendingDisabled: boolean selectApiConfigDisabled: boolean placeholderText: string @@ -85,6 +87,7 @@ export const ChatTextArea = forwardRef( { inputValue, setInputValue, + setInputValuePreservingAuthorization, selectApiConfigDisabled, placeholderText, selectedImages, @@ -155,6 +158,12 @@ export const ChatTextArea = forwardRef( return () => document.removeEventListener("mousedown", handleClickOutside) }, [showDropdown]) + // Generated text routes through this setter so it never clears the + // chat box authorization scope. + const setInputValueProgrammatic = setInputValuePreservingAuthorization ?? setInputValue + + const programmaticInsertRef = useRef(false) + // Handle enhanced prompt response and search results. useEffect(() => { const messageHandler = (event: MessageEvent) => { @@ -173,12 +182,14 @@ export const ChatTextArea = forwardRef( // Select all text first textarea.select() + programmaticInsertRef.current = true document.execCommand("insertText", false, message.text) } else { - setInputValue(message.text) + setInputValueProgrammatic(message.text) } } catch { - setInputValue(message.text) + programmaticInsertRef.current = false + setInputValueProgrammatic(message.text) } } @@ -202,7 +213,7 @@ export const ChatTextArea = forwardRef( message.text + " " + currentValue.slice(cursorPos) - setInputValue(newValue) + setInputValueProgrammatic(newValue) // Set cursor position after the inserted text const newCursorPos = cursorPos + prefix.length + message.text.length + 1 @@ -233,7 +244,7 @@ export const ChatTextArea = forwardRef( window.addEventListener("message", messageHandler) return () => window.removeEventListener("message", messageHandler) - }, [setInputValue, searchRequestId, normalizedInputValue]) + }, [setInputValueProgrammatic, searchRequestId, normalizedInputValue]) const [isDraggingOver, setIsDraggingOver] = useState(false) const [textAreaBaseHeight, setTextAreaBaseHeight] = useState(undefined) @@ -278,9 +289,9 @@ export const ChatTextArea = forwardRef( setIsEnhancingPrompt(true) vscode.postMessage({ type: "enhancePrompt" as const, text: trimmedInput }) } else { - setInputValue(t("chat:enhancePromptDescription")) + setInputValueProgrammatic(t("chat:enhancePromptDescription")) } - }, [normalizedInputValue, setInputValue, t]) + }, [normalizedInputValue, setInputValueProgrammatic, t]) const allModes = useMemo(() => getAllModes(customModes), [customModes]) @@ -613,7 +624,12 @@ export const ChatTextArea = forwardRef( const handleInputChange = useCallback( (e: React.ChangeEvent) => { const newValue = e.target.value - setInputValue(newValue) + if (programmaticInsertRef.current) { + programmaticInsertRef.current = false + setInputValueProgrammatic(newValue) + } else { + setInputValue(newValue) + } // Reset history navigation when user types resetOnInputChange() @@ -675,7 +691,14 @@ export const ChatTextArea = forwardRef( setFileSearchResults([]) // Clear file search results. } }, - [setInputValue, setSearchRequestId, setFileSearchResults, setSearchLoading, resetOnInputChange], + [ + setInputValue, + setInputValueProgrammatic, + setSearchRequestId, + setFileSearchResults, + setSearchLoading, + resetOnInputChange, + ], ) useEffect(() => { diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index b9430bd722..19f28b856f 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -668,7 +668,12 @@ const ChatViewComponent: React.ForwardRefRenderFunction ({ Trans: ({ children }: { children?: React.ReactNode }) => <>{children}, })) +// Narrow subset of the real ChatTextArea props the double consumes. +interface MockChatTextAreaProps { + onSend: () => void + inputValue?: string + setInputValue?: (value: string) => void + setInputValuePreservingAuthorization?: (value: string) => void +} + // ChatTextArea double: an input whose Enter triggers onSend and whose -// changes flow through the provided setInputValue. +// changes flow through the provided setInputValue. Programmatic updates +// mirror the real component and keep the authorization scope. vi.mock("../ChatTextArea", async () => { const React = await import("react") - const ChatTextAreaComponent = React.forwardRef(function MockChatTextArea(props: any, _ref: unknown) { + const ChatTextAreaComponent = React.forwardRef(function MockChatTextArea( + props: MockChatTextAreaProps, + _ref: unknown, + ) { + React.useEffect(() => { + const listener = (event: MessageEvent) => { + if (event.data?.type === "enhancedPrompt" && typeof event.data.text === "string") { + const update = props.setInputValuePreservingAuthorization ?? props.setInputValue + update?.(event.data.text) + } + } + window.addEventListener("message", listener) + return () => window.removeEventListener("message", listener) + }, [props]) + return (
) => props.setInputValue(e.target.value)} + onChange={(e: React.ChangeEvent) => props.setInputValue?.(e.target.value)} onKeyDown={(e: React.KeyboardEvent) => { if (e.key === "Enter" && !e.shiftKey) { e.preventDefault() @@ -94,13 +117,29 @@ vi.mock("@vscode/webview-ui-toolkit/react", () => ({ VSCodeButton: ({ children, onClick }: { children: React.ReactNode; onClick?: () => void }) => ( ), - VSCodeCheckbox: ({ children, checked, onChange }: any) => ( + VSCodeCheckbox: ({ + children, + checked, + onChange, + }: { + children?: React.ReactNode + checked?: boolean + onChange?: (event: React.ChangeEvent) => void + }) => ( ), - VSCodeTextField: ({ value, onInput, placeholder }: any) => ( + VSCodeTextField: ({ + value, + onInput, + placeholder, + }: { + value?: string + onInput?: (e: { target: { value: string } }) => void + placeholder?: string + }) => ( { expect(message).toMatchObject({ authorizedMentionText: "explain this" }) }) - it("authorizes the whole value when no authorization was recorded", async () => { + it("authorizes nothing from inserted text when the sender omits the field", async () => { const { getByTestId } = renderChatView() await addGeneratedContext({}) const message = await sendNewTask(getByTestId) - expect(message).toMatchObject({ authorizedMentionText: "context with @/secret code" }) + expect(message).toMatchObject({ authorizedMentionText: "" }) }) it("re-authorizes the whole value after a manual edit", async () => { @@ -190,4 +229,20 @@ describe("ChatView - add-to-context mention authorization", () => { const message = await sendNewTask(getByTestId) expect(message).toMatchObject({ authorizedMentionText: "my own task with @/ok" }) }) + + it("keeps the authorization scope when an enhanced prompt replaces the text", async () => { + const { getByTestId } = renderChatView() + + await addGeneratedContext({ authorizedMentionText: "" }) + + await act(async () => { + window.postMessage({ type: "enhancedPrompt", text: "enhanced text with @/path" }, "*") + }) + await waitFor(() => { + expect(getChatBoxValue()).toBe("enhanced text with @/path") + }) + + const message = await sendNewTask(getByTestId) + expect(message).toMatchObject({ authorizedMentionText: "" }) + }) }) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index 42edc94b9f..a8029fa9bd 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -1243,6 +1243,7 @@ describe("ChatView - Message Queueing Tests", () => { type: "queueMessage", text: "follow-up question during spinner", images: [], + authorizedMentionText: "follow-up question during spinner", }) }) @@ -1314,6 +1315,7 @@ describe("ChatView - Message Queueing Tests", () => { askResponse: "messageResponse", text: "follow-up after completion", images: [], + authorizedMentionText: "follow-up after completion", }) }) @@ -1373,6 +1375,7 @@ describe("ChatView - Message Queueing Tests", () => { type: "queueMessage", text: "message during queue drain", images: [], + authorizedMentionText: "message during queue drain", }) }) @@ -1436,6 +1439,7 @@ describe("ChatView - Message Queueing Tests", () => { type: "queueMessage", text: "message during command execution", images: [], + authorizedMentionText: "message during command execution", }) }) @@ -1556,6 +1560,7 @@ describe("ChatView - Follow-up Suggestions", () => { askResponse: "messageResponse", text: "Use code mode", images: [], + authorizedMentionText: "Use code mode", }) }) @@ -1596,6 +1601,7 @@ describe("ChatView - Follow-up Suggestions", () => { askResponse: "messageResponse", text: "Use invalid mode", images: [], + authorizedMentionText: "Use invalid mode", }) }) expect(vscode.postMessage).not.toHaveBeenCalledWith(expect.objectContaining({ type: "mode" })) @@ -1656,6 +1662,7 @@ describe("ChatView - Follow-up Suggestions", () => { askResponse: "messageResponse", text: "Valid answer", images: [], + authorizedMentionText: "Valid answer", }) }) }) From d6e6d41872fe56879510962ed0da272b14d12b4d Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sat, 14 Nov 2026 22:01:38 +0000 Subject: [PATCH 05/15] fix(mentions): scope image, queue, and generated-text authorization Scan only the authorized substring for image mentions. Pass the stored scope through queued messages and submitUserMessage. Fail closed when a prompt enhancement replaces typed text or a suggestion is sent. --- src/core/task/Task.ts | 13 +++++-- src/core/task/__tests__/Task.spec.ts | 33 ++++++++++++---- .../webviewMessageHandler.newTask.spec.ts | 26 +++++++++++++ src/core/webview/webviewMessageHandler.ts | 38 ++++++++++++++----- .../__tests__/api-start-new-task.spec.ts | 6 ++- src/extension/api.ts | 2 +- webview-ui/src/components/chat/ChatView.tsx | 16 +++++++- .../chat/__tests__/ChatTextArea.spec.tsx | 22 +++++++++++ ...View.add-to-context-authorization.spec.tsx | 18 +++++++++ .../chat/__tests__/ChatView.spec.tsx | 6 +-- 10 files changed, 152 insertions(+), 28 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index ab475d096e..db84c8e471 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2637,6 +2637,7 @@ export class Task extends EventEmitter implements TaskLike { images?: string[], mode?: string, providerProfile?: string, + authorizedMentionText = "", ): Promise { try { text = (text ?? "").trim() @@ -2671,7 +2672,7 @@ export class Task extends EventEmitter implements TaskLike { // Handle the message directly instead of routing through the webview. // This avoids a race condition where the webview's message state hasn't // hydrated yet, causing it to interpret the message as a new task request. - this.handleWebviewAskResponse("messageResponse", text, images) + this.handleWebviewAskResponse("messageResponse", text, images, authorizedMentionText) } else { console.error("[Task#submitUserMessage] Provider reference lost") } @@ -6554,9 +6555,13 @@ export class Task extends EventEmitter implements TaskLike { const queued = this.messageQueueService.dequeueMessage() if (queued) { setTimeout(() => { - this.submitUserMessage(queued.text, queued.images).catch((err) => - console.error(`[Task] Failed to submit queued message:`, err), - ) + this.submitUserMessage( + queued.text, + queued.images, + undefined, + undefined, + queued.authorizedMentionText ?? "", + ).catch((err) => console.error(`[Task] Failed to submit queued message:`, err)) }, 0) } } diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 7f2c96e258..bb24d0755e 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -4261,11 +4261,25 @@ describe("Cline", () => { await task.submitUserMessage("test message", ["image1.png"]) // Verify handleWebviewAskResponse was called directly (not webview) - expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "test message", ["image1.png"]) + expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "test message", ["image1.png"], "") // Should NOT route through webview anymore expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() }) + it("forwards the authorized substring to the ask response", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "initial task", + startTask: false, + }) + const handleResponseSpy = vi.spyOn(task, "handleWebviewAskResponse") + + await task.submitUserMessage("a @/x b", [], undefined, undefined, "b") + + expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "a @/x b", [], "b") + }) + it("uses a mode selected through submitUserMessage in the next API request", async () => { vi.spyOn(mockProvider, "getState").mockResolvedValue(providerStateWith({ mode: "ask" })) vi.spyOn(mockProvider, "setMode").mockResolvedValue(undefined) @@ -4356,7 +4370,7 @@ describe("Cline", () => { task.clineMessages = [] await task.submitUserMessage("new task", ["image1.png"]) - expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "new task", ["image1.png"]) + expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "new task", ["image1.png"], "") // Clear mock handleResponseSpy.mockClear() @@ -4372,7 +4386,12 @@ describe("Cline", () => { ] await task.submitUserMessage("follow-up message", ["image2.png"]) - expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "follow-up message", ["image2.png"]) + expect(handleResponseSpy).toHaveBeenCalledWith( + "messageResponse", + "follow-up message", + ["image2.png"], + "", + ) }) it("should handle undefined provider gracefully", async () => { @@ -8133,7 +8152,7 @@ describe("Queued message processing after condense", () => { const submitSpy = vi.spyOn(task, "submitUserMessage").mockResolvedValue(undefined) // Queue a message during condensing - task.messageQueueService.addMessage("queued text", ["img1.png"]) + task.messageQueueService.addMessage("queued text", ["img1.png"], "queued") // Use fake timers to capture setTimeout(0) in processQueuedMessages vi.useFakeTimers() @@ -8143,7 +8162,7 @@ describe("Queued message processing after condense", () => { vi.runAllTimers() vi.useRealTimers() - expect(submitSpy).toHaveBeenCalledWith("queued text", ["img1.png"]) + expect(submitSpy).toHaveBeenCalledWith("queued text", ["img1.png"], undefined, undefined, "queued") expect(task.messageQueueService.isEmpty()).toBe(true) }) @@ -8179,7 +8198,7 @@ describe("Queued message processing after condense", () => { vi.runAllTimers() vi.useRealTimers() - expect(spyA).toHaveBeenCalledWith("A message", undefined) + expect(spyA).toHaveBeenCalledWith("A message", undefined, undefined, undefined, "") expect(spyB).not.toHaveBeenCalled() expect(taskB.messageQueueService.isEmpty()).toBe(false) @@ -8189,7 +8208,7 @@ describe("Queued message processing after condense", () => { vi.runAllTimers() vi.useRealTimers() - expect(spyB).toHaveBeenCalledWith("B message", undefined) + expect(spyB).toHaveBeenCalledWith("B message", undefined, undefined, undefined, "") expect(taskB.messageQueueService.isEmpty()).toBe(true) }) }) diff --git a/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts index c5de2d35e1..e108daa723 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts @@ -30,6 +30,11 @@ vi.mock("vscode", () => ({ commands: { executeCommand: vi.fn() }, })) +const { resolveImageMentions } = vi.hoisted(() => ({ + resolveImageMentions: vi.fn(async ({ images }: { images?: string[] }) => ({ text: "", images: images ?? [] })), +})) +vi.mock("../../mentions/resolveImageMentions", () => ({ resolveImageMentions })) + describe("webviewMessageHandler mention authorization forwarding", () => { // Narrow double: the handler under test only touches these members. // The single cast at construction keeps every use site type-checked. @@ -142,4 +147,25 @@ describe("webviewMessageHandler mention authorization forwarding", () => { "user line", ) }) + + it.each([ + { label: "an authorized substring", authorizedMentionText: "user @/a.png", expected: "user @/a.png" }, + { label: "an omitted field", authorizedMentionText: undefined, expected: "" }, + ])("scans only the authorized text for image mentions with $label", async ({ authorizedMentionText, expected }) => { + const message: WebviewMessage = { + type: "newTask", + text: "generated @/b.png user @/a.png", + authorizedMentionText, + } + await webviewMessageHandler(providerAsClineProvider, message) + + expect(resolveImageMentions).toHaveBeenCalledWith(expect.objectContaining({ text: expected })) + expect(provider.createTask).toHaveBeenCalledWith( + "generated @/b.png user @/a.png", + [], + undefined, + expect.anything(), + undefined, + ) + }) }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index f3790df67e..0f7c8f9516 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -226,21 +226,28 @@ export const webviewMessageHandler = async ( * Resolves image file mentions in incoming messages. * Matches read_file behavior: respects size limits and model capabilities. */ - const resolveIncomingImages = async (payload: { text?: string; images?: string[] }) => { + const resolveIncomingImages = async (payload: { + text?: string + images?: string[] + authorizedMentionText?: string + }) => { const text = payload.text ?? "" const images = payload.images const currentTask = provider.getCurrentTask() const state = await provider.getState() + // Scan only the authorized substring, so generated mentions attach no images. const resolved = await resolveImageMentions({ - text, + text: payload.authorizedMentionText ?? "", images, cwd: getCurrentCwd(), rooIgnoreController: currentTask?.rooIgnoreController, maxImageFileSize: state.maxImageFileSize, maxTotalImageSize: state.maxTotalImageSize, }) - return resolved + return { text, images: resolved.images } } + const authorizedTextOf = (message: { authorizedMentionText?: unknown }) => + typeof message.authorizedMentionText === "string" ? message.authorizedMentionText : "" /** * Shared utility to find message indices based on timestamp. * When multiple messages share the same timestamp (e.g., after condense), @@ -711,7 +718,11 @@ export const webviewMessageHandler = async ( // agentically running promises in old instance don't affect our new // task. This essentially creates a fresh slate for the new task. try { - const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) + const resolved = await resolveIncomingImages({ + text: message.text, + images: message.images, + authorizedMentionText: authorizedTextOf(message), + }) await provider.createTask( resolved.text, resolved.images, @@ -720,8 +731,7 @@ export const webviewMessageHandler = async ( taskId: message.taskId, // Fail closed when the webview omits the authorized // substring, so generated context cannot expand. - authorizedMentionText: - typeof message.authorizedMentionText === "string" ? message.authorizedMentionText : "", + authorizedMentionText: authorizedTextOf(message), }, message.taskConfiguration, ) @@ -742,14 +752,18 @@ export const webviewMessageHandler = async ( case "askResponse": { - const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) + const resolved = await resolveIncomingImages({ + text: message.text, + images: message.images, + authorizedMentionText: authorizedTextOf(message), + }) provider.getCurrentTask()?.handleWebviewAskResponse( message.askResponse!, resolved.text, resolved.images, // Fail closed when the webview omits the authorized // substring. - typeof message.authorizedMentionText === "string" ? message.authorizedMentionText : "", + authorizedTextOf(message), ) } break @@ -4037,12 +4051,16 @@ export const webviewMessageHandler = async ( */ case "queueMessage": { - const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) + const resolved = await resolveIncomingImages({ + text: message.text, + images: message.images, + authorizedMentionText: authorizedTextOf(message), + }) provider.getCurrentTask()?.messageQueueService.addMessage( resolved.text, resolved.images, // Fail closed when the webview omits the authorized substring. - typeof message.authorizedMentionText === "string" ? message.authorizedMentionText : "", + authorizedTextOf(message), ) break } diff --git a/src/extension/__tests__/api-start-new-task.spec.ts b/src/extension/__tests__/api-start-new-task.spec.ts index e33f2b3553..c2b26841b9 100644 --- a/src/extension/__tests__/api-start-new-task.spec.ts +++ b/src/extension/__tests__/api-start-new-task.spec.ts @@ -86,7 +86,11 @@ describe("API - startNewTask routing", () => { "new task", images, undefined, - { consecutiveMistakeLimit: Number.MAX_SAFE_INTEGER, toolRepetitionSoftLimit: 0 }, + { + consecutiveMistakeLimit: Number.MAX_SAFE_INTEGER, + toolRepetitionSoftLimit: 0, + authorizedMentionText: "new task", + }, configuration, ) expect(taskId).toBe(newTab ? "editor-task" : "sidebar-task") diff --git a/src/extension/api.ts b/src/extension/api.ts index e05513576a..6e52bac2c6 100644 --- a/src/extension/api.ts +++ b/src/extension/api.ts @@ -411,7 +411,7 @@ export class API extends EventEmitter implements RooCodeAPI { return } - await currentTask.submitUserMessage(text ?? "", images) + await currentTask.submitUserMessage(text ?? "", images, undefined, undefined, text ?? "") return } diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 19f28b856f..5c7c403da0 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -786,6 +786,12 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + authorizedMentionRef.current ??= inputValueRef.current + setInputValue(value) + }, []) + // Handle stop button click from textarea const handleStopTask = useCallback(() => { vscode.postMessage({ type: "cancelTask" }) @@ -800,7 +806,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction { expect(setInputValue).toHaveBeenCalledWith("Enhanced test prompt") }) + it("routes the enhanced prompt through the authorization-preserving setter when provided", () => { + const setInputValue = vi.fn() + const setInputValuePreservingAuthorization = vi.fn() + Object.defineProperty(document, "execCommand", { value: undefined, writable: true }) + + render( + , + ) + + window.dispatchEvent( + new MessageEvent("message", { data: { type: "enhancedPrompt", text: "Enhanced test prompt" } }), + ) + + expect(setInputValuePreservingAuthorization).toHaveBeenCalledWith("Enhanced test prompt") + expect(setInputValue).not.toHaveBeenCalled() + }) + it("should not crash when textarea ref is not available", () => { const setInputValue = vi.fn() diff --git a/webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx index 97f497a836..31e80b1040 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx @@ -245,4 +245,22 @@ describe("ChatView - add-to-context mention authorization", () => { const message = await sendNewTask(getByTestId) expect(message).toMatchObject({ authorizedMentionText: "" }) }) + + it("authorizes only the typed text when an enhanced prompt replaces a manual draft", async () => { + const { getByTestId } = renderChatView() + + const input = getByTestId("chat-textarea").querySelector("input")! + await act(async () => { + fireEvent.change(input, { target: { value: "my draft" } }) + }) + await act(async () => { + window.postMessage({ type: "enhancedPrompt", text: "enhanced with @/secret" }, "*") + }) + await waitFor(() => { + expect(getChatBoxValue()).toBe("enhanced with @/secret") + }) + + const message = await sendNewTask(getByTestId) + expect(message).toMatchObject({ text: "enhanced with @/secret", authorizedMentionText: "my draft" }) + }) }) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index a8029fa9bd..85e99f5185 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -1560,7 +1560,7 @@ describe("ChatView - Follow-up Suggestions", () => { askResponse: "messageResponse", text: "Use code mode", images: [], - authorizedMentionText: "Use code mode", + authorizedMentionText: "", }) }) @@ -1601,7 +1601,7 @@ describe("ChatView - Follow-up Suggestions", () => { askResponse: "messageResponse", text: "Use invalid mode", images: [], - authorizedMentionText: "Use invalid mode", + authorizedMentionText: "", }) }) expect(vscode.postMessage).not.toHaveBeenCalledWith(expect.objectContaining({ type: "mode" })) @@ -1662,7 +1662,7 @@ describe("ChatView - Follow-up Suggestions", () => { askResponse: "messageResponse", text: "Valid answer", images: [], - authorizedMentionText: "Valid answer", + authorizedMentionText: "", }) }) }) From 0c6092e69bdf58ca54804826ea48a31dc8020e33 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sun, 15 Nov 2026 00:16:50 +0000 Subject: [PATCH 06/15] fix(mentions): keep scope for edits, suggestions, and queued input Scan edited text for images and authorize it as user typed. Keep only the typed draft authorized when a suggestion is appended. Forward the scope for command-output queueing. Trim the authorized text with the message text. --- src/core/task/Task.ts | 1 + src/core/task/__tests__/Task.spec.ts | 14 +++++++ .../webview/__tests__/ClineProvider.spec.ts | 24 +++++++++-- .../webviewMessageHandler.newTask.spec.ts | 16 ++++++++ src/core/webview/webviewMessageHandler.ts | 11 ++++- webview-ui/src/components/chat/ChatView.tsx | 13 +++++- .../chat/__tests__/ChatTextArea.spec.tsx | 40 +++++++++++++++++++ .../chat/__tests__/ChatView.spec.tsx | 39 ++++++++++++++++++ 8 files changed, 151 insertions(+), 7 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index db84c8e471..40e2d616b2 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2641,6 +2641,7 @@ export class Task extends EventEmitter implements TaskLike { ): Promise { try { text = (text ?? "").trim() + authorizedMentionText = authorizedMentionText.trim() images = images ?? [] if (text.length === 0 && images.length === 0) { diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index bb24d0755e..1504d3997e 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -4280,6 +4280,20 @@ describe("Cline", () => { expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "a @/x b", [], "b") }) + it("trims the authorized substring the same way as the text", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "initial task", + startTask: false, + }) + const handleResponseSpy = vi.spyOn(task, "handleWebviewAskResponse") + + await task.submitUserMessage(" @/chosen.txt ", [], undefined, undefined, " @/chosen.txt ") + + expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "@/chosen.txt", [], "@/chosen.txt") + }) + it("uses a mode selected through submitUserMessage in the next API request", async () => { vi.spyOn(mockProvider, "getState").mockResolvedValue(providerStateWith({ mode: "ask" })) vi.spyOn(mockProvider, "setMode").mockResolvedValue(undefined) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 5a8c3a8fb2..609b3c1ce9 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -7245,7 +7245,13 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { expect(mockCline.overwriteClineMessages).toHaveBeenCalledWith([mockMessages[0]]) expect(mockCline.overwriteApiConversationHistory).toHaveBeenCalledWith([{ ts: 1000 }]) // Verify submitUserMessage was called with the edited content - expect(mockCline.submitUserMessage).toHaveBeenCalledWith("Edited message with preserved images", []) + expect(mockCline.submitUserMessage).toHaveBeenCalledWith( + "Edited message with preserved images", + [], + undefined, + undefined, + "Edited message with preserved images", + ) }) test("handles editing messages with file attachments", async () => { @@ -7298,7 +7304,13 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { }) expect(mockCline.overwriteClineMessages).toHaveBeenCalled() - expect(mockCline.submitUserMessage).toHaveBeenCalledWith("Edited message with file attachment", []) + expect(mockCline.submitUserMessage).toHaveBeenCalledWith( + "Edited message with file attachment", + [], + undefined, + undefined, + "Edited message with file attachment", + ) }) }) @@ -7829,7 +7841,13 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { await messageHandler({ type: "editMessageConfirm", messageTs: 2000, text: largeEditedContent }) expect(mockCline.overwriteClineMessages).toHaveBeenCalled() - expect(mockCline.submitUserMessage).toHaveBeenCalledWith(largeEditedContent, []) + expect(mockCline.submitUserMessage).toHaveBeenCalledWith( + largeEditedContent, + [], + undefined, + undefined, + largeEditedContent, + ) }) test("handles deleting messages with large payloads", async () => { diff --git a/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts index e108daa723..52c4dc0637 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts @@ -168,4 +168,20 @@ describe("webviewMessageHandler mention authorization forwarding", () => { undefined, ) }) + + it("scans the edited text for images and authorizes it as user typed", async () => { + const submitUserMessage = vi.fn() + const editTask = { + clineMessages: [{ ts: 1, type: "say", say: "user_feedback", text: "old" }], + apiConversationHistory: [], + overwriteClineMessages: vi.fn(), + overwriteApiConversationHistory: vi.fn(), + submitUserMessage, + } + provider.getCurrentTask.mockReturnValue(editTask) + const message: WebviewMessage = { type: "editMessageConfirm", messageTs: 1, text: "see @/a.png" } + await webviewMessageHandler(providerAsClineProvider, message).catch(() => {}) + + expect(resolveImageMentions).toHaveBeenCalledWith(expect.objectContaining({ text: "see @/a.png" })) + }) }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 0f7c8f9516..76195363ea 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -444,6 +444,7 @@ export const webviewMessageHandler = async ( editedContent: string, restoreCheckpoint?: boolean, images?: string[], + authorizedMentionText = "", ): Promise => { const currentCline = provider.getCurrentTask() if (!currentCline) { @@ -563,7 +564,7 @@ export const webviewMessageHandler = async ( // Update the UI to reflect the deletion await provider.postStateToWebview() - await currentCline.submitUserMessage(editedContent, images) + await currentCline.submitUserMessage(editedContent, images, undefined, undefined, authorizedMentionText) } catch (error) { console.error("Error in edit message:", error) vscode.window.showErrorMessage( @@ -2637,12 +2638,18 @@ export const webviewMessageHandler = async ( break case "editMessageConfirm": if (message.messageTs && message.text) { - const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) + // The edit box holds only text the user typed, so all of it is authorized. + const resolved = await resolveIncomingImages({ + text: message.text, + images: message.images, + authorizedMentionText: message.text, + }) await handleEditMessageConfirm( message.messageTs, resolved.text, message.restoreCheckpoint, resolved.images, + message.text, ) } break diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 5c7c403da0..0b93539591 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -1514,7 +1514,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction { return currentValue !== "" ? `${currentValue} \n${answer}` : answer }) @@ -1683,7 +1685,14 @@ const ChatViewComponent: React.ForwardRefRenderFunction { expect(setInputValue).not.toHaveBeenCalled() }) + it("routes the native enhanced-prompt input event through the preserving setter, then manual edits through setInputValue", () => { + const setInputValue = vi.fn() + const setInputValuePreservingAuthorization = vi.fn() + // Mimic the browser: execCommand fires an input event on the textarea. + const target: { el?: HTMLTextAreaElement } = {} + Object.defineProperty(document, "execCommand", { + value: vi.fn((_cmd: string, _ui: boolean, value: string) => { + if (target.el) { + fireEvent.change(target.el, { target: { value } }) + } + return true + }), + writable: true, + }) + + const { container } = render( + , + ) + const textarea = container.querySelector("textarea")! + target.el = textarea + + act(() => { + window.dispatchEvent( + new MessageEvent("message", { data: { type: "enhancedPrompt", text: "Enhanced test prompt" } }), + ) + }) + + expect(setInputValuePreservingAuthorization).toHaveBeenCalledWith("Enhanced test prompt") + expect(setInputValue).not.toHaveBeenCalled() + + fireEvent.change(textarea, { target: { value: "typed by hand" } }) + + expect(setInputValue).toHaveBeenCalledWith("typed by hand") + }) + it("should not crash when textarea ref is not available", () => { const setInputValue = vi.fn() diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index 85e99f5185..d2bd8ec7ca 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -1564,6 +1564,45 @@ describe("ChatView - Follow-up Suggestions", () => { }) }) + it("authorizes only the typed draft when a shift-clicked suggestion is appended", async () => { + const { getByRole, getByTestId } = renderChatView() + + mockPostMessage({ + mode: "ask", + customModes: [], + clineMessages: [ + { type: "say", say: "task", ts: Date.now() - 1000, text: "Initial task" }, + { + type: "ask", + ask: "followup", + ts: Date.now(), + text: JSON.stringify({ question: "Next?", suggest: [{ answer: "Use @/secret" }] }), + partial: false, + }, + ], + }) + + const suggestion = await waitFor(() => getByRole("button", { name: "Use @/secret" })) + const input = getByTestId("chat-textarea").querySelector("input")! as HTMLInputElement + await act(async () => { + fireEvent.change(input, { target: { value: "mine" } }) + }) + vscodePostMessageMock.cleanup() + + await act(async () => { + fireEvent.click(suggestion, { shiftKey: true }) + }) + await act(async () => { + fireEvent.keyDown(input, { key: "Enter", code: "Enter" }) + }) + + await waitFor(() => { + expect(vscode.postMessage).toHaveBeenCalledWith( + expect.objectContaining({ type: "askResponse", authorizedMentionText: "mine" }), + ) + }) + }) + it("does not switch modes for an unknown malformed object mode suggestion", async () => { const { getByRole } = renderChatView() From 2fa2bc11d0b59a51b92359bf39046acc4d4f2feb Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sun, 15 Nov 2026 00:40:53 +0000 Subject: [PATCH 07/15] refactor(mentions): carry typed text segments instead of an authorized substring Messages now carry segments with a source of user or generated. The extension expands mentions per user segment and never searches the text again. The chat box records manual edits as user text with a diff, so typing near generated context does not authorize it. Enhanced prompts, suggestions, and model text stay generated. Missing or mismatched segments fail closed. --- packages/types/src/message.ts | 16 +- packages/types/src/vscode-extension-host.ts | 10 +- .../processUserContentMentions.spec.ts | 44 ++++- .../mentions/processUserContentMentions.ts | 91 +++++----- src/core/message-queue/MessageQueueService.ts | 12 +- .../__tests__/MessageQueueService.spec.ts | 20 +++ src/core/task/Task.ts | 71 ++++---- src/core/task/__tests__/Task.spec.ts | 80 ++++++--- src/core/tools/AskFollowupQuestionTool.ts | 16 +- src/core/tools/AttemptCompletionTool.ts | 14 +- .../__tests__/askFollowupQuestionTool.spec.ts | 2 +- .../__tests__/attemptCompletionTool.spec.ts | 2 +- src/core/webview/ClineProvider.ts | 28 ++- .../webview/__tests__/ClineProvider.spec.ts | 34 ++-- .../webviewMessageHandler.newTask.spec.ts | 162 +++++++----------- .../__tests__/webviewMessageHandler.spec.ts | 4 +- src/core/webview/webviewMessageHandler.ts | 62 +++---- .../__tests__/api-start-new-task.spec.ts | 2 +- src/extension/api.ts | 8 +- src/shared/__tests__/mentionSegments.spec.ts | 79 +++++++++ src/shared/__tests__/support-prompts.spec.ts | 24 +++ src/shared/mentionSegments.ts | 93 ++++++++++ src/shared/support-prompt.ts | 22 +++ .../src/components/chat/ChatTextArea.tsx | 15 +- webview-ui/src/components/chat/ChatView.tsx | 143 ++++++++++------ .../chat/__tests__/ChatTextArea.spec.tsx | 16 +- ...View.add-to-context-authorization.spec.tsx | 86 +++++++--- .../chat/__tests__/ChatView.spec.tsx | 24 ++- 28 files changed, 751 insertions(+), 429 deletions(-) create mode 100644 src/shared/__tests__/mentionSegments.spec.ts create mode 100644 src/shared/mentionSegments.ts diff --git a/packages/types/src/message.ts b/packages/types/src/message.ts index 8a9842b454..f50142602d 100644 --- a/packages/types/src/message.ts +++ b/packages/types/src/message.ts @@ -367,6 +367,18 @@ export const tokenUsageSchema = z.object({ export type TokenUsage = z.infer +/** + * MentionSegment + */ + +/** A part of a message with a known origin. Only `user` parts may have mentions expanded. */ +export const mentionSegmentSchema = z.object({ + text: z.string(), + source: z.enum(["user", "generated"]), +}) + +export type MentionSegment = z.infer + /** * QueuedMessage */ @@ -376,8 +388,8 @@ export const queuedMessageSchema = z.object({ id: z.string(), text: z.string(), images: z.array(z.string()).optional(), - /** User-authored substring of `text` that may have mentions expanded. */ - authorizedMentionText: z.string().optional(), + /** Origin of each part of `text`. Absent means all generated. */ + segments: z.array(mentionSegmentSchema).optional(), }) export type QueuedMessage = z.infer diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index e7a6d4b6d9..516cfe2479 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -5,7 +5,7 @@ import type { ProviderSettings, ProviderSettingsEntry } from "./provider-setting import type { HistoryItem } from "./history.js" import type { ModeConfig, PromptComponent } from "./mode.js" import type { Experiments } from "./experiment.js" -import type { ClineMessage, QueuedMessage } from "./message.js" +import type { ClineMessage, MentionSegment, QueuedMessage } from "./message.js" import type { MarketplaceItem, MarketplaceInstalledMetadata, InstallMarketplaceItemOptions } from "./marketplace.js" import type { TodoItem } from "./todo.js" import type { CloudUserInfo, CloudOrganizationMembership, OrganizationAllowList, ShareVisibility } from "./cloud.js" @@ -133,8 +133,8 @@ export interface ExtensionMessage { | "switchTab" | "toggleAutoApprove" invoke?: "newChat" | "sendMessage" | "primaryButtonClick" | "secondaryButtonClick" | "setChatBoxMessage" - /** With `invoke: "setChatBoxMessage"`: user-authored substring of `text` that may have mentions expanded. */ - authorizedMentionText?: string + /** With `invoke: "setChatBoxMessage"`: origin of each part of `text`. Absent means all generated. */ + segments?: MentionSegment[] /** * Partial state updates are allowed to reduce message size (e.g. omit large fields like taskHistory). * The webview is responsible for merging. @@ -819,8 +819,8 @@ export interface WebviewMessage { updatedSettings?: RooCodeSettings /** Task configuration applied via `createTask()`. */ taskConfiguration?: RooCodeSettings - /** User-authored substring of `text` that may have mentions expanded. Empty disables expansion. */ - authorizedMentionText?: string + /** Origin of each part of `text`. Absent or not matching `text` means all generated. */ + segments?: MentionSegment[] // Worktree properties worktreePath?: string worktreeBranch?: string diff --git a/src/core/mentions/__tests__/processUserContentMentions.spec.ts b/src/core/mentions/__tests__/processUserContentMentions.spec.ts index de45ee237f..15a1bd3e6e 100644 --- a/src/core/mentions/__tests__/processUserContentMentions.spec.ts +++ b/src/core/mentions/__tests__/processUserContentMentions.spec.ts @@ -1,10 +1,14 @@ import type Anthropic from "@anthropic-ai/sdk" -import { processUserContentMentions, userMentionContent } from "../processUserContentMentions" +import type { MentionSegment } from "@roo-code/types" +import { processUserContentMentions, userMentionContent, wrapUserMessage } from "../processUserContentMentions" import { parseMentions } from "../index" import { FileContextTracker } from "../../context-tracking/FileContextTracker" vi.mock("../index", () => ({ parseMentions: vi.fn() })) +const user = (text: string): MentionSegment => ({ text, source: "user" }) +const gen = (text: string): MentionSegment => ({ text, source: "generated" }) + describe("processUserContentMentions authorization", () => { beforeEach(() => { vi.clearAllMocks() @@ -34,7 +38,7 @@ describe("processUserContentMentions authorization", () => { }) it("expands explicit user input without requiring a textual marker", async () => { - const result = await process([userMentionContent("@/allowed", "@/allowed")]) + const result = await process([userMentionContent([user("@/allowed")])]) expect(parseMentions).toHaveBeenCalledWith( "@/allowed", "/test", @@ -56,18 +60,46 @@ describe("processUserContentMentions authorization", () => { }) }) - it("expands only the user-authored portion of a support prompt", async () => { + it("expands only the user segments of a support prompt", async () => { const context = "@/secret @terminal" - const result = await process([userMentionContent(`@/allowed\n${context}`, "@/allowed")]) + const result = await process([userMentionContent([user("@/allowed"), gen(`\n${context}`)])]) expect(vi.mocked(parseMentions).mock.calls.map(([text]) => text)).toEqual(["@/allowed"]) expect(result.content[0]).toEqual({ type: "text", text: `parsed: @/allowed\n${context}` }) }) + it("expands each user segment on its own and leaves generated segments unchanged", async () => { + const result = await process([ + userMentionContent([gen("a @/x "), user("@/one"), gen(" b @/y "), user("@/two")]), + ]) + expect(vi.mocked(parseMentions).mock.calls.map(([text]) => text)).toEqual(["@/one", "@/two"]) + expect(result.content[0]).toEqual({ type: "text", text: "a @/x parsed: @/one b @/y parsed: @/two" }) + }) + + it("expands the same text only where it is a user segment", async () => { + const result = await process([userMentionContent([gen("@/same "), user("@/same")])]) + expect(vi.mocked(parseMentions).mock.calls.map(([text]) => text)).toEqual(["@/same"]) + expect(result.content[0]).toEqual({ type: "text", text: "@/same parsed: @/same" }) + }) + + it("does not expand all generated segments", async () => { + await process([userMentionContent([gen("@/secret")])]) + expect(parseMentions).not.toHaveBeenCalled() + }) + + it("wraps segments in tags that stay generated", async () => { + const result = await process([userMentionContent(wrapUserMessage([user("@/allowed")]))]) + expect(vi.mocked(parseMentions).mock.calls.map(([text]) => text)).toEqual(["@/allowed"]) + expect(result.content[0]).toEqual({ + type: "text", + text: "\nparsed: @/allowed\n", + }) + }) + it("does not trust restored, copied, or modified blocks", async () => { - const block = userMentionContent("@/allowed", "@/allowed") + const block = userMentionContent([user("@/allowed")]) const copy = { ...block } block.text += " @/secret" - await process([copy, block, userMentionContent("context @/secret", "")]) + await process([copy, block, userMentionContent([gen("context @/secret")])]) expect(parseMentions).not.toHaveBeenCalled() }) }) diff --git a/src/core/mentions/processUserContentMentions.ts b/src/core/mentions/processUserContentMentions.ts index 0803d4a3df..90d9c255ed 100644 --- a/src/core/mentions/processUserContentMentions.ts +++ b/src/core/mentions/processUserContentMentions.ts @@ -1,4 +1,5 @@ import Anthropic from "@anthropic-ai/sdk" +import type { MentionSegment } from "@roo-code/types" import { parseMentions, MentionContentBlock } from "./index" import { FileContextTracker } from "../context-tracking/FileContextTracker" @@ -8,16 +9,26 @@ import type { SkillLookup } from "../../services/skills/skillInvocation" type TextPart = Anthropic.Messages.TextBlockParam type ImagePart = Anthropic.Messages.ImageBlockParam -// Authorization stays in memory and cannot be forged by message text or restored history. -const authorizedInputs = new WeakMap() +// Origin labels stay in memory and cannot be forged by message text or restored history. +const segmentedBlocks = new WeakMap() -/** Call only at a user-input boundary, before adding lower-trust context. */ -export function userMentionContent(text: string, input: string): TextPart { +/** Call only at a user-input boundary. Only `user` segments may have mentions expanded. */ +export function userMentionContent(segments: MentionSegment[]): TextPart { + const text = segments.map((s) => s.text).join("") const block: TextPart = { type: "text", text } - if (input && text.includes(input)) authorizedInputs.set(block, { text, input }) + segmentedBlocks.set(block, { text, segments }) return block } +/** Wraps segments in the `` tags. The tags are generated text. */ +export function wrapUserMessage(segments: MentionSegment[]): MentionSegment[] { + return [ + { text: "\n", source: "generated" }, + ...segments, + { text: "\n", source: "generated" }, + ] +} + export interface ProcessUserContentMentionsResult { content: Anthropic.Messages.ContentBlockParam[] mode?: string // Mode from the first slash command that has one @@ -72,46 +83,38 @@ export async function processUserContentMentions({ await Promise.all( userContent.map(async (block) => { if (block.type === "text") { - const authorized = authorizedInputs.get(block) - if (authorized && authorized.text === block.text) { - const result = await parseMentions( - authorized.input, - cwd, - fileContextTracker, - rooIgnoreController, - showRooIgnoredFiles, - includeDiagnosticMessages, - maxDiagnosticMessages, - skillsManager, - currentMode, - ) - // Capture the first mode found - if (!commandMode && result.mode) { - commandMode = result.mode - } - - // Build the blocks array: - // 1. User's text (with @ mentions replaced by clean paths) - // 2. File/folder content blocks (formatted like read_file results) - // 3. Slash command help (if any) - const blocks: Array = [ - { - ...block, - text: block.text.replace(authorized.input, () => result.text), - }, - ] - - // Add file/folder content as separate blocks - if (result.contentBlocks.length > 0) { - blocks.push(...contentBlocksToTextParts(result.contentBlocks)) - } - - if (result.slashCommandHelp) { - blocks.push({ - type: "text" as const, - text: result.slashCommandHelp, - }) + const labeled = segmentedBlocks.get(block) + if (labeled && labeled.text === block.text) { + let text = "" + const extra: TextPart[] = [] + for (const segment of labeled.segments) { + if (segment.source !== "user") { + text += segment.text + continue + } + const result = await parseMentions( + segment.text, + cwd, + fileContextTracker, + rooIgnoreController, + showRooIgnoredFiles, + includeDiagnosticMessages, + maxDiagnosticMessages, + skillsManager, + currentMode, + ) + // Capture the first mode found + if (!commandMode && result.mode) { + commandMode = result.mode + } + text += result.text + // File/folder content as separate blocks (formatted like read_file results) + extra.push(...contentBlocksToTextParts(result.contentBlocks)) + if (result.slashCommandHelp) { + extra.push({ type: "text" as const, text: result.slashCommandHelp }) + } } + const blocks: Array = [{ ...block, text }, ...extra] return blocks } diff --git a/src/core/message-queue/MessageQueueService.ts b/src/core/message-queue/MessageQueueService.ts index 57fb7781b0..869aeee9f1 100644 --- a/src/core/message-queue/MessageQueueService.ts +++ b/src/core/message-queue/MessageQueueService.ts @@ -2,7 +2,9 @@ import { EventEmitter } from "events" import { v4 as uuidv4 } from "uuid" -import { QueuedMessage } from "@roo-code/types" +import type { MentionSegment, QueuedMessage } from "@roo-code/types" + +import { userSegments } from "../../shared/mentionSegments" export interface QueueEvents { stateChanged: [messages: QueuedMessage[]] @@ -28,7 +30,7 @@ export class MessageQueueService extends EventEmitter { return { index, message: this._messages[index] } } - public addMessage(text: string, images?: string[], authorizedMentionText?: string): QueuedMessage | undefined { + public addMessage(text: string, images?: string[], segments?: MentionSegment[]): QueuedMessage | undefined { if (!text && !images?.length) { return undefined } @@ -38,7 +40,7 @@ export class MessageQueueService extends EventEmitter { id: uuidv4(), text, images, - authorizedMentionText, + segments, } this._messages.push(message) @@ -70,8 +72,8 @@ export class MessageQueueService extends EventEmitter { message.timestamp = Date.now() message.text = text message.images = images - // The edited text is user-authored, so it replaces any recorded scope. - message.authorizedMentionText = text + // The edited text is user-authored, so it replaces any recorded segments. + message.segments = userSegments(text) this.emit("stateChanged", this._messages) return true } diff --git a/src/core/message-queue/__tests__/MessageQueueService.spec.ts b/src/core/message-queue/__tests__/MessageQueueService.spec.ts index 567dca314b..a81d4c06f7 100644 --- a/src/core/message-queue/__tests__/MessageQueueService.spec.ts +++ b/src/core/message-queue/__tests__/MessageQueueService.spec.ts @@ -68,3 +68,23 @@ describe("MessageQueueService claims", () => { expect(queue.hasUnclaimed()).toBe(true) }) }) + +describe("MessageQueueService segments", () => { + it("stores the segments with the message", () => { + const queue = new MessageQueueService() + const segments = [ + { text: "ctx ", source: "generated" as const }, + { text: "mine", source: "user" as const }, + ] + expect(queue.addMessage("ctx mine", undefined, segments)?.segments).toEqual(segments) + }) + + it("replaces the segments with user text when the user edits the message", () => { + const queue = new MessageQueueService() + const message = queue.addMessage("ctx @/x", undefined, [{ text: "ctx @/x", source: "generated" }])! + + queue.updateMessage(message.id, "edited @/y") + + expect(queue.messages[0]?.segments).toEqual([{ text: "edited @/y", source: "user" }]) + }) +}) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 40e2d616b2..9f38d6d24d 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -57,6 +57,7 @@ import { MAX_MCP_TOOLS_THRESHOLD, countEnabledMcpTools, providerIdentifiers, + type MentionSegment, } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" import { CloudService } from "@roo-code/cloud" @@ -134,7 +135,8 @@ import { checkpointRestore, checkpointDiff, } from "../checkpoints" -import { processUserContentMentions, userMentionContent } from "../mentions/processUserContentMentions" +import { joinSegments, normalizeSegments, trimSegments } from "../../shared/mentionSegments" +import { processUserContentMentions, userMentionContent, wrapUserMessage } from "../mentions/processUserContentMentions" import { getMessagesSinceLastSummary, summarizeConversation, getEffectiveApiHistory } from "../condense" import { MessageQueueService } from "../message-queue/MessageQueueService" import { type AutoDenyDetail, AutoApprovalHandler, checkAutoApproval } from "../auto-approval" @@ -254,8 +256,8 @@ export interface TaskOptions extends CreateTaskOptions { checkpointTimeout?: number consecutiveMistakeLimit?: number task?: string - /** User-authored portion of a generated support prompt; empty disables expansion. */ - authorizedMentionText?: string + /** Origin of each part of `task`. Absent means all generated, so no mention expands. */ + segments?: MentionSegment[] images?: string[] historyItem?: HistoryItem experiments?: Record @@ -452,7 +454,7 @@ export class Task extends EventEmitter implements TaskLike { private askResponse?: ClineAskResponse private askResponseText?: string private askResponseImages?: string[] - private askResponseAuthorizedMentionText?: string + private askResponseSegments?: MentionSegment[] /** * Structured detail of the automatic denial that resolved the current ask, * set when `checkAutoApproval` denies via policy (denylist, blanket @@ -696,10 +698,10 @@ export class Task extends EventEmitter implements TaskLike { // MessageManager for high-level message operations (lazy initialized) private _messageManager?: MessageManager - private readonly authorizedMentionText?: string + private readonly segments?: MentionSegment[] constructor({ - authorizedMentionText, + segments, provider, apiConfiguration, enableCheckpoints = true, @@ -724,7 +726,7 @@ export class Task extends EventEmitter implements TaskLike { handoffExecutionContext, }: TaskOptions) { super() - this.authorizedMentionText = authorizedMentionText + this.segments = segments this.resetAssistantMessagePersistence() @@ -1259,12 +1261,7 @@ export class Task extends EventEmitter implements TaskLike { } private handleQueuedAskResponse(message: QueuedMessage, resolution: QueuedAskResolution): string | undefined { - this.handleWebviewAskResponse( - resolution.response, - message.text, - message.images, - message.authorizedMentionText ?? "", - ) + this.handleWebviewAskResponse(resolution.response, message.text, message.images, message.segments) if (resolution.requiresDurableAck) { return message.id } @@ -1934,8 +1931,8 @@ export class Task extends EventEmitter implements TaskLike { response: ClineAskResponse text?: string images?: string[] - /** User-authored substring of `text` that may have mentions expanded. */ - authorizedMentionText?: string + /** Origin of each part of `text`. */ + segments?: MentionSegment[] queuedMessageId?: string /** * Present when the ask was resolved by an automatic (policy) denial @@ -2151,7 +2148,7 @@ export class Task extends EventEmitter implements TaskLike { // Store the auto-approval timeout so it can be cancelled if user interacts this.autoApprovalTimeoutRef = setTimeout(() => { const { askResponse, text, images } = approval.fn() - this.handleWebviewAskResponse(askResponse, text, images, "") + this.handleWebviewAskResponse(askResponse, text, images) this.autoApprovalTimeoutRef = undefined }, approval.timeout) timeouts.push(this.autoApprovalTimeoutRef) @@ -2454,14 +2451,14 @@ export class Task extends EventEmitter implements TaskLike { response: this.askResponse!, text: this.askResponseText, images: this.askResponseImages, - authorizedMentionText: this.askResponseAuthorizedMentionText ?? "", + segments: this.askResponseSegments, queuedMessageId, autoDenyDetail: this.pendingAutoDenyDetail, } this.askResponse = undefined this.askResponseText = undefined this.askResponseImages = undefined - this.askResponseAuthorizedMentionText = undefined + this.askResponseSegments = undefined this.pendingAutoDenyDetail = undefined // Switch back to an active state. @@ -2488,7 +2485,7 @@ export class Task extends EventEmitter implements TaskLike { askResponse: ClineAskResponse, text?: string, images?: string[], - authorizedMentionText = "", + segments?: MentionSegment[], ) { // Clear any pending auto-approval timeout when user responds this.cancelAutoApprovalTimeout() @@ -2496,7 +2493,7 @@ export class Task extends EventEmitter implements TaskLike { this.askResponse = askResponse this.askResponseText = text this.askResponseImages = images - this.askResponseAuthorizedMentionText = authorizedMentionText + this.askResponseSegments = segments // Create a checkpoint whenever the user sends a message. // Use allowEmpty=true to ensure a checkpoint is recorded even if there are no file changes. @@ -2637,11 +2634,12 @@ export class Task extends EventEmitter implements TaskLike { images?: string[], mode?: string, providerProfile?: string, - authorizedMentionText = "", + segments?: MentionSegment[], ): Promise { try { - text = (text ?? "").trim() - authorizedMentionText = authorizedMentionText.trim() + // Trim the text and its segments together so they stay equal. + const trimmed = trimSegments(normalizeSegments(text ?? "", segments)) + text = joinSegments(trimmed) images = images ?? [] if (text.length === 0 && images.length === 0) { @@ -2673,7 +2671,7 @@ export class Task extends EventEmitter implements TaskLike { // Handle the message directly instead of routing through the webview. // This avoids a race condition where the webview's message state hasn't // hydrated yet, causing it to interpret the message as a new task request. - this.handleWebviewAskResponse("messageResponse", text, images, authorizedMentionText) + this.handleWebviewAskResponse("messageResponse", text, images, trimmed) } else { console.error("[Task#submitUserMessage] Provider reference lost") } @@ -3183,10 +3181,9 @@ export class Task extends EventEmitter implements TaskLike { // Task starting await this.initiateTaskLoop([ - // Without explicit authorization nothing expands. Task text - // can carry generated context, so callers must state the - // user-authored part. - userMentionContent(`\n${task}\n`, this.authorizedMentionText ?? ""), + // Task text can carry generated context. Without segments + // nothing expands. + userMentionContent(wrapUserMessage(normalizeSegments(task ?? "", this.segments))), ...imageBlocks, ]).catch((error) => { // Swallow loop rejection when the task was intentionally abandoned/aborted @@ -3326,7 +3323,7 @@ export class Task extends EventEmitter implements TaskLike { this.markInitialized() - const { response, text, images, authorizedMentionText } = await this.ask(askType) // Calls `postStateToWebview`. + const { response, text, images, segments } = await this.ask(askType) // Calls `postStateToWebview`. let responseText: string | undefined let responseImages: string[] | undefined @@ -3448,10 +3445,8 @@ export class Task extends EventEmitter implements TaskLike { const newUserContent: Anthropic.Messages.ContentBlockParam[] = [...modifiedOldUserContent] if (responseText) { - // Only the chat-box substring the webview authorized expands. - newUserContent.push( - userMentionContent(`\n${responseText}\n`, authorizedMentionText ?? ""), - ) + // Only user segments expand. + newUserContent.push(userMentionContent(wrapUserMessage(normalizeSegments(responseText, segments)))) } if (responseImages && responseImages.length > 0) { @@ -6556,13 +6551,9 @@ export class Task extends EventEmitter implements TaskLike { const queued = this.messageQueueService.dequeueMessage() if (queued) { setTimeout(() => { - this.submitUserMessage( - queued.text, - queued.images, - undefined, - undefined, - queued.authorizedMentionText ?? "", - ).catch((err) => console.error(`[Task] Failed to submit queued message:`, err)) + this.submitUserMessage(queued.text, queued.images, undefined, undefined, queued.segments).catch( + (err) => console.error(`[Task] Failed to submit queued message:`, err), + ) }, 0) } } diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 1504d3997e..eaf3065af0 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -3528,10 +3528,14 @@ describe("Cline", () => { type: "text", text: "Regular text with 'some/path' (see below for file content)", } as const, - userMentionContent( - "Text with 'some/path' (see below for file content) in user_message tags", - "Text with 'some/path' (see below for file content) in user_message tags", - ), + userMentionContent([ + { text: "", source: "generated" }, + { + text: "Text with 'some/path' (see below for file content) in user_message tags", + source: "user", + }, + { text: "", source: "generated" }, + ]), { type: "tool_result", tool_use_id: "test-id", @@ -3592,14 +3596,18 @@ describe("Cline", () => { }) it.each([ - { label: "empty string", authorizedMentionText: "" }, - { label: "omitted option", authorizedMentionText: undefined }, - ])("does not expand task text when authorization is $label", async ({ authorizedMentionText }) => { + { + label: "all generated", + segments: [{ text: "read '@/secret' please", source: "generated" as const }], + }, + { label: "omitted", segments: undefined }, + { label: "not matching the text", segments: [{ text: "other", source: "user" as const }] }, + ])("does not expand task text when segments are $label", async ({ segments }) => { const [cline, task] = Task.create({ provider: mockProvider, apiConfiguration: mockApiConfig, task: "read '@/secret' please", - authorizedMentionText, + segments, }) // Wait until the loop reached mention processing, then stop it. @@ -3616,7 +3624,7 @@ describe("Cline", () => { provider: mockProvider, apiConfiguration: mockApiConfig, task: taskText, - authorizedMentionText: taskText, + segments: [{ text: taskText, source: "user" }], }) await vi.waitFor(() => expect(parseMentions).toHaveBeenCalled()) @@ -3631,7 +3639,10 @@ describe("Cline", () => { provider: mockProvider, apiConfiguration: mockApiConfig, task: "context @/context.txt\nchosen @/chosen.txt", - authorizedMentionText: "chosen @/chosen.txt", + segments: [ + { text: "context @/context.txt\n", source: "generated" }, + { text: "chosen @/chosen.txt", source: "user" }, + ], }) await vi.waitFor(() => expect(parseMentions).toHaveBeenCalled()) @@ -4261,12 +4272,17 @@ describe("Cline", () => { await task.submitUserMessage("test message", ["image1.png"]) // Verify handleWebviewAskResponse was called directly (not webview) - expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "test message", ["image1.png"], "") + expect(handleResponseSpy).toHaveBeenCalledWith( + "messageResponse", + "test message", + ["image1.png"], + [{ text: "test message", source: "generated" }], + ) // Should NOT route through webview anymore expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() }) - it("forwards the authorized substring to the ask response", async () => { + it("forwards the segments to the ask response", async () => { const task = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, @@ -4275,12 +4291,16 @@ describe("Cline", () => { }) const handleResponseSpy = vi.spyOn(task, "handleWebviewAskResponse") - await task.submitUserMessage("a @/x b", [], undefined, undefined, "b") + const segments = [ + { text: "a @/x ", source: "generated" as const }, + { text: "b", source: "user" as const }, + ] + await task.submitUserMessage("a @/x b", [], undefined, undefined, segments) - expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "a @/x b", [], "b") + expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "a @/x b", [], segments) }) - it("trims the authorized substring the same way as the text", async () => { + it("trims the segments the same way as the text", async () => { const task = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, @@ -4289,9 +4309,16 @@ describe("Cline", () => { }) const handleResponseSpy = vi.spyOn(task, "handleWebviewAskResponse") - await task.submitUserMessage(" @/chosen.txt ", [], undefined, undefined, " @/chosen.txt ") + await task.submitUserMessage(" @/chosen.txt ", [], undefined, undefined, [ + { text: " @/chosen.txt ", source: "user" }, + ]) - expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "@/chosen.txt", [], "@/chosen.txt") + expect(handleResponseSpy).toHaveBeenCalledWith( + "messageResponse", + "@/chosen.txt", + [], + [{ text: "@/chosen.txt", source: "user" }], + ) }) it("uses a mode selected through submitUserMessage in the next API request", async () => { @@ -4384,7 +4411,12 @@ describe("Cline", () => { task.clineMessages = [] await task.submitUserMessage("new task", ["image1.png"]) - expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "new task", ["image1.png"], "") + expect(handleResponseSpy).toHaveBeenCalledWith( + "messageResponse", + "new task", + ["image1.png"], + [{ text: "new task", source: "generated" }], + ) // Clear mock handleResponseSpy.mockClear() @@ -4404,7 +4436,7 @@ describe("Cline", () => { "messageResponse", "follow-up message", ["image2.png"], - "", + [{ text: "follow-up message", source: "generated" }], ) }) @@ -8166,7 +8198,7 @@ describe("Queued message processing after condense", () => { const submitSpy = vi.spyOn(task, "submitUserMessage").mockResolvedValue(undefined) // Queue a message during condensing - task.messageQueueService.addMessage("queued text", ["img1.png"], "queued") + task.messageQueueService.addMessage("queued text", ["img1.png"], [{ text: "queued text", source: "user" }]) // Use fake timers to capture setTimeout(0) in processQueuedMessages vi.useFakeTimers() @@ -8176,7 +8208,9 @@ describe("Queued message processing after condense", () => { vi.runAllTimers() vi.useRealTimers() - expect(submitSpy).toHaveBeenCalledWith("queued text", ["img1.png"], undefined, undefined, "queued") + expect(submitSpy).toHaveBeenCalledWith("queued text", ["img1.png"], undefined, undefined, [ + { text: "queued text", source: "user" }, + ]) expect(task.messageQueueService.isEmpty()).toBe(true) }) @@ -8212,7 +8246,7 @@ describe("Queued message processing after condense", () => { vi.runAllTimers() vi.useRealTimers() - expect(spyA).toHaveBeenCalledWith("A message", undefined, undefined, undefined, "") + expect(spyA).toHaveBeenCalledWith("A message", undefined, undefined, undefined, undefined) expect(spyB).not.toHaveBeenCalled() expect(taskB.messageQueueService.isEmpty()).toBe(false) @@ -8222,7 +8256,7 @@ describe("Queued message processing after condense", () => { vi.runAllTimers() vi.useRealTimers() - expect(spyB).toHaveBeenCalledWith("B message", undefined, undefined, undefined, "") + expect(spyB).toHaveBeenCalledWith("B message", undefined, undefined, undefined, undefined) expect(taskB.messageQueueService.isEmpty()).toBe(true) }) }) diff --git a/src/core/tools/AskFollowupQuestionTool.ts b/src/core/tools/AskFollowupQuestionTool.ts index 3504765747..54c46a0dce 100644 --- a/src/core/tools/AskFollowupQuestionTool.ts +++ b/src/core/tools/AskFollowupQuestionTool.ts @@ -1,4 +1,5 @@ -import { userMentionContent } from "../mentions/processUserContentMentions" +import { userMentionContent, wrapUserMessage } from "../mentions/processUserContentMentions" +import { normalizeSegments } from "../../shared/mentionSegments" import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import type { ToolUse } from "../../shared/tools" @@ -71,18 +72,11 @@ export class AskFollowupQuestionTool extends BaseTool<"ask_followup_question"> { } task.consecutiveMistakeCount = 0 - const { text, images, authorizedMentionText } = await task.ask( - "followup", - JSON.stringify(follow_up_json), - false, - ) + const { text, images, segments } = await task.ask("followup", JSON.stringify(follow_up_json), false) const safeText = text ?? "" await task.say("user_feedback", safeText, images) - // Authorizes only the chat-box substring the webview named. The - // tool result repeats the text by design. - task.userMessageContent.push( - userMentionContent(`\n${safeText}\n`, authorizedMentionText ?? ""), - ) + // Only user segments expand. The tool result repeats the text by design. + task.userMessageContent.push(userMentionContent(wrapUserMessage(normalizeSegments(safeText, segments)))) pushToolResult(formatResponse.toolResult(`\n${safeText}\n`, images)) } catch (error) { await handleError("asking question", error as Error) diff --git a/src/core/tools/AttemptCompletionTool.ts b/src/core/tools/AttemptCompletionTool.ts index 446204c5fc..0ff84f9d0f 100644 --- a/src/core/tools/AttemptCompletionTool.ts +++ b/src/core/tools/AttemptCompletionTool.ts @@ -1,4 +1,5 @@ -import { userMentionContent } from "../mentions/processUserContentMentions" +import { userMentionContent, wrapUserMessage } from "../mentions/processUserContentMentions" +import { normalizeSegments } from "../../shared/mentionSegments" import * as vscode from "vscode" import { RooCodeEventName, type HistoryItem, type PendingTaskAction } from "@roo-code/types" @@ -219,11 +220,7 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { task.flushTelemetryInstallment("attempt_completion") } - const { response, text, images, queuedMessageId, authorizedMentionText } = await task.ask( - "completion_result", - "", - false, - ) + const { response, text, images, queuedMessageId, segments } = await task.ask("completion_result", "", false) if (response === "yesButtonClicked") { // A stale history replay reruns this handler on a fresh Task instance for a @@ -250,9 +247,8 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { } const feedbackText = `\n${text}\n` - // Authorizes only the chat-box substring the webview named. The - // tool result repeats the text by design. - task.userMessageContent.push(userMentionContent(feedbackText, authorizedMentionText ?? "")) + // Only user segments expand. The tool result repeats the text by design. + task.userMessageContent.push(userMentionContent(wrapUserMessage(normalizeSegments(String(text), segments)))) pushToolResult(formatResponse.toolResult(feedbackText, images)) } catch (error) { await handleError("inspecting site", error as Error) diff --git a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts index e0e884a6e8..11c53a8e05 100644 --- a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts +++ b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts @@ -212,7 +212,7 @@ describe("AskFollowupQuestionTool", () => { it("pushes an authorized feedback block that mention processing expands", async () => { const params = { question: "What?", follow_up: [{ text: "A" }] } - askMock.mockResolvedValue({ text: "@/allowed", images: [], authorizedMentionText: "@/allowed" }) + askMock.mockResolvedValue({ text: "@/allowed", images: [], segments: [{ text: "@/allowed", source: "user" }] }) await tool.execute(params, mockTask, mockCallbacks) diff --git a/src/core/tools/__tests__/attemptCompletionTool.spec.ts b/src/core/tools/__tests__/attemptCompletionTool.spec.ts index a0a4224eba..8bde95d371 100644 --- a/src/core/tools/__tests__/attemptCompletionTool.spec.ts +++ b/src/core/tools/__tests__/attemptCompletionTool.spec.ts @@ -1179,7 +1179,7 @@ describe("attemptCompletionTool", () => { response: "messageResponse", text: "@/allowed", images: [], - authorizedMentionText: "@/allowed", + segments: [{ text: "@/allowed", source: "user" }], }) vi.mocked(parseMentions).mockImplementation(async (text) => ({ text: `parsed: ${text}`, diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 7640afe666..63bb9eea06 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -90,6 +90,7 @@ import { CloudService, getRooCodeApiUrl } from "@roo-code/cloud" import { Package } from "../../shared/package" import { findLast } from "../../shared/array" import { supportPrompt } from "../../shared/support-prompt" +import { generatedSegments, joinSegments } from "../../shared/mentionSegments" import { GlobalFileNames } from "../../shared/globalFileNames" import { Mode, defaultModeSlug, getModeBySlug } from "../../shared/modes" import { experimentDefault } from "../../shared/experiments" @@ -138,7 +139,7 @@ import { PendingActionSettlementError, Task } from "../task/Task" import { webviewMessageHandler } from "./webviewMessageHandler" import type { WebviewFocusTracker } from "./WebviewFocusTracker" -import type { ClineMessage, TodoItem } from "@roo-code/types" +import type { ClineMessage, MentionSegment, TodoItem } from "@roo-code/types" import { type ApiMessage, readApiMessages, @@ -1068,24 +1069,22 @@ export class ClineProvider const { customSupportPrompts } = await this.getState() // TODO: Improve type safety for promptType. - const prompt = supportPrompt.create(promptType, params, customSupportPrompts) + // Only the typed input is a user segment. The template and its other params are generated. + const segments = supportPrompt.createSegments(promptType, params, customSupportPrompts) + const prompt = joinSegments(segments) if (command === "addToContext") { await this.postMessageToWebview({ type: "invoke", invoke: "setChatBoxMessage", text: `${prompt}\n\n`, - // The generated prompt is context. Only the typed input may - // have mentions expanded. - authorizedMentionText: typeof params.userInput === "string" ? params.userInput : "", + segments: [...segments, ...generatedSegments("\n\n")], }) await this.postMessageToWebview({ type: "action", action: "focusInput" }) return } - await this.createTask(prompt, undefined, undefined, { - authorizedMentionText: typeof params.userInput === "string" ? params.userInput : "", - }) + await this.createTask(prompt, undefined, undefined, { segments }) } public static async handleTerminalAction( @@ -1102,25 +1101,22 @@ export class ClineProvider } const { customSupportPrompts } = await visibleProvider.getState() - const prompt = supportPrompt.create(promptType, params, customSupportPrompts) + const segments = supportPrompt.createSegments(promptType, params, customSupportPrompts) + const prompt = joinSegments(segments) if (command === "terminalAddToContext") { await visibleProvider.postMessageToWebview({ type: "invoke", invoke: "setChatBoxMessage", text: `${prompt}\n\n`, - // The template interpolates the typed input, so only that - // part may have mentions expanded. - authorizedMentionText: typeof params.userInput === "string" ? params.userInput : "", + segments: [...segments, ...generatedSegments("\n\n")], }) await visibleProvider.postMessageToWebview({ type: "action", action: "focusInput" }) return } try { - await visibleProvider.createTask(prompt, undefined, undefined, { - authorizedMentionText: typeof params.userInput === "string" ? params.userInput : "", - }) + await visibleProvider.createTask(prompt, undefined, undefined, { segments }) } catch (error) { if (error instanceof OrganizationAllowListViolationError) { // Errors from terminal commands seem to get swallowed / ignored. @@ -4280,7 +4276,7 @@ export class ClineProvider parentTask?: Task, options: CreateTaskOptions & { handoffExecutionContext?: DelegatedChildContext - authorizedMentionText?: string + segments?: MentionSegment[] } = {}, configuration: RooCodeSettings = {}, ): Promise { diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 609b3c1ce9..fa390df067 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -425,6 +425,8 @@ afterAll(() => { vi.restoreAllMocks() }) +const noUserSegments = expect.not.arrayContaining([expect.objectContaining({ source: "user" })]) + describe("ClineProvider", () => { beforeAll(() => { vi.mocked(Task).mockImplementation(function (options: any) { @@ -743,8 +745,8 @@ describe("ClineProvider", () => { type: "invoke", invoke: "setChatBoxMessage", text: expect.stringContaining("selected code"), - // Generated context is never the authorized substring. - authorizedMentionText: "", + // Generated context has no user segment. + segments: noUserSegments, }) expect(sidebar.postMessage).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) expect(tab.postMessage).not.toHaveBeenCalled() @@ -936,7 +938,7 @@ describe("ClineProvider", () => { invoke: "setChatBoxMessage", text: expect.stringContaining("selected code"), // Generated context is never the authorized substring. - authorizedMentionText: "", + segments: noUserSegments, }) expect(visible.postMessage).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) expect(vscode.commands.executeCommand).not.toHaveBeenCalled() @@ -966,7 +968,7 @@ describe("ClineProvider", () => { invoke: "setChatBoxMessage", text: expect.stringContaining("selected code"), // Generated context is never the authorized substring. - authorizedMentionText: "", + segments: noUserSegments, }) expect(tab.postMessage).not.toHaveBeenCalled() expect(panel.reveal).not.toHaveBeenCalled() @@ -1027,7 +1029,7 @@ describe("ClineProvider", () => { expect.stringContaining("selected code"), undefined, undefined, - { authorizedMentionText: "" }, + { segments: noUserSegments }, ) expect(tabTask).not.toHaveBeenCalled() } finally { @@ -1053,7 +1055,7 @@ describe("ClineProvider", () => { ["fixCode", "FIX"], ["improveCode", "IMPROVE"], ] as const)( - "forwards user-authored code action input as authorizedMentionText for %s", + "forwards user-authored code action input as a user segment for %s", async (command, promptType) => { const task = new Task(defaultTaskOptions) const createTask = vi.spyOn(provider, "createTask").mockResolvedValue(task) @@ -1064,7 +1066,7 @@ describe("ClineProvider", () => { }) // A regression that always sends "" would fail this assertion. expect(createTask).toHaveBeenCalledWith(expect.any(String), undefined, undefined, { - authorizedMentionText: "@/allowed", + segments: expect.arrayContaining([{ text: "@/allowed", source: "user" }]), }) } finally { createTask.mockRestore() @@ -1072,7 +1074,7 @@ describe("ClineProvider", () => { }, ) - test("forwards user-authored terminal action input as authorizedMentionText", async () => { + test("forwards user-authored terminal action input as a user segment", async () => { const task = new Task(defaultTaskOptions) const sidebarTask = vi.spyOn(provider, "createTask").mockResolvedValue(task) const tabTask = vi.spyOn(tabProvider, "createTask").mockResolvedValue(task) @@ -1085,7 +1087,7 @@ describe("ClineProvider", () => { // A regression that always sends "" would fail this assertion. expect(tabTask).toHaveBeenCalledOnce() expect(tabTask).toHaveBeenCalledWith(expect.any(String), undefined, undefined, { - authorizedMentionText: "@/allowed", + segments: expect.arrayContaining([{ text: "@/allowed", source: "user" }]), }) expect(sidebarTask).not.toHaveBeenCalled() } finally { @@ -7250,7 +7252,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { [], undefined, undefined, - "Edited message with preserved images", + [{ text: "Edited message with preserved images", source: "user" }], ) }) @@ -7309,7 +7311,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { [], undefined, undefined, - "Edited message with file attachment", + [{ text: "Edited message with file attachment", source: "user" }], ) }) }) @@ -7841,13 +7843,9 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { await messageHandler({ type: "editMessageConfirm", messageTs: 2000, text: largeEditedContent }) expect(mockCline.overwriteClineMessages).toHaveBeenCalled() - expect(mockCline.submitUserMessage).toHaveBeenCalledWith( - largeEditedContent, - [], - undefined, - undefined, - largeEditedContent, - ) + expect(mockCline.submitUserMessage).toHaveBeenCalledWith(largeEditedContent, [], undefined, undefined, [ + { text: largeEditedContent, source: "user" }, + ]) }) test("handles deleting messages with large payloads", async () => { diff --git a/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts index 52c4dc0637..805f9c1fbc 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts @@ -1,6 +1,7 @@ import { describe, it, expect, beforeEach, vi } from "vitest" import { webviewMessageHandler } from "../webviewMessageHandler" import type { ClineProvider } from "../ClineProvider" +import type { MentionSegment } from "@roo-code/types" import type { WebviewMessage } from "../../../shared/WebviewMessage" vi.mock("../../../i18n", () => ({ @@ -35,7 +36,10 @@ const { resolveImageMentions } = vi.hoisted(() => ({ })) vi.mock("../../mentions/resolveImageMentions", () => ({ resolveImageMentions })) -describe("webviewMessageHandler mention authorization forwarding", () => { +const generated = (text: string): MentionSegment => ({ text, source: "generated" }) +const user = (text: string): MentionSegment => ({ text, source: "user" }) + +describe("webviewMessageHandler mention segment forwarding", () => { // Narrow double: the handler under test only touches these members. // The single cast at construction keeps every use site type-checked. let provider: { @@ -65,118 +69,72 @@ describe("webviewMessageHandler mention authorization forwarding", () => { providerAsClineProvider = provider as unknown as ClineProvider }) - it("forwards the authorized mention substring from the webview message", async () => { - const message: WebviewMessage = { - type: "newTask", - text: "context @/secret\nuser line", - authorizedMentionText: "user line", - } - await webviewMessageHandler(providerAsClineProvider, message) - - expect(provider.createTask).toHaveBeenCalledWith( - "context @/secret\nuser line", - [], - undefined, - expect.objectContaining({ authorizedMentionText: "user line" }), - undefined, - ) + const text = "context @/secret\nuser line" + const segments = [generated("context @/secret\n"), user("user line")] + // Segments that the handler must replace with all generated text. + const rejected = [ + { label: "omitted", segments: undefined }, + { label: "not matching the text", segments: [user("other")] }, + { label: "invalid", segments: [{ text, source: "system" }] }, + ] + + describe.each([ + { + type: "newTask" as const, + read: () => provider.createTask.mock.calls[0], + segmentsOf: (call: unknown[]) => (call[3] as { segments: MentionSegment[] }).segments, + }, + { + type: "askResponse" as const, + read: () => currentTask.handleWebviewAskResponse.mock.calls[0], + segmentsOf: (call: unknown[]) => call[3] as MentionSegment[], + }, + { + type: "queueMessage" as const, + read: () => currentTask.messageQueueService.addMessage.mock.calls[0], + segmentsOf: (call: unknown[]) => call[2] as MentionSegment[], + }, + ])("$type", ({ type, read, segmentsOf }) => { + const send = (extra: Record) => + webviewMessageHandler(providerAsClineProvider, { + type, + askResponse: "messageResponse", + ...extra, + } as WebviewMessage) + + it("forwards matching segments", async () => { + await send({ text, segments }) + expect(segmentsOf(read()!)).toEqual(segments) + }) + + it.each(rejected)("treats segments that are $label as all generated", async ({ segments: bad }) => { + await send({ text, segments: bad }) + expect(segmentsOf(read()!)).toEqual([generated(text)]) + }) + + it.each([ + { label: "user segment", segments: [user("see @/a.png")], expected: "see @/a.png" }, + { label: "generated segment", segments: [generated("see @/a.png")], expected: "" }, + ])("scans only user text for images with a $label", async ({ segments: given, expected }) => { + await send({ text: "see @/a.png", segments: given }) + expect(resolveImageMentions).toHaveBeenCalledWith(expect.objectContaining({ text: expected })) + }) }) - it("passes an empty authorized substring when the webview omits it", async () => { - const message: WebviewMessage = { - type: "newTask", - text: "generated @/path from selected code", - } - await webviewMessageHandler(providerAsClineProvider, message) - - expect(provider.createTask).toHaveBeenCalledWith( - "generated @/path from selected code", - [], - undefined, - expect.objectContaining({ authorizedMentionText: "" }), - undefined, - ) - }) - - it("forwards the authorized substring with the ask response", async () => { - const message: WebviewMessage = { - type: "askResponse", - askResponse: "messageResponse", - text: "context @/secret\nuser line", - authorizedMentionText: "user line", - } - await webviewMessageHandler(providerAsClineProvider, message) - - expect(currentTask.handleWebviewAskResponse).toHaveBeenCalledWith( - "messageResponse", - "context @/secret\nuser line", - [], - "user line", - ) - }) - - it("treats an omitted authorized substring on an ask response as empty", async () => { - const message: WebviewMessage = { - type: "askResponse", - askResponse: "messageResponse", - text: "generated @/path", - } - await webviewMessageHandler(providerAsClineProvider, message) - - expect(currentTask.handleWebviewAskResponse).toHaveBeenCalledWith("messageResponse", "generated @/path", [], "") - }) - - it("passes an empty authorized substring when a queued message omits it", async () => { - const message: WebviewMessage = { type: "queueMessage", text: "generated @/path" } - await webviewMessageHandler(providerAsClineProvider, message) - - expect(currentTask.messageQueueService.addMessage).toHaveBeenCalledWith("generated @/path", [], "") - }) - - it("forwards the authorized substring with a queued message", async () => { - const message: WebviewMessage = { - type: "queueMessage", - text: "context @/secret\nuser line", - authorizedMentionText: "user line", - } - await webviewMessageHandler(providerAsClineProvider, message) - - expect(currentTask.messageQueueService.addMessage).toHaveBeenCalledWith( - "context @/secret\nuser line", - [], - "user line", - ) - }) - - it.each([ - { label: "an authorized substring", authorizedMentionText: "user @/a.png", expected: "user @/a.png" }, - { label: "an omitted field", authorizedMentionText: undefined, expected: "" }, - ])("scans only the authorized text for image mentions with $label", async ({ authorizedMentionText, expected }) => { - const message: WebviewMessage = { - type: "newTask", - text: "generated @/b.png user @/a.png", - authorizedMentionText, - } + it("keeps the original text and images when it forwards", async () => { + const message: WebviewMessage = { type: "newTask", text, segments } await webviewMessageHandler(providerAsClineProvider, message) - expect(resolveImageMentions).toHaveBeenCalledWith(expect.objectContaining({ text: expected })) - expect(provider.createTask).toHaveBeenCalledWith( - "generated @/b.png user @/a.png", - [], - undefined, - expect.anything(), - undefined, - ) + expect(provider.createTask).toHaveBeenCalledWith(text, [], undefined, expect.anything(), undefined) }) - it("scans the edited text for images and authorizes it as user typed", async () => { - const submitUserMessage = vi.fn() + it("scans the edited text for images and treats it as user text", async () => { const editTask = { clineMessages: [{ ts: 1, type: "say", say: "user_feedback", text: "old" }], apiConversationHistory: [], overwriteClineMessages: vi.fn(), overwriteApiConversationHistory: vi.fn(), - submitUserMessage, + submitUserMessage: vi.fn(), } provider.getCurrentTask.mockReturnValue(editTask) const message: WebviewMessage = { type: "editMessageConfirm", messageTs: 1, text: "see @/a.png" } diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index 82dd500c77..d49a9f9c1a 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -964,12 +964,12 @@ describe("webviewMessageHandler - image mentions", () => { }) expect(vi.mocked(resolveImageMentions)).toHaveBeenCalled() - // An omitted authorized substring fails closed. + // Omitted segments fail closed, so the text is all generated. expect(mockHandleWebviewAskResponse).toHaveBeenCalledWith( "messageResponse", "See @/img.png", ["data:image/png;base64,from-mention"], - "", + [{ text: "See @/img.png", source: "generated" }], ) }) }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 76195363ea..e93ebd4280 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -30,6 +30,7 @@ import { RouterModelsMessageType, VsCodeLmModelsMessageType, isTelemetryOptedIn, + type MentionSegment, } from "@roo-code/types" import { customToolRegistry } from "@roo-code/core" import { CloudService } from "@roo-code/cloud" @@ -90,6 +91,7 @@ import { openGitHubAccountManagement, } from "../../api/providers/github-copilot" import { openMention } from "../mentions" +import { normalizeSegments, userSegments, userText } from "../../shared/mentionSegments" import { resolveImageMentions } from "../mentions/resolveImageMentions" import { RooIgnoreController } from "../ignore/RooIgnoreController" import { getWorkspacePath } from "../../utils/path" @@ -226,18 +228,14 @@ export const webviewMessageHandler = async ( * Resolves image file mentions in incoming messages. * Matches read_file behavior: respects size limits and model capabilities. */ - const resolveIncomingImages = async (payload: { - text?: string - images?: string[] - authorizedMentionText?: string - }) => { + const resolveIncomingImages = async (payload: { text?: string; images?: string[]; segments: MentionSegment[] }) => { const text = payload.text ?? "" const images = payload.images const currentTask = provider.getCurrentTask() const state = await provider.getState() - // Scan only the authorized substring, so generated mentions attach no images. + // Scan only user segments, so generated mentions attach no images. const resolved = await resolveImageMentions({ - text: payload.authorizedMentionText ?? "", + text: userText(payload.segments), images, cwd: getCurrentCwd(), rooIgnoreController: currentTask?.rooIgnoreController, @@ -246,8 +244,9 @@ export const webviewMessageHandler = async ( }) return { text, images: resolved.images } } - const authorizedTextOf = (message: { authorizedMentionText?: unknown }) => - typeof message.authorizedMentionText === "string" ? message.authorizedMentionText : "" + // Fails closed: segments that are missing or do not match the text are all generated. + const segmentsOf = (message: { text?: string; segments?: unknown }) => + normalizeSegments(message.text ?? "", message.segments) /** * Shared utility to find message indices based on timestamp. * When multiple messages share the same timestamp (e.g., after condense), @@ -444,7 +443,7 @@ export const webviewMessageHandler = async ( editedContent: string, restoreCheckpoint?: boolean, images?: string[], - authorizedMentionText = "", + segments?: MentionSegment[], ): Promise => { const currentCline = provider.getCurrentTask() if (!currentCline) { @@ -564,7 +563,7 @@ export const webviewMessageHandler = async ( // Update the UI to reflect the deletion await provider.postStateToWebview() - await currentCline.submitUserMessage(editedContent, images, undefined, undefined, authorizedMentionText) + await currentCline.submitUserMessage(editedContent, images, undefined, undefined, segments) } catch (error) { console.error("Error in edit message:", error) vscode.window.showErrorMessage( @@ -722,7 +721,7 @@ export const webviewMessageHandler = async ( const resolved = await resolveIncomingImages({ text: message.text, images: message.images, - authorizedMentionText: authorizedTextOf(message), + segments: segmentsOf(message), }) await provider.createTask( resolved.text, @@ -730,9 +729,7 @@ export const webviewMessageHandler = async ( undefined, { taskId: message.taskId, - // Fail closed when the webview omits the authorized - // substring, so generated context cannot expand. - authorizedMentionText: authorizedTextOf(message), + segments: segmentsOf(message), }, message.taskConfiguration, ) @@ -753,19 +750,15 @@ export const webviewMessageHandler = async ( case "askResponse": { + const segments = segmentsOf(message) const resolved = await resolveIncomingImages({ text: message.text, images: message.images, - authorizedMentionText: authorizedTextOf(message), + segments, }) - provider.getCurrentTask()?.handleWebviewAskResponse( - message.askResponse!, - resolved.text, - resolved.images, - // Fail closed when the webview omits the authorized - // substring. - authorizedTextOf(message), - ) + provider + .getCurrentTask() + ?.handleWebviewAskResponse(message.askResponse!, resolved.text, resolved.images, segments) } break @@ -2638,18 +2631,19 @@ export const webviewMessageHandler = async ( break case "editMessageConfirm": if (message.messageTs && message.text) { - // The edit box holds only text the user typed, so all of it is authorized. + // The edit box holds only text the user typed. + const editSegments = userSegments(message.text) const resolved = await resolveIncomingImages({ text: message.text, images: message.images, - authorizedMentionText: message.text, + segments: editSegments, }) await handleEditMessageConfirm( message.messageTs, resolved.text, message.restoreCheckpoint, resolved.images, - message.text, + editSegments, ) } break @@ -4058,17 +4052,9 @@ export const webviewMessageHandler = async ( */ case "queueMessage": { - const resolved = await resolveIncomingImages({ - text: message.text, - images: message.images, - authorizedMentionText: authorizedTextOf(message), - }) - provider.getCurrentTask()?.messageQueueService.addMessage( - resolved.text, - resolved.images, - // Fail closed when the webview omits the authorized substring. - authorizedTextOf(message), - ) + const segments = segmentsOf(message) + const resolved = await resolveIncomingImages({ text: message.text, images: message.images, segments }) + provider.getCurrentTask()?.messageQueueService.addMessage(resolved.text, resolved.images, segments) break } case "removeQueuedMessage": { diff --git a/src/extension/__tests__/api-start-new-task.spec.ts b/src/extension/__tests__/api-start-new-task.spec.ts index c2b26841b9..d1a473d1d0 100644 --- a/src/extension/__tests__/api-start-new-task.spec.ts +++ b/src/extension/__tests__/api-start-new-task.spec.ts @@ -89,7 +89,7 @@ describe("API - startNewTask routing", () => { { consecutiveMistakeLimit: Number.MAX_SAFE_INTEGER, toolRepetitionSoftLimit: 0, - authorizedMentionText: "new task", + segments: [{ text: "new task", source: "user" }], }, configuration, ) diff --git a/src/extension/api.ts b/src/extension/api.ts index 6e52bac2c6..2566006033 100644 --- a/src/extension/api.ts +++ b/src/extension/api.ts @@ -24,9 +24,11 @@ import { IpcMessageType, TASK_START_FAILURE_ERROR_CODE, TASK_START_FAILURE_ERROR_MESSAGE, + type MentionSegment, } from "@roo-code/types" import { IpcServer } from "@roo-code/ipc" +import { userSegments } from "../shared/mentionSegments" import { Package } from "../shared/package" import type { Mode } from "../shared/modes" @@ -250,11 +252,11 @@ export class API extends EventEmitter implements RooCodeAPI { // effectively unlimited. The soft repetition tier is disabled too: // without a reachable hard stop it would reject every identical call // with no escalation. - const options: CreateTaskOptions & { authorizedMentionText?: string } = { + const options: CreateTaskOptions & { segments?: MentionSegment[] } = { consecutiveMistakeLimit: Number.MAX_SAFE_INTEGER, toolRepetitionSoftLimit: 0, // The API caller authored the task text. - authorizedMentionText: text ?? "", + segments: userSegments(text ?? ""), } const task = await provider.createTask(text, images, undefined, options, configuration) @@ -411,7 +413,7 @@ export class API extends EventEmitter implements RooCodeAPI { return } - await currentTask.submitUserMessage(text ?? "", images, undefined, undefined, text ?? "") + await currentTask.submitUserMessage(text ?? "", images, undefined, undefined, userSegments(text ?? "")) return } diff --git a/src/shared/__tests__/mentionSegments.spec.ts b/src/shared/__tests__/mentionSegments.spec.ts new file mode 100644 index 0000000000..b9887ff23b --- /dev/null +++ b/src/shared/__tests__/mentionSegments.spec.ts @@ -0,0 +1,79 @@ +import type { MentionSegment } from "@roo-code/types" + +import { + applyEdit, + generatedSegments, + joinSegments, + normalizeSegments, + trimSegments, + userSegments, + userText, +} from "../mentionSegments" + +const user = (text: string): MentionSegment => ({ text, source: "user" }) +const gen = (text: string): MentionSegment => ({ text, source: "generated" }) + +describe("mentionSegments", () => { + describe("normalizeSegments", () => { + it("keeps valid segments that match the text", () => { + expect(normalizeSegments("ab", [gen("a"), user("b")])).toEqual([gen("a"), user("b")]) + }) + + it.each([ + { label: "omitted", segments: undefined }, + { label: "not a list", segments: "user" }, + { label: "bad source", segments: [{ text: "ab", source: "system" }] }, + { label: "text mismatch", segments: [user("abc")] }, + ])("fails closed when segments are $label", ({ segments }) => { + expect(normalizeSegments("ab", segments)).toEqual([gen("ab")]) + }) + + it("returns no segments for empty text", () => { + expect(normalizeSegments("", undefined)).toEqual([]) + }) + }) + + it("joins text and selects only user text", () => { + const segments = [gen("a @/x "), user("b @/y"), gen(" c"), user("d")] + expect(joinSegments(segments)).toBe("a @/x b @/y cd") + expect(userText(segments)).toBe("b @/y\nd") + }) + + it("trims both ends across segments", () => { + expect(trimSegments([gen(" "), user(" a "), gen(" b ")])).toEqual([user("a "), gen(" b")]) + }) + + describe("applyEdit", () => { + it("marks inserted text as user and keeps the rest", () => { + expect(applyEdit([gen("context @/s")], "context @/s", "context @/s mine")).toEqual([ + gen("context @/s"), + user(" mine"), + ]) + }) + + it("marks text inserted in the middle as user and keeps both sides generated", () => { + expect(applyEdit([gen("ab")], "ab", "aXb")).toEqual([gen("a"), user("X"), gen("b")]) + }) + + it("removes deleted text and keeps its neighbors", () => { + expect(applyEdit([user("ab"), gen("cd")], "abcd", "acd")).toEqual([user("a"), gen("cd")]) + }) + + it("marks replaced generated text as user", () => { + expect(applyEdit([gen("abc")], "abc", "aZc")).toEqual([gen("a"), user("Z"), gen("c")]) + }) + + it("keeps all user text user", () => { + expect(applyEdit(userSegments("ab"), "ab", "abc")).toEqual([user("abc")]) + }) + + it("fails closed when the segments do not match the old value", () => { + expect(applyEdit([gen("x")], "ab", "abc")).toEqual(generatedSegments("abc")) + }) + + it("does not authorize a generated mention after an unrelated edit", () => { + const result = applyEdit([gen("see @/secret")], "see @/secret", "see @/secret now") + expect(userText(result)).toBe(" now") + }) + }) +}) diff --git a/src/shared/__tests__/support-prompts.spec.ts b/src/shared/__tests__/support-prompts.spec.ts index ea6a193d5a..2a003eb662 100644 --- a/src/shared/__tests__/support-prompts.spec.ts +++ b/src/shared/__tests__/support-prompts.spec.ts @@ -265,3 +265,27 @@ describe("Code Action Prompts", () => { }) }) }) + +describe("createSegments", () => { + it("marks only the user input as a user segment and matches create()", () => { + const params = { filePath: "a.ts", selectedText: "code @/secret", userInput: "mine @/ok" } + const segments = supportPrompt.createSegments("EXPLAIN", params) + + expect(segments.map((s) => s.text).join("")).toBe(supportPrompt.create("EXPLAIN", params)) + expect(segments.filter((s) => s.source === "user")).toEqual([{ text: "mine @/ok", source: "user" }]) + expect(segments.filter((s) => s.source === "generated").some((s) => s.text.includes("@/secret"))).toBe(true) + }) + + it("has no user segment without user input", () => { + const params = { filePath: "a.ts", selectedText: "code @/secret" } + const segments = supportPrompt.createSegments("EXPLAIN", params) + + expect(segments.some((s) => s.source === "user")).toBe(false) + expect(segments.map((s) => s.text).join("")).toBe(supportPrompt.create("EXPLAIN", params)) + }) + + it("does not treat a parameter that holds user-like text as a user segment", () => { + const segments = supportPrompt.createSegments("EXPLAIN", { selectedText: "@/secret", userInput: "" }) + expect(segments.some((s) => s.source === "user")).toBe(false) + }) +}) diff --git a/src/shared/mentionSegments.ts b/src/shared/mentionSegments.ts new file mode 100644 index 0000000000..5c46fb9443 --- /dev/null +++ b/src/shared/mentionSegments.ts @@ -0,0 +1,93 @@ +import { mentionSegmentSchema, type MentionSegment } from "@roo-code/types" + +const segmentsSchema = mentionSegmentSchema.array() + +export const joinSegments = (segments: MentionSegment[]): string => segments.map((s) => s.text).join("") + +/** The text that may have mentions expanded, one user part per line. */ +export const userText = (segments: MentionSegment[]): string => + segments + .filter((s) => s.source === "user") + .map((s) => s.text) + .join("\n") + +export const userSegments = (text: string): MentionSegment[] => (text ? [{ text, source: "user" }] : []) + +export const generatedSegments = (text: string): MentionSegment[] => (text ? [{ text, source: "generated" }] : []) + +/** Drops empty parts and joins neighbors that have the same source. */ +const merge = (segments: MentionSegment[]): MentionSegment[] => { + const result: MentionSegment[] = [] + for (const segment of segments) { + if (!segment.text) continue + const last = result[result.length - 1] + if (last?.source === segment.source) { + last.text += segment.text + } else { + result.push({ ...segment }) + } + } + return result +} + +/** + * Checks segments that arrive from outside. Fails closed: anything that is + * not a valid list whose text matches `text` becomes all generated. + */ +export const normalizeSegments = (text: string, segments: unknown): MentionSegment[] => { + const parsed = segmentsSchema.safeParse(segments) + return parsed.success && joinSegments(parsed.data) === text ? merge(parsed.data) : generatedSegments(text) +} + +export const trimSegments = (segments: MentionSegment[]): MentionSegment[] => { + const result = merge(segments) + while (result.length > 0) { + const first = result[0]! + first.text = first.text.trimStart() + if (first.text) break + result.shift() + } + while (result.length > 0) { + const last = result[result.length - 1]! + last.text = last.text.trimEnd() + if (last.text) break + result.pop() + } + return result +} + +const slice = (segments: MentionSegment[], start: number, end: number): MentionSegment[] => { + const result: MentionSegment[] = [] + let offset = 0 + for (const segment of segments) { + const from = Math.max(start - offset, 0) + const to = Math.min(end - offset, segment.text.length) + if (from < to) result.push({ text: segment.text.slice(from, to), source: segment.source }) + offset += segment.text.length + } + return result +} + +/** + * Records a manual edit. The changed range is the part between the common + * prefix and the common suffix. The new text there is `user`. The rest keeps + * its source. If the segments do not match `oldValue`, the result is all + * generated. + */ +export const applyEdit = (segments: MentionSegment[], oldValue: string, newValue: string): MentionSegment[] => { + if (joinSegments(segments) !== oldValue) return generatedSegments(newValue) + + const max = Math.min(oldValue.length, newValue.length) + let prefix = 0 + while (prefix < max && oldValue[prefix] === newValue[prefix]) prefix++ + let suffix = 0 + while (suffix < max - prefix && oldValue[oldValue.length - 1 - suffix] === newValue[newValue.length - 1 - suffix]) { + suffix++ + } + + return merge([ + ...slice(segments, 0, prefix), + ...userSegments(newValue.slice(prefix, newValue.length - suffix)), + ...slice(segments, oldValue.length - suffix, oldValue.length), + ]) +} diff --git a/src/shared/support-prompt.ts b/src/shared/support-prompt.ts index 6fe48fa364..7acd9bb9a9 100644 --- a/src/shared/support-prompt.ts +++ b/src/shared/support-prompt.ts @@ -1,3 +1,5 @@ +import type { MentionSegment } from "@roo-code/types" + // Support prompts type PromptDiagnostic = { source?: string @@ -261,6 +263,26 @@ export const supportPrompt = { const template = supportPrompt.get(customSupportPrompts, type) return createPrompt(template, params) }, + /** + * Builds the prompt as segments. Only the `userInput` parameter is `user`. + * Every other part of the template and its parameters is `generated`. + */ + createSegments: ( + type: SupportPromptType, + params: PromptParams, + customSupportPrompts?: Record, + ): MentionSegment[] => { + const template = supportPrompt.get(customSupportPrompts, type) + const mark = `\u0000${globalThis.crypto.randomUUID()}\u0000` + const userInput = typeof params["userInput"] === "string" ? params["userInput"] : "" + const parts = createPrompt(template, { ...params, userInput: mark }).split(mark) + const segments: MentionSegment[] = [] + parts.forEach((part, index) => { + if (part) segments.push({ text: part, source: "generated" }) + if (index < parts.length - 1 && userInput) segments.push({ text: userInput, source: "user" }) + }) + return segments + }, } as const export type { SupportPromptType } diff --git a/webview-ui/src/components/chat/ChatTextArea.tsx b/webview-ui/src/components/chat/ChatTextArea.tsx index d0e285f740..79660cb421 100644 --- a/webview-ui/src/components/chat/ChatTextArea.tsx +++ b/webview-ui/src/components/chat/ChatTextArea.tsx @@ -60,7 +60,8 @@ interface ChatTextAreaProps { inputValue: string setInputValue: (value: string) => void /** Programmatic updates preserve the chat box authorization scope. */ - setInputValuePreservingAuthorization?: (value: string) => void + /** Sets generated text, such as an enhanced prompt. Falls back to `setInputValue`. */ + setInputValueGenerated?: (value: string) => void sendingDisabled: boolean selectApiConfigDisabled: boolean placeholderText: string @@ -87,7 +88,7 @@ export const ChatTextArea = forwardRef( { inputValue, setInputValue, - setInputValuePreservingAuthorization, + setInputValueGenerated, selectApiConfigDisabled, placeholderText, selectedImages, @@ -158,9 +159,8 @@ export const ChatTextArea = forwardRef( return () => document.removeEventListener("mousedown", handleClickOutside) }, [showDropdown]) - // Generated text routes through this setter so it never clears the - // chat box authorization scope. - const setInputValueProgrammatic = setInputValuePreservingAuthorization ?? setInputValue + // Generated text routes through this setter so it is never marked as user text. + const setInputValueProgrammatic = setInputValueGenerated ?? setInputValue const programmaticInsertRef = useRef(false) @@ -213,7 +213,8 @@ export const ChatTextArea = forwardRef( message.text + " " + currentValue.slice(cursorPos) - setInputValueProgrammatic(newValue) + // The user chose this text, so it is a normal edit. + setInputValue(newValue) // Set cursor position after the inserted text const newCursorPos = cursorPos + prefix.length + message.text.length + 1 @@ -244,7 +245,7 @@ export const ChatTextArea = forwardRef( window.addEventListener("message", messageHandler) return () => window.removeEventListener("message", messageHandler) - }, [setInputValueProgrammatic, searchRequestId, normalizedInputValue]) + }, [setInputValue, setInputValueProgrammatic, searchRequestId, normalizedInputValue]) const [isDraggingOver, setIsDraggingOver] = useState(false) const [textAreaBaseHeight, setTextAreaBaseHeight] = useState(undefined) diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 0b93539591..ad835e592f 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -20,10 +20,26 @@ import { getCostBreakdownIfNeeded } from "@src/utils/costFormatting" import { batchNearby } from "@src/utils/batchNearby" import { isBoundary, isIgnorableBetweenTargets } from "@src/utils/chatBatchingPredicates" -import type { ClineAsk, ClineSayTool, ClineMessage, ExtensionMessage, AudioType, SuggestionItem } from "@roo-code/types" +import type { + ClineAsk, + ClineSayTool, + ClineMessage, + ExtensionMessage, + AudioType, + SuggestionItem, + MentionSegment, +} from "@roo-code/types" import { getCompletionCheckpoint, getSuggestionMode, hasUsableAnswer, isRetiredProvider } from "@roo-code/types" import { findLast } from "@roo/array" +import { + applyEdit, + generatedSegments, + joinSegments, + normalizeSegments, + trimSegments, + userSegments, +} from "@roo/mentionSegments" import { combineApiRequests } from "@roo/combineApiRequests" import { combineCommandSequences } from "@roo/combineCommandSequences" import { getApiMetrics } from "@roo/getApiMetrics" @@ -202,9 +218,8 @@ const ChatViewComponent: React.ForwardRefRenderFunction(null) const userRespondedRef = useRef(false) - // Null means the chat box value is fully user-authored. A string names - // the only substring that may have mentions expanded. - const authorizedMentionRef = useRef(null) + // Origin of each part of the chat box value. Null means all of it is user text. + const segmentsRef = useRef(null) const [currentFollowUpTs, setCurrentFollowUpTs] = useState(null) const [aggregatedCostsMap, setAggregatedCostsMap] = useState< Map< @@ -632,7 +647,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + (text: string, images: string[], segmentsOverride?: MentionSegment[]) => { text = text.trim() + // Trimmed chat box segments. A value that does not match the text is all generated. + const trimmed = segmentsRef.current ? trimSegments(segmentsRef.current) : userSegments(text) + const segments = segmentsOverride + ? trimSegments(segmentsOverride) + : joinSegments(trimmed) === text + ? trimmed + : generatedSegments(text) if (text || images.length > 0) { // Intercept when the active provider is retired — show a @@ -672,11 +694,11 @@ const ChatViewComponent: React.ForwardRefRenderFunction { - // Avoid nested template literals by breaking down the logic - let newValue = text - - if (inputValue !== "") { - newValue = inputValue + " " + text + (text: string, images: string[], inserted?: MentionSegment[]) => { + // Segments that are missing or do not match the text are all generated. + const insertedSegments = normalizeSegments(text, inserted) + if (inputValue === "") { + segmentsRef.current = insertedSegments + setInputValue(text) + } else { + segmentsRef.current = [ + ...(segmentsRef.current ?? userSegments(inputValue)), + ...generatedSegments(" "), + ...insertedSegments, + ] + setInputValue(inputValue + " " + text) } - - // An omitted field authorizes nothing from the inserted text. - const insertedAuthorization = typeof authorizedMentionText === "string" ? authorizedMentionText : "" - // Empty box: the named substring is the only authorized part. - // Typed box text is user-authored, so it stays the authorized - // part and the inserted substring is dropped (fail closed). - authorizedMentionRef.current = - inputValue === "" ? insertedAuthorization : (authorizedMentionRef.current ?? inputValue) - - setInputValue(newValue) setSelectedImages([...selectedImages, ...images]) }, [inputValue, selectedImages], @@ -777,18 +795,24 @@ const ChatViewComponent: React.ForwardRefRenderFunction string)) => { - authorizedMentionRef.current = null - setInputValue(value) + const previous = inputValueRef.current + const next = typeof value === "function" ? value(previous) : value + if (segmentsRef.current) { + segmentsRef.current = applyEdit(segmentsRef.current, previous, next) + } + inputValueRef.current = next + setInputValue(next) }, [setInputValue], ) - // Generated text never widens the scope: a null scope keeps only the prior text. + // Generated text replaces the whole value and authorizes nothing. const setInputValueGenerated = useCallback((value: string) => { - authorizedMentionRef.current ??= inputValueRef.current + segmentsRef.current = generatedSegments(value) + inputValueRef.current = value setInputValue(value) }, []) @@ -806,9 +830,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction { - return currentValue !== "" ? `${currentValue} \n${answer}` : answer - }) + // The suggestion is model text, so it stays generated. + const currentValue = inputValueRef.current + const separator = currentValue !== "" ? " \n" : "" + segmentsRef.current = [ + ...(segmentsRef.current ?? userSegments(currentValue)), + ...generatedSegments(separator + answer), + ] + inputValueRef.current = currentValue + separator + answer + setInputValue(inputValueRef.current) } else { // Don't clear the input value when sending a follow-up choice // The message should be sent but the text area should preserve what the user typed const preservedInput = inputValueRef.current - const preservedAuthorization = authorizedMentionRef.current - // A suggestion is model text, so it authorizes no mention. - authorizedMentionRef.current = "" - handleSendMessage(answer, []) - // Restore the input value and its scope after sending + const preservedSegments = segmentsRef.current + // A suggestion is model text, so it is generated. + handleSendMessage(answer, [], generatedSegments(answer)) + // Restore the input value and its segments after sending setInputValue(preservedInput) - authorizedMentionRef.current = preservedAuthorization + segmentsRef.current = preservedSegments } }, [handleSendMessage, setInputValue, switchToMode, alwaysAllowModeSwitch, clineAsk, markFollowUpAsAnswered], @@ -1686,13 +1716,14 @@ const ChatViewComponent: React.ForwardRefRenderFunction { expect(setInputValue).toHaveBeenCalledWith("Enhanced test prompt") }) - it("routes the enhanced prompt through the authorization-preserving setter when provided", () => { + it("routes the enhanced prompt through the generated-text setter when provided", () => { const setInputValue = vi.fn() - const setInputValuePreservingAuthorization = vi.fn() + const setInputValueGenerated = vi.fn() Object.defineProperty(document, "execCommand", { value: undefined, writable: true }) render( , ) @@ -300,13 +300,13 @@ describe("ChatTextArea", () => { new MessageEvent("message", { data: { type: "enhancedPrompt", text: "Enhanced test prompt" } }), ) - expect(setInputValuePreservingAuthorization).toHaveBeenCalledWith("Enhanced test prompt") + expect(setInputValueGenerated).toHaveBeenCalledWith("Enhanced test prompt") expect(setInputValue).not.toHaveBeenCalled() }) - it("routes the native enhanced-prompt input event through the preserving setter, then manual edits through setInputValue", () => { + it("routes the native enhanced-prompt input event through the generated-text setter, then manual edits through setInputValue", () => { const setInputValue = vi.fn() - const setInputValuePreservingAuthorization = vi.fn() + const setInputValueGenerated = vi.fn() // Mimic the browser: execCommand fires an input event on the textarea. const target: { el?: HTMLTextAreaElement } = {} Object.defineProperty(document, "execCommand", { @@ -323,7 +323,7 @@ describe("ChatTextArea", () => { , ) @@ -336,7 +336,7 @@ describe("ChatTextArea", () => { ) }) - expect(setInputValuePreservingAuthorization).toHaveBeenCalledWith("Enhanced test prompt") + expect(setInputValueGenerated).toHaveBeenCalledWith("Enhanced test prompt") expect(setInputValue).not.toHaveBeenCalled() fireEvent.change(textarea, { target: { value: "typed by hand" } }) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx index 31e80b1040..f479f09e7e 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx @@ -69,7 +69,7 @@ interface MockChatTextAreaProps { onSend: () => void inputValue?: string setInputValue?: (value: string) => void - setInputValuePreservingAuthorization?: (value: string) => void + setInputValueGenerated?: (value: string) => void } // ChatTextArea double: an input whose Enter triggers onSend and whose @@ -85,7 +85,7 @@ vi.mock("../ChatTextArea", async () => { React.useEffect(() => { const listener = (event: MessageEvent) => { if (event.data?.type === "enhancedPrompt" && typeof event.data.text === "string") { - const update = props.setInputValuePreservingAuthorization ?? props.setInputValue + const update = props.setInputValueGenerated ?? props.setInputValue update?.(event.data.text) } } @@ -184,69 +184,108 @@ const getChatBoxValue = () => { return input?.value ?? "" } -describe("ChatView - add-to-context mention authorization", () => { +const generated = (text: string) => ({ text, source: "generated" as const }) +const user = (text: string) => ({ text, source: "user" as const }) + +describe("ChatView - add-to-context mention segments", () => { beforeEach(() => { vi.clearAllMocks() }) - it("sends no authorized substring when add-to-context carries no user input", async () => { + it("sends all generated segments when add-to-context carries no user input", async () => { const { getByTestId } = renderChatView() - await addGeneratedContext({ authorizedMentionText: "" }) + await addGeneratedContext({ segments: [generated("context with @/secret code")] }) const message = await sendNewTask(getByTestId) - expect(message).toMatchObject({ text: "context with @/secret code", authorizedMentionText: "" }) + expect(message).toMatchObject({ + text: "context with @/secret code", + segments: [generated("context with @/secret code")], + }) }) - it("sends only the user-authored substring the extension authorized", async () => { + it("keeps the user segment the extension named", async () => { const { getByTestId } = renderChatView() - await addGeneratedContext({ authorizedMentionText: "explain this" }) + await addGeneratedContext({ segments: [generated("context with @/secret "), user("code")] }) const message = await sendNewTask(getByTestId) - expect(message).toMatchObject({ authorizedMentionText: "explain this" }) + expect(message).toMatchObject({ segments: [generated("context with @/secret "), user("code")] }) }) - it("authorizes nothing from inserted text when the sender omits the field", async () => { + it("treats text as generated when the sender omits segments", async () => { const { getByTestId } = renderChatView() await addGeneratedContext({}) const message = await sendNewTask(getByTestId) - expect(message).toMatchObject({ authorizedMentionText: "" }) + expect(message).toMatchObject({ segments: [generated("context with @/secret code")] }) + }) + + it("treats text as generated when the segments do not match it", async () => { + const { getByTestId } = renderChatView() + + await addGeneratedContext({ segments: [user("something else")] }) + const message = await sendNewTask(getByTestId) + + expect(message).toMatchObject({ segments: [generated("context with @/secret code")] }) }) - it("re-authorizes the whole value after a manual edit", async () => { + it("marks only the typed text as user text after a manual edit", async () => { const { getByTestId } = renderChatView() - await addGeneratedContext({ authorizedMentionText: "" }) + await addGeneratedContext({ segments: [generated("context with @/secret code")] }) const input = getByTestId("chat-textarea").querySelector("input")! await act(async () => { - fireEvent.change(input, { target: { value: "my own task with @/ok" } }) + fireEvent.change(input, { target: { value: "context with @/secret code and @/ok" } }) }) const message = await sendNewTask(getByTestId) - expect(message).toMatchObject({ authorizedMentionText: "my own task with @/ok" }) + expect(message).toMatchObject({ + segments: [generated("context with @/secret code"), user(" and @/ok")], + }) }) - it("keeps the authorization scope when an enhanced prompt replaces the text", async () => { + it("keeps all text user text when nothing generated was added", async () => { const { getByTestId } = renderChatView() - await addGeneratedContext({ authorizedMentionText: "" }) + const input = getByTestId("chat-textarea").querySelector("input")! + await act(async () => { + fireEvent.change(input, { target: { value: "my own task with @/ok" } }) + }) + + const message = await sendNewTask(getByTestId) + expect(message).toMatchObject({ segments: [user("my own task with @/ok")] }) + }) + it("appends inserted segments after typed text", async () => { + const { getByTestId } = renderChatView() + + const input = getByTestId("chat-textarea").querySelector("input")! await act(async () => { - window.postMessage({ type: "enhancedPrompt", text: "enhanced text with @/path" }, "*") + fireEvent.change(input, { target: { value: "mine" } }) + }) + await act(async () => { + window.postMessage( + { + type: "invoke", + invoke: "setChatBoxMessage", + text: "ctx @/secret", + segments: [generated("ctx @/secret")], + }, + "*", + ) }) await waitFor(() => { - expect(getChatBoxValue()).toBe("enhanced text with @/path") + expect(getChatBoxValue()).toBe("mine ctx @/secret") }) const message = await sendNewTask(getByTestId) - expect(message).toMatchObject({ authorizedMentionText: "" }) + expect(message).toMatchObject({ segments: [user("mine"), generated(" ctx @/secret")] }) }) - it("authorizes only the typed text when an enhanced prompt replaces a manual draft", async () => { + it("makes an enhanced prompt all generated, even over a typed draft", async () => { const { getByTestId } = renderChatView() const input = getByTestId("chat-textarea").querySelector("input")! @@ -261,6 +300,9 @@ describe("ChatView - add-to-context mention authorization", () => { }) const message = await sendNewTask(getByTestId) - expect(message).toMatchObject({ text: "enhanced with @/secret", authorizedMentionText: "my draft" }) + expect(message).toMatchObject({ + text: "enhanced with @/secret", + segments: [generated("enhanced with @/secret")], + }) }) }) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index d2bd8ec7ca..0fb9435cba 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -1243,7 +1243,7 @@ describe("ChatView - Message Queueing Tests", () => { type: "queueMessage", text: "follow-up question during spinner", images: [], - authorizedMentionText: "follow-up question during spinner", + segments: [{ text: "follow-up question during spinner", source: "user" }], }) }) @@ -1315,7 +1315,7 @@ describe("ChatView - Message Queueing Tests", () => { askResponse: "messageResponse", text: "follow-up after completion", images: [], - authorizedMentionText: "follow-up after completion", + segments: [{ text: "follow-up after completion", source: "user" }], }) }) @@ -1375,7 +1375,7 @@ describe("ChatView - Message Queueing Tests", () => { type: "queueMessage", text: "message during queue drain", images: [], - authorizedMentionText: "message during queue drain", + segments: [{ text: "message during queue drain", source: "user" }], }) }) @@ -1439,7 +1439,7 @@ describe("ChatView - Message Queueing Tests", () => { type: "queueMessage", text: "message during command execution", images: [], - authorizedMentionText: "message during command execution", + segments: [{ text: "message during command execution", source: "user" }], }) }) @@ -1560,11 +1560,11 @@ describe("ChatView - Follow-up Suggestions", () => { askResponse: "messageResponse", text: "Use code mode", images: [], - authorizedMentionText: "", + segments: [{ text: "Use code mode", source: "generated" }], }) }) - it("authorizes only the typed draft when a shift-clicked suggestion is appended", async () => { + it("keeps a shift-clicked suggestion generated and the typed draft user text", async () => { const { getByRole, getByTestId } = renderChatView() mockPostMessage({ @@ -1598,7 +1598,13 @@ describe("ChatView - Follow-up Suggestions", () => { await waitFor(() => { expect(vscode.postMessage).toHaveBeenCalledWith( - expect.objectContaining({ type: "askResponse", authorizedMentionText: "mine" }), + expect.objectContaining({ + type: "askResponse", + segments: [ + { text: "mine", source: "user" }, + { text: " \nUse @/secret", source: "generated" }, + ], + }), ) }) }) @@ -1640,7 +1646,7 @@ describe("ChatView - Follow-up Suggestions", () => { askResponse: "messageResponse", text: "Use invalid mode", images: [], - authorizedMentionText: "", + segments: [{ text: "Use invalid mode", source: "generated" }], }) }) expect(vscode.postMessage).not.toHaveBeenCalledWith(expect.objectContaining({ type: "mode" })) @@ -1701,7 +1707,7 @@ describe("ChatView - Follow-up Suggestions", () => { askResponse: "messageResponse", text: "Valid answer", images: [], - authorizedMentionText: "", + segments: [{ text: "Valid answer", source: "generated" }], }) }) }) From 2d6c62ed15d530b8f2980c3c4754b3746a672b53 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sun, 15 Nov 2026 01:08:13 +0000 Subject: [PATCH 08/15] fix(mentions): keep generated text generated across edits and history Queue edits mark only the changed text as user text. Prompt history saves and restores the draft with its segments. Stored prompts and edited messages carry no segments, so they expand nothing. Add tests for mixed-origin and rejected segments in both tools. --- src/core/message-queue/MessageQueueService.ts | 6 +- .../__tests__/MessageQueueService.spec.ts | 27 ++++++++- .../__tests__/askFollowupQuestionTool.spec.ts | 39 +++++++++++++ .../__tests__/attemptCompletionTool.spec.ts | 55 +++++++++++++++++++ .../webview/__tests__/ClineProvider.spec.ts | 6 +- .../webviewMessageHandler.newTask.spec.ts | 23 ++++++-- src/core/webview/webviewMessageHandler.ts | 6 +- .../src/components/chat/ChatTextArea.tsx | 10 +++- webview-ui/src/components/chat/ChatView.tsx | 10 ++++ .../chat/__tests__/ChatTextArea.spec.tsx | 25 ++++++++- ...View.add-to-context-authorization.spec.tsx | 16 +++++- .../components/chat/hooks/usePromptHistory.ts | 32 ++++++++--- 12 files changed, 229 insertions(+), 26 deletions(-) diff --git a/src/core/message-queue/MessageQueueService.ts b/src/core/message-queue/MessageQueueService.ts index 869aeee9f1..d87d149d68 100644 --- a/src/core/message-queue/MessageQueueService.ts +++ b/src/core/message-queue/MessageQueueService.ts @@ -4,7 +4,7 @@ import { v4 as uuidv4 } from "uuid" import type { MentionSegment, QueuedMessage } from "@roo-code/types" -import { userSegments } from "../../shared/mentionSegments" +import { applyEdit, generatedSegments } from "../../shared/mentionSegments" export interface QueueEvents { stateChanged: [messages: QueuedMessage[]] @@ -69,11 +69,11 @@ export class MessageQueueService extends EventEmitter { return false } + // Only the changed text becomes user text. Unchanged generated text stays generated. + message.segments = applyEdit(message.segments ?? generatedSegments(message.text), message.text, text) message.timestamp = Date.now() message.text = text message.images = images - // The edited text is user-authored, so it replaces any recorded segments. - message.segments = userSegments(text) this.emit("stateChanged", this._messages) return true } diff --git a/src/core/message-queue/__tests__/MessageQueueService.spec.ts b/src/core/message-queue/__tests__/MessageQueueService.spec.ts index a81d4c06f7..2a2d7c7275 100644 --- a/src/core/message-queue/__tests__/MessageQueueService.spec.ts +++ b/src/core/message-queue/__tests__/MessageQueueService.spec.ts @@ -79,12 +79,33 @@ describe("MessageQueueService segments", () => { expect(queue.addMessage("ctx mine", undefined, segments)?.segments).toEqual(segments) }) - it("replaces the segments with user text when the user edits the message", () => { + it("keeps unchanged generated text generated when the user saves without edits", () => { const queue = new MessageQueueService() const message = queue.addMessage("ctx @/x", undefined, [{ text: "ctx @/x", source: "generated" }])! - queue.updateMessage(message.id, "edited @/y") + queue.updateMessage(message.id, "ctx @/x") - expect(queue.messages[0]?.segments).toEqual([{ text: "edited @/y", source: "user" }]) + expect(queue.messages[0]?.segments).toEqual([{ text: "ctx @/x", source: "generated" }]) + }) + + it("marks only the appended text as user text", () => { + const queue = new MessageQueueService() + const message = queue.addMessage("ctx @/x", undefined, [{ text: "ctx @/x", source: "generated" }])! + + queue.updateMessage(message.id, "ctx @/x mine") + + expect(queue.messages[0]?.segments).toEqual([ + { text: "ctx @/x", source: "generated" }, + { text: " mine", source: "user" }, + ]) + }) + + it("treats a message without segments as generated", () => { + const queue = new MessageQueueService() + const message = queue.addMessage("old @/x")! + + queue.updateMessage(message.id, "old @/x") + + expect(queue.messages[0]?.segments).toEqual([{ text: "old @/x", source: "generated" }]) }) }) diff --git a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts index 11c53a8e05..414a17212a 100644 --- a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts +++ b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts @@ -231,6 +231,45 @@ describe("AskFollowupQuestionTool", () => { expect(result.content).toEqual([{ type: "text", text: "\nparsed: @/allowed\n" }]) }) + it("expands only the user segment of a mixed answer", async () => { + const params = { question: "What?", follow_up: [{ text: "A" }] } + askMock.mockResolvedValue({ + text: "@/secret @/mine", + images: [], + segments: [ + { text: "@/secret ", source: "generated" }, + { text: "@/mine", source: "user" }, + ], + }) + + await tool.execute(params, mockTask, mockCallbacks) + + await processUserContentMentions({ + userContent: [mockTask.userMessageContent[0]], + cwd: "/test", + fileContextTracker: {} as FileContextTracker, + }) + expect(vi.mocked(parseMentions).mock.calls.map((call) => call[0])).toEqual(["@/mine"]) + }) + + it.each([ + { label: "missing", segments: undefined }, + { label: "not matching the text", segments: [{ text: "@/other", source: "user" }] }, + { label: "invalid", segments: [{ text: "@/secret", source: "system" }] }, + ])("expands nothing when the segments are $label", async ({ segments }) => { + const params = { question: "What?", follow_up: [{ text: "A" }] } + askMock.mockResolvedValue({ text: "@/secret", images: [], segments }) + + await tool.execute(params, mockTask, mockCallbacks) + + await processUserContentMentions({ + userContent: [mockTask.userMessageContent[0]], + cwd: "/test", + fileContextTracker: {} as FileContextTracker, + }) + expect(parseMentions).not.toHaveBeenCalled() + }) + it("does not authorize the feedback block when the user answer is null", async () => { const params = { question: "What?", follow_up: [{ text: "A" }] } askMock.mockResolvedValue({ text: null, images: [] }) diff --git a/src/core/tools/__tests__/attemptCompletionTool.spec.ts b/src/core/tools/__tests__/attemptCompletionTool.spec.ts index 8bde95d371..4b069618af 100644 --- a/src/core/tools/__tests__/attemptCompletionTool.spec.ts +++ b/src/core/tools/__tests__/attemptCompletionTool.spec.ts @@ -1167,6 +1167,61 @@ describe("attemptCompletionTool", () => { expect(mockPushToolResult).toHaveBeenCalledTimes(1) }) + it.each([ + { + label: "mixed origin", + segments: [ + { text: "@/secret ", source: "generated" }, + { text: "@/mine", source: "user" }, + ], + text: "@/secret @/mine", + expanded: ["@/mine"], + }, + { label: "missing", segments: undefined, text: "@/secret", expanded: [] }, + { + label: "not matching the text", + segments: [{ text: "@/other", source: "user" }], + text: "@/secret", + expanded: [], + }, + { + label: "invalid", + segments: [{ text: "@/secret", source: "system" }], + text: "@/secret", + expanded: [], + }, + ])("expands only authorized text when the segments are $label", async ({ segments, text, expanded }) => { + const block: ToolUse<"attempt_completion"> = { + type: "tool_use", + name: "attempt_completion", + params: { result: "Done" }, + nativeArgs: { result: "Done" }, + partial: false, + } + mockTask.ask = vi.fn().mockResolvedValue({ response: "messageResponse", text, images: [], segments }) + vi.mocked(parseMentions).mockImplementation(async (input) => ({ + text: `parsed: ${input}`, + contentBlocks: [], + })) + vi.mocked(parseMentions).mockClear() + + await attemptCompletionTool.handle(mockTask as Task, block, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + toolCallId: "", + askFinishSubTaskApproval: mockAskFinishSubTaskApproval, + toolDescription: mockToolDescription, + }) + + await processUserContentMentions({ + userContent: [mockTask.userMessageContent![0]], + cwd: "/test", + fileContextTracker: {} as FileContextTracker, + }) + expect(vi.mocked(parseMentions).mock.calls.map((call) => call[0])).toEqual(expanded) + }) + it("pushes an authorized feedback block that mention processing expands", async () => { const block: ToolUse<"attempt_completion"> = { type: "tool_use", diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index fa390df067..911a1d289c 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -7252,7 +7252,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { [], undefined, undefined, - [{ text: "Edited message with preserved images", source: "user" }], + [{ text: "Edited message with preserved images", source: "generated" }], ) }) @@ -7311,7 +7311,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { [], undefined, undefined, - [{ text: "Edited message with file attachment", source: "user" }], + [{ text: "Edited message with file attachment", source: "generated" }], ) }) }) @@ -7844,7 +7844,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { expect(mockCline.overwriteClineMessages).toHaveBeenCalled() expect(mockCline.submitUserMessage).toHaveBeenCalledWith(largeEditedContent, [], undefined, undefined, [ - { text: largeEditedContent, source: "user" }, + { text: largeEditedContent, source: "generated" }, ]) }) diff --git a/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts index 805f9c1fbc..ba0f3625ec 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.newTask.spec.ts @@ -36,6 +36,11 @@ const { resolveImageMentions } = vi.hoisted(() => ({ })) vi.mock("../../mentions/resolveImageMentions", () => ({ resolveImageMentions })) +vi.mock("../../task-persistence", async (importOriginal) => ({ + ...(await importOriginal()), + saveTaskMessages: vi.fn(), +})) + const generated = (text: string): MentionSegment => ({ text, source: "generated" }) const user = (text: string): MentionSegment => ({ text, source: "user" }) @@ -47,6 +52,8 @@ describe("webviewMessageHandler mention segment forwarding", () => { postMessageToWebview: ReturnType getCurrentTask: ReturnType getState: ReturnType + postStateToWebview: ReturnType + contextProxy: { globalStorageUri: { fsPath: string } } } let providerAsClineProvider: ClineProvider let currentTask: { @@ -65,6 +72,8 @@ describe("webviewMessageHandler mention segment forwarding", () => { postMessageToWebview: vi.fn().mockResolvedValue(undefined), getCurrentTask: vi.fn(() => currentTask), getState: vi.fn().mockResolvedValue({ maxImageFileSize: 5, maxTotalImageSize: 20 }), + postStateToWebview: vi.fn().mockResolvedValue(undefined), + contextProxy: { globalStorageUri: { fsPath: "/tmp/storage" } }, } providerAsClineProvider = provider as unknown as ClineProvider }) @@ -128,18 +137,24 @@ describe("webviewMessageHandler mention segment forwarding", () => { expect(provider.createTask).toHaveBeenCalledWith(text, [], undefined, expect.anything(), undefined) }) - it("scans the edited text for images and treats it as user text", async () => { + it("authorizes nothing in an edited message", async () => { + const submitUserMessage = vi.fn() const editTask = { + taskId: "task-1", clineMessages: [{ ts: 1, type: "say", say: "user_feedback", text: "old" }], apiConversationHistory: [], overwriteClineMessages: vi.fn(), overwriteApiConversationHistory: vi.fn(), - submitUserMessage: vi.fn(), + submitUserMessage, + messageManager: { rewindToTimestamp: vi.fn() }, } provider.getCurrentTask.mockReturnValue(editTask) const message: WebviewMessage = { type: "editMessageConfirm", messageTs: 1, text: "see @/a.png" } - await webviewMessageHandler(providerAsClineProvider, message).catch(() => {}) + await webviewMessageHandler(providerAsClineProvider, message) - expect(resolveImageMentions).toHaveBeenCalledWith(expect.objectContaining({ text: "see @/a.png" })) + expect(resolveImageMentions).toHaveBeenCalledWith(expect.objectContaining({ text: "" })) + expect(submitUserMessage).toHaveBeenCalledWith("see @/a.png", expect.anything(), undefined, undefined, [ + generated("see @/a.png"), + ]) }) }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index e93ebd4280..c5f67e164e 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -91,7 +91,7 @@ import { openGitHubAccountManagement, } from "../../api/providers/github-copilot" import { openMention } from "../mentions" -import { normalizeSegments, userSegments, userText } from "../../shared/mentionSegments" +import { generatedSegments, normalizeSegments, userText } from "../../shared/mentionSegments" import { resolveImageMentions } from "../mentions/resolveImageMentions" import { RooIgnoreController } from "../ignore/RooIgnoreController" import { getWorkspacePath } from "../../utils/path" @@ -2631,8 +2631,8 @@ export const webviewMessageHandler = async ( break case "editMessageConfirm": if (message.messageTs && message.text) { - // The edit box holds only text the user typed. - const editSegments = userSegments(message.text) + // Stored messages carry no segments, so fail closed: nothing is expanded. + const editSegments = generatedSegments(message.text) const resolved = await resolveIncomingImages({ text: message.text, images: message.images, diff --git a/webview-ui/src/components/chat/ChatTextArea.tsx b/webview-ui/src/components/chat/ChatTextArea.tsx index 79660cb421..cf28e4d4b9 100644 --- a/webview-ui/src/components/chat/ChatTextArea.tsx +++ b/webview-ui/src/components/chat/ChatTextArea.tsx @@ -3,7 +3,7 @@ import { useEvent } from "react-use" import DynamicTextArea from "react-textarea-autosize" import { VolumeX, Image, WandSparkles, SendHorizontal, X, ListEnd, Square } from "lucide-react" -import type { ExtensionMessage } from "@roo-code/types" +import type { ExtensionMessage, MentionSegment } from "@roo-code/types" import { mentionRegex, mentionRegexGlobal, commandRegexGlobal, unescapeSpaces } from "@roo/context-mentions" import { WebviewMessage } from "@roo/WebviewMessage" @@ -62,6 +62,10 @@ interface ChatTextAreaProps { /** Programmatic updates preserve the chat box authorization scope. */ /** Sets generated text, such as an enhanced prompt. Falls back to `setInputValue`. */ setInputValueGenerated?: (value: string) => void + /** Reads the draft's segments. Null means all user text. */ + getInputSegments?: () => MentionSegment[] | null + /** Sets the value with its segments, for prompt history. */ + setInputValueWithSegments?: (value: string, segments: MentionSegment[] | null) => void sendingDisabled: boolean selectApiConfigDisabled: boolean placeholderText: string @@ -89,6 +93,8 @@ export const ChatTextArea = forwardRef( inputValue, setInputValue, setInputValueGenerated, + getInputSegments, + setInputValueWithSegments, selectApiConfigDisabled, placeholderText, selectedImages, @@ -270,6 +276,8 @@ export const ChatTextArea = forwardRef( cwd, inputValue: normalizedInputValue, setInputValue, + getSegments: getInputSegments, + setInputValueWithSegments, }) // Fetch git commits when Git is selected or when typing a hash. diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index ad835e592f..12d3f017d3 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -816,6 +816,14 @@ const ChatViewComponent: React.ForwardRefRenderFunction segmentsRef.current, []) + const setInputValueWithSegments = useCallback((value: string, segments: MentionSegment[] | null) => { + segmentsRef.current = segments + inputValueRef.current = value + setInputValue(value) + }, []) + // Handle stop button click from textarea const handleStopTask = useCallback(() => { vscode.postMessage({ type: "cancelTask" }) @@ -1972,6 +1980,8 @@ const ChatViewComponent: React.ForwardRefRenderFunction { Object.defineProperty(document, "execCommand", { value: vi.fn((_cmd: string, _ui: boolean, value: string) => { if (target.el) { - fireEvent.change(target.el, { target: { value } }) + fireEvent.input(target.el, { target: { value } }) } return true }), @@ -686,6 +686,29 @@ describe("ChatTextArea", () => { expect(setInputValue).toHaveBeenCalledWith("Third prompt") }) + it("restores the draft with its segments after history navigation", () => { + const draft = [{ text: "ctx @/a", source: "generated" as const }] + const setInputValueWithSegments = vi.fn() + const { container } = render( + draft} + setInputValueWithSegments={setInputValueWithSegments} + />, + ) + const textarea = container.querySelector("textarea")! + textarea.setSelectionRange(0, 0) + + fireEvent.keyDown(textarea, { key: "ArrowUp" }) + expect(setInputValueWithSegments).toHaveBeenLastCalledWith("Third prompt", [ + { text: "Third prompt", source: "generated" }, + ]) + + fireEvent.keyDown(textarea, { key: "ArrowDown" }) + expect(setInputValueWithSegments).toHaveBeenLastCalledWith("ctx @/a", draft) + }) + it("should navigate through history with multiple arrow up presses", () => { const setInputValue = vi.fn() const { container } = render( diff --git a/webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx index f479f09e7e..163eeba235 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.add-to-context-authorization.spec.tsx @@ -4,7 +4,7 @@ import React from "react" import { describe, it, expect, vi, beforeEach } from "vitest" import { renderWithExtensionState, waitFor, act, fireEvent } from "@/utils/test-utils" -import ChatView, { ChatViewProps } from "../ChatView" +import ChatView, { ChatViewProps, ChatViewRef } from "../ChatView" // Mock vscode API const { mockPostMessage } = vi.hoisted(() => ({ mockPostMessage: vi.fn() })) @@ -305,4 +305,18 @@ describe("ChatView - add-to-context mention segments", () => { segments: [generated("enhanced with @/secret")], }) }) + + it("sends generated segments when the imperative acceptInput submits", async () => { + const ref = React.createRef() + renderWithExtensionState() + + await addGeneratedContext({ segments: [generated("context with @/secret code")] }) + await act(async () => { + ref.current!.acceptInput() + }) + await waitFor(() => expect(mockPostMessage).toHaveBeenCalledWith(expect.objectContaining({ type: "newTask" }))) + + const message = mockPostMessage.mock.calls.find(([m]) => m.type === "newTask")?.[0] + expect(message).toMatchObject({ segments: [generated("context with @/secret code")] }) + }) }) diff --git a/webview-ui/src/components/chat/hooks/usePromptHistory.ts b/webview-ui/src/components/chat/hooks/usePromptHistory.ts index 402538182a..c4e64e35db 100644 --- a/webview-ui/src/components/chat/hooks/usePromptHistory.ts +++ b/webview-ui/src/components/chat/hooks/usePromptHistory.ts @@ -1,5 +1,5 @@ -import { ClineMessage, HistoryItem } from "@roo-code/types" -import { useCallback, useEffect, useMemo, useState } from "react" +import { ClineMessage, HistoryItem, MentionSegment } from "@roo-code/types" +import { useCallback, useEffect, useMemo, useRef, useState } from "react" interface UsePromptHistoryProps { clineMessages: ClineMessage[] | undefined @@ -7,6 +7,10 @@ interface UsePromptHistoryProps { cwd: string | undefined inputValue: string setInputValue: (value: string) => void + /** Reads the draft's segments. Null means all user text. */ + getSegments?: () => MentionSegment[] | null + /** Sets the value with its segments. Falls back to `setInputValue`. */ + setInputValueWithSegments?: (value: string, segments: MentionSegment[] | null) => void } export interface UsePromptHistoryReturn { @@ -30,6 +34,8 @@ export const usePromptHistory = ({ cwd, inputValue, setInputValue, + getSegments, + setInputValueWithSegments, }: UsePromptHistoryProps): UsePromptHistoryReturn => { // Maximum number of prompts to keep in history for memory management const MAX_PROMPT_HISTORY_SIZE = 100 @@ -37,6 +43,8 @@ export const usePromptHistory = ({ // Prompt history navigation state const [historyIndex, setHistoryIndex] = useState(-1) const [tempInput, setTempInput] = useState("") + // Segments of the draft that history navigation replaced. + const tempSegmentsRef = useRef(null) const [promptHistory, setPromptHistory] = useState([]) // Initialize prompt history with hybrid approach: conversation messages if in task, otherwise task history @@ -111,22 +119,31 @@ export const usePromptHistory = ({ if (!historicalPrompt) return false setHistoryIndex(newIndex) - setInputValue(historicalPrompt) + // Stored prompts carry no segments, so they authorize nothing. + if (setInputValueWithSegments) { + setInputValueWithSegments(historicalPrompt, [{ text: historicalPrompt, source: "generated" }]) + } else { + setInputValue(historicalPrompt) + } setCursorPosition(textarea, cursorPos, historicalPrompt.length) return true }, - [promptHistory, setInputValue, setCursorPosition], + [promptHistory, setInputValue, setInputValueWithSegments, setCursorPosition], ) // Helper to return to current input const returnToCurrentInput = useCallback( (textarea: HTMLTextAreaElement, cursorPos: "start" | "end" = "end") => { setHistoryIndex(-1) - setInputValue(tempInput) + if (setInputValueWithSegments) { + setInputValueWithSegments(tempInput, tempSegmentsRef.current) + } else { + setInputValue(tempInput) + } setCursorPosition(textarea, cursorPos, tempInput.length) }, - [tempInput, setInputValue, setCursorPosition], + [tempInput, setInputValue, setInputValueWithSegments, setCursorPosition], ) const handleHistoryNavigation = useCallback( @@ -147,6 +164,7 @@ export const usePromptHistory = ({ // Save current input if starting navigation if (historyIndex === -1) { setTempInput(inputValue) + tempSegmentsRef.current = getSegments?.() ?? null } return navigateToHistory(historyIndex + 1, textarea, "start") } @@ -167,7 +185,7 @@ export const usePromptHistory = ({ } return false }, - [promptHistory, historyIndex, inputValue, navigateToHistory, returnToCurrentInput], + [promptHistory, historyIndex, inputValue, getSegments, navigateToHistory, returnToCurrentInput], ) const resetHistoryNavigation = useCallback(() => { From cfc13e608853fb8b28a2db02552d202dbd63923f Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sun, 15 Nov 2026 02:29:02 +0000 Subject: [PATCH 09/15] test(mentions): cover interleaved segments and resume responses Assert the full processed output for user, generated, user segments in both tools. Add resumeTaskFromHistory tests for accept, omitted, mismatched, and empty responses. --- .../task/__tests__/Task.persistence.spec.ts | 70 +++++++++++++++++++ .../__tests__/askFollowupQuestionTool.spec.ts | 25 +++++++ .../__tests__/attemptCompletionTool.spec.ts | 42 +++++++++++ 3 files changed, 137 insertions(+) diff --git a/src/core/task/__tests__/Task.persistence.spec.ts b/src/core/task/__tests__/Task.persistence.spec.ts index b7c97a357e..17724e1fb6 100644 --- a/src/core/task/__tests__/Task.persistence.spec.ts +++ b/src/core/task/__tests__/Task.persistence.spec.ts @@ -16,6 +16,9 @@ import { TelemetryService } from "@roo-code/telemetry" import type { Anthropic } from "@anthropic-ai/sdk" import { PendingActionSettlementError, Task } from "../Task" +import { parseMentions } from "../../mentions" +import { processUserContentMentions } from "../../mentions/processUserContentMentions" +import type { FileContextTracker } from "../../context-tracking/FileContextTracker" import { ClineProvider } from "../../webview/ClineProvider" import { ContextProxy } from "../../config/ContextProxy" import { providerIdentifiers } from "@roo-code/types/provider-identifiers" @@ -2219,6 +2222,73 @@ describe("Task persistence", () => { }) }) + describe("resumeTaskFromHistory response mentions", () => { + const resumeWith = async (response: { text: string; segments?: unknown }) => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + historyItem: { + id: "resume-mentions", + number: 1, + ts: Date.now(), + task: "Resumed task", + tokensIn: 10, + tokensOut: 5, + totalCost: 0.001, + }, + startTask: false, + initialStatus: "interrupted", + }) + const initiate = vi.spyOn(getTaskPersistenceAccess(task), "initiateTaskLoop").mockResolvedValue(undefined) + vi.spyOn(task, "ask").mockResolvedValue({ response: "messageResponse", ...response } as never) + mockReadApiMessages.mockResolvedValue([{ role: "assistant", content: [{ type: "text", text: "Hi" }] }]) + mockProvider.taskHistoryStore.get = vi + .fn() + .mockReturnValue({ id: "resume-mentions", status: "interrupted" }) + + await getTaskPersistenceAccess(task).resumeTaskFromHistory() + + vi.mocked(parseMentions).mockClear() + const processed = await processUserContentMentions({ + userContent: initiate.mock.calls[0][0], + cwd: "/test", + fileContextTracker: {} as FileContextTracker, + }) + return { processed, parsed: vi.mocked(parseMentions).mock.calls.map((call) => call[0]) } + } + + it("expands a user segment and keeps the wrapper", async () => { + const text = "read @/allowed please" + const { processed, parsed } = await resumeWith({ text, segments: [{ text, source: "user" }] }) + + expect(parsed).toEqual([text]) + expect(processed.content).toContainEqual({ + type: "text", + text: `\nprocessed: ${text}\n`, + }) + }) + + it.each([ + { label: "omitted", segments: undefined }, + { label: "not matching the text", segments: [{ text: "other", source: "user" }] }, + ])("expands nothing when the segments are $label", async ({ segments }) => { + const { processed, parsed } = await resumeWith({ text: "read @/secret", segments }) + + expect(parsed).toEqual([]) + expect(processed.content).toContainEqual({ + type: "text", + text: "\nread @/secret\n", + }) + }) + + it("pushes no response block for an empty response", async () => { + const { processed, parsed } = await resumeWith({ text: "" }) + + expect(parsed).toEqual([]) + expect(JSON.stringify(processed.content)).not.toContain("") + }) + }) + describe("resumeTaskFromHistory", () => { it.each(["not_found", "invalid", "io_error"] as const)( "does not persist when hydration fails with %s", diff --git a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts index 414a17212a..6af1b91ff5 100644 --- a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts +++ b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts @@ -252,6 +252,31 @@ describe("AskFollowupQuestionTool", () => { expect(vi.mocked(parseMentions).mock.calls.map((call) => call[0])).toEqual(["@/mine"]) }) + it("keeps generated text between user segments and expands both user segments", async () => { + const params = { question: "What?", follow_up: [{ text: "A" }] } + askMock.mockResolvedValue({ + text: "@/a @/secret @/b", + images: [], + segments: [ + { text: "@/a ", source: "user" }, + { text: "@/secret ", source: "generated" }, + { text: "@/b", source: "user" }, + ], + }) + vi.mocked(parseMentions).mockImplementation(async (input) => ({ text: `parsed: ${input}`, contentBlocks: [] })) + + await tool.execute(params, mockTask, mockCallbacks) + + const result = await processUserContentMentions({ + userContent: [mockTask.userMessageContent[0]], + cwd: "/test", + fileContextTracker: {} as FileContextTracker, + }) + expect(result.content).toEqual([ + { type: "text", text: "\nparsed: @/a @/secret parsed: @/b\n" }, + ]) + }) + it.each([ { label: "missing", segments: undefined }, { label: "not matching the text", segments: [{ text: "@/other", source: "user" }] }, diff --git a/src/core/tools/__tests__/attemptCompletionTool.spec.ts b/src/core/tools/__tests__/attemptCompletionTool.spec.ts index 4b069618af..a0354d3858 100644 --- a/src/core/tools/__tests__/attemptCompletionTool.spec.ts +++ b/src/core/tools/__tests__/attemptCompletionTool.spec.ts @@ -1167,6 +1167,48 @@ describe("attemptCompletionTool", () => { expect(mockPushToolResult).toHaveBeenCalledTimes(1) }) + it("keeps generated text between user segments and expands both user segments", async () => { + const block: ToolUse<"attempt_completion"> = { + type: "tool_use", + name: "attempt_completion", + params: { result: "Done" }, + nativeArgs: { result: "Done" }, + partial: false, + } + mockTask.ask = vi.fn().mockResolvedValue({ + response: "messageResponse", + text: "@/a @/secret @/b", + images: [], + segments: [ + { text: "@/a ", source: "user" }, + { text: "@/secret ", source: "generated" }, + { text: "@/b", source: "user" }, + ], + }) + vi.mocked(parseMentions).mockImplementation(async (input) => ({ + text: `parsed: ${input}`, + contentBlocks: [], + })) + + await attemptCompletionTool.handle(mockTask as Task, block, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + toolCallId: "", + askFinishSubTaskApproval: mockAskFinishSubTaskApproval, + toolDescription: mockToolDescription, + }) + + const result = await processUserContentMentions({ + userContent: [mockTask.userMessageContent![0]], + cwd: "/test", + fileContextTracker: {} as FileContextTracker, + }) + expect(result.content).toEqual([ + { type: "text", text: "\nparsed: @/a @/secret parsed: @/b\n" }, + ]) + }) + it.each([ { label: "mixed origin", From 380466b0e454582e21de9b6ae936f98d34ac71e9 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sun, 15 Nov 2026 03:56:10 +0000 Subject: [PATCH 10/15] test(mentions): cover headless sendMessage and resume source checks Test the headless sendMessage path with text, empty text, and no task. Add interleaved, generated-only, and invalid-source cases to the resume tests. --- .../task/__tests__/Task.persistence.spec.ts | 20 +++++++++++++ .../__tests__/api-send-message.spec.ts | 30 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/src/core/task/__tests__/Task.persistence.spec.ts b/src/core/task/__tests__/Task.persistence.spec.ts index 17724e1fb6..e896d13a7c 100644 --- a/src/core/task/__tests__/Task.persistence.spec.ts +++ b/src/core/task/__tests__/Task.persistence.spec.ts @@ -2268,9 +2268,29 @@ describe("Task persistence", () => { }) }) + it("expands only the user segments of an interleaved answer", async () => { + const text = "@/a @/secret @/b" + const { processed, parsed } = await resumeWith({ + text, + segments: [ + { text: "@/a ", source: "user" }, + { text: "@/secret ", source: "generated" }, + { text: "@/b", source: "user" }, + ], + }) + + expect(parsed).toEqual(["@/a ", "@/b"]) + expect(processed.content).toContainEqual({ + type: "text", + text: "\nprocessed: @/a @/secret processed: @/b\n", + }) + }) + it.each([ { label: "omitted", segments: undefined }, { label: "not matching the text", segments: [{ text: "other", source: "user" }] }, + { label: "matching but generated only", segments: [{ text: "read @/secret", source: "generated" }] }, + { label: "matching but of an invalid source", segments: [{ text: "read @/secret", source: "system" }] }, ])("expands nothing when the segments are $label", async ({ segments }) => { const { processed, parsed } = await resumeWith({ text: "read @/secret", segments }) diff --git a/src/extension/__tests__/api-send-message.spec.ts b/src/extension/__tests__/api-send-message.spec.ts index dc51d06445..1f7f76df54 100644 --- a/src/extension/__tests__/api-send-message.spec.ts +++ b/src/extension/__tests__/api-send-message.spec.ts @@ -153,4 +153,34 @@ describe("API - SendMessage Command", () => { }) expect(mockPostMessageToWebview).toHaveBeenCalledTimes(1) }) + + describe("when the webview is not launched", () => { + const user = (text: string) => ({ text, source: "user" as const }) + + it.each([ + { label: "text and images", text: "see @/file.ts", images: ["img"], expected: [user("see @/file.ts")] }, + { label: "empty text", text: "", images: undefined, expected: [] }, + { label: "missing text", text: undefined, images: undefined, expected: [] }, + ])("submits the user text directly with $label", async ({ text, images, expected }) => { + const submitUserMessage = vi.fn().mockResolvedValue(undefined) + Object.assign(mockProvider, { + viewLaunched: false, + getCurrentTask: vi.fn().mockReturnValue({ submitUserMessage }), + }) + + await api.sendMessage(text, images) + + expect(submitUserMessage).toHaveBeenCalledWith(text ?? "", images, undefined, undefined, expected) + expect(mockPostMessageToWebview).not.toHaveBeenCalled() + }) + + it("logs and drops the message when there is no current task", async () => { + Object.assign(mockProvider, { viewLaunched: false }) + + await api.sendMessage("see @/file.ts") + + expect(mockLog).toHaveBeenCalledWith(expect.stringContaining("no current task")) + expect(mockPostMessageToWebview).not.toHaveBeenCalled() + }) + }) }) From f3f3f551c1a933dca573399542dacb772d26c1cf Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sun, 15 Nov 2026 04:14:31 +0000 Subject: [PATCH 11/15] fix(mentions): send segments from the API and approval feedback The sendMessage API and the approve and reject feedback now carry user segments, so typed mentions expand as in the headless path. Add tests for queued answers, edit overlap, and mention processing. --- .../processUserContentMentions.spec.ts | 52 +++++++++++++++++++ .../ask-queued-message-drain.spec.ts | 11 ++++ src/core/tools/AttemptCompletionTool.ts | 2 +- .../__tests__/attemptCompletionTool.spec.ts | 2 +- .../webview/__tests__/ClineProvider.spec.ts | 7 ++- .../__tests__/api-send-message.spec.ts | 7 +++ src/extension/api.ts | 8 ++- src/shared/__tests__/mentionSegments.spec.ts | 10 ++++ webview-ui/src/components/chat/ChatView.tsx | 9 ++++ 9 files changed, 104 insertions(+), 4 deletions(-) diff --git a/src/core/mentions/__tests__/processUserContentMentions.spec.ts b/src/core/mentions/__tests__/processUserContentMentions.spec.ts index 15a1bd3e6e..f863756b54 100644 --- a/src/core/mentions/__tests__/processUserContentMentions.spec.ts +++ b/src/core/mentions/__tests__/processUserContentMentions.spec.ts @@ -102,4 +102,56 @@ describe("processUserContentMentions authorization", () => { await process([copy, block, userMentionContent([gen("context @/secret")])]) expect(parseMentions).not.toHaveBeenCalled() }) + + it("passes image blocks through unchanged next to a user block", async () => { + const image: Anthropic.Messages.ImageBlockParam = { + type: "image", + source: { type: "base64", media_type: "image/png", data: "abc" }, + } + const result = await process([userMentionContent([user("@/allowed")]), image]) + expect(result.content[result.content.length - 1]).toBe(image) + }) + + it.each([ + { showRooIgnoredFiles: true, includeDiagnosticMessages: false, maxDiagnosticMessages: 3, currentMode: "ask" }, + { showRooIgnoredFiles: false, includeDiagnosticMessages: true, maxDiagnosticMessages: 50, currentMode: "code" }, + ])("passes the flags to parseMentions: %o", async (flags) => { + await processUserContentMentions({ + userContent: [userMentionContent([user("@/allowed")])], + cwd: "/test", + fileContextTracker: {} as FileContextTracker, + ...flags, + }) + expect(parseMentions).toHaveBeenCalledWith( + "@/allowed", + "/test", + expect.anything(), + undefined, + flags.showRooIgnoredFiles, + flags.includeDiagnosticMessages, + flags.maxDiagnosticMessages, + undefined, + flags.currentMode, + ) + }) + + it("keeps the mode of the first slash command", async () => { + vi.mocked(parseMentions) + .mockResolvedValueOnce({ text: "one", contentBlocks: [], mode: "architect" }) + .mockResolvedValueOnce({ text: "two", contentBlocks: [], mode: "debug" }) + const result = await process([userMentionContent([user("/a"), gen(" "), user("/b")])]) + expect(result.mode).toBe("architect") + }) + + it("orders the extra blocks by segment after the text block", async () => { + vi.mocked(parseMentions) + .mockResolvedValueOnce({ text: "one", contentBlocks: [{ type: "file", path: "a", content: "A" }] }) + .mockResolvedValueOnce({ text: "two", contentBlocks: [{ type: "file", path: "b", content: "B" }] }) + const result = await process([userMentionContent([user("@/a"), gen(" "), user("@/b")])]) + expect(result.content).toEqual([ + { type: "text", text: "one two" }, + { type: "text", text: "A" }, + { type: "text", text: "B" }, + ]) + }) }) diff --git a/src/core/task/__tests__/ask-queued-message-drain.spec.ts b/src/core/task/__tests__/ask-queued-message-drain.spec.ts index 16fbb1c93e..a0b4ac91fa 100644 --- a/src/core/task/__tests__/ask-queued-message-drain.spec.ts +++ b/src/core/task/__tests__/ask-queued-message-drain.spec.ts @@ -53,6 +53,17 @@ describe("Task.ask queued message drain", () => { expect(result.text).toBe("picked answer") }) + it("forwards the queued message segments to the ask response", async () => { + const task = await createTask() + const segments = [{ text: "@/a", source: "user" as const }] + + const askPromise = task.ask("followup", "Q?", false) + task.messageQueueService.addMessage("@/a", undefined, segments) + + const result = await askPromise + expect(result.segments).toEqual(segments) + }) + it("does not consume queued messages for command_output asks", async () => { const task = await createTask() diff --git a/src/core/tools/AttemptCompletionTool.ts b/src/core/tools/AttemptCompletionTool.ts index 0ff84f9d0f..e8fe148d57 100644 --- a/src/core/tools/AttemptCompletionTool.ts +++ b/src/core/tools/AttemptCompletionTool.ts @@ -248,7 +248,7 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { const feedbackText = `\n${text}\n` // Only user segments expand. The tool result repeats the text by design. - task.userMessageContent.push(userMentionContent(wrapUserMessage(normalizeSegments(String(text), segments)))) + task.userMessageContent.push(userMentionContent(wrapUserMessage(normalizeSegments(text ?? "", segments)))) pushToolResult(formatResponse.toolResult(feedbackText, images)) } catch (error) { await handleError("inspecting site", error as Error) diff --git a/src/core/tools/__tests__/attemptCompletionTool.spec.ts b/src/core/tools/__tests__/attemptCompletionTool.spec.ts index a0354d3858..c1a5689034 100644 --- a/src/core/tools/__tests__/attemptCompletionTool.spec.ts +++ b/src/core/tools/__tests__/attemptCompletionTool.spec.ts @@ -1345,7 +1345,7 @@ describe("attemptCompletionTool", () => { fileContextTracker: {} as FileContextTracker, }) expect(parseMentions).not.toHaveBeenCalled() - expect(result.content).toEqual([{ type: "text", text: "\nundefined\n" }]) + expect(result.content).toEqual([{ type: "text", text: "\n\n" }]) }) it("does not clear pending metadata when stale delegation continues without an action id", async () => { diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 911a1d289c..255af5788c 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -425,7 +425,12 @@ afterAll(() => { vi.restoreAllMocks() }) -const noUserSegments = expect.not.arrayContaining([expect.objectContaining({ source: "user" })]) +// Matches a non-empty segment list that holds only generated text. +const noUserSegments = { + asymmetricMatch: (value: unknown) => + Array.isArray(value) && value.length > 0 && value.every((segment) => segment?.source === "generated"), + toString: () => "onlyGeneratedSegments", +} describe("ClineProvider", () => { beforeAll(() => { diff --git a/src/extension/__tests__/api-send-message.spec.ts b/src/extension/__tests__/api-send-message.spec.ts index 1f7f76df54..1804315c85 100644 --- a/src/extension/__tests__/api-send-message.spec.ts +++ b/src/extension/__tests__/api-send-message.spec.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest" import * as vscode from "vscode" import { API } from "../api" +import { userSegments } from "../../shared/mentionSegments" import { ClineProvider } from "../../core/webview/ClineProvider" import { makeClineProviderFactory } from "../../test-utils/provider" @@ -53,6 +54,7 @@ describe("API - SendMessage Command", () => { invoke: "sendMessage", text: messageText, images: undefined, + segments: userSegments(messageText ?? ""), }) }) @@ -72,6 +74,7 @@ describe("API - SendMessage Command", () => { invoke: "sendMessage", text: messageText, images, + segments: userSegments(messageText ?? ""), }) }) @@ -90,6 +93,7 @@ describe("API - SendMessage Command", () => { invoke: "sendMessage", text: undefined, images, + segments: userSegments(""), }) }) @@ -103,6 +107,7 @@ describe("API - SendMessage Command", () => { invoke: "sendMessage", text: undefined, images: undefined, + segments: userSegments(""), }) }) @@ -129,6 +134,7 @@ describe("API - SendMessage Command", () => { invoke: "sendMessage", text: messageText, images: undefined, + segments: userSegments(messageText ?? ""), }) }) @@ -150,6 +156,7 @@ describe("API - SendMessage Command", () => { invoke: "sendMessage", text: messageText, images, + segments: userSegments(messageText ?? ""), }) expect(mockPostMessageToWebview).toHaveBeenCalledTimes(1) }) diff --git a/src/extension/api.ts b/src/extension/api.ts index 2566006033..e34a7fdf97 100644 --- a/src/extension/api.ts +++ b/src/extension/api.ts @@ -417,7 +417,13 @@ export class API extends EventEmitter implements RooCodeAPI { return } - await this.sidebarProvider.postMessageToWebview({ type: "invoke", invoke: "sendMessage", text, images }) + await this.sidebarProvider.postMessageToWebview({ + type: "invoke", + invoke: "sendMessage", + text, + images, + segments: userSegments(text ?? ""), + }) } public deleteQueuedMessage(messageId: string) { diff --git a/src/shared/__tests__/mentionSegments.spec.ts b/src/shared/__tests__/mentionSegments.spec.ts index b9887ff23b..10ca7f3b82 100644 --- a/src/shared/__tests__/mentionSegments.spec.ts +++ b/src/shared/__tests__/mentionSegments.spec.ts @@ -71,6 +71,16 @@ describe("mentionSegments", () => { expect(applyEdit([gen("x")], "ab", "abc")).toEqual(generatedSegments("abc")) }) + it.each([ + ["aa", "aaa"], + ["aaa", "aa"], + ["abab", "ab"], + ["ab", "abab"], + ])("keeps the joined text equal to the new value for %s -> %s", (oldValue, newValue) => { + const result = applyEdit([gen(oldValue)], oldValue, newValue) + expect(joinSegments(result)).toBe(newValue) + }) + it("does not authorize a generated mention after an unrelated edit", () => { const result = applyEdit([gen("see @/secret")], "see @/secret", "see @/secret now") expect(userText(result)).toBe(" now") diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 12d3f017d3..c4b7dc09c4 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -881,6 +881,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction Date: Sun, 15 Nov 2026 04:17:42 +0000 Subject: [PATCH 12/15] test(mentions): cover approve and reject feedback segments --- .../ChatView.clear-approval-buttons.spec.tsx | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.clear-approval-buttons.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.clear-approval-buttons.spec.tsx index 14ccce9751..bdd26b9cfa 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.clear-approval-buttons.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.clear-approval-buttons.spec.tsx @@ -195,6 +195,34 @@ describe("ChatView approval button behavior", () => { }) }) + it.each([ + { label: RUN_BUTTON_LABEL, askResponse: "yesButtonClicked" }, + { label: DENY_BUTTON_LABEL, askResponse: "noButtonClicked" }, + ])("sends the typed feedback as user segments with $askResponse", async ({ label, askResponse }) => { + const { getByText, getByRole } = renderChatView() + + await act(async () => { + hydrateState(commandAsk()) + }) + await waitFor(() => expect(getByText(label)).toBeInTheDocument()) + + await act(async () => { + fireEvent.input(getByRole("textbox"), { target: { value: "use @/ok" } }) + }) + await act(async () => { + fireEvent.click(getByText(label)) + }) + + expect(vscode.postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "askResponse", + askResponse, + text: "use @/ok", + segments: [{ text: "use @/ok", source: "user" }], + }), + ) + }) + it("never shows Run/Deny buttons when the command ask is already answered (auto-approved)", async () => { const { queryByText } = renderChatView() From b7930218fc6aad66fa94971442da0133f9cf98ed Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sun, 15 Nov 2026 18:41:03 +0000 Subject: [PATCH 13/15] refactor(mentions): share one segment helper for chat box sends Approve, reject, resume and queue sends now use one helper. The helper checks that the segments match the text. Tests cover generated text and the resume_task ask. --- webview-ui/src/components/chat/ChatView.tsx | 37 +++----- .../ChatView.clear-approval-buttons.spec.tsx | 93 ++++++++++++++----- 2 files changed, 83 insertions(+), 47 deletions(-) diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index c4b7dc09c4..321f6d0070 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -220,6 +220,11 @@ const ChatViewComponent: React.ForwardRefRenderFunction(false) // Origin of each part of the chat box value. Null means all of it is user text. const segmentsRef = useRef(null) + // Trimmed chat box segments for the text. A value that does not match the text is all generated. + const segmentsFor = useCallback((text: string) => { + const trimmed = segmentsRef.current ? trimSegments(segmentsRef.current) : userSegments(text) + return joinSegments(trimmed) === text ? trimmed : generatedSegments(text) + }, []) const [currentFollowUpTs, setCurrentFollowUpTs] = useState(null) const [aggregatedCostsMap, setAggregatedCostsMap] = useState< Map< @@ -661,13 +666,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction { text = text.trim() - // Trimmed chat box segments. A value that does not match the text is all generated. - const trimmed = segmentsRef.current ? trimSegments(segmentsRef.current) : userSegments(text) - const segments = segmentsOverride - ? trimSegments(segmentsOverride) - : joinSegments(trimmed) === text - ? trimmed - : generatedSegments(text) + const segments = segmentsOverride ? trimSegments(segmentsOverride) : segmentsFor(text) if (text || images.length > 0) { // Intercept when the active provider is retired — show a @@ -767,6 +766,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction