diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 2f1222f8ee..f440c9f037 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -473,6 +473,7 @@ export interface WebviewMessage { | "getListApiConfiguration" | "customInstructions" | "webviewDidLaunch" + | "webviewDidFocus" | "newTask" | "askResponse" | "terminalOperation" diff --git a/src/__tests__/api-subtask.spec.ts b/src/__tests__/api-subtask.spec.ts index 2b5a1a86e9..eb0c6177d4 100644 --- a/src/__tests__/api-subtask.spec.ts +++ b/src/__tests__/api-subtask.spec.ts @@ -29,6 +29,7 @@ vi.mock("@roo-code/ipc", () => ({ vi.mock("../services/command/commands", () => ({ getCommands: vi.fn().mockResolvedValue([]) })) import { API } from "../extension/api" +import { makeClineProviderFactory } from "../test-utils/provider" function makeProviderMock() { const emitter = new EventEmitter() @@ -63,7 +64,7 @@ describe("API.clearCurrentTask()", () => { beforeEach(() => { vi.clearAllMocks() provider = makeProviderMock() - api = new API({} as any, provider as any) + api = new API({} as any, provider as any, makeClineProviderFactory()) }) it("calls evictCurrentTask then postStateToWebview on sidebarProvider", async () => { @@ -84,7 +85,7 @@ describe("API.abandonSubtask()", () => { beforeEach(() => { vi.clearAllMocks() provider = makeProviderMock() - api = new API({} as any, provider as any) + api = new API({} as any, provider as any, makeClineProviderFactory()) }) it("delegates to sidebarProvider.abandonSubtask and returns its result", async () => { diff --git a/src/__tests__/extension.spec.ts b/src/__tests__/extension.spec.ts index d0a8b274b8..889f780c9d 100644 --- a/src/__tests__/extension.spec.ts +++ b/src/__tests__/extension.spec.ts @@ -1,8 +1,12 @@ // npx vitest run __tests__/extension.spec.ts import type * as vscode from "vscode" +import type { WebviewMessage } from "@roo-code/types" + +import { makeCompositeDisposable, makeEventEmitter, makeExtensionContext } from "../test-utils/vscode" vi.mock("vscode", () => ({ + Disposable: { from: (...subscriptions: vscode.Disposable[]) => makeCompositeDisposable(...subscriptions) }, window: { createOutputChannel: vi.fn().mockReturnValue({ appendLine: vi.fn(), @@ -166,6 +170,8 @@ vi.mock("../extension/api", () => ({ }), })) +vi.mock("../activate/registerCommands", () => ({ openClineInNewTab: vi.fn() })) + vi.mock("../activate", () => ({ handleUri: vi.fn(), registerCommands: vi.fn(), @@ -239,6 +245,103 @@ describe("extension.ts", () => { settingsUpdatedHandler = undefined }) + test("shares the extension-owned focus tracker with the sidebar and code actions and disposes its listeners", async () => { + vi.resetModules() + const { WebviewFocusTracker } = await import("../core/webview/WebviewFocusTracker") + const { ClineProvider } = await import("../core/webview/ClineProvider") + const { ContextProxy } = await import("../core/config/ContextProxy") + const { registerCodeActions } = await import("../activate") + const vscode = await import("vscode") + const { activate } = await import("../extension") + + await activate(mockContext) + + const trackers = mockContext.subscriptions.filter((entry) => entry instanceof WebviewFocusTracker) + expect(trackers).toHaveLength(1) + const tracker = trackers[0] + const outputChannel = vi.mocked(vscode.window.createOutputChannel).mock.results[0].value + expect(ClineProvider).toHaveBeenCalledExactlyOnceWith( + mockContext, + outputChannel, + "sidebar", + await ContextProxy.getInstance(mockContext), + tracker, + null, + ) + expect(registerCodeActions).toHaveBeenCalledExactlyOnceWith(mockContext, tracker) + + const messages = makeEventEmitter() + const disposed = makeEventEmitter() + const provider = ClineProvider.getVisibleInstance()! + tracker.init(provider, { webview: { onDidReceiveMessage: messages.event }, onDidDispose: disposed.event }) + messages.fire({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBe(provider) + + // Dispose the exact instance owned by the extension, rather than a separate test tracker. + tracker.dispose() + messages.fire({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBeUndefined() + }) + + test("keeps focus trackers independent across extension activations", async () => { + vi.resetModules() + const { WebviewFocusTracker } = await import("../core/webview/WebviewFocusTracker") + const { registerCodeActions } = await import("../activate") + const { activate } = await import("../extension") + const secretChanges = makeEventEmitter() + const secondContext = makeExtensionContext({ + secrets: { + get: vi.fn().mockResolvedValue(undefined), + store: vi.fn().mockResolvedValue(undefined), + delete: vi.fn().mockResolvedValue(undefined), + onDidChange: secretChanges.event, + }, + }) + + await activate(mockContext) + await activate(secondContext) + + const firstTracker = mockContext.subscriptions.find((entry) => entry instanceof WebviewFocusTracker) + const secondTracker = secondContext.subscriptions.find((entry) => entry instanceof WebviewFocusTracker) + expect(firstTracker).toBeInstanceOf(WebviewFocusTracker) + expect(secondTracker).toBeInstanceOf(WebviewFocusTracker) + expect(secondTracker).not.toBe(firstTracker) + expect(registerCodeActions).toHaveBeenNthCalledWith(1, mockContext, firstTracker) + expect(registerCodeActions).toHaveBeenNthCalledWith(2, secondContext, secondTracker) + firstTracker?.dispose() + secondTracker?.dispose() + secretChanges.dispose() + }) + + test("injects a factory instance with this activation's dependencies without reading the sidebar tracker", async () => { + vi.resetModules() + const { API } = await import("../extension/api") + const { openClineInNewTab } = await import("../activate/registerCommands") + const { ClineProviderFactory } = await import("../core/webview/ClineProviderFactory") + const { WebviewFocusTracker } = await import("../core/webview/WebviewFocusTracker") + const { ClineProvider } = await import("../core/webview/ClineProvider") + const { activate } = await import("../extension") + const vscode = await import("vscode") + await activate(mockContext) + + const tracker = mockContext.subscriptions.find((entry) => entry instanceof WebviewFocusTracker) + const outputChannel = vi.mocked(vscode.window.createOutputChannel).mock.results[0].value + const sidebar = vi.mocked(ClineProvider).mock.results[0].value + const providerFactory = vi.mocked(API).mock.calls[0][2] + expect(providerFactory).toBeInstanceOf(ClineProviderFactory) + expect(sidebar.webviewFocusTracker).toBeUndefined() + expect(openClineInNewTab).not.toHaveBeenCalled() + vi.mocked(openClineInNewTab).mockResolvedValueOnce(sidebar) + + await expect(providerFactory.createInNewTab()).resolves.toBe(sidebar) + expect(openClineInNewTab).toHaveBeenCalledExactlyOnceWith({ + context: mockContext, + outputChannel, + webviewFocusTracker: tracker, + }) + tracker?.dispose() + }) + test("initializes the code index scope and registers it for extension cleanup", async () => { vi.resetModules() const { CodeIndexScope } = await import("../services/code-index/code-index-scope") diff --git a/src/__tests__/single-open-invariant.spec.ts b/src/__tests__/single-open-invariant.spec.ts index af1631df9c..13fc4bc6fb 100644 --- a/src/__tests__/single-open-invariant.spec.ts +++ b/src/__tests__/single-open-invariant.spec.ts @@ -9,6 +9,7 @@ import { type Task } from "../core/task/Task" import { API } from "../extension/api" import * as ProfileValidatorMod from "../shared/ProfileValidator" import { providerIdentifiers } from "@roo-code/types/provider-identifiers" +import { makeClineProviderFactory } from "../test-utils/provider" type PrivateClineProviderMethods = { createTask: ( @@ -406,7 +407,7 @@ describe("Single-open-task invariant", () => { } as unknown as ClineProvider const output = { appendLine: vi.fn() } as unknown as OutputChannel - const api = new API(output, provider, undefined, false) + const api = new API(output, provider, makeClineProviderFactory(), undefined, false) const taskId = await api.startNewTask({ configuration: {}, diff --git a/src/activate/__tests__/handleTask.spec.ts b/src/activate/__tests__/handleTask.spec.ts new file mode 100644 index 0000000000..a3da94e0a8 --- /dev/null +++ b/src/activate/__tests__/handleTask.spec.ts @@ -0,0 +1,120 @@ +import * as vscode from "vscode" + +import { handleNewTask } from "../handleTask" +import { ClineProvider } from "../../core/webview/ClineProvider" +import { WebviewFocusTracker } from "../../core/webview/WebviewFocusTracker" +import { Package } from "../../shared/package" + +vi.mock("vscode", () => ({ + window: { showInputBox: vi.fn() }, + commands: { executeCommand: vi.fn().mockResolvedValue(undefined) }, +})) +vi.mock("../../core/webview/ClineProvider", () => ({ + ClineProvider: class { + static getInstance = vi.fn() + get isViewVisible() { + return true + } + }, +})) +vi.mock("../../i18n", () => ({ t: (key: string) => key })) + +describe("handleNewTask", () => { + let provider: ClineProvider + let tracker: WebviewFocusTracker + + beforeEach(() => { + vi.clearAllMocks() + tracker = new WebviewFocusTracker() + provider = Object.create(ClineProvider.prototype) as ClineProvider + provider.handleCodeAction = vi.fn().mockResolvedValue(undefined) + vi.mocked(ClineProvider.getInstance).mockResolvedValue(provider) + vi.mocked(vscode.window.showInputBox).mockResolvedValue(undefined) + }) + + it("executes the supplied prompt on the resolved provider", async () => { + await handleNewTask({ prompt: "new task" }, tracker) + expect(ClineProvider.getInstance).toHaveBeenCalledOnce() + expect(vscode.window.showInputBox).not.toHaveBeenCalled() + expect(provider.handleCodeAction).toHaveBeenCalledWith("newTask", "NEW_TASK", { userInput: "new task" }) + }) + + it("executes the prompt entered in the input dialog", async () => { + vi.mocked(vscode.window.showInputBox).mockResolvedValue("entered task") + await handleNewTask(undefined, tracker) + expect(provider.handleCodeAction).toHaveBeenCalledWith("newTask", "NEW_TASK", { userInput: "entered task" }) + }) + + it("focuses the sidebar without executing an action when input is cancelled", async () => { + const getLastActive = vi.spyOn(tracker, "getLastActiveProvider") + await handleNewTask(undefined, tracker) + expect(vscode.commands.executeCommand).toHaveBeenCalledWith(`${Package.name}.SidebarProvider.focus`) + expect(ClineProvider.getInstance).not.toHaveBeenCalled() + expect(getLastActive).not.toHaveBeenCalled() + expect(provider.handleCodeAction).not.toHaveBeenCalled() + }) + + it("does nothing when no provider can be resolved", async () => { + vi.mocked(ClineProvider.getInstance).mockResolvedValue(undefined) + await handleNewTask({ prompt: "new task" }, tracker) + expect(provider.handleCodeAction).not.toHaveBeenCalled() + }) + + it("awaits the provider action and propagates its error", async () => { + const error = new Error("Task creation failed") + vi.mocked(provider.handleCodeAction).mockRejectedValue(error) + await expect(handleNewTask({ prompt: "new task" }, tracker)).rejects.toBe(error) + }) + + it.each(["supplied", "dialog"])( + "uses the last active visible chat for a %s prompt without resolving another provider", + async (source) => { + vi.spyOn(tracker, "getLastActiveProvider").mockReturnValue(provider) + const fallback = Object.create(ClineProvider.prototype) as ClineProvider + fallback.handleCodeAction = vi.fn().mockResolvedValue(undefined) + vi.mocked(ClineProvider.getInstance).mockResolvedValue(fallback) + vi.mocked(vscode.window.showInputBox).mockResolvedValue("new task") + await handleNewTask(source === "supplied" ? { prompt: "new task" } : undefined, tracker) + expect(provider.handleCodeAction).toHaveBeenCalledWith("newTask", "NEW_TASK", { userInput: "new task" }) + expect(ClineProvider.getInstance).not.toHaveBeenCalled() + expect(fallback.handleCodeAction).not.toHaveBeenCalled() + }, + ) + + it.each(["supplied", "dialog"])( + "uses the fallback for a %s prompt when the last active chat is hidden", + async (source) => { + vi.spyOn(provider, "isViewVisible", "get").mockReturnValue(false) + vi.spyOn(tracker, "getLastActiveProvider").mockReturnValue(provider) + const fallback = Object.create(ClineProvider.prototype) as ClineProvider + fallback.handleCodeAction = vi.fn().mockResolvedValue(undefined) + vi.mocked(ClineProvider.getInstance).mockResolvedValue(fallback) + vi.mocked(vscode.window.showInputBox).mockResolvedValue("new task") + + await handleNewTask(source === "supplied" ? { prompt: "new task" } : undefined, tracker) + + expect(ClineProvider.getInstance).toHaveBeenCalledOnce() + expect(provider.handleCodeAction).not.toHaveBeenCalled() + expect(fallback.handleCodeAction).toHaveBeenCalledExactlyOnceWith("newTask", "NEW_TASK", { + userInput: "new task", + }) + }, + ) + + it.each(["supplied", "dialog"])( + "does not execute a %s prompt on a hidden chat when fallback finds no provider", + async (source) => { + vi.spyOn(provider, "isViewVisible", "get").mockReturnValue(false) + vi.spyOn(tracker, "getLastActiveProvider").mockReturnValue(provider) + vi.mocked(ClineProvider.getInstance).mockResolvedValue(undefined) + vi.mocked(vscode.window.showInputBox).mockResolvedValue("new task") + + await expect( + handleNewTask(source === "supplied" ? { prompt: "new task" } : undefined, tracker), + ).resolves.toBeUndefined() + + expect(ClineProvider.getInstance).toHaveBeenCalledOnce() + expect(provider.handleCodeAction).not.toHaveBeenCalled() + }, + ) +}) diff --git a/src/activate/__tests__/registerCodeActions.spec.ts b/src/activate/__tests__/registerCodeActions.spec.ts new file mode 100644 index 0000000000..6fd9b4efc5 --- /dev/null +++ b/src/activate/__tests__/registerCodeActions.spec.ts @@ -0,0 +1,163 @@ +import * as vscode from "vscode" + +import { registerCodeActions } from "../registerCodeActions" +import { ClineProvider } from "../../core/webview/ClineProvider" +import { WebviewFocusTracker } from "../../core/webview/WebviewFocusTracker" +import { getCodeActionCommand } from "../../utils/commands" +import { makeExtensionContext } from "../../test-utils/vscode" +import { EditorUtils } from "../../integrations/editor/EditorUtils" + +const codeActions = [ + ["addToContext", "ADD_TO_CONTEXT"], + ["explainCode", "EXPLAIN"], + ["fixCode", "FIX"], + ["improveCode", "IMPROVE"], +] as const + +vi.mock("vscode", () => ({ commands: { registerCommand: vi.fn() } })) +vi.mock("../../core/webview/ClineProvider", () => ({ + ClineProvider: class { + static getInstance = vi.fn() + get isViewVisible() { + return true + } + }, +})) +vi.mock("../../integrations/editor/EditorUtils", () => ({ EditorUtils: { getEditorContext: vi.fn() } })) + +describe("registerCodeActions destination", () => { + let tracker: WebviewFocusTracker + let handlers: Map unknown> + + function createProvider() { + const provider = Object.create(ClineProvider.prototype) as ClineProvider + provider.handleCodeAction = vi.fn().mockResolvedValue(undefined) + return provider + } + + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(ClineProvider.getInstance).mockResolvedValue(undefined) + tracker = new WebviewFocusTracker() + handlers = new Map() + vi.mocked(vscode.commands.registerCommand).mockImplementation((command, handler) => { + handlers.set(command, handler) + return { dispose: vi.fn() } + }) + registerCodeActions(makeExtensionContext(), tracker) + }) + + it.each(codeActions)("executes %s directly on the last active visible chat", async (command, promptType) => { + const provider = createProvider() + const getLastActive = vi.spyOn(tracker, "getLastActiveProvider").mockReturnValue(provider) + await handlers.get(getCodeActionCommand(command))!("file.ts", "selected code", 1, 2) + + expect(getLastActive).toHaveBeenCalledOnce() + expect(ClineProvider.getInstance).not.toHaveBeenCalled() + expect(provider.handleCodeAction).toHaveBeenCalledOnce() + expect(provider.handleCodeAction).toHaveBeenCalledWith(command, promptType, { + filePath: "file.ts", + selectedText: "selected code", + startLine: "1", + endLine: "2", + }) + }) + + it.each(codeActions)("preserves fallback for %s when no chat has been focused", async (command, promptType) => { + const provider = createProvider() + vi.mocked(ClineProvider.getInstance).mockResolvedValue(provider) + await handlers.get(getCodeActionCommand(command))!("file.ts", "selected code") + expect(ClineProvider.getInstance).toHaveBeenCalledOnce() + expect(provider.handleCodeAction).toHaveBeenCalledOnce() + expect(provider.handleCodeAction).toHaveBeenCalledWith(command, promptType, { + filePath: "file.ts", + selectedText: "selected code", + }) + }) + + it.each(codeActions)("uses the last active chat for %s from the command palette", async (command, promptType) => { + const provider = createProvider() + vi.spyOn(tracker, "getLastActiveProvider").mockReturnValue(provider) + vi.mocked(EditorUtils.getEditorContext).mockReturnValue({ + filePath: "file.ts", + selectedText: "selected code", + startLine: 1, + endLine: 2, + diagnostics: [], + }) + await handlers.get(getCodeActionCommand(command))!() + expect(ClineProvider.getInstance).not.toHaveBeenCalled() + expect(provider.handleCodeAction).toHaveBeenCalledWith(command, promptType, { + filePath: "file.ts", + selectedText: "selected code", + startLine: "1", + endLine: "2", + diagnostics: [], + }) + }) + + describe.each(["code action", "command palette"])("hidden destination from %s", (source) => { + beforeEach(() => { + vi.mocked(EditorUtils.getEditorContext).mockReturnValue({ + filePath: "file.ts", + selectedText: "selected code", + startLine: 1, + endLine: 2, + diagnostics: [], + }) + }) + + it.each(codeActions)("uses the fallback for %s instead of the hidden chat", async (command, promptType) => { + const hidden = createProvider() + vi.spyOn(hidden, "isViewVisible", "get").mockReturnValue(false) + vi.spyOn(tracker, "getLastActiveProvider").mockReturnValue(hidden) + const fallback = createProvider() + vi.mocked(ClineProvider.getInstance).mockResolvedValue(fallback) + const args = source === "code action" ? ["file.ts", "selected code", 1, 2] : [] + + await handlers.get(getCodeActionCommand(command))!(...args) + + expect(ClineProvider.getInstance).toHaveBeenCalledOnce() + expect(hidden.handleCodeAction).not.toHaveBeenCalled() + expect(fallback.handleCodeAction).toHaveBeenCalledExactlyOnceWith( + command, + promptType, + expect.objectContaining({ + filePath: "file.ts", + selectedText: "selected code", + startLine: "1", + endLine: "2", + }), + ) + }) + + it.each(codeActions)( + "does not execute %s on a hidden chat when fallback finds no provider", + async (command) => { + const hidden = createProvider() + vi.spyOn(hidden, "isViewVisible", "get").mockReturnValue(false) + vi.spyOn(tracker, "getLastActiveProvider").mockReturnValue(hidden) + const args = source === "code action" ? ["file.ts", "selected code"] : [] + + await expect(handlers.get(getCodeActionCommand(command))!(...args)).resolves.toBeUndefined() + + expect(ClineProvider.getInstance).toHaveBeenCalledOnce() + expect(hidden.handleCodeAction).not.toHaveBeenCalled() + }, + ) + }) + + it.each(codeActions)("does nothing for %s when fallback cannot find a provider", async (command) => { + await expect(handlers.get(getCodeActionCommand(command))!("file.ts", "selected code")).resolves.toBeUndefined() + expect(ClineProvider.getInstance).toHaveBeenCalledOnce() + }) + + it("awaits execution on the selected provider and propagates its error", async () => { + const provider = createProvider() + const error = new Error("Code action failed") + vi.mocked(provider.handleCodeAction).mockRejectedValue(error) + vi.spyOn(tracker, "getLastActiveProvider").mockReturnValue(provider) + await expect(handlers.get(getCodeActionCommand("fixCode"))!("file.ts", "selected code")).rejects.toBe(error) + expect(ClineProvider.getInstance).not.toHaveBeenCalled() + }) +}) diff --git a/src/activate/__tests__/registerCommands.spec.ts b/src/activate/__tests__/registerCommands.spec.ts index 7088560700..d041dada2e 100644 --- a/src/activate/__tests__/registerCommands.spec.ts +++ b/src/activate/__tests__/registerCommands.spec.ts @@ -1,6 +1,9 @@ import type { Mock } from "vitest" import * as vscode from "vscode" import { ClineProvider } from "../../core/webview/ClineProvider" +import { WebviewFocusTracker } from "../../core/webview/WebviewFocusTracker" +import { ContextProxy } from "../../core/config/ContextProxy" +import { makeExtensionContext } from "../../test-utils/vscode" import { getVisibleProviderOrLog, openClineInNewTab, registerCommands, setPanel } from "../registerCommands" @@ -134,7 +137,7 @@ describe("registerCommands handlers", () => { let mockOutputChannel: vscode.OutputChannel let mockContext: vscode.ExtensionContext let mockVisibleProvider: { postMessageToWebview: Mock } - let mockProvider: { postMessageToWebview: Mock } + let mockProvider: { postMessageToWebview: Mock; webviewFocusTracker: WebviewFocusTracker } let handlers: Record unknown> beforeEach(() => { @@ -152,9 +155,7 @@ describe("registerCommands handlers", () => { dispose: vi.fn(), } - mockContext = { - subscriptions: [], - } as unknown as vscode.ExtensionContext + mockContext = makeExtensionContext() mockVisibleProvider = { postMessageToWebview: vi.fn().mockResolvedValue(undefined), @@ -162,6 +163,7 @@ describe("registerCommands handlers", () => { mockProvider = { postMessageToWebview: vi.fn().mockResolvedValue(undefined), + webviewFocusTracker: new WebviewFocusTracker(), } ;(ClineProvider.getVisibleInstance as Mock).mockReturnValue(mockVisibleProvider) ;(vscode.commands.registerCommand as Mock).mockImplementation( @@ -192,6 +194,54 @@ describe("registerCommands handlers", () => { expect(mockContext.subscriptions).toContain(disposable) }) + it("passes the shared focus tracker to the newTask handler", async () => { + const { handleNewTask } = await import("../handleTask") + const params = { prompt: "new task" } + await handlers["zoo-code.newTask"](params) + expect(handleNewTask).toHaveBeenCalledWith(params, mockProvider.webviewFocusTracker) + }) + + it.each(["popoutButtonClicked", "openInNewTab"])( + "%s opens an editor chat using the registered provider's focus tracker", + async (command) => { + const panel: vscode.WebviewPanel = { + viewType: "zoo-code.TabPanelProvider", + title: "Zoo Code", + webview: { + options: {}, + html: "", + cspSource: "test-webview", + postMessage: vi.fn().mockResolvedValue(true), + onDidReceiveMessage: vi.fn(), + asWebviewUri: vi.fn((uri) => uri), + }, + options: {}, + viewColumn: vscode.ViewColumn.Two, + active: true, + visible: true, + onDidChangeViewState: vi.fn(), + onDidDispose: vi.fn(), + reveal: vi.fn(), + dispose: vi.fn(), + } + vi.mocked(vscode.window.createWebviewPanel).mockReturnValue(panel) + + const result = await handlers[`zoo-code.${command}`]() + + expect(ClineProvider).toHaveBeenCalledExactlyOnceWith( + mockContext, + mockOutputChannel, + "editor", + await ContextProxy.getInstance(mockContext), + mockProvider.webviewFocusTracker, + undefined, + ) + const tabProvider = vi.mocked(ClineProvider).mock.results[0].value + expect(result).toBe(tabProvider) + expect(tabProvider.resolveWebviewView).toHaveBeenCalledExactlyOnceWith(panel) + }, + ) + it("settingsButtonClicked posts both settingsButtonClicked and didBecomeVisible actions", () => { handlers["zoo-code.settingsButtonClicked"]() @@ -412,7 +462,16 @@ describe("openClineInNewTab", () => { }) it("creates a webview panel with title 'Zoo Code'", async () => { - await openClineInNewTab({ context: mockContext, outputChannel: mockOutputChannel }) + const webviewFocusTracker = new WebviewFocusTracker() + await openClineInNewTab({ context: mockContext, outputChannel: mockOutputChannel, webviewFocusTracker }) + expect(ClineProvider).toHaveBeenCalledWith( + mockContext, + mockOutputChannel, + "editor", + await ContextProxy.getInstance(mockContext), + webviewFocusTracker, + undefined, + ) expect(vscode.window.createWebviewPanel).toHaveBeenCalledWith( "zoo-code.TabPanelProvider", diff --git a/src/activate/handleTask.ts b/src/activate/handleTask.ts index bc2aed4beb..4493b1bada 100644 --- a/src/activate/handleTask.ts +++ b/src/activate/handleTask.ts @@ -1,10 +1,14 @@ import * as vscode from "vscode" import { Package } from "../shared/package" -import { ClineProvider } from "../core/webview/ClineProvider" +import type { WebviewFocusTracker } from "../core/webview/WebviewFocusTracker" import { t } from "../i18n" +import { resolveChatProvider } from "./resolveChatProvider" -export const handleNewTask = async (params: { prompt?: string } | null | undefined) => { +export const handleNewTask = async ( + params: { prompt?: string } | null | undefined, + webviewFocusTracker: WebviewFocusTracker, +) => { let prompt = params?.prompt if (!prompt) { @@ -19,5 +23,6 @@ export const handleNewTask = async (params: { prompt?: string } | null | undefin return } - await ClineProvider.handleCodeAction("newTask", "NEW_TASK", { userInput: prompt }) + const provider = await resolveChatProvider(webviewFocusTracker) + await provider?.handleCodeAction("newTask", "NEW_TASK", { userInput: prompt }) } diff --git a/src/activate/registerCodeActions.ts b/src/activate/registerCodeActions.ts index 6c0a65b9e0..66fc4c5582 100644 --- a/src/activate/registerCodeActions.ts +++ b/src/activate/registerCodeActions.ts @@ -4,16 +4,22 @@ import { CodeActionId, CodeActionName } from "@roo-code/types" import { getCodeActionCommand } from "../utils/commands" import { EditorUtils } from "../integrations/editor/EditorUtils" -import { ClineProvider } from "../core/webview/ClineProvider" - -export const registerCodeActions = (context: vscode.ExtensionContext) => { - registerCodeAction(context, "explainCode", "EXPLAIN") - registerCodeAction(context, "fixCode", "FIX") - registerCodeAction(context, "improveCode", "IMPROVE") - registerCodeAction(context, "addToContext", "ADD_TO_CONTEXT") +import type { WebviewFocusTracker } from "../core/webview/WebviewFocusTracker" +import { resolveChatProvider } from "./resolveChatProvider" + +export const registerCodeActions = (context: vscode.ExtensionContext, webviewFocusTracker: WebviewFocusTracker) => { + registerCodeAction(context, "explainCode", "EXPLAIN", webviewFocusTracker) + registerCodeAction(context, "fixCode", "FIX", webviewFocusTracker) + registerCodeAction(context, "improveCode", "IMPROVE", webviewFocusTracker) + registerCodeAction(context, "addToContext", "ADD_TO_CONTEXT", webviewFocusTracker) } -const registerCodeAction = (context: vscode.ExtensionContext, command: CodeActionId, promptType: CodeActionName) => { +const registerCodeAction = ( + context: vscode.ExtensionContext, + command: CodeActionId, + promptType: CodeActionName, + webviewFocusTracker: WebviewFocusTracker, +) => { let userInput: string | undefined context.subscriptions.push( @@ -47,7 +53,9 @@ const registerCodeAction = (context: vscode.ExtensionContext, command: CodeActio ...(userInput ? { userInput } : {}), } - await ClineProvider.handleCodeAction(command, promptType, params) + // Capture the destination before focus returns from the source editor to a chat. + const targetProvider = await resolveChatProvider(webviewFocusTracker) + await targetProvider?.handleCodeAction(command, promptType, params) }), ) } diff --git a/src/activate/registerCommands.ts b/src/activate/registerCommands.ts index da98be291b..79e629f479 100644 --- a/src/activate/registerCommands.ts +++ b/src/activate/registerCommands.ts @@ -7,6 +7,7 @@ import { TelemetryService } from "@roo-code/telemetry" import { Package } from "../shared/package" import { getCommand } from "../utils/commands" import { ClineProvider } from "../core/webview/ClineProvider" +import type { WebviewFocusTracker } from "../core/webview/WebviewFocusTracker" import { ContextProxy } from "../core/config/ContextProxy" import { focusPanel } from "../utils/focusPanel" import { handleNewTask } from "./handleTask" @@ -109,9 +110,10 @@ const getCommandsMap = ({ popoutButtonClicked: () => { TelemetryService.instance.captureTitleButtonClicked("popout") - return openClineInNewTab({ context, outputChannel }) + return openClineInNewTab({ context, outputChannel, webviewFocusTracker: provider.webviewFocusTracker }) }, - openInNewTab: () => openClineInNewTab({ context, outputChannel }), + openInNewTab: () => + openClineInNewTab({ context, outputChannel, webviewFocusTracker: provider.webviewFocusTracker }), settingsButtonClicked: () => { const visibleProvider = getVisibleProviderOrLog(outputChannel) @@ -151,7 +153,7 @@ const getCommandsMap = ({ outputChannel.appendLine(`[marketplaceButtonClicked] postMessageToWebview failed: ${error}`), ) }, - newTask: handleNewTask, + newTask: (params: { prompt?: string } | null | undefined) => handleNewTask(params, provider.webviewFocusTracker), setCustomStoragePath: async () => { const { promptForCustomStoragePath } = await import("../utils/storage") await promptForCustomStoragePath() @@ -220,7 +222,13 @@ const getCommandsMap = ({ }, }) -export const openClineInNewTab = async ({ context, outputChannel }: Omit) => { +type OpenClineInNewTabOptions = { + context: vscode.ExtensionContext + outputChannel: vscode.OutputChannel + webviewFocusTracker: WebviewFocusTracker +} + +export const openClineInNewTab = async ({ context, outputChannel, webviewFocusTracker }: OpenClineInNewTabOptions) => { // (This example uses webviewProvider activation event which is necessary to // deserialize cached webview, but since we use retainContextWhenHidden, we // don't need to use that event). @@ -236,7 +244,14 @@ export const openClineInNewTab = async ({ context, outputChannel }: Omit editor.viewColumn || 0)) // Check if there are any visible text editors, otherwise open a new group diff --git a/src/activate/resolveChatProvider.ts b/src/activate/resolveChatProvider.ts new file mode 100644 index 0000000000..62b8327197 --- /dev/null +++ b/src/activate/resolveChatProvider.ts @@ -0,0 +1,10 @@ +import { ClineProvider } from "../core/webview/ClineProvider" +import type { WebviewFocusTracker } from "../core/webview/WebviewFocusTracker" + +export async function resolveChatProvider( + webviewFocusTracker: WebviewFocusTracker, +): Promise { + const provider = webviewFocusTracker.getLastActiveProvider() + // Remember chat interaction across editor focus changes, but never target a hidden chat. + return provider?.isViewVisible ? provider : ClineProvider.getInstance() +} diff --git a/src/core/task/__tests__/Task.persistence.spec.ts b/src/core/task/__tests__/Task.persistence.spec.ts index 25e97f2d44..109eee8dc0 100644 --- a/src/core/task/__tests__/Task.persistence.spec.ts +++ b/src/core/task/__tests__/Task.persistence.spec.ts @@ -1,3 +1,4 @@ +import { WebviewFocusTracker } from "../../webview/WebviewFocusTracker" // cd src && npx vitest run core/task/__tests__/Task.persistence.spec.ts import * as os from "os" @@ -285,6 +286,7 @@ describe("Task persistence", () => { mockOutputChannel, "sidebar", new ContextProxy(mockExtensionContext), + new WebviewFocusTracker(), ) as ClineProvider & Record mockApiConfig = { diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 06f07d2e02..b484d327d2 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -1,3 +1,4 @@ +import { WebviewFocusTracker } from "../../webview/WebviewFocusTracker" // npx vitest core/task/__tests__/Task.spec.ts import * as os from "os" @@ -398,6 +399,7 @@ describe("Cline", () => { mockOutputChannel, "sidebar", new ContextProxy(mockExtensionContext), + new WebviewFocusTracker(), ) // Setup mock API configuration @@ -6069,6 +6071,7 @@ describe("Queued message processing after condense", () => { output as unknown as vscode.OutputChannel, "sidebar", new ContextProxy(ctx), + new WebviewFocusTracker(), ) provider.postMessageToWebview = vi.fn().mockResolvedValue(undefined) provider.postStateToWebview = vi.fn().mockResolvedValue(undefined) @@ -6206,6 +6209,7 @@ describe("Telemetry installments (idle/shutdown flush)", () => { } as unknown as vscode.OutputChannel, "sidebar", new ContextProxy(mockExtensionContext), + new WebviewFocusTracker(), ) mockProvider.postMessageToWebview = vi.fn().mockResolvedValue(undefined) mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined) @@ -6460,6 +6464,7 @@ describe("pushToolResultToUserContent", () => { mockOutputChannel, "sidebar", new ContextProxy(mockExtensionContext), + new WebviewFocusTracker(), ) mockProvider.postMessageToWebview = vi.fn().mockResolvedValue(undefined) diff --git a/src/core/task/__tests__/flushPendingToolResultsToHistory.spec.ts b/src/core/task/__tests__/flushPendingToolResultsToHistory.spec.ts index f32a6e2507..d8f9fba5db 100644 --- a/src/core/task/__tests__/flushPendingToolResultsToHistory.spec.ts +++ b/src/core/task/__tests__/flushPendingToolResultsToHistory.spec.ts @@ -1,3 +1,4 @@ +import { WebviewFocusTracker } from "../../webview/WebviewFocusTracker" // npx vitest run core/task/__tests__/flushPendingToolResultsToHistory.spec.ts import * as os from "os" @@ -211,6 +212,7 @@ describe("flushPendingToolResultsToHistory", () => { mockOutputChannel, "sidebar", new ContextProxy(mockExtensionContext), + new WebviewFocusTracker(), ) as any mockApiConfig = { diff --git a/src/core/task/__tests__/grace-retry-errors.spec.ts b/src/core/task/__tests__/grace-retry-errors.spec.ts index 3f72924f21..dc057957bc 100644 --- a/src/core/task/__tests__/grace-retry-errors.spec.ts +++ b/src/core/task/__tests__/grace-retry-errors.spec.ts @@ -1,3 +1,4 @@ +import { WebviewFocusTracker } from "../../webview/WebviewFocusTracker" // npx vitest core/task/__tests__/grace-retry-errors.spec.ts import * as os from "os" @@ -199,6 +200,7 @@ describe("Grace Retry Error Handling", () => { mockOutputChannel, "sidebar", new ContextProxy(mockExtensionContext), + new WebviewFocusTracker(), ) as any mockApiConfig = { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 874e2f8f08..07772816b4 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -115,6 +115,7 @@ import { CustomModesManager } from "../config/CustomModesManager" 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 ApiMessage, @@ -333,6 +334,7 @@ export class ClineProvider private readonly outputChannel: vscode.OutputChannel, private readonly renderContext: "sidebar" | "editor" = "sidebar", public readonly contextProxy: ContextProxy, + public readonly webviewFocusTracker: WebviewFocusTracker, mdmService?: MdmService, ) { super() @@ -902,8 +904,12 @@ export class ClineProvider McpServerManager.unregisterProvider(this) } + public get isViewVisible(): boolean { + return this.view?.visible === true + } + public static getVisibleInstance(): ClineProvider | undefined { - return findLast(Array.from(this.activeInstances), (instance) => instance.view?.visible === true) + return findLast(Array.from(this.activeInstances), (instance) => instance.isViewVisible) } public static getAllInstances(): ClineProvider[] { @@ -944,7 +950,7 @@ export class ClineProvider return false } - public static async handleCodeAction( + public async handleCodeAction( command: CodeActionId, promptType: CodeActionName, params: Record, @@ -952,28 +958,22 @@ export class ClineProvider // Capture telemetry for code action usage TelemetryService.instance.captureCodeActionUsed(promptType) - const visibleProvider = await ClineProvider.getInstance() - - if (!visibleProvider) { - return - } - - const { customSupportPrompts } = await visibleProvider.getState() + const { customSupportPrompts } = await this.getState() // TODO: Improve type safety for promptType. const prompt = supportPrompt.create(promptType, params, customSupportPrompts) if (command === "addToContext") { - await visibleProvider.postMessageToWebview({ + await this.postMessageToWebview({ type: "invoke", invoke: "setChatBoxMessage", text: `${prompt}\n\n`, }) - await visibleProvider.postMessageToWebview({ type: "action", action: "focusInput" }) + await this.postMessageToWebview({ type: "action", action: "focusInput" }) return } - await visibleProvider.createTask(prompt) + await this.createTask(prompt) } public static async handleTerminalAction( @@ -1076,6 +1076,7 @@ export class ClineProvider // Sets up an event listener to listen for messages passed from the webview view context // and executes code based on the message that is received. this.setWebviewMessageListener(webviewView.webview) + this.webviewDisposables.push(this.webviewFocusTracker.init(this, webviewView)) // Listen for when the panel becomes visible. // https://github.com/microsoft/vscode-discussions/discussions/840 diff --git a/src/core/webview/ClineProviderFactory.ts b/src/core/webview/ClineProviderFactory.ts new file mode 100644 index 0000000000..4ed6aba814 --- /dev/null +++ b/src/core/webview/ClineProviderFactory.ts @@ -0,0 +1,21 @@ +import type * as vscode from "vscode" + +import { openClineInNewTab } from "../../activate/registerCommands" +import type { ClineProvider } from "./ClineProvider" +import type { WebviewFocusTracker } from "./WebviewFocusTracker" + +export class ClineProviderFactory { + constructor( + private readonly context: vscode.ExtensionContext, + private readonly outputChannel: vscode.OutputChannel, + private readonly webviewFocusTracker: WebviewFocusTracker, + ) {} + + public createInNewTab(): Promise { + return openClineInNewTab({ + context: this.context, + outputChannel: this.outputChannel, + webviewFocusTracker: this.webviewFocusTracker, + }) + } +} diff --git a/src/core/webview/WebviewFocusTracker.ts b/src/core/webview/WebviewFocusTracker.ts new file mode 100644 index 0000000000..d209461ee2 --- /dev/null +++ b/src/core/webview/WebviewFocusTracker.ts @@ -0,0 +1,82 @@ +import type * as vscode from "vscode" +import type { WebviewMessage } from "@roo-code/types" + +import type { ClineProvider } from "./ClineProvider" + +interface WebviewFocusSource { + readonly webview: Pick + readonly onDidDispose: vscode.Event +} + +interface TrackedWebview extends vscode.Disposable { + readonly provider: ClineProvider + subscriptions: vscode.Disposable[] +} + +export class WebviewFocusTracker implements vscode.Disposable { + private lastFocusedWebview?: TrackedWebview + private readonly trackedWebviews = new Set() + + public getLastActiveProvider(): ClineProvider | undefined { + return this.lastFocusedWebview?.provider + } + + public init(provider: ClineProvider, view: WebviewFocusSource): vscode.Disposable { + const trackedWebview: TrackedWebview = { + provider, + subscriptions: [], + dispose: () => this.untrackWebview(trackedWebview), + } + this.trackedWebviews.add(trackedWebview) + + trackedWebview.subscriptions = [ + view.webview.onDidReceiveMessage((message: WebviewMessage) => this.handleMessage(trackedWebview, message)), + view.onDidDispose(() => trackedWebview.dispose()), + ] + + // An event source may dispose the view while its listeners are being registered. + if (!this.trackedWebviews.has(trackedWebview)) { + this.disposeSubscriptions(trackedWebview) + } + + return trackedWebview + } + + public dispose() { + for (const trackedWebview of [...this.trackedWebviews]) { + trackedWebview.dispose() + } + this.lastFocusedWebview = undefined + } + + private handleMessage(trackedWebview: TrackedWebview, message: WebviewMessage): void { + if (message.type !== "webviewDidFocus" || !this.trackedWebviews.has(trackedWebview)) { + return + } + + this.lastFocusedWebview = trackedWebview + } + + private untrackWebview(trackedWebview: TrackedWebview): void { + if (!this.trackedWebviews.delete(trackedWebview)) { + return + } + + if (this.lastFocusedWebview === trackedWebview) { + this.lastFocusedWebview = undefined + } + + this.disposeSubscriptions(trackedWebview) + } + + private disposeSubscriptions(trackedWebview: TrackedWebview): void { + // Drain ownership before invoking callbacks, so failures and reentrant disposal cannot retry listeners. + for (const subscription of trackedWebview.subscriptions.splice(0)) { + try { + subscription.dispose() + } catch (error) { + console.error("[WebviewFocusTracker] Failed to dispose webview subscription:", error) + } + } + } +} diff --git a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts index 99d254cb9a..4561d688fe 100644 --- a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts @@ -1,6 +1,8 @@ +import { WebviewFocusTracker } from "../WebviewFocusTracker" // npx vitest core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts import * as vscode from "vscode" +import { makeCompositeDisposable } from "../../../test-utils/vscode" import { TelemetryService } from "@roo-code/telemetry" import { getModelId, RooCodeEventName } from "@roo-code/types" @@ -43,6 +45,7 @@ vi.mock("vscode", () => ({ ExtensionContext: vi.fn(), OutputChannel: vi.fn(), WebviewView: vi.fn(), + Disposable: { from: (...subscriptions: vscode.Disposable[]) => makeCompositeDisposable(...subscriptions) }, Uri: { joinPath: vi.fn(), file: vi.fn(), @@ -230,7 +233,13 @@ describe("ClineProvider - API Handler Rebuild Guard", () => { }), } as unknown as vscode.WebviewView - provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) // Mock providerSettingsManager ;(provider as any).providerSettingsManager = { diff --git a/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts b/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts index f0b2f46722..5625beaa54 100644 --- a/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts @@ -1,3 +1,4 @@ +import { WebviewFocusTracker } from "../WebviewFocusTracker" import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest" import * as vscode from "vscode" @@ -350,6 +351,7 @@ describe("ClineProvider flicker-free cancel", () => { mockOutputChannel, "sidebar", mockContextProxy as unknown as ContextProxy, + new WebviewFocusTracker(), ) // Mock provider methods diff --git a/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts b/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts index f42eb401f2..6472b77749 100644 --- a/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts @@ -1,6 +1,8 @@ +import { WebviewFocusTracker } from "../WebviewFocusTracker" // npx vitest run core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts import * as vscode from "vscode" +import { makeCompositeDisposable } from "../../../test-utils/vscode" import { TelemetryService } from "@roo-code/telemetry" import { ClineProvider } from "../ClineProvider" import { ContextProxy } from "../../config/ContextProxy" @@ -10,6 +12,7 @@ vi.mock("vscode", () => ({ ExtensionContext: vi.fn(), OutputChannel: vi.fn(), WebviewView: vi.fn(), + Disposable: { from: (...subscriptions: vscode.Disposable[]) => makeCompositeDisposable(...subscriptions) }, Uri: { joinPath: vi.fn(), file: vi.fn(), @@ -322,7 +325,13 @@ describe("ClineProvider - Lock API Config Across Modes", () => { }), } as unknown as vscode.WebviewView - provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) // Mock getMcpHub method provider.getMcpHub = vi.fn().mockReturnValue({ diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 57fe347acf..0e4aa4aa18 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -37,6 +37,9 @@ import { webviewMessageHandler } from "../webviewMessageHandler" import { Terminal } from "../../../integrations/terminal/Terminal" import { MessageManager } from "../../message-manager" import { forceFullModelDetailsLoad, hasLoadedFullDetails } from "../../../api/providers/fetchers/lmstudio" +import { makeCompositeDisposable, makeEventEmitter } from "../../../test-utils/vscode" +import { WebviewFocusTracker } from "../WebviewFocusTracker" +import { resolveChatProvider } from "../../../activate/resolveChatProvider" // Mock setup must come before imports. vi.mock("../../prompts/sections/custom-instructions") @@ -156,6 +159,7 @@ vi.mock("vscode", () => ({ ExtensionContext: vi.fn(), OutputChannel: vi.fn(), WebviewView: vi.fn(), + Disposable: { from: (...subscriptions: vscode.Disposable[]) => makeCompositeDisposable(...subscriptions) }, EventEmitter: vi.fn().mockImplementation(function () { return { event: vi.fn(), @@ -541,7 +545,13 @@ describe("ClineProvider", () => { }), } - provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) defaultTaskOptions = { provider, @@ -583,6 +593,364 @@ describe("ClineProvider", () => { expect(ClineProvider.getVisibleInstance()).toBe(provider) }) + test("reports an unresolved webview as not visible", () => { + expect(provider.isViewVisible).toBe(false) + }) + + describe("Add to Context destination", () => { + let originalInstances: Set + let tabProvider: ClineProvider + let sidebar: ReturnType + let tab: ReturnType + let panel: vscode.WebviewPanel + let panelState: vscode.EventEmitter + + function createView() { + const messages = makeEventEmitter() + const visibility = makeEventEmitter() + const disposed = makeEventEmitter() + const postMessage = vi.fn<(message: ExtensionMessage) => Promise>().mockResolvedValue(true) + const view: vscode.WebviewView = { + viewType: ClineProvider.sideBarId, + visible: true, + webview: { + html: "", + options: {}, + cspSource: "vscode-webview://test-csp-source", + asWebviewUri: (uri) => uri, + postMessage, + onDidReceiveMessage: messages.event, + }, + onDidChangeVisibility: visibility.event, + onDidDispose: disposed.event, + show: vi.fn(), + } + return { view, messages, visibility, disposed, postMessage } + } + + beforeEach(async () => { + originalInstances = ClineProvider["activeInstances"] + ClineProvider["activeInstances"] = new Set([provider]) + tabProvider = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + provider.webviewFocusTracker, + ) + sidebar = createView() + tab = createView() + panelState = makeEventEmitter() + panel = { + viewType: ClineProvider.tabPanelId, + title: "Zoo Code", + viewColumn: undefined, + webview: tab.view.webview, + options: {}, + active: false, + visible: true, + onDidChangeViewState: panelState.event, + onDidDispose: tab.disposed.event, + reveal: vi.fn(), + dispose: vi.fn(), + } + await provider.resolveWebviewView(sidebar.view) + await tabProvider.resolveWebviewView(panel) + sidebar.postMessage.mockClear() + tab.postMessage.mockClear() + }) + + afterEach(async () => { + await provider.dispose() + await tabProvider.dispose() + ClineProvider["activeInstances"] = originalInstances + }) + + const addSelection = async () => { + const target = await resolveChatProvider(provider.webviewFocusTracker) + await target?.handleCodeAction("addToContext", "ADD_TO_CONTEXT", { selectedText: "selected code" }) + } + + test("reports current visibility independently of panel activation", () => { + expect(provider.isViewVisible).toBe(true) + expect(tabProvider.isViewVisible).toBe(true) + expect(panel.active).toBe(false) + Object.defineProperty(sidebar.view, "visible", { value: false, configurable: true }) + Object.defineProperty(panel, "visible", { value: false, configurable: true }) + expect(provider.isViewVisible).toBe(false) + expect(tabProvider.isViewVisible).toBe(false) + Object.defineProperty(sidebar.view, "visible", { value: true, configurable: true }) + Object.defineProperty(panel, "visible", { value: true, configurable: true }) + expect(provider.isViewVisible).toBe(true) + expect(tabProvider.isViewVisible).toBe(true) + }) + + test("follows chat focus rather than registration order after returning to the source editor", async () => { + sidebar.messages.fire({ type: "webviewDidFocus" }) + // Both views stay visible while the source editor has focus. + expect(ClineProvider.getVisibleInstance()).toBe(tabProvider) + await addSelection() + expect(sidebar.postMessage).toHaveBeenCalledWith({ + type: "invoke", + invoke: "setChatBoxMessage", + text: expect.stringContaining("selected code"), + }) + expect(sidebar.postMessage).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) + expect(tab.postMessage).not.toHaveBeenCalled() + + sidebar.postMessage.mockClear() + tab.messages.fire({ type: "webviewDidFocus" }) + await addSelection() + expect(tab.postMessage).toHaveBeenCalledTimes(2) + expect(sidebar.postMessage).not.toHaveBeenCalled() + }) + + test("remembers explicit chat focus after panel deactivation and background visibility events", async () => { + tab.messages.fire({ type: "webviewDidFocus" }) + Object.defineProperty(panel, "active", { value: false, configurable: true }) + panelState.fire({ webviewPanel: panel }) + sidebar.visibility.fire() + sidebar.postMessage.mockClear() + tab.postMessage.mockClear() + await addSelection() + expect(tab.postMessage).toHaveBeenCalledTimes(2) + expect(sidebar.postMessage).not.toHaveBeenCalled() + }) + + test.each([ + { change: "activation", property: "active", value: true }, + { change: "visibility", property: "visible", value: false }, + { change: "position", property: "viewColumn", value: 2 }, + ])("panel $change does not overwrite a more recent sidebar interaction", async ({ property, value }) => { + Object.defineProperty(panel, "active", { value: property !== "active", configurable: true }) + tab.messages.fire({ type: "webviewDidFocus" }) + sidebar.messages.fire({ type: "webviewDidFocus" }) + Object.defineProperty(panel, property, { value, configurable: true }) + panelState.fire({ webviewPanel: panel }) + expect(provider.webviewFocusTracker.getLastActiveProvider()).toBe(provider) + + sidebar.postMessage.mockClear() + tab.postMessage.mockClear() + await addSelection() + expect(sidebar.postMessage).toHaveBeenCalledTimes(2) + expect(tab.postMessage).not.toHaveBeenCalled() + }) + + test("background messages reach the provider handler without changing the last focused chat", async () => { + sidebar.messages.fire({ type: "webviewDidFocus" }) + const handler = vi + .spyOn(await import("../webviewMessageHandler"), "webviewMessageHandler") + .mockResolvedValue(undefined) + try { + const message: WebviewMessage = { type: "themeFixtureProbeResponse" } + tab.messages.fire(message) + expect(handler).toHaveBeenCalledOnce() + expect(handler.mock.calls[0].slice(0, 2)).toEqual([tabProvider, message]) + expect(provider.webviewFocusTracker.getLastActiveProvider()).toBe(provider) + } finally { + handler.mockRestore() + } + await addSelection() + expect(sidebar.postMessage).toHaveBeenCalledTimes(2) + expect(tab.postMessage).not.toHaveBeenCalled() + }) + + test("waits for explicit focus even when a panel is already active during registration", () => { + const state = makeEventEmitter() + const activePanel = { ...panel, active: true, onDidChangeViewState: state.event } + const tracker = new WebviewFocusTracker() + tracker.init(tabProvider, activePanel) + expect(tracker.getLastActiveProvider()).toBeUndefined() + state.fire({ webviewPanel: activePanel }) + expect(tracker.getLastActiveProvider()).toBeUndefined() + tab.messages.fire({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBe(tabProvider) + tracker.dispose() + expect(tracker.getLastActiveProvider()).toBeUndefined() + tab.messages.fire({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBeUndefined() + }) + + test("closing a different chat does not clear the last focused chat", async () => { + tab.messages.fire({ type: "webviewDidFocus" }) + sidebar.disposed.fire() + await addSelection() + expect(tab.postMessage).toHaveBeenCalledTimes(2) + expect(sidebar.postMessage).not.toHaveBeenCalled() + }) + + test("tracks focus using only message and disposal events", () => { + const tracker = new WebviewFocusTracker() + const messages = makeEventEmitter() + const disposed = makeEventEmitter() + tracker.init(provider, { + webview: { onDidReceiveMessage: messages.event }, + onDidDispose: disposed.event, + }) + messages.fire({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBe(provider) + disposed.fire() + expect(tracker.getLastActiveProvider()).toBeUndefined() + messages.fire({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBeUndefined() + tracker.dispose() + }) + + test("tracker instances keep focus state and disposal independent", () => { + const first = new WebviewFocusTracker() + const second = new WebviewFocusTracker() + const firstView = createView() + const secondView = createView() + const firstRegistration = first.init(provider, firstView.view) + second.init(tabProvider, secondView.view) + firstView.messages.fire({ type: "webviewDidFocus" }) + secondView.messages.fire({ type: "webviewDidFocus" }) + + expect(first.getLastActiveProvider()).toBe(provider) + expect(second.getLastActiveProvider()).toBe(tabProvider) + firstRegistration.dispose() + firstRegistration.dispose() + firstView.messages.fire({ type: "webviewDidFocus" }) + expect(first.getLastActiveProvider()).toBeUndefined() + expect(second.getLastActiveProvider()).toBe(tabProvider) + first.dispose() + second.dispose() + second.dispose() + secondView.messages.fire({ type: "webviewDidFocus" }) + expect(second.getLastActiveProvider()).toBeUndefined() + }) + + test("preserves the visible-provider fallback when no chat has been focused", async () => { + await addSelection() + expect(tab.postMessage).toHaveBeenCalledTimes(2) + expect(sidebar.postMessage).not.toHaveBeenCalled() + }) + + test.each(["sidebar", "tab"])( + "falls back to the visible chat when the last focused %s is hidden", + async (source) => { + const hidden = source === "sidebar" ? sidebar : tab + const visible = source === "sidebar" ? tab : sidebar + hidden.messages.fire({ type: "webviewDidFocus" }) + Object.defineProperty(source === "sidebar" ? sidebar.view : panel, "visible", { + value: false, + configurable: true, + }) + + const target = await resolveChatProvider(provider.webviewFocusTracker) + expect(target).toBe(source === "sidebar" ? tabProvider : provider) + await target?.handleCodeAction("addToContext", "ADD_TO_CONTEXT", { selectedText: "selected code" }) + + expect(hidden.postMessage).not.toHaveBeenCalled() + expect(visible.postMessage).toHaveBeenCalledWith({ + type: "invoke", + invoke: "setChatBoxMessage", + text: expect.stringContaining("selected code"), + }) + expect(visible.postMessage).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) + expect(vscode.commands.executeCommand).not.toHaveBeenCalled() + }, + ) + + test.each(["sidebar", "tab"])( + "opens the sidebar when the last focused %s and all other chats are hidden", + async (source) => { + const focused = source === "sidebar" ? sidebar : tab + focused.messages.fire({ type: "webviewDidFocus" }) + Object.defineProperty(sidebar.view, "visible", { value: false, configurable: true }) + Object.defineProperty(panel, "visible", { value: false, configurable: true }) + vi.mocked(vscode.commands.executeCommand).mockImplementationOnce(async () => { + Object.defineProperty(sidebar.view, "visible", { value: true, configurable: true }) + }) + + const target = await resolveChatProvider(provider.webviewFocusTracker) + expect(target).toBe(provider) + await target?.handleCodeAction("addToContext", "ADD_TO_CONTEXT", { selectedText: "selected code" }) + + expect(vscode.commands.executeCommand).toHaveBeenCalledExactlyOnceWith( + `${ClineProvider.sideBarId}.focus`, + ) + expect(sidebar.postMessage).toHaveBeenCalledWith({ + type: "invoke", + invoke: "setChatBoxMessage", + text: expect.stringContaining("selected code"), + }) + expect(tab.postMessage).not.toHaveBeenCalled() + expect(panel.reveal).not.toHaveBeenCalled() + }, + ) + + test("remembers the last focused chat across hiding and showing it without another interaction", async () => { + sidebar.messages.fire({ type: "webviewDidFocus" }) + Object.defineProperty(sidebar.view, "visible", { value: false, configurable: true }) + expect(await resolveChatProvider(provider.webviewFocusTracker)).toBe(tabProvider) + expect(provider.webviewFocusTracker.getLastActiveProvider()).toBe(provider) + Object.defineProperty(sidebar.view, "visible", { value: true, configurable: true }) + expect(await resolveChatProvider(provider.webviewFocusTracker)).toBe(provider) + }) + + test("preserves single-view behavior", async () => { + await tabProvider.dispose() + await addSelection() + expect(sidebar.postMessage).toHaveBeenCalledTimes(2) + }) + + test("preserves the sidebar fallback when the last focused tab is disposed", async () => { + tab.messages.fire({ type: "webviewDidFocus" }) + await tabProvider.dispose() + Object.defineProperty(sidebar.view, "visible", { value: false, configurable: true }) + vi.mocked(vscode.commands.executeCommand).mockImplementationOnce(async () => { + Object.defineProperty(sidebar.view, "visible", { value: true, configurable: true }) + }) + await addSelection() + expect(vscode.commands.executeCommand).toHaveBeenCalledWith(`${ClineProvider.sideBarId}.focus`) + expect(sidebar.postMessage).toHaveBeenCalledTimes(2) + expect(tab.postMessage).not.toHaveBeenCalled() + }) + + test("forgets a disposed sidebar webview even while its provider remains registered", async () => { + sidebar.messages.fire({ type: "webviewDidFocus" }) + sidebar.disposed.fire() + expect(provider.webviewFocusTracker.getLastActiveProvider()).toBeUndefined() + await addSelection() + expect(tab.postMessage).toHaveBeenCalledTimes(2) + expect(sidebar.postMessage).not.toHaveBeenCalled() + }) + + test.each([ + ["explainCode", "EXPLAIN"], + ["fixCode", "FIX"], + ["improveCode", "IMPROVE"], + ] as const)("creates a task only in the last focused chat for %s", async (command, promptType) => { + sidebar.messages.fire({ type: "webviewDidFocus" }) + const task = new Task(defaultTaskOptions) + const sidebarTask = vi.spyOn(provider, "createTask").mockResolvedValue(task) + const tabTask = vi.spyOn(tabProvider, "createTask").mockResolvedValue(task) + try { + const target = provider.webviewFocusTracker.getLastActiveProvider()! + await target.handleCodeAction(command, promptType, { selectedText: "selected code" }) + expect(sidebarTask).toHaveBeenCalledOnce() + expect(sidebarTask).toHaveBeenCalledWith(expect.stringContaining("selected code")) + expect(tabTask).not.toHaveBeenCalled() + } finally { + sidebarTask.mockRestore() + tabTask.mockRestore() + } + }) + + test("preserves visible-provider fallback for task creation when no chat has been focused", async () => { + const task = new Task(defaultTaskOptions) + const sidebarTask = vi.spyOn(provider, "createTask").mockResolvedValue(task) + const tabTask = vi.spyOn(tabProvider, "createTask").mockResolvedValue(task) + const target = await ClineProvider.getInstance() + await target?.handleCodeAction("explainCode", "EXPLAIN", { selectedText: "selected code" }) + expect(tabTask).toHaveBeenCalledOnce() + expect(sidebarTask).not.toHaveBeenCalled() + sidebarTask.mockRestore() + tabTask.mockRestore() + }) + }) + test("loads full model details when preparing an LM Studio task", async () => { await provider.performPreparationTasks({ apiConfiguration: { @@ -684,6 +1052,7 @@ describe("ClineProvider", () => { mockOutputChannel, "sidebar", new ContextProxy(developmentContext), + new WebviewFocusTracker(), ) // The dev-server probe fails, so the HMR path falls back to the production HTML. ;(axios.get as any).mockRejectedValueOnce(new Error("Network error")) @@ -732,6 +1101,7 @@ describe("ClineProvider", () => { mockOutputChannel, "sidebar", new ContextProxy({ ...mockContext, extensionMode: vscode.ExtensionMode.Development }), + new WebviewFocusTracker(), ) // The default axios mock resolves, so the dev-server probe succeeds and the // HMR HTML branch (instead of the production fallback) is taken. @@ -916,6 +1286,7 @@ describe("ClineProvider", () => { mockOutputChannel, "sidebar", new ContextProxy(mockContext), + new WebviewFocusTracker(), mdmService, ) @@ -2189,7 +2560,13 @@ describe("ClineProvider", () => { } as unknown as vscode.ExtensionContext // Create new provider with updated mock context - provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] @@ -3655,7 +4032,13 @@ describe("Project MCP Settings", () => { } ;(vscode.window as any).activeTextEditor = undefined ;(vscode.workspace.getWorkspaceFolder as any).mockReset() - provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) }) test("handles openProjectMcpSettings message", async () => { @@ -3784,7 +4167,13 @@ describe("getTelemetryProperties", () => { } as unknown as vscode.ExtensionContext mockOutputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel - provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) defaultTaskOptions = { provider, @@ -3992,7 +4381,13 @@ describe("ClineProvider - Router Models", () => { TelemetryService.createInstance([]) } - provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) }) test("handles requestRouterModels with successful responses", async () => { @@ -4342,7 +4737,13 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { }), } - provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) defaultTaskOptions = { provider, diff --git a/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts b/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts index 55e26a9667..b60ec498fb 100644 --- a/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts @@ -1,6 +1,8 @@ +import { WebviewFocusTracker } from "../WebviewFocusTracker" // npx vitest core/webview/__tests__/ClineProvider.sticky-mode.spec.ts import * as vscode from "vscode" +import { makeCompositeDisposable } from "../../../test-utils/vscode" import { TelemetryService } from "@roo-code/telemetry" import { ClineProvider } from "../ClineProvider" import { ContextProxy } from "../../config/ContextProxy" @@ -12,6 +14,7 @@ vi.mock("vscode", () => ({ ExtensionContext: vi.fn(), OutputChannel: vi.fn(), WebviewView: vi.fn(), + Disposable: { from: (...subscriptions: vscode.Disposable[]) => makeCompositeDisposable(...subscriptions) }, Uri: { joinPath: vi.fn(), file: vi.fn(), @@ -301,7 +304,13 @@ describe("ClineProvider - Sticky Mode", () => { }), } as unknown as vscode.WebviewView - provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) // Wait for the async TaskHistoryStore initialization to complete await new Promise((resolve) => setTimeout(resolve, 10)) diff --git a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts index 7d8493fba3..01864c0234 100644 --- a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts @@ -1,6 +1,8 @@ +import { WebviewFocusTracker } from "../WebviewFocusTracker" // npx vitest run core/webview/__tests__/ClineProvider.sticky-profile.spec.ts import * as vscode from "vscode" +import { makeCompositeDisposable } from "../../../test-utils/vscode" import { TelemetryService } from "@roo-code/telemetry" import { ClineProvider } from "../ClineProvider" import { ContextProxy } from "../../config/ContextProxy" @@ -11,6 +13,7 @@ vi.mock("vscode", () => ({ ExtensionContext: vi.fn(), OutputChannel: vi.fn(), WebviewView: vi.fn(), + Disposable: { from: (...subscriptions: vscode.Disposable[]) => makeCompositeDisposable(...subscriptions) }, Uri: { joinPath: vi.fn(), file: vi.fn(), @@ -300,7 +303,13 @@ describe("ClineProvider - Sticky Provider Profile", () => { }), } as unknown as vscode.WebviewView - provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) // Wait for the async TaskHistoryStore initialization to complete await new Promise((resolve) => setTimeout(resolve, 10)) diff --git a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts index 753332dd5b..14e765bf9e 100644 --- a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts @@ -1,6 +1,8 @@ +import { WebviewFocusTracker } from "../WebviewFocusTracker" // pnpm --filter roo-cline test core/webview/__tests__/ClineProvider.taskHistory.spec.ts import * as vscode from "vscode" +import { makeCompositeDisposable } from "../../../test-utils/vscode" import type { HistoryItem, ExtensionMessage } from "@roo-code/types" import { providerIdentifiers, RooCodeEventName } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" @@ -104,6 +106,7 @@ vi.mock("vscode", () => ({ ExtensionContext: vi.fn(), OutputChannel: vi.fn(), WebviewView: vi.fn(), + Disposable: { from: (...subscriptions: vscode.Disposable[]) => makeCompositeDisposable(...subscriptions) }, Uri: { joinPath: vi.fn(), file: vi.fn(), @@ -349,7 +352,13 @@ describe("ClineProvider Task History Synchronization", () => { }), } as unknown as vscode.WebviewView - provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) // Wait for the async TaskHistoryStore initialization to complete // (fire-and-forget from the constructor; microtasks need to flush) diff --git a/src/core/webview/__tests__/ClineProviderFactory.spec.ts b/src/core/webview/__tests__/ClineProviderFactory.spec.ts new file mode 100644 index 0000000000..6545835b2b --- /dev/null +++ b/src/core/webview/__tests__/ClineProviderFactory.spec.ts @@ -0,0 +1,73 @@ +import type * as vscode from "vscode" + +import { ClineProviderFactory } from "../ClineProviderFactory" +import { ClineProvider } from "../ClineProvider" +import { WebviewFocusTracker } from "../WebviewFocusTracker" +import { openClineInNewTab } from "../../../activate/registerCommands" +import { makeExtensionContext } from "../../../test-utils/vscode" + +vi.mock("../../../activate/registerCommands", () => ({ openClineInNewTab: vi.fn() })) +vi.mock("../ClineProvider", () => ({ ClineProvider: class {} })) + +describe("ClineProviderFactory", () => { + let context: vscode.ExtensionContext + let outputChannel: vscode.OutputChannel + let tracker: WebviewFocusTracker + let factory: ClineProviderFactory + let provider: ClineProvider + + beforeEach(() => { + vi.clearAllMocks() + context = makeExtensionContext() + outputChannel = { + name: "test-output", + append: vi.fn(), + appendLine: vi.fn(), + replace: vi.fn(), + clear: vi.fn(), + show: vi.fn(), + hide: vi.fn(), + dispose: vi.fn(), + } + tracker = new WebviewFocusTracker() + factory = new ClineProviderFactory(context, outputChannel, tracker) + provider = Object.create(ClineProvider.prototype) as ClineProvider + vi.mocked(openClineInNewTab).mockResolvedValue(provider) + }) + + it("creates a tab with its injected dependencies and returns the provider", async () => { + await expect(factory.createInNewTab()).resolves.toBe(provider) + expect(openClineInNewTab).toHaveBeenCalledExactlyOnceWith({ + context, + outputChannel, + webviewFocusTracker: tracker, + }) + }) + + it("keeps dependencies independent between factory instances", async () => { + const secondContext = makeExtensionContext() + const secondTracker = new WebviewFocusTracker() + const secondFactory = new ClineProviderFactory(secondContext, outputChannel, secondTracker) + const secondProvider = Object.create(ClineProvider.prototype) as ClineProvider + vi.mocked(openClineInNewTab).mockResolvedValueOnce(provider).mockResolvedValueOnce(secondProvider) + + await expect(factory.createInNewTab()).resolves.toBe(provider) + await expect(secondFactory.createInNewTab()).resolves.toBe(secondProvider) + expect(openClineInNewTab).toHaveBeenNthCalledWith(1, { + context, + outputChannel, + webviewFocusTracker: tracker, + }) + expect(openClineInNewTab).toHaveBeenNthCalledWith(2, { + context: secondContext, + outputChannel, + webviewFocusTracker: secondTracker, + }) + }) + + it("propagates tab creation errors", async () => { + const error = new Error("Cannot create a tab") + vi.mocked(openClineInNewTab).mockRejectedValueOnce(error) + await expect(factory.createInNewTab()).rejects.toBe(error) + }) +}) diff --git a/src/core/webview/__tests__/WebviewFocusTracker.spec.ts b/src/core/webview/__tests__/WebviewFocusTracker.spec.ts new file mode 100644 index 0000000000..6c7c2d9742 --- /dev/null +++ b/src/core/webview/__tests__/WebviewFocusTracker.spec.ts @@ -0,0 +1,327 @@ +import type { WebviewMessage } from "@roo-code/types" + +import type { ClineProvider } from "../ClineProvider" +import { WebviewFocusTracker } from "../WebviewFocusTracker" +import { makeEventEmitter } from "../../../test-utils/vscode" + +function createView() { + const messages = makeEventEmitter() + const disposed = makeEventEmitter() + const view = { + webview: { onDidReceiveMessage: vi.fn(messages.event) }, + onDidDispose: vi.fn(disposed.event), + } + return { view, messages, disposed } +} + +describe("WebviewFocusTracker", () => { + let tracker: WebviewFocusTracker + let provider: ClineProvider + + beforeEach(() => { + tracker = new WebviewFocusTracker() + // The tracker only stores provider identity; no provider methods are needed. + provider = {} as ClineProvider + }) + + afterEach(() => tracker.dispose()) + + it("waits for a focus message and ignores other messages", () => { + const { view, messages } = createView() + tracker.init(provider, view) + expect(tracker.getLastActiveProvider()).toBeUndefined() + + messages.fire({ type: "themeFixtureProbeResponse" }) + expect(tracker.getLastActiveProvider()).toBeUndefined() + + messages.fire({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBe(provider) + }) + + it("follows focus messages rather than registration order", () => { + const first = createView() + const second = createView() + const secondProvider = {} as ClineProvider + tracker.init(provider, first.view) + tracker.init(secondProvider, second.view) + + first.messages.fire({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBe(provider) + second.messages.fire({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBe(secondProvider) + first.messages.fire({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBe(provider) + }) + + it("does not clear focus when another view using the same provider is disposed", () => { + const first = createView() + const second = createView() + tracker.init(provider, first.view) + tracker.init(provider, second.view) + + first.messages.fire({ type: "webviewDidFocus" }) + second.disposed.fire() + expect(tracker.getLastActiveProvider()).toBe(provider) + first.disposed.fire() + expect(tracker.getLastActiveProvider()).toBeUndefined() + }) + + it("unsubscribes both listeners exactly once when a registration is disposed", () => { + const { view, messages, disposed } = createView() + const registration = tracker.init(provider, view) + const disposeMessages = vi.spyOn(view.webview.onDidReceiveMessage.mock.results[0].value, "dispose") + const disposeView = vi.spyOn(view.onDidDispose.mock.results[0].value, "dispose") + messages.fire({ type: "webviewDidFocus" }) + + registration.dispose() + registration.dispose() + disposed.fire() + tracker.dispose() + expect(tracker.getLastActiveProvider()).toBeUndefined() + expect(disposeMessages).toHaveBeenCalledOnce() + expect(disposeView).toHaveBeenCalledOnce() + messages.fire({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBeUndefined() + }) + + it.each(["onDidReceiveMessage", "onDidDispose"] as const)( + "continues registration cleanup and logs a failing %s listener", + (eventName) => { + const { view, messages, disposed } = createView() + const registration = tracker.init(provider, view) + const error = new Error(`${eventName} cleanup failed`) + const listeners = { + onDidReceiveMessage: vi.spyOn(view.webview.onDidReceiveMessage.mock.results[0].value, "dispose"), + onDidDispose: vi.spyOn(view.onDidDispose.mock.results[0].value, "dispose"), + } + listeners[eventName].mockImplementation(() => { + throw error + }) + const log = vi.spyOn(console, "error").mockImplementation(() => {}) + try { + messages.fire({ type: "webviewDidFocus" }) + expect(() => registration.dispose()).not.toThrow() + expect(tracker.getLastActiveProvider()).toBeUndefined() + expect(listeners.onDidReceiveMessage).toHaveBeenCalledOnce() + expect(listeners.onDidDispose).toHaveBeenCalledOnce() + expect(log).toHaveBeenCalledExactlyOnceWith( + "[WebviewFocusTracker] Failed to dispose webview subscription:", + error, + ) + + // A failed unsubscription may leave the source alive, but it must not restore focus. + messages.fire({ type: "webviewDidFocus" }) + disposed.fire() + registration.dispose() + tracker.dispose() + expect(tracker.getLastActiveProvider()).toBeUndefined() + expect(listeners.onDidReceiveMessage).toHaveBeenCalledOnce() + expect(listeners.onDidDispose).toHaveBeenCalledOnce() + expect(log).toHaveBeenCalledOnce() + } finally { + log.mockRestore() + } + }, + ) + + it("disposes each listener once when cleanup reenters tracker disposal", () => { + const first = createView() + const second = createView() + first.view.webview.onDidReceiveMessage.mockImplementation((listener) => { + const subscription = first.messages.event(listener) + return { + dispose: () => { + subscription.dispose() + tracker.dispose() + }, + } + }) + tracker.init(provider, first.view) + tracker.init(provider, second.view) + const listeners = [first, second].flatMap(({ view }) => [ + vi.spyOn(view.webview.onDidReceiveMessage.mock.results[0].value, "dispose"), + vi.spyOn(view.onDidDispose.mock.results[0].value, "dispose"), + ]) + second.messages.fire({ type: "webviewDidFocus" }) + + tracker.dispose() + + for (const listener of listeners) { + expect(listener).toHaveBeenCalledOnce() + } + expect(tracker.getLastActiveProvider()).toBeUndefined() + first.messages.fire({ type: "webviewDidFocus" }) + second.messages.fire({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBeUndefined() + }) + + it("attempts every listener across views even when all listener disposals fail", () => { + const first = createView() + const second = createView() + const secondProvider = {} as ClineProvider + tracker.init(provider, first.view) + tracker.init(secondProvider, second.view) + const errors = [new Error("message listener failed"), "dispose listener failed"] + const listeners = [first, second].flatMap(({ view }) => [ + vi.spyOn(view.webview.onDidReceiveMessage.mock.results[0].value, "dispose").mockImplementation(() => { + throw errors[0] + }), + vi.spyOn(view.onDidDispose.mock.results[0].value, "dispose").mockImplementation(() => { + throw errors[1] + }), + ]) + const log = vi.spyOn(console, "error").mockImplementation(() => {}) + try { + second.messages.fire({ type: "webviewDidFocus" }) + expect(() => tracker.dispose()).not.toThrow() + expect(tracker.getLastActiveProvider()).toBeUndefined() + for (const listener of listeners) { + expect(listener).toHaveBeenCalledOnce() + } + expect(log).toHaveBeenCalledTimes(4) + for (const [index, error] of [...errors, ...errors].entries()) { + expect(log).toHaveBeenNthCalledWith( + index + 1, + "[WebviewFocusTracker] Failed to dispose webview subscription:", + error, + ) + } + + const third = createView() + const thirdProvider = {} as ClineProvider + tracker.init(thirdProvider, third.view) + third.messages.fire({ type: "webviewDidFocus" }) + for (const previous of [first, second]) { + previous.messages.fire({ type: "webviewDidFocus" }) + previous.disposed.fire() + } + expect(tracker.getLastActiveProvider()).toBe(thirdProvider) + tracker.dispose() + tracker.dispose() + for (const listener of listeners) { + expect(listener).toHaveBeenCalledOnce() + } + expect(log).toHaveBeenCalledTimes(4) + } finally { + log.mockRestore() + } + }) + + it("ignores a queued focus callback from a disposed registration", () => { + const first = createView() + const second = createView() + const secondProvider = {} as ClineProvider + const registration = tracker.init(provider, first.view) + const onMessage = first.view.webview.onDidReceiveMessage.mock.calls[0][0] + tracker.init(secondProvider, second.view) + second.messages.fire({ type: "webviewDidFocus" }) + + registration.dispose() + onMessage({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBe(secondProvider) + }) + + it("cleans up when the view is disposed while listeners are being registered", () => { + const { view, messages } = createView() + const disposeView = vi.fn() + view.onDidDispose.mockImplementation((listener) => { + listener() + return { dispose: disposeView } + }) + const registration = tracker.init(provider, view) + const onMessage = view.webview.onDidReceiveMessage.mock.calls[0][0] + messages.fire({ type: "webviewDidFocus" }) + onMessage({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBeUndefined() + expect(disposeView).toHaveBeenCalledOnce() + + registration.dispose() + tracker.dispose() + expect(disposeView).toHaveBeenCalledOnce() + }) + + it("contains listener cleanup errors when a view closes during registration", () => { + const { view, messages } = createView() + const error = new Error("registration cleanup failed") + const disposeMessages = vi.fn() + view.webview.onDidReceiveMessage.mockImplementation((listener) => { + const subscription = messages.event(listener) + disposeMessages.mockImplementation(() => subscription.dispose()) + return { dispose: disposeMessages } + }) + const disposeView = vi.fn(() => { + throw error + }) + view.onDidDispose.mockImplementation((listener) => { + listener() + return { dispose: disposeView } + }) + const log = vi.spyOn(console, "error").mockImplementation(() => {}) + try { + expect(() => tracker.init(provider, view)).not.toThrow() + expect(disposeMessages).toHaveBeenCalledOnce() + expect(disposeView).toHaveBeenCalledOnce() + expect(log).toHaveBeenCalledExactlyOnceWith( + "[WebviewFocusTracker] Failed to dispose webview subscription:", + error, + ) + messages.fire({ type: "webviewDidFocus" }) + view.webview.onDidReceiveMessage.mock.calls[0][0]({ type: "webviewDidFocus" }) + expect(tracker.getLastActiveProvider()).toBeUndefined() + tracker.dispose() + expect(disposeView).toHaveBeenCalledOnce() + } finally { + log.mockRestore() + } + }) + + it("disposes all registrations and can track a new view afterward", () => { + const first = createView() + const second = createView() + tracker.init(provider, first.view) + tracker.init(provider, second.view) + const listeners = [first, second].flatMap(({ view }) => [ + vi.spyOn(view.webview.onDidReceiveMessage.mock.results[0].value, "dispose"), + vi.spyOn(view.onDidDispose.mock.results[0].value, "dispose"), + ]) + const queuedCallbacks = [first, second].map(({ view }) => view.webview.onDidReceiveMessage.mock.calls[0][0]) + first.messages.fire({ type: "webviewDidFocus" }) + + tracker.dispose() + tracker.dispose() + for (const listener of listeners) { + expect(listener).toHaveBeenCalledOnce() + } + first.messages.fire({ type: "webviewDidFocus" }) + second.messages.fire({ type: "webviewDidFocus" }) + for (const callback of queuedCallbacks) { + callback({ type: "webviewDidFocus" }) + } + expect(tracker.getLastActiveProvider()).toBeUndefined() + + const third = createView() + const thirdProvider = {} as ClineProvider + tracker.init(thirdProvider, third.view) + third.messages.fire({ type: "webviewDidFocus" }) + for (const callback of queuedCallbacks) { + callback({ type: "webviewDidFocus" }) + } + expect(tracker.getLastActiveProvider()).toBe(thirdProvider) + }) + + it("keeps focus and disposal independent between trackers", () => { + const otherTracker = new WebviewFocusTracker() + const first = createView() + const second = createView() + tracker.init(provider, first.view) + otherTracker.init(provider, second.view) + first.messages.fire({ type: "webviewDidFocus" }) + second.messages.fire({ type: "webviewDidFocus" }) + + tracker.dispose() + expect(tracker.getLastActiveProvider()).toBeUndefined() + expect(otherTracker.getLastActiveProvider()).toBe(provider) + otherTracker.dispose() + expect(otherTracker.getLastActiveProvider()).toBeUndefined() + }) +}) diff --git a/src/extension.ts b/src/extension.ts index 1d71f834df..aa82c5388f 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -28,6 +28,8 @@ import { Package } from "./shared/package" import { formatLanguage } from "./shared/language" import { ContextProxy } from "./core/config/ContextProxy" import { ClineProvider } from "./core/webview/ClineProvider" +import { ClineProviderFactory } from "./core/webview/ClineProviderFactory" +import { WebviewFocusTracker } from "./core/webview/WebviewFocusTracker" import { DIFF_VIEW_URI_SCHEME } from "./integrations/editor/DiffViewProvider" import { Terminal } from "./integrations/terminal/Terminal" import { TerminalRegistry } from "./integrations/terminal/TerminalRegistry" @@ -225,7 +227,9 @@ export async function activate(context: vscode.ExtensionContext) { } // Initialize the provider *before* the Roo Code Cloud service. - const provider = new ClineProvider(context, outputChannel, "sidebar", contextProxy, mdmService) + const webviewFocusTracker = new WebviewFocusTracker() + context.subscriptions.push(webviewFocusTracker) + const provider = new ClineProvider(context, outputChannel, "sidebar", contextProxy, webviewFocusTracker, mdmService) // Initialize Roo Code Cloud service. settingsUpdatedHandler = () => { @@ -314,7 +318,7 @@ export async function activate(context: vscode.ExtensionContext) { }), ) - registerCodeActions(context) + registerCodeActions(context, webviewFocusTracker) registerTerminalActions(context) // Allows other extensions to activate once Roo is ready. @@ -383,7 +387,8 @@ export async function activate(context: vscode.ExtensionContext) { ) }) - return new API(outputChannel, provider, socketPath, enableLogging) + const providerFactory = new ClineProviderFactory(context, outputChannel, webviewFocusTracker) + return new API(outputChannel, provider, providerFactory, socketPath, enableLogging) } // This method is called when your extension is deactivated. diff --git a/src/extension/__tests__/api-configuration.spec.ts b/src/extension/__tests__/api-configuration.spec.ts index 80f8cad9cd..f4ed8af469 100644 --- a/src/extension/__tests__/api-configuration.spec.ts +++ b/src/extension/__tests__/api-configuration.spec.ts @@ -3,6 +3,7 @@ import type * as vscode from "vscode" import { API } from "../api" import type { ClineProvider } from "../../core/webview/ClineProvider" +import { makeClineProviderFactory } from "../../test-utils/provider" vi.mock("@roo-code/ipc", () => ({ IpcServer: class {}, @@ -22,7 +23,7 @@ describe("API - configuration", () => { postStateToWebview, } as unknown as ClineProvider const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel - const api = new API(outputChannel, provider) + const api = new API(outputChannel, provider, makeClineProviderFactory()) await api.setConfiguration({ currentApiConfigName: "default", @@ -55,7 +56,7 @@ describe("API - configuration", () => { postStateToWebview, } as unknown as ClineProvider const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel - const api = new API(outputChannel, provider) + const api = new API(outputChannel, provider, makeClineProviderFactory()) await api.setConfiguration({ currentApiConfigName: "default" }) diff --git a/src/extension/__tests__/api-delete-queued-message.spec.ts b/src/extension/__tests__/api-delete-queued-message.spec.ts index 6bf6014bf8..91c945df39 100644 --- a/src/extension/__tests__/api-delete-queued-message.spec.ts +++ b/src/extension/__tests__/api-delete-queued-message.spec.ts @@ -3,6 +3,7 @@ import * as vscode from "vscode" import { API } from "../api" import { ClineProvider } from "../../core/webview/ClineProvider" +import { makeClineProviderFactory } from "../../test-utils/provider" vi.mock("vscode") vi.mock("../../core/webview/ClineProvider") @@ -36,7 +37,7 @@ describe("API - DeleteQueuedMessage Command", () => { mockLog = vi.fn() - api = new API(mockOutputChannel, mockProvider, undefined, true) + api = new API(mockOutputChannel, mockProvider, makeClineProviderFactory(), undefined, true) ;(api as any).log = mockLog }) diff --git a/src/extension/__tests__/api-send-message.spec.ts b/src/extension/__tests__/api-send-message.spec.ts index 23677b1218..5158a218e7 100644 --- a/src/extension/__tests__/api-send-message.spec.ts +++ b/src/extension/__tests__/api-send-message.spec.ts @@ -4,6 +4,7 @@ import * as vscode from "vscode" import { API } from "../api" import { ClineProvider } from "../../core/webview/ClineProvider" import { TaskCommandName } from "@roo-code/types" +import { makeClineProviderFactory } from "../../test-utils/provider" vi.mock("vscode") vi.mock("../../core/webview/ClineProvider") @@ -35,7 +36,7 @@ describe("API - SendMessage Command", () => { mockLog = vi.fn<(...args: any[]) => void>() // Create API instance with logging enabled for testing - api = new API(mockOutputChannel, mockProvider, undefined, true) + api = new API(mockOutputChannel, mockProvider, makeClineProviderFactory(), undefined, true) // Override the log method to use our mock ;(api as any).log = mockLog }) diff --git a/src/extension/__tests__/api-start-new-task.spec.ts b/src/extension/__tests__/api-start-new-task.spec.ts new file mode 100644 index 0000000000..e7ff6968b3 --- /dev/null +++ b/src/extension/__tests__/api-start-new-task.spec.ts @@ -0,0 +1,123 @@ +import * as vscode from "vscode" +import { RooCodeEventName } from "@roo-code/types" + +import { API } from "../api" +import { ClineProvider } from "../../core/webview/ClineProvider" +import type { ClineProviderFactory } from "../../core/webview/ClineProviderFactory" +import type { Task } from "../../core/task/Task" +import { Package } from "../../shared/package" +import { makeClineProviderFactory } from "../../test-utils/provider" + +vi.mock("vscode", () => ({ commands: { executeCommand: vi.fn().mockResolvedValue(undefined) } })) +vi.mock("@roo-code/ipc", () => ({ IpcServer: class {} })) +vi.mock("../../core/webview/ClineProvider", () => ({ ClineProvider: class {} })) + +function createProvider(taskId: string) { + // Only the task identity is consumed by this API path; task execution is outside this test's scope. + const task = { taskId } as Task + return Object.assign(Object.create(ClineProvider.prototype) as ClineProvider, { + on: vi.fn(), + evictCurrentTask: vi.fn().mockResolvedValue(undefined), + postStateToWebview: vi.fn().mockResolvedValue(undefined), + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + createTask: vi.fn().mockResolvedValue(task), + }) +} + +describe("API - startNewTask routing", () => { + let sidebar: ReturnType + let editor: ReturnType + let outputChannel: vscode.OutputChannel + let api: API + let providerFactory: ClineProviderFactory + + beforeEach(() => { + vi.clearAllMocks() + sidebar = createProvider("sidebar-task") + editor = createProvider("editor-task") + outputChannel = { + name: "test-output", + append: vi.fn(), + appendLine: vi.fn(), + replace: vi.fn(), + clear: vi.fn(), + show: vi.fn(), + hide: vi.fn(), + dispose: vi.fn(), + } + providerFactory = makeClineProviderFactory() + vi.mocked(providerFactory.createInNewTab).mockResolvedValue(editor) + api = new API(outputChannel, sidebar, providerFactory) + }) + + it.each([true, false, undefined])("routes a task with newTab=%s to the requested provider", async (newTab) => { + const configuration = { currentApiConfigName: "test-profile" } + const images = ["data:image/png;base64,test"] + const target = newTab ? editor : sidebar + const other = newTab ? sidebar : editor + + const taskId = await api.startNewTask({ configuration, text: "new task", images, newTab }) + + if (newTab) { + expect(vscode.commands.executeCommand).toHaveBeenNthCalledWith(1, "workbench.action.files.revert") + expect(vscode.commands.executeCommand).toHaveBeenNthCalledWith(2, "workbench.action.closeAllEditors") + expect(vscode.commands.executeCommand).toHaveBeenCalledTimes(2) + expect(providerFactory.createInNewTab).toHaveBeenCalledExactlyOnceWith() + expect(vi.mocked(providerFactory.createInNewTab).mock.contexts[0]).toBe(providerFactory) + expect(editor.on).toHaveBeenCalledWith(RooCodeEventName.TaskCreated, expect.any(Function)) + expect(editor.on).toHaveBeenCalledWith(RooCodeEventName.TaskCompleted, expect.any(Function)) + } else { + expect(vscode.commands.executeCommand).toHaveBeenCalledExactlyOnceWith( + `${Package.name}.SidebarProvider.focus`, + ) + expect(providerFactory.createInNewTab).not.toHaveBeenCalled() + expect(editor.on).not.toHaveBeenCalled() + } + expect(target.evictCurrentTask).toHaveBeenCalledOnce() + expect(target.postStateToWebview).toHaveBeenCalledOnce() + expect(target.postMessageToWebview).toHaveBeenNthCalledWith(1, { type: "action", action: "chatButtonClicked" }) + expect(target.postMessageToWebview).toHaveBeenNthCalledWith(2, { + type: "invoke", + invoke: "newChat", + text: "new task", + images, + }) + expect(target.createTask).toHaveBeenCalledExactlyOnceWith( + "new task", + images, + undefined, + { consecutiveMistakeLimit: Number.MAX_SAFE_INTEGER }, + configuration, + ) + expect(taskId).toBe(newTab ? "editor-task" : "sidebar-task") + expect(other.evictCurrentTask).not.toHaveBeenCalled() + expect(other.postStateToWebview).not.toHaveBeenCalled() + expect(other.postMessageToWebview).not.toHaveBeenCalled() + expect(other.createTask).not.toHaveBeenCalled() + }) + + it("propagates tab-opening failures without starting a task in another chat", async () => { + const error = new Error("Cannot open editor chat") + vi.mocked(providerFactory.createInNewTab).mockRejectedValueOnce(error) + + await expect(api.startNewTask({ configuration: {}, newTab: true })).rejects.toBe(error) + + expect(sidebar.evictCurrentTask).not.toHaveBeenCalled() + expect(editor.evictCurrentTask).not.toHaveBeenCalled() + expect(sidebar.createTask).not.toHaveBeenCalled() + expect(editor.createTask).not.toHaveBeenCalled() + expect(editor.on).not.toHaveBeenCalled() + }) + + it.each([true, false])("propagates task-creation failures without falling back with newTab=%s", async (newTab) => { + const target = newTab ? editor : sidebar + const other = newTab ? sidebar : editor + const error = new Error("Task creation rejected") + target.createTask.mockRejectedValueOnce(error) + + await expect(api.startNewTask({ configuration: {}, text: "blocked task", newTab })).rejects.toBe(error) + + expect(target.createTask).toHaveBeenCalledOnce() + expect(other.createTask).not.toHaveBeenCalled() + }) +}) diff --git a/src/extension/__tests__/api-task-conversation-history-length.spec.ts b/src/extension/__tests__/api-task-conversation-history-length.spec.ts index b60c5eb21a..a3b8f16151 100644 --- a/src/extension/__tests__/api-task-conversation-history-length.spec.ts +++ b/src/extension/__tests__/api-task-conversation-history-length.spec.ts @@ -4,6 +4,7 @@ import { RooCodeEventName } from "@roo-code/types" import { API } from "../api" import { ClineProvider } from "../../core/webview/ClineProvider" +import { makeClineProviderFactory } from "../../test-utils/provider" vi.mock("vscode") vi.mock("../../core/webview/ClineProvider") @@ -38,7 +39,7 @@ describe("API#getTaskApiConversationHistoryLength", () => { }), } as unknown as ClineProvider - api = new API(mockOutputChannel, mockProvider, undefined, true) + api = new API(mockOutputChannel, mockProvider, makeClineProviderFactory(), undefined, true) }) it("returns the persisted api conversation history length", async () => { diff --git a/src/extension/__tests__/api-terminal-profile.spec.ts b/src/extension/__tests__/api-terminal-profile.spec.ts index 46ef3541ab..58c88075c3 100644 --- a/src/extension/__tests__/api-terminal-profile.spec.ts +++ b/src/extension/__tests__/api-terminal-profile.spec.ts @@ -5,6 +5,7 @@ import { API } from "../api" import type { ClineProvider } from "../../core/webview/ClineProvider" import { Terminal } from "../../integrations/terminal/Terminal" import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry" +import { makeClineProviderFactory } from "../../test-utils/provider" vi.mock("@roo-code/ipc", () => ({ IpcServer: class {}, @@ -21,7 +22,7 @@ describe("API - terminal profile", () => { } as unknown as ClineProvider Terminal.setTerminalProfile(undefined) - api = new API(outputChannel, provider) + api = new API(outputChannel, provider, makeClineProviderFactory()) }) afterEach(() => { diff --git a/src/extension/__tests__/api-theme-fixture.spec.ts b/src/extension/__tests__/api-theme-fixture.spec.ts index 52058e1b4e..2db4c13937 100644 --- a/src/extension/__tests__/api-theme-fixture.spec.ts +++ b/src/extension/__tests__/api-theme-fixture.spec.ts @@ -3,6 +3,7 @@ import type * as vscode from "vscode" import { API } from "../api" import type { ClineProvider } from "../../core/webview/ClineProvider" +import { makeClineProviderFactory } from "../../test-utils/provider" vi.mock("@roo-code/ipc", () => ({ IpcServer: class {}, @@ -22,7 +23,7 @@ describe("API - theme fixture probe", () => { requestWebviewThemeFixture, } as unknown as ClineProvider const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel - const api = new API(outputChannel, provider) + const api = new API(outputChannel, provider, makeClineProviderFactory()) await expect(api.captureWebviewThemeFixture()).resolves.toEqual(fixture) expect(requestWebviewThemeFixture).toHaveBeenCalledOnce() @@ -35,7 +36,7 @@ describe("API - theme fixture probe", () => { latestAnnouncementId: "current-announcement", } as unknown as ClineProvider const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel - const api = new API(outputChannel, provider) + const api = new API(outputChannel, provider, makeClineProviderFactory()) expect(api.getLatestAnnouncementId()).toBe("current-announcement") }) diff --git a/src/extension/api.ts b/src/extension/api.ts index 316e7a6c9d..9ae696adb1 100644 --- a/src/extension/api.ts +++ b/src/extension/api.ts @@ -26,17 +26,16 @@ import { IpcServer } from "@roo-code/ipc" import { Package } from "../shared/package" import type { Mode } from "../shared/modes" -import { ClineProvider } from "../core/webview/ClineProvider" +import type { ClineProvider } from "../core/webview/ClineProvider" +import type { ClineProviderFactory } from "../core/webview/ClineProviderFactory" import { Terminal } from "../integrations/terminal/Terminal" import { TerminalRegistry } from "../integrations/terminal/TerminalRegistry" -import { openClineInNewTab } from "../activate/registerCommands" import { getCommands } from "../services/command/commands" import { getModels } from "../api/providers/fetchers/modelCache" export class API extends EventEmitter implements RooCodeAPI { private readonly outputChannel: vscode.OutputChannel private readonly sidebarProvider: ClineProvider - private readonly context: vscode.ExtensionContext private readonly ipc?: IpcServer private readonly log: (...args: unknown[]) => void private logfile?: string @@ -44,6 +43,7 @@ export class API extends EventEmitter implements RooCodeAPI { constructor( outputChannel: vscode.OutputChannel, provider: ClineProvider, + private readonly providerFactory: ClineProviderFactory, socketPath?: string, enableLogging = false, ) { @@ -51,7 +51,6 @@ export class API extends EventEmitter implements RooCodeAPI { this.outputChannel = outputChannel this.sidebarProvider = provider - this.context = provider.context if (enableLogging) { this.log = (...args: unknown[]) => { @@ -187,7 +186,7 @@ export class API extends EventEmitter implements RooCodeAPI { await vscode.commands.executeCommand("workbench.action.files.revert") await vscode.commands.executeCommand("workbench.action.closeAllEditors") - provider = await openClineInNewTab({ context: this.context, outputChannel: this.outputChannel }) + provider = await this.providerFactory.createInNewTab() this.registerListeners(provider) } else { await vscode.commands.executeCommand(`${Package.name}.SidebarProvider.focus`) diff --git a/src/test-utils/__tests__/provider.spec.ts b/src/test-utils/__tests__/provider.spec.ts new file mode 100644 index 0000000000..a9ba29fb6c --- /dev/null +++ b/src/test-utils/__tests__/provider.spec.ts @@ -0,0 +1,17 @@ +import { ClineProviderFactory } from "../../core/webview/ClineProviderFactory" +import { makeClineProviderFactory } from "../provider" + +vi.mock("../../activate/registerCommands", () => ({ openClineInNewTab: vi.fn() })) + +describe("provider test utilities", () => { + it("returns independent factory instances and fails on unexpected tab creation", async () => { + const first = makeClineProviderFactory() + const second = makeClineProviderFactory() + expect(first).toBeInstanceOf(ClineProviderFactory) + expect(second).toBeInstanceOf(ClineProviderFactory) + expect(second).not.toBe(first) + expect(first.createInNewTab).not.toBe(second.createInNewTab) + await expect(first.createInNewTab()).rejects.toThrow("Unexpected editor-tab creation in this test") + expect(second.createInNewTab).not.toHaveBeenCalled() + }) +}) diff --git a/src/test-utils/__tests__/vscode.spec.ts b/src/test-utils/__tests__/vscode.spec.ts index 81ce4a7468..b490bef765 100644 --- a/src/test-utils/__tests__/vscode.spec.ts +++ b/src/test-utils/__tests__/vscode.spec.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from "vitest" import { + makeCompositeDisposable, makeDisposable, makeEventEmitter, makeExtensionContext, @@ -67,6 +68,22 @@ describe("VS Code test utilities", () => { emitter.dispose() }) + it("disposes composite subscriptions only once and keeps composites independent", () => { + const first = makeDisposable() + const second = makeDisposable() + const other = makeDisposable() + const composite = makeCompositeDisposable(first, second) + const independent = makeCompositeDisposable(other) + + composite.dispose() + composite.dispose() + expect(first.dispose).toHaveBeenCalledOnce() + expect(second.dispose).toHaveBeenCalledOnce() + expect(other.dispose).not.toHaveBeenCalled() + independent.dispose() + expect(other.dispose).toHaveBeenCalledOnce() + }) + it("creates configurable workspace settings", async () => { const configuration = makeWorkspaceConfiguration({ enabled: true }) diff --git a/src/test-utils/provider.ts b/src/test-utils/provider.ts new file mode 100644 index 0000000000..4a7633a2c9 --- /dev/null +++ b/src/test-utils/provider.ts @@ -0,0 +1,24 @@ +import { vi } from "vitest" +import type * as vscode from "vscode" + +import { ClineProviderFactory } from "../core/webview/ClineProviderFactory" +import { WebviewFocusTracker } from "../core/webview/WebviewFocusTracker" +import { makeExtensionContext } from "./vscode" + +export function makeClineProviderFactory(): ClineProviderFactory { + const outputChannel: vscode.OutputChannel = { + name: "test-output", + append: vi.fn(), + appendLine: vi.fn(), + replace: vi.fn(), + clear: vi.fn(), + show: vi.fn(), + hide: vi.fn(), + dispose: vi.fn(), + } + const factory = new ClineProviderFactory(makeExtensionContext(), outputChannel, new WebviewFocusTracker()) + factory.createInNewTab = vi + .fn() + .mockRejectedValue(new Error("Unexpected editor-tab creation in this test")) + return factory +} diff --git a/src/test-utils/vscode.ts b/src/test-utils/vscode.ts index c61106f8f1..edb41f701c 100644 --- a/src/test-utils/vscode.ts +++ b/src/test-utils/vscode.ts @@ -98,6 +98,12 @@ export function makeDisposable(overrides: Partial = {}): vsco return { dispose: vi.fn(), ...overrides } } +export function makeCompositeDisposable(...subscriptions: vscode.Disposable[]): vscode.Disposable { + return makeDisposable({ + dispose: () => subscriptions.splice(0).forEach((subscription) => subscription?.dispose()), + }) +} + export function makeEventEmitter(): vscode.EventEmitter { const listeners = new Set<(value: T) => unknown>() const event = ((listener: (value: T) => unknown) => { diff --git a/webview-ui/src/App.tsx b/webview-ui/src/App.tsx index b1fbf82999..3e05ce64da 100644 --- a/webview-ui/src/App.tsx +++ b/webview-ui/src/App.tsx @@ -206,6 +206,17 @@ const App = () => { // Tell the extension that we are ready to receive messages. useEffect(() => vscode.postMessage({ type: "webviewDidLaunch" }), []) + useEffect(() => { + // Window activation and programmatic focus do not identify a chat interaction. + const onInteraction = () => vscode.postMessage({ type: "webviewDidFocus" }) + document.addEventListener("pointerdown", onInteraction, true) + document.addEventListener("keydown", onInteraction, true) + return () => { + document.removeEventListener("pointerdown", onInteraction, true) + document.removeEventListener("keydown", onInteraction, true) + } + }, []) + // Initialize source map support for better error reporting useEffect(() => { // Initialize source maps for better error reporting in production diff --git a/webview-ui/src/__tests__/App.spec.tsx b/webview-ui/src/__tests__/App.spec.tsx index 137bed5d70..4b4cbcd5fe 100644 --- a/webview-ui/src/__tests__/App.spec.tsx +++ b/webview-ui/src/__tests__/App.spec.tsx @@ -4,6 +4,7 @@ import React from "react" import { render, screen, act, cleanup } from "@/utils/test-utils" import AppWithProviders from "../App" +import { vscode } from "../utils/vscode" vi.mock("@src/utils/vscode", () => ({ vscode: { @@ -205,6 +206,65 @@ describe("App", () => { expect(chatView.getAttribute("data-hidden")).toBe("false") }, 10000) + it.each(["sidebar", "editor"])( + "reports direct pointer and keyboard interaction in the %s webview", + (renderContext) => { + mockUseExtensionState.mockReturnValue({ + didHydrateState: true, + showWelcome: false, + renderContext, + }) + const { unmount } = render() + vi.mocked(vscode.postMessage).mockClear() + + document.dispatchEvent(new Event("pointerdown")) + document.dispatchEvent(new KeyboardEvent("keydown", { key: "a" })) + expect(vscode.postMessage).toHaveBeenCalledTimes(2) + expect(vscode.postMessage).toHaveBeenNthCalledWith(1, { type: "webviewDidFocus" }) + expect(vscode.postMessage).toHaveBeenNthCalledWith(2, { type: "webviewDidFocus" }) + + window.dispatchEvent(new Event("blur")) + expect(vscode.postMessage).toHaveBeenCalledTimes(2) + + unmount() + document.dispatchEvent(new Event("pointerdown")) + document.dispatchEvent(new KeyboardEvent("keydown", { key: "a" })) + expect(vscode.postMessage).toHaveBeenCalledTimes(2) + }, + ) + + it("does not report chat interaction on initial focus, window activation, or programmatic focus", () => { + const hasFocus = vi.spyOn(document, "hasFocus").mockReturnValue(true) + try { + render() + expect(vscode.postMessage).not.toHaveBeenCalledWith({ type: "webviewDidFocus" }) + window.dispatchEvent(new Event("focus")) + window.dispatchEvent(new Event("blur")) + window.dispatchEvent(new Event("focus")) + document.dispatchEvent(new Event("visibilitychange")) + document.dispatchEvent(new FocusEvent("focusin")) + const chat = screen.getByTestId("chat-view") + chat.tabIndex = 0 + chat.focus() + expect(vscode.postMessage).not.toHaveBeenCalledWith({ type: "webviewDidFocus" }) + } finally { + hasFocus.mockRestore() + } + }) + + it("reports direct interaction even when a child stops event propagation", () => { + render() + vi.mocked(vscode.postMessage).mockClear() + const chat = screen.getByTestId("chat-view") + const stopPropagation = (event: Event) => event.stopPropagation() + chat.addEventListener("pointerdown", stopPropagation) + chat.addEventListener("keydown", stopPropagation) + chat.dispatchEvent(new Event("pointerdown", { bubbles: true })) + chat.dispatchEvent(new KeyboardEvent("keydown", { key: "a", bubbles: true })) + expect(vscode.postMessage).toHaveBeenCalledTimes(2) + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "webviewDidFocus" }) + }) + it("shows welcome view when setup is incomplete", () => { mockUseExtensionState.mockReturnValue({ didHydrateState: true, diff --git a/webview-ui/src/components/chat/ChatTextArea.tsx b/webview-ui/src/components/chat/ChatTextArea.tsx index 2c539766fe..a78f3f0f8e 100644 --- a/webview-ui/src/components/chat/ChatTextArea.tsx +++ b/webview-ui/src/components/chat/ChatTextArea.tsx @@ -1097,7 +1097,7 @@ export const ChatTextArea = forwardRef( placeholder={placeholderText} minRows={3} maxRows={15} - autoFocus={true} + autoFocus={document.hasFocus()} className={cn( "w-full", "text-vscode-input-foreground", diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 4694eeabf7..613e2c1b4d 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -913,7 +913,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction { - if (!isHidden && !sendingDisabled && !enableButtons) { + if (document.hasFocus() && !isHidden && !sendingDisabled && !enableButtons) { textAreaRef.current?.focus() } }, diff --git a/webview-ui/src/components/chat/__tests__/ChatTextArea.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatTextArea.spec.tsx index aae9ac3cc6..5cced6a92f 100644 --- a/webview-ui/src/components/chat/__tests__/ChatTextArea.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatTextArea.spec.tsx @@ -76,6 +76,38 @@ describe("ChatTextArea", () => { }) }) + it.each([false, true])("only autofocuses the mounted input when this document has focus=%s", (hasFocus) => { + const documentFocus = vi.spyOn(document, "hasFocus").mockReturnValue(hasFocus) + const focusedElements: HTMLElement[] = [] + const originalFocus = Object.getOwnPropertyDescriptor(HTMLElement.prototype, "focus") + if (!originalFocus) { + throw new Error("Expected the test setup to define HTMLElement.focus") + } + // The shared setup replaces focus with an accessor, so restore its descriptor after the test. + Object.defineProperty(HTMLElement.prototype, "focus", { + configurable: true, + get: () => + function (this: HTMLElement) { + focusedElements.push(this) + }, + }) + const outsideInput = document.createElement("input") + document.body.append(outsideInput) + outsideInput.focus() + try { + render() + expect(focusedElements[0]).toBe(outsideInput) + expect(focusedElements).toHaveLength(hasFocus ? 2 : 1) + if (hasFocus) { + expect(focusedElements[1]).toBeInstanceOf(HTMLTextAreaElement) + } + } finally { + documentFocus.mockRestore() + Object.defineProperty(HTMLElement.prototype, "focus", originalFocus) + outsideInput.remove() + } + }) + describe("enhance prompt button", () => { it("should be enabled even when sendingDisabled is true (for message queueing)", () => { ;(useExtensionState as ReturnType).mockReturnValue({ diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index a1e918b63c..bbad494170 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -755,7 +755,83 @@ describe("ChatView - Virtualization Configuration", () => { }) describe("ChatView - Focus Grabbing Tests", () => { - beforeEach(() => vi.clearAllMocks()) + beforeEach(() => { + vi.clearAllMocks() + vi.spyOn(document, "hasFocus").mockReturnValue(false) + }) + + afterEach(() => vi.mocked(document.hasFocus).mockRestore()) + + it("does not focus input when mounted or shown while this document is unfocused", async () => { + vi.useFakeTimers() + try { + const { rerender } = renderChatView() + await act(async () => vi.advanceTimersByTime(100)) + expect(mockFocus).not.toHaveBeenCalled() + rerender() + rerender() + await act(async () => vi.advanceTimersByTime(100)) + expect(mockFocus).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + + it("does not focus input on passive visibility notifications but preserves explicit focus requests", async () => { + renderChatView() + mockFocus.mockClear() + await dispatchExtensionMessage({ type: "action", action: "didBecomeVisible" }) + expect(mockFocus).not.toHaveBeenCalled() + await dispatchExtensionMessage({ type: "action", action: "focusInput" }) + expect(mockFocus).toHaveBeenCalledTimes(1) + }) + + it("autofocuses input when the user opens a focused chat", async () => { + vi.mocked(document.hasFocus).mockReturnValue(true) + vi.useFakeTimers() + try { + renderChatView() + await act(async () => vi.advanceTimersByTime(100)) + expect(mockFocus).toHaveBeenCalledTimes(1) + mockFocus.mockClear() + await dispatchExtensionMessage({ type: "action", action: "didBecomeVisible" }) + expect(mockFocus).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) + + it("does not run delayed autofocus after focus moves back to the editor", async () => { + vi.mocked(document.hasFocus).mockReturnValue(true) + vi.useFakeTimers() + try { + renderChatView() + vi.mocked(document.hasFocus).mockReturnValue(false) + await act(async () => vi.advanceTimersByTime(100)) + expect(mockFocus).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + + it.each([false, true])("only refocuses re-enabled input when this document has focus=%s", async (hasFocus) => { + vi.mocked(document.hasFocus).mockReturnValue(hasFocus) + vi.useFakeTimers() + try { + const { getByRole } = renderChatView() + await dispatchTaskState("background task", 1) + await dispatchExtensionMessage({ type: "invoke", invoke: "newChat" }) + expect(getByRole("textbox")).toHaveAttribute("data-sending-disabled", "true") + await act(async () => vi.advanceTimersByTime(100)) + mockFocus.mockClear() + await dispatchExtensionMessage({ type: "state", state: makeExtensionState({ clineMessages: [] }) }) + expect(getByRole("textbox")).toHaveAttribute("data-sending-disabled", "false") + await act(async () => vi.advanceTimersByTime(100)) + expect(mockFocus).toHaveBeenCalledTimes(hasFocus ? 1 : 0) + } finally { + vi.useRealTimers() + } + }) it("does not grab focus when follow-up question presented", async () => { const { getByTestId } = renderChatView() @@ -777,11 +853,6 @@ describe("ChatView - Focus Grabbing Tests", () => { expect(getByTestId("chat-textarea")).toBeInTheDocument() }) - // Wait for the debounced focus effect to fire (50ms debounce + buffer for CI variability) - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 100)) - }) - // Clear any initial calls after state has settled mockFocus.mockClear()