mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: prevent stale selection context by capturing at send-time
- Move selection context request from mount-time to send-time in ChatView - Remove storage of selection context in Task instance to prevent staleness - Pass selection context through message flow as ephemeral parameter - Add getAndClearSelectionContext() method to Task for one-time use - Update tests to reflect new architecture with mocked getter method - Add selectionContext to CreateTaskOptions interface This ensures selection context is always fresh when sent and automatically cleared after use, preventing context leakage between messages.
This commit is contained in:
parent
e25c2d66f4
commit
338d8d01a3
6 changed files with 126 additions and 81 deletions
|
|
@ -92,6 +92,12 @@ export interface CreateTaskOptions {
|
|||
consecutiveMistakeLimit?: number
|
||||
experiments?: Record<string, boolean>
|
||||
initialTodos?: TodoItem[]
|
||||
selectionContext?: {
|
||||
selectedText: string
|
||||
selectionFilePath: string
|
||||
selectionStartLine: number
|
||||
selectionEndLine: number
|
||||
}
|
||||
}
|
||||
|
||||
export enum TaskStatus {
|
||||
|
|
|
|||
|
|
@ -116,6 +116,7 @@ describe("getEnvironmentDetails", () => {
|
|||
deref: vi.fn().mockReturnValue(mockProvider),
|
||||
[Symbol.toStringTag]: "WeakRef",
|
||||
} as unknown as WeakRef<ClineProvider>,
|
||||
getAndClearSelectionContext: vi.fn().mockReturnValue(undefined),
|
||||
}
|
||||
|
||||
// Mock other dependencies.
|
||||
|
|
@ -393,46 +394,57 @@ describe("getEnvironmentDetails", () => {
|
|||
|
||||
describe("Selection Context", () => {
|
||||
it("should include selection context when available", async () => {
|
||||
const clineWithSelection = {
|
||||
...mockCline,
|
||||
selectionContext: {
|
||||
selectedText: "const x = 1;\nconst y = 2;",
|
||||
selectionFilePath: "src/test.ts",
|
||||
selectionStartLine: 10,
|
||||
selectionEndLine: 11,
|
||||
},
|
||||
const selectionContext = {
|
||||
selectedText: "const x = 1;\nconst y = 2;",
|
||||
selectionFilePath: "src/test.ts",
|
||||
selectionStartLine: 10,
|
||||
selectionEndLine: 11,
|
||||
}
|
||||
|
||||
const result = await getEnvironmentDetails(clineWithSelection as Task)
|
||||
const clineWithSelection = {
|
||||
...mockCline,
|
||||
getAndClearSelectionContext: vi.fn().mockReturnValueOnce(selectionContext),
|
||||
}
|
||||
|
||||
const result = await getEnvironmentDetails(clineWithSelection as unknown as Task)
|
||||
|
||||
expect(result).toContain("# Current Selection")
|
||||
expect(result).toContain("File: src/test.ts:10-11")
|
||||
expect(result).toContain("```")
|
||||
expect(result).toContain("const x = 1;")
|
||||
expect(result).toContain("const y = 2;")
|
||||
expect(clineWithSelection.getAndClearSelectionContext).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it("should clear selection context after including it", async () => {
|
||||
const clineWithSelection = {
|
||||
...mockCline,
|
||||
selectionContext: {
|
||||
selectedText: "test code",
|
||||
selectionFilePath: "src/app.ts",
|
||||
selectionStartLine: 5,
|
||||
selectionEndLine: 5,
|
||||
},
|
||||
const selectionContext = {
|
||||
selectedText: "test code",
|
||||
selectionFilePath: "src/app.ts",
|
||||
selectionStartLine: 5,
|
||||
selectionEndLine: 5,
|
||||
}
|
||||
|
||||
await getEnvironmentDetails(clineWithSelection as Task)
|
||||
const clineWithSelection = {
|
||||
...mockCline,
|
||||
getAndClearSelectionContext: vi.fn().mockReturnValueOnce(selectionContext),
|
||||
}
|
||||
|
||||
// Selection context should be cleared after use
|
||||
expect(clineWithSelection.selectionContext).toBeUndefined()
|
||||
await getEnvironmentDetails(clineWithSelection as unknown as Task)
|
||||
|
||||
// Selection context should be cleared after use (method called once)
|
||||
expect(clineWithSelection.getAndClearSelectionContext).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it("should not include selection section when no context is available", async () => {
|
||||
const result = await getEnvironmentDetails(mockCline as Task)
|
||||
const clineWithoutSelection = {
|
||||
...mockCline,
|
||||
getAndClearSelectionContext: vi.fn().mockReturnValueOnce(undefined),
|
||||
}
|
||||
|
||||
const result = await getEnvironmentDetails(clineWithoutSelection as unknown as Task)
|
||||
|
||||
expect(result).not.toContain("# Current Selection")
|
||||
expect(clineWithoutSelection.getAndClearSelectionContext).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -32,15 +32,13 @@ export async function getEnvironmentDetails(cline: Task, includeFileDetails: boo
|
|||
maxWorkspaceFiles = 200,
|
||||
} = state ?? {}
|
||||
|
||||
// Include selection context if available
|
||||
if (cline.selectionContext) {
|
||||
const { selectedText, selectionFilePath, selectionStartLine, selectionEndLine } = cline.selectionContext
|
||||
// Include selection context if available (and automatically clear it)
|
||||
const selectionContext = cline.getAndClearSelectionContext()
|
||||
if (selectionContext) {
|
||||
const { selectedText, selectionFilePath, selectionStartLine, selectionEndLine } = selectionContext
|
||||
details += "\n\n# Current Selection"
|
||||
details += `\nFile: ${selectionFilePath}:${selectionStartLine}-${selectionEndLine}`
|
||||
details += `\n\`\`\`\n${selectedText}\n\`\`\``
|
||||
|
||||
// Clear the selection context after including it once
|
||||
cline.selectionContext = undefined
|
||||
}
|
||||
|
||||
// It could be useful for cline to know if the user went from one or no
|
||||
|
|
|
|||
|
|
@ -142,6 +142,12 @@ export interface TaskOptions extends CreateTaskOptions {
|
|||
onCreated?: (task: Task) => void
|
||||
initialTodos?: TodoItem[]
|
||||
workspacePath?: string
|
||||
selectionContext?: {
|
||||
selectedText: string
|
||||
selectionFilePath: string
|
||||
selectionStartLine: number
|
||||
selectionEndLine: number
|
||||
}
|
||||
}
|
||||
|
||||
export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
||||
|
|
@ -155,7 +161,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
|
||||
todoList?: TodoItem[]
|
||||
|
||||
selectionContext?: {
|
||||
// Temporary storage for selection context during message processing
|
||||
// This is cleared after being used in getEnvironmentDetails
|
||||
private _currentSelectionContext?: {
|
||||
selectedText: string
|
||||
selectionFilePath: string
|
||||
selectionStartLine: number
|
||||
|
|
@ -326,6 +334,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
onCreated,
|
||||
initialTodos,
|
||||
workspacePath,
|
||||
selectionContext,
|
||||
}: TaskOptions) {
|
||||
super()
|
||||
|
||||
|
|
@ -447,7 +456,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
|
||||
if (startTask) {
|
||||
if (task || images) {
|
||||
this.startTask(task, images)
|
||||
this.startTask(task, images, selectionContext)
|
||||
} else if (historyItem) {
|
||||
this.resumeTaskFromHistory()
|
||||
} else {
|
||||
|
|
@ -456,6 +465,23 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get and clear the current selection context.
|
||||
* This ensures selection context is only used once and doesn't persist.
|
||||
*/
|
||||
public getAndClearSelectionContext():
|
||||
| {
|
||||
selectedText: string
|
||||
selectionFilePath: string
|
||||
selectionStartLine: number
|
||||
selectionEndLine: number
|
||||
}
|
||||
| undefined {
|
||||
const context = this._currentSelectionContext
|
||||
this._currentSelectionContext = undefined
|
||||
return context
|
||||
}
|
||||
|
||||
/**
|
||||
* Initialize the task mode from the provider state.
|
||||
* This method handles async initialization with proper error handling.
|
||||
|
|
@ -971,11 +997,24 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
return result
|
||||
}
|
||||
|
||||
handleWebviewAskResponse(askResponse: ClineAskResponse, text?: string, images?: string[]) {
|
||||
handleWebviewAskResponse(
|
||||
askResponse: ClineAskResponse,
|
||||
text?: string,
|
||||
images?: string[],
|
||||
selectionContext?: {
|
||||
selectedText: string
|
||||
selectionFilePath: string
|
||||
selectionStartLine: number
|
||||
selectionEndLine: number
|
||||
},
|
||||
) {
|
||||
this.askResponse = askResponse
|
||||
this.askResponseText = text
|
||||
this.askResponseImages = images
|
||||
|
||||
// Store selection context temporarily for use in the next getEnvironmentDetails call
|
||||
this._currentSelectionContext = selectionContext
|
||||
|
||||
// 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.
|
||||
// Suppress the checkpoint_saved chat row for this particular checkpoint to keep the timeline clean.
|
||||
|
|
@ -1251,7 +1290,16 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
// Lifecycle
|
||||
// Start / Resume / Abort / Dispose
|
||||
|
||||
private async startTask(task?: string, images?: string[]): Promise<void> {
|
||||
private async startTask(
|
||||
task?: string,
|
||||
images?: string[],
|
||||
selectionContext?: {
|
||||
selectedText: string
|
||||
selectionFilePath: string
|
||||
selectionStartLine: number
|
||||
selectionEndLine: number
|
||||
},
|
||||
): Promise<void> {
|
||||
if (this.enableBridge) {
|
||||
try {
|
||||
await BridgeOrchestrator.subscribeToTask(this)
|
||||
|
|
@ -1271,6 +1319,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
this.clineMessages = []
|
||||
this.apiConversationHistory = []
|
||||
|
||||
// Store selection context temporarily for use in the first getEnvironmentDetails call
|
||||
this._currentSelectionContext = selectionContext
|
||||
|
||||
// The todo list is already set in the constructor if initialTodos were provided
|
||||
// No need to add any messages - the todoList property is already set
|
||||
|
||||
|
|
|
|||
|
|
@ -450,7 +450,7 @@ export const webviewMessageHandler = async (
|
|||
const startLine = selection.start.line + 1
|
||||
const endLine = selection.end.line + 1
|
||||
|
||||
// Send selection context to webview
|
||||
// Send selection context to webview only - don't store in task
|
||||
await provider.postMessageToWebview({
|
||||
type: "selectionContext",
|
||||
selectedText,
|
||||
|
|
@ -458,28 +458,11 @@ export const webviewMessageHandler = async (
|
|||
selectionStartLine: startLine,
|
||||
selectionEndLine: endLine,
|
||||
})
|
||||
|
||||
// Store selection context in current task for use in environment details
|
||||
const currentTask = provider.getCurrentTask()
|
||||
if (currentTask) {
|
||||
currentTask.selectionContext = {
|
||||
selectedText,
|
||||
selectionFilePath: relativeFilePath,
|
||||
selectionStartLine: startLine,
|
||||
selectionEndLine: endLine,
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// No selection, send empty context
|
||||
await provider.postMessageToWebview({
|
||||
type: "selectionContext",
|
||||
})
|
||||
|
||||
// Clear selection context in current task
|
||||
const currentTask = provider.getCurrentTask()
|
||||
if (currentTask) {
|
||||
currentTask.selectionContext = undefined
|
||||
}
|
||||
}
|
||||
break
|
||||
}
|
||||
|
|
@ -562,24 +545,20 @@ 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 {
|
||||
await provider.createTask(message.text, message.images)
|
||||
|
||||
// Store selection context in the newly created task
|
||||
const newTask = provider.getCurrentTask()
|
||||
if (
|
||||
newTask &&
|
||||
message.selectedText &&
|
||||
message.selectionFilePath &&
|
||||
typeof message.selectionStartLine === "number" &&
|
||||
typeof message.selectionEndLine === "number"
|
||||
) {
|
||||
newTask.selectionContext = {
|
||||
selectedText: message.selectedText,
|
||||
selectionFilePath: message.selectionFilePath,
|
||||
selectionStartLine: message.selectionStartLine,
|
||||
selectionEndLine: message.selectionEndLine,
|
||||
}
|
||||
}
|
||||
await provider.createTask(message.text, message.images, undefined, {
|
||||
selectionContext:
|
||||
message.selectedText &&
|
||||
message.selectionFilePath &&
|
||||
typeof message.selectionStartLine === "number" &&
|
||||
typeof message.selectionEndLine === "number"
|
||||
? {
|
||||
selectedText: message.selectedText,
|
||||
selectionFilePath: message.selectionFilePath,
|
||||
selectionStartLine: message.selectionStartLine,
|
||||
selectionEndLine: message.selectionEndLine,
|
||||
}
|
||||
: undefined,
|
||||
})
|
||||
|
||||
// Task created successfully - notify the UI to reset
|
||||
await provider.postMessageToWebview({ type: "invoke", invoke: "newChat" })
|
||||
|
|
@ -597,23 +576,21 @@ export const webviewMessageHandler = async (
|
|||
break
|
||||
|
||||
case "askResponse": {
|
||||
// Store selection context in current task before handling response
|
||||
const task = provider.getCurrentTask()
|
||||
if (
|
||||
task &&
|
||||
// Pass selection context through to handleWebviewAskResponse
|
||||
const selectionContext =
|
||||
message.selectedText &&
|
||||
message.selectionFilePath &&
|
||||
typeof message.selectionStartLine === "number" &&
|
||||
typeof message.selectionEndLine === "number"
|
||||
) {
|
||||
task.selectionContext = {
|
||||
selectedText: message.selectedText,
|
||||
selectionFilePath: message.selectionFilePath,
|
||||
selectionStartLine: message.selectionStartLine,
|
||||
selectionEndLine: message.selectionEndLine,
|
||||
}
|
||||
}
|
||||
task?.handleWebviewAskResponse(message.askResponse!, message.text, message.images)
|
||||
? {
|
||||
selectedText: message.selectedText,
|
||||
selectionFilePath: message.selectionFilePath,
|
||||
selectionStartLine: message.selectionStartLine,
|
||||
selectionEndLine: message.selectionEndLine,
|
||||
}
|
||||
: undefined
|
||||
task?.handleWebviewAskResponse(message.askResponse!, message.text, message.images, selectionContext)
|
||||
break
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -568,6 +568,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
|
|||
// Mark that user has responded - this prevents any pending auto-approvals.
|
||||
userRespondedRef.current = true
|
||||
|
||||
// Request fresh selection context before sending
|
||||
vscode.postMessage({ type: "requestSelectionContext" })
|
||||
|
||||
if (messagesRef.current.length === 0) {
|
||||
vscode.postMessage({
|
||||
type: "newTask",
|
||||
|
|
@ -838,8 +841,6 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
|
|||
// NOTE: the VSCode window needs to be focused for this to work.
|
||||
useMount(() => {
|
||||
textAreaRef.current?.focus()
|
||||
// Request initial selection context when component mounts
|
||||
vscode.postMessage({ type: "requestSelectionContext" })
|
||||
})
|
||||
|
||||
const visibleMessages = useMemo(() => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue