diff --git a/packages/types/src/vscode.ts b/packages/types/src/vscode.ts index fd4e31116d..6a1a08b821 100644 --- a/packages/types/src/vscode.ts +++ b/packages/types/src/vscode.ts @@ -35,6 +35,14 @@ export const commandIds = [ "popoutButtonClicked", "settingsButtonClicked", + // Editor-tab (popped-out) surface variants of the title-bar buttons. The + // shared ids above target the sidebar click origin, so the tab surface + // needs its own ids (see registerCommands.ts getTabProvider). + "plusButtonClickedInTab", + "settingsButtonClickedInTab", + "marketplaceButtonClickedInTab", + "historyButtonClickedInTab", + "openInNewTab", "newTask", diff --git a/src/activate/__tests__/registerCommands.spec.ts b/src/activate/__tests__/registerCommands.spec.ts index d041dada2e..03432de768 100644 --- a/src/activate/__tests__/registerCommands.spec.ts +++ b/src/activate/__tests__/registerCommands.spec.ts @@ -1,11 +1,22 @@ import type { Mock } from "vitest" import * as vscode from "vscode" -import { ClineProvider } from "../../core/webview/ClineProvider" +import { TelemetryService } from "@roo-code/telemetry" 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" +import { ContextProxy } from "../../core/config/ContextProxy" +import { ClineProvider } from "../../core/webview/ClineProvider" +import { MdmService } from "../../services/mdm/MdmService" + +import { + clearSidebarPanel, + getPanel, + getVisibleProviderOrLog, + openClineInNewTab, + registerCommands, + setPanel, +} from "../registerCommands" +import { focusPanel } from "../../utils/focusPanel" vi.mock("execa", () => ({ execa: vi.fn(), @@ -137,7 +148,12 @@ describe("registerCommands handlers", () => { let mockOutputChannel: vscode.OutputChannel let mockContext: vscode.ExtensionContext let mockVisibleProvider: { postMessageToWebview: Mock } - let mockProvider: { postMessageToWebview: Mock; webviewFocusTracker: WebviewFocusTracker } + let mockProvider: { + postMessageToWebview: Mock + evictCurrentTask: Mock + refreshWorkspace: Mock + webviewFocusTracker: WebviewFocusTracker + } let handlers: Record unknown> beforeEach(() => { @@ -163,6 +179,8 @@ describe("registerCommands handlers", () => { mockProvider = { postMessageToWebview: vi.fn().mockResolvedValue(undefined), + evictCurrentTask: vi.fn().mockResolvedValue(undefined), + refreshWorkspace: vi.fn().mockResolvedValue(undefined), webviewFocusTracker: new WebviewFocusTracker(), } ;(ClineProvider.getVisibleInstance as Mock).mockReturnValue(mockVisibleProvider) @@ -194,6 +212,8 @@ describe("registerCommands handlers", () => { expect(mockContext.subscriptions).toContain(disposable) }) + // The sidebar title-bar handlers target the registered provider (the + // sidebar click origin) directly, not the visible-instance heuristic. it("passes the shared focus tracker to the newTask handler", async () => { const { handleNewTask } = await import("../handleTask") const params = { prompt: "new task" } @@ -241,45 +261,110 @@ describe("registerCommands handlers", () => { expect(tabProvider.resolveWebviewView).toHaveBeenCalledExactlyOnceWith(panel) }, ) - - it("settingsButtonClicked posts both settingsButtonClicked and didBecomeVisible actions", () => { + it("settingsButtonClicked posts both settingsButtonClicked and didBecomeVisible actions on the registered provider", () => { handlers["zoo-code.settingsButtonClicked"]() - expect(mockVisibleProvider.postMessageToWebview).toHaveBeenCalledWith({ + expect(TelemetryService.instance.captureTitleButtonClicked).toHaveBeenCalledWith("settings") + expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "action", action: "settingsButtonClicked", }) - expect(mockVisibleProvider.postMessageToWebview).toHaveBeenCalledWith({ + expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "action", action: "didBecomeVisible", }) - expect(mockVisibleProvider.postMessageToWebview).toHaveBeenCalledTimes(2) - }) - - it("settingsButtonClicked is a no-op when no visible provider", () => { - ;(ClineProvider.getVisibleInstance as Mock).mockReturnValue(undefined) - - handlers["zoo-code.settingsButtonClicked"]() - + expect(mockProvider.postMessageToWebview).toHaveBeenCalledTimes(2) expect(mockVisibleProvider.postMessageToWebview).not.toHaveBeenCalled() }) - it("historyButtonClicked posts historyButtonClicked action", () => { + it("historyButtonClicked posts historyButtonClicked action on the registered provider", () => { handlers["zoo-code.historyButtonClicked"]() - expect(mockVisibleProvider.postMessageToWebview).toHaveBeenCalledWith({ + expect(TelemetryService.instance.captureTitleButtonClicked).toHaveBeenCalledWith("history") + expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "action", action: "historyButtonClicked", }) + expect(mockVisibleProvider.postMessageToWebview).not.toHaveBeenCalled() }) - it("marketplaceButtonClicked posts marketplaceButtonClicked action", () => { + it("marketplaceButtonClicked posts marketplaceButtonClicked action on the registered provider", () => { handlers["zoo-code.marketplaceButtonClicked"]() - expect(mockVisibleProvider.postMessageToWebview).toHaveBeenCalledWith({ + expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "action", action: "marketplaceButtonClicked", }) + expect(mockVisibleProvider.postMessageToWebview).not.toHaveBeenCalled() + }) + + // The `*InTab` handlers serve the `editor/title` menu: they target the + // instance that owns the tracked tab panel, resolved via + // ClineProvider.getInstanceForView. + const tabHandlerCases: { command: string; actions: string[]; telemetry?: string }[] = [ + { + command: "zoo-code.settingsButtonClickedInTab", + actions: ["settingsButtonClicked", "didBecomeVisible"], + telemetry: "settings", + }, + { command: "zoo-code.historyButtonClickedInTab", actions: ["historyButtonClicked"], telemetry: "history" }, + { command: "zoo-code.marketplaceButtonClickedInTab", actions: ["marketplaceButtonClicked"] }, + ] + it.each(tabHandlerCases)( + "$command targets the tab instance for the tracked tab panel", + ({ command, actions, telemetry }) => { + const mockTabProvider = { postMessageToWebview: vi.fn().mockResolvedValue(undefined) } + // Retain the tracked tab panel and pin the instance lookup + // against its identity: a handler that resolved the sidebar view + // or any other view must fail instead of passing on the stubbed + // provider result alone. + const tabPanel = {} as vscode.WebviewPanel + setPanel(tabPanel, "tab") + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(mockTabProvider) + + handlers[command]() + + expect(ClineProvider.getInstanceForView as Mock).toHaveBeenCalledWith(tabPanel) + for (const action of actions) { + expect(mockTabProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "action", action }) + } + expect(mockTabProvider.postMessageToWebview).toHaveBeenCalledTimes(actions.length) + if (telemetry) { + expect(TelemetryService.instance.captureTitleButtonClicked).toHaveBeenCalledWith(telemetry) + } + expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() + }, + ) + + // The `*InTab` handlers must no-op when there is no live tab instance: a + // missing or disposed tab must not crash the handler or fall back to + // another instance. Every handler is awaited, so an async handler that + // slipped past its guard (rejecting on the missing instance) fails the + // test instead of settling as an unhandled rejection. + const inTabNoOpCommands = [ + "zoo-code.plusButtonClickedInTab", + "zoo-code.settingsButtonClickedInTab", + "zoo-code.historyButtonClickedInTab", + "zoo-code.marketplaceButtonClickedInTab", + ] + it.each(inTabNoOpCommands)("%s is a no-op when no tab panel is tracked", async (command) => { + await handlers[command]() + + expect(ClineProvider.getInstanceForView as Mock).not.toHaveBeenCalled() + expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() + expect(mockVisibleProvider.postMessageToWebview).not.toHaveBeenCalled() + }) + + it.each(inTabNoOpCommands)("%s is a no-op when the tab instance is disposed", async (command) => { + const disposedPanel = {} as vscode.WebviewPanel + setPanel(disposedPanel, "tab") + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(undefined) + + await handlers[command]() + + expect(ClineProvider.getInstanceForView as Mock).toHaveBeenCalledWith(disposedPanel) + expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() + expect(mockVisibleProvider.postMessageToWebview).not.toHaveBeenCalled() }) it("acceptInput posts acceptInput message", () => { @@ -322,6 +407,8 @@ describe("registerCommands handlers", () => { it("focusInput awaits postMessage on the registered provider when a sidebar panel is active", async () => { const fakeSidebar = {} as vscode.WebviewView setPanel(fakeSidebar, "sidebar") + // The sidebar target is resolved through the live-instance lookup. + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(mockProvider) // Same deferred-promise pattern as above. focusInput first awaits // focusPanel() (mocked to resolve sync) and then awaits @@ -352,44 +439,245 @@ describe("registerCommands handlers", () => { }) }) - it("focusInput does not post when no sidebar panel is active", async () => { + it("focusInput does not post when no sidebar panel is tracked", async () => { + await handlers["zoo-code.focusInput"]() + + expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() + }) + + it("focusInput posts to the tab provider when a tab panel is tracked alongside the sidebar", async () => { + setPanel({} as vscode.WebviewView, "sidebar") + const tabPanel = {} as vscode.WebviewPanel + setPanel(tabPanel, "tab") + const tabProvider = { postMessageToWebview: vi.fn().mockResolvedValue(undefined) } + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(tabProvider) + + await handlers["zoo-code.focusInput"]() + + // focusPanel selects the tab when one is tracked, so the focus message follows that + // selection: the tab webview is the one that became visible and must receive it. + expect(ClineProvider.getInstanceForView as Mock).toHaveBeenCalledWith(tabPanel) + expect(tabProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) + expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() + }) + + it("focusInput posts to neither provider when the tracked tab has no live instance", async () => { + // Negative coverage for the both-tracked lookup: focusPanel selects the tab, + // and the focus target is resolved through getInstanceForView(tabPanel). When + // that lookup finds no live instance (tab provider disposed or untracked), the + // sidebar must not receive the post - focusPanel made the tab visible, so a + // sidebar post would focus an input the user cannot see. + setPanel({} as vscode.WebviewView, "sidebar") + const tabPanel = {} as vscode.WebviewPanel + setPanel(tabPanel, "tab") + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(undefined) + + await handlers["zoo-code.focusInput"]() + + expect(ClineProvider.getInstanceForView as Mock).toHaveBeenCalledWith(tabPanel) + expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() + expect(mockVisibleProvider.postMessageToWebview).not.toHaveBeenCalled() + }) + + it("setPanel keeps independent refs: clearing only the tab ref re-enables the sidebar post", async () => { + const sidebarPanel = {} as vscode.WebviewView + setPanel(sidebarPanel, "sidebar") + const tabPanel = {} as vscode.WebviewPanel + setPanel(tabPanel, "tab") + const tabProvider = { postMessageToWebview: vi.fn().mockResolvedValue(undefined) } + ;(ClineProvider.getInstanceForView as Mock).mockImplementation((view: unknown) => + view === tabPanel ? tabProvider : mockProvider, + ) + + // The tab ref does not wipe the sidebar ref, and while it is set the tab is the + // focus target... + await handlers["zoo-code.focusInput"]() + expect(tabProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) + expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() + + // ...and clearing only the tab ref re-enables the sidebar post. + setPanel(undefined, "tab") + await handlers["zoo-code.focusInput"]() + expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) + }) + + // Bidirectional independence: resolving the sidebar AFTER the tab must keep the tab + // as the tracked surface (the reverse order is covered above). + it("setPanel keeps the tab target when the sidebar resolves after it", async () => { + const tabPanel = {} as vscode.WebviewPanel + setPanel(tabPanel, "tab") + const tabProvider = { postMessageToWebview: vi.fn().mockResolvedValue(undefined) } + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(tabProvider) + + const sidebarPanel = {} as vscode.WebviewView + setPanel(sidebarPanel, "sidebar") + + await handlers["zoo-code.focusInput"]() + // getPanel still reports the tab and the tab keeps the focus target: the sidebar + // resolution must not displace it. + expect(getPanel()).toBe(tabPanel) + expect(tabProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) + expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() + }) + + it("clearSidebarPanel ignores a late disposal of a replaced sidebar view", () => { + const first = {} as vscode.WebviewView + const second = {} as vscode.WebviewView + setPanel(first, "sidebar") + setPanel(second, "sidebar") + + // The replaced view's late disposal must not clear the replacement. + clearSidebarPanel(first) + expect(getPanel()).toBe(second) + + // The current view's disposal does clear the slot. + clearSidebarPanel(second) + expect(getPanel()).toBeUndefined() + }) + + it("focusInput skips a tracked sidebar whose view is no longer live", async () => { + setPanel({} as vscode.WebviewView, "sidebar") + // The tracked view's instance is gone: nothing may receive the input focus. + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(undefined) + vi.mocked(focusPanel).mockClear() + await handlers["zoo-code.focusInput"]() expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() + expect(mockVisibleProvider.postMessageToWebview).not.toHaveBeenCalled() + // The disposed view must not even be handed to focusPanel: no post is not enough. + expect(focusPanel).toHaveBeenCalledWith(undefined, undefined) + }) + + it("focusPanel command skips a tracked sidebar whose view is no longer live", async () => { + setPanel({} as vscode.WebviewView, "sidebar") + // The tracked sidebar's instance is gone: the handler must not reveal it. + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(undefined) + vi.mocked(focusPanel).mockClear() + + await handlers["zoo-code.focusPanel"]() + + expect(focusPanel).toHaveBeenCalledWith(undefined, undefined) }) - // Representative coverage for the .catch arm on all five void-prefixed - // postMessageToWebview sites in registerCommands.ts (settingsButtonClicked - // posts twice, plus historyButtonClicked, marketplaceButtonClicked, and - // acceptInput). Each handler is synchronous, so the .catch arm runs on a - // microtask; setImmediate ensures all microtasks are flushed before we assert. The + it("focusPanel command targets the tracked live surfaces", async () => { + const sidebarPanel = {} as vscode.WebviewView + setPanel(sidebarPanel, "sidebar") + const tabPanel = {} as vscode.WebviewPanel + setPanel(tabPanel, "tab") + ;(ClineProvider.getInstanceForView as Mock).mockImplementation((view: unknown) => + view === tabPanel || view === sidebarPanel ? {} : undefined, + ) + vi.mocked(focusPanel).mockClear() + + await handlers["zoo-code.focusPanel"]() + + expect(focusPanel).toHaveBeenCalledWith(tabPanel, sidebarPanel) + }) + + // Coverage for the .catch arm on the sidebar title-bar post sites + // (settingsButtonClicked posts twice, plus historyButtonClicked and + // marketplaceButtonClicked) and acceptInput (the visible-provider path). + // Each handler is synchronous, so the .catch arm runs on a microtask; + // setImmediate ensures all microtasks are flushed before we assert. The // log messages carry a `[]` prefix so multi-failure logs // remain unambiguous; the prefix is per-handler, not per-call (both of - // settingsButtonClicked's posts share the same prefix). + // settingsButtonClicked's posts share the same prefix). Each post rejects + // with its own error and call N is pinned to post N, so a mutant that + // alters one catch's message cannot hide behind the other post's + // identical log. it.each([ - { command: "zoo-code.settingsButtonClicked", prefix: "settingsButtonClicked", expectedCalls: 2 }, - { command: "zoo-code.historyButtonClicked", prefix: "historyButtonClicked", expectedCalls: 1 }, - { command: "zoo-code.marketplaceButtonClicked", prefix: "marketplaceButtonClicked", expectedCalls: 1 }, - { command: "zoo-code.acceptInput", prefix: "acceptInput", expectedCalls: 1 }, + { + command: "zoo-code.settingsButtonClicked", + prefix: "settingsButtonClicked", + errorLabels: ["first post", "second post"], + target: "sidebar" as const, + }, + { + command: "zoo-code.historyButtonClicked", + prefix: "historyButtonClicked", + errorLabels: ["post"], + target: "sidebar" as const, + }, + { + command: "zoo-code.marketplaceButtonClicked", + prefix: "marketplaceButtonClicked", + errorLabels: ["post"], + target: "sidebar" as const, + }, + { command: "zoo-code.acceptInput", prefix: "acceptInput", errorLabels: ["post"], target: "visible" as const }, ])( "$command logs to outputChannel when postMessageToWebview rejects", - async ({ command, prefix, expectedCalls }) => { - const boom = new Error("boom") - mockVisibleProvider.postMessageToWebview.mockReset() - mockVisibleProvider.postMessageToWebview.mockRejectedValue(boom) + async ({ command, prefix, errorLabels, target }) => { + const post = + target === "sidebar" ? mockProvider.postMessageToWebview : mockVisibleProvider.postMessageToWebview + post.mockReset() + const booms = errorLabels.map((label) => new Error(label)) + booms.forEach((boom) => post.mockRejectedValueOnce(boom)) handlers[command]() - // Flush microtasks so the chained .catch arm runs. + // Flush microtasks so the chained .catch arms run. await new Promise((resolve) => setImmediate(resolve)) - expect(mockOutputChannel.appendLine).toHaveBeenCalledTimes(expectedCalls) - expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( - `[${prefix}] postMessageToWebview failed: ${boom}`, - ) + expect(mockOutputChannel.appendLine).toHaveBeenCalledTimes(booms.length) + booms.forEach((boom, index) => { + expect(mockOutputChannel.appendLine).toHaveBeenNthCalledWith( + index + 1, + `[${prefix}] postMessageToWebview failed: ${boom}`, + ) + }) }, ) + // The two posts reject with distinct errors and the nth-call assertions + // pin each catch's message, so neither template literal can survive + // behind the other post's identical log. + it("settingsButtonClickedInTab logs to outputChannel when postMessageToWebview rejects", async () => { + const booms = [new Error("first post"), new Error("second post")] + const mockTabProvider = { + postMessageToWebview: vi.fn().mockRejectedValueOnce(booms[0]).mockRejectedValueOnce(booms[1]), + } + setPanel({} as vscode.WebviewPanel, "tab") + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(mockTabProvider) + + handlers["zoo-code.settingsButtonClickedInTab"]() + + // Flush microtasks so the chained .catch arms run. + await new Promise((resolve) => setImmediate(resolve)) + + expect(mockOutputChannel.appendLine).toHaveBeenCalledTimes(2) + expect(mockOutputChannel.appendLine).toHaveBeenNthCalledWith( + 1, + `[settingsButtonClickedInTab] postMessageToWebview failed: ${booms[0]}`, + ) + expect(mockOutputChannel.appendLine).toHaveBeenNthCalledWith( + 2, + `[settingsButtonClickedInTab] postMessageToWebview failed: ${booms[1]}`, + ) + }) + + // The history and marketplace InTab catch sites share the identical + // single-post pattern (their sidebar equivalents are covered by the + // it.each above); pin their exact messages too. + it.each([ + { command: "zoo-code.historyButtonClickedInTab", prefix: "historyButtonClickedInTab" }, + { command: "zoo-code.marketplaceButtonClickedInTab", prefix: "marketplaceButtonClickedInTab" }, + ])("$command logs to outputChannel when the tab postMessageToWebview rejects", async ({ command, prefix }) => { + const boom = new Error("post") + const mockTabProvider = { postMessageToWebview: vi.fn().mockRejectedValue(boom) } + setPanel({} as vscode.WebviewPanel, "tab") + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(mockTabProvider) + + handlers[command]() + + // Flush microtasks so the chained .catch arm runs. + await new Promise((resolve) => setImmediate(resolve)) + + expect(mockOutputChannel.appendLine).toHaveBeenCalledTimes(1) + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith(`[${prefix}] postMessageToWebview failed: ${boom}`) + }) + it("toggleAutoApprove logs to outputChannel when postMessageToWebview rejects", async () => { // toggleAutoApprove is `async` and awaits postMessageToWebview inside a // try/catch (rather than relying on a `.catch` microtask like the @@ -407,31 +695,52 @@ describe("registerCommands handlers", () => { ) }) - it("plusButtonClicked calls evictCurrentTask on the visible provider", async () => { - const evictCurrentTask = vi.fn().mockResolvedValue(undefined) - const refreshWorkspace = vi.fn().mockResolvedValue(undefined) - ;(mockVisibleProvider as any).evictCurrentTask = evictCurrentTask - ;(mockVisibleProvider as any).refreshWorkspace = refreshWorkspace - + it("plusButtonClicked calls evictCurrentTask on the registered sidebar provider", async () => { await handlers["zoo-code.plusButtonClicked"]() - expect(evictCurrentTask).toHaveBeenCalledTimes(1) + expect(TelemetryService.instance.captureTitleButtonClicked).toHaveBeenCalledWith("plus") + expect(mockProvider.evictCurrentTask).toHaveBeenCalledTimes(1) + expect(mockProvider.refreshWorkspace).toHaveBeenCalledTimes(1) + expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "action", action: "chatButtonClicked" }) + expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) }) - it("plusButtonClicked is a no-op when no visible provider", async () => { - ;(ClineProvider.getVisibleInstance as Mock).mockReturnValue(undefined) - - // Should not throw even with no visible provider - await handlers["zoo-code.plusButtonClicked"]() + it("plusButtonClickedInTab evicts and posts on the tab instance for the tracked tab panel", async () => { + const mockTabProvider = { + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + evictCurrentTask: vi.fn().mockResolvedValue(undefined), + refreshWorkspace: vi.fn().mockResolvedValue(undefined), + } + // Same identity pin as the other InTab cases: the eviction must run + // against the provider resolved from the exact tracked tab panel. + const tabPanel = {} as vscode.WebviewPanel + setPanel(tabPanel, "tab") + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(mockTabProvider) + + await handlers["zoo-code.plusButtonClickedInTab"]() + + expect(ClineProvider.getInstanceForView as Mock).toHaveBeenCalledWith(tabPanel) + expect(TelemetryService.instance.captureTitleButtonClicked).toHaveBeenCalledWith("plus") + expect(mockTabProvider.evictCurrentTask).toHaveBeenCalledTimes(1) + expect(mockTabProvider.refreshWorkspace).toHaveBeenCalledTimes(1) + expect(mockTabProvider.postMessageToWebview).toHaveBeenCalledWith({ + type: "action", + action: "chatButtonClicked", + }) + expect(mockTabProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) }) }) describe("openClineInNewTab", () => { let mockOutputChannel: vscode.OutputChannel let mockContext: vscode.ExtensionContext + // Upstream's openClineInNewTab now takes a focus tracker; every call in this + // block passes the same instance so the constructor assertions can name it. + let tabFocusTracker: WebviewFocusTracker beforeEach(() => { vi.clearAllMocks() + tabFocusTracker = new WebviewFocusTracker() mockOutputChannel = { appendLine: vi.fn(), @@ -473,6 +782,9 @@ describe("openClineInNewTab", () => { undefined, ) + // No tab was tracked, so the reuse path (and its instance lookup) + // must not run. + expect(ClineProvider.getInstanceForView as Mock).not.toHaveBeenCalled() expect(vscode.window.createWebviewPanel).toHaveBeenCalledWith( "zoo-code.TabPanelProvider", "Zoo Code", @@ -483,4 +795,427 @@ describe("openClineInNewTab", () => { }), ) }) + + it("reveals the existing tab instead of creating a second panel", async () => { + const mockExistingProvider = { postMessageToWebview: vi.fn().mockResolvedValue(undefined) } + const mockPanel = { + webview: { postMessage: vi.fn() }, + onDidChangeViewState: vi.fn(), + onDidDispose: vi.fn(), + reveal: vi.fn().mockResolvedValue(undefined), + } as unknown as vscode.WebviewPanel + setPanel(mockPanel, "tab") + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(mockExistingProvider) + + const result = await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + + expect(result).toBe(mockExistingProvider) + expect(mockPanel.reveal).toHaveBeenCalledTimes(1) + expect(vscode.window.createWebviewPanel).not.toHaveBeenCalled() + expect(mockExistingProvider.postMessageToWebview).toHaveBeenCalledWith({ + type: "action", + action: "didBecomeVisible", + }) + }) + + it("creates a new tab panel when the tracked tab's provider has been disposed", async () => { + const mockPanel = { + webview: { postMessage: vi.fn() }, + onDidChangeViewState: vi.fn(), + onDidDispose: vi.fn(), + reveal: vi.fn().mockResolvedValue(undefined), + } as unknown as vscode.WebviewPanel + setPanel(mockPanel, "tab") + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(undefined) + + await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + + expect(mockPanel.reveal).not.toHaveBeenCalled() + expect(vscode.window.createWebviewPanel).toHaveBeenCalledTimes(1) + }) + + it("falls back to an undefined MdmService when MdmService.getInstance throws", async () => { + ;(MdmService.getInstance as Mock).mockImplementation(() => { + throw new Error("MDM service not initialized") + }) + + const provider = await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + + // The creation must survive the MDM lookup failure: the provider is + // constructed with an undefined MDM service and the tab panel is + // still created. + const ctor = ClineProvider as unknown as Mock + expect(ctor.mock.instances[0]).toBeDefined() + expect(ctor).toHaveBeenCalledWith( + mockContext, + mockOutputChannel, + "editor", + undefined, + tabFocusTracker, + undefined, + ) + expect(provider).toBe(ctor.mock.instances[0]) + expect(vscode.window.createWebviewPanel).toHaveBeenCalledTimes(1) + + // The fallback is observable in the output channel. + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + "[openClineInNewTab] MDM service unavailable, continuing without it: Error: MDM service not initialized", + ) + }) + + it("opens a new group to the right and targets ViewColumn.Two when no editors are visible", async () => { + ;(vscode.window as unknown as { visibleTextEditors: vscode.TextEditor[] }).visibleTextEditors = [] + + await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + + expect(vscode.commands.executeCommand).toHaveBeenCalledWith("workbench.action.newGroupRight") + expect(vscode.commands.executeCommand).toHaveBeenCalledWith("workbench.action.lockEditorGroup") + expect(vscode.window.createWebviewPanel).toHaveBeenCalledWith( + "zoo-code.TabPanelProvider", + "Zoo Code", + vscode.ViewColumn.Two, + { + enableScripts: true, + retainContextWhenHidden: true, + localResourceRoots: [mockContext.extensionUri], + }, + ) + + // The panel icon points at the extension's asset files. + const panel = (vscode.window.createWebviewPanel as Mock).mock.results[0].value as { + iconPath?: { light: { path: string }; dark: { path: string } } + } + expect(panel.iconPath).toEqual({ + light: { path: "assets/icons/panel_light.png" }, + dark: { path: "assets/icons/panel_dark.png" }, + }) + }) + + it("treats editors without a viewColumn as column 0 when computing the target column", async () => { + // openClineInNewTab only reads viewColumn from each editor, so the + // fixture keeps that single field. + const editorWithoutColumn = { viewColumn: undefined } as unknown as vscode.TextEditor + ;(vscode.window as unknown as { visibleTextEditors: vscode.TextEditor[] }).visibleTextEditors = [ + editorWithoutColumn, + ] + + await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + + // lastCol falls back to 0, so the panel lands on column 1 instead of + // opening a new editor group. + expect(vscode.commands.executeCommand).not.toHaveBeenCalledWith("workbench.action.newGroupRight") + expect(vscode.window.createWebviewPanel).toHaveBeenCalledWith( + "zoo-code.TabPanelProvider", + "Zoo Code", + 1, + expect.objectContaining({ enableScripts: true }), + ) + }) + + it("places the tab panel one column right of the rightmost visible editor", async () => { + // openClineInNewTab only reads viewColumn from each editor, so the + // fixtures keep that single field. + ;(vscode.window as unknown as { visibleTextEditors: vscode.TextEditor[] }).visibleTextEditors = [ + { viewColumn: 1 } as unknown as vscode.TextEditor, + { viewColumn: 3 } as unknown as vscode.TextEditor, + ] + + await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + + // lastCol is 3, so the panel lands on column 4 without opening a new + // editor group. + expect(vscode.commands.executeCommand).not.toHaveBeenCalledWith("workbench.action.newGroupRight") + expect(vscode.window.createWebviewPanel).toHaveBeenCalledWith( + "zoo-code.TabPanelProvider", + "Zoo Code", + 4, + expect.objectContaining({ enableScripts: true }), + ) + }) + + it("constructs the tab provider with the 'editor' context and the live MdmService instance", async () => { + const mockMdm = { name: "mock-mdm" } + // MdmService has a private constructor, so pin a sentinel stand-in. + ;(MdmService.getInstance as Mock).mockReturnValue(mockMdm as unknown as MdmService) + + const provider = await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + + const ctor = ClineProvider as unknown as Mock + expect(ctor).toHaveBeenCalledTimes(1) + expect(ctor).toHaveBeenCalledWith(mockContext, mockOutputChannel, "editor", undefined, tabFocusTracker, mockMdm) + expect(provider).toBe(ctor.mock.instances[0]) + }) + + it("posts didBecomeVisible only for visible state changes and clears the tracked tab on dispose", async () => { + await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + + // Retain the panel returned during creation and pin the tracked tab + // against it with identity (not a weak defined check), so a wrong or + // duplicated tracked panel fails before the dispose assertions. + const panel = (vscode.window.createWebviewPanel as Mock).mock.results[0].value as { + onDidChangeViewState: Mock + onDidDispose: Mock + } + expect(getPanel()).toBe(panel) + + const stateHandler = panel.onDidChangeViewState.mock.calls[0][0] as (event: { + webviewPanel: { visible: boolean; webview: { postMessage: (message: unknown) => void } } + }) => void + const visibleEvent = { webviewPanel: { visible: true, webview: { postMessage: vi.fn() } } } + stateHandler(visibleEvent) + expect(visibleEvent.webviewPanel.webview.postMessage).toHaveBeenCalledWith({ + type: "action", + action: "didBecomeVisible", + }) + + const hiddenEvent = { webviewPanel: { visible: false, webview: { postMessage: vi.fn() } } } + stateHandler(hiddenEvent) + expect(hiddenEvent.webviewPanel.webview.postMessage).not.toHaveBeenCalled() + + const disposeHandler = panel.onDidDispose.mock.calls[0][0] as () => void + disposeHandler() + expect(getPanel()).toBeUndefined() + }) + + it("serializes concurrent opens so overlapping calls create one panel and share one provider", async () => { + const [first, second] = await Promise.all([ + openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }), + openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }), + ]) + + // Overlapping "Open in editor" calls must share the in-flight + // creation: exactly one tab panel is created and both callers receive + // the same constructed provider. Pinning both results against the + // mocked constructor (not just against each other) keeps the test + // failing if the shared result is undefined. + const ctor = ClineProvider as unknown as Mock + const constructed = ctor.mock.instances[0] + expect(constructed).toBeDefined() + expect(first).toBe(constructed) + expect(second).toBe(constructed) + expect(vscode.window.createWebviewPanel).toHaveBeenCalledTimes(1) + }) + + it("shares one in-flight creation when openInNewTab and popoutButtonClicked start before it resolves", async () => { + // Defer the first creation at ContextProxy.getInstance so both command + // handlers can start while the creation is still in flight. + let resolveContextProxy!: () => void + ;(ContextProxy.getInstance as Mock).mockReturnValue( + new Promise((resolve) => { + resolveContextProxy = resolve + }), + ) + + const commandHandlers: Record unknown> = {} + ;(vscode.commands.registerCommand as Mock).mockImplementation( + (id: string, cb: (...args: unknown[]) => unknown) => { + commandHandlers[id] = cb + return { dispose: vi.fn() } + }, + ) + const sidebarProvider = { postMessageToWebview: vi.fn().mockResolvedValue(undefined) } + registerCommands({ + context: mockContext, + outputChannel: mockOutputChannel, + provider: sidebarProvider as unknown as ClineProvider, + }) + + const started = [commandHandlers["zoo-code.openInNewTab"](), commandHandlers["zoo-code.popoutButtonClicked"]()] + + // While the shared creation is suspended at ContextProxy.getInstance, + // neither caller has created a panel yet. + expect(vscode.window.createWebviewPanel).not.toHaveBeenCalled() + + resolveContextProxy() + const [first, second] = await Promise.all(started) + + // Both command entry points await the shared in-flight creation: + // exactly one tab panel is created and both results are the same + // constructed provider. + const ctor = ClineProvider as unknown as Mock + const constructed = ctor.mock.instances[0] + expect(constructed).toBeDefined() + expect(first).toBe(constructed) + expect(second).toBe(constructed) + expect(vscode.window.createWebviewPanel).toHaveBeenCalledTimes(1) + }) + + it("creates a fresh panel for a new call once the previous creation settled and its provider disposed", async () => { + // The first open settles and tracks its panel. + await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + expect(vscode.window.createWebviewPanel).toHaveBeenCalledTimes(1) + + // The tracked provider is disposed, so the next open cannot reuse the + // existing tab: the settled (and cleared) in-flight promise must not + // be returned, and a fresh panel is created. + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(undefined) + + const secondProvider = await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + + const ctor = ClineProvider as unknown as Mock + const second = ctor.mock.instances[1] + expect(second).toBeDefined() + expect(secondProvider).toBe(second) + expect(vscode.window.createWebviewPanel).toHaveBeenCalledTimes(2) + }) + + it("does not reuse a rejected creation: a later call creates a fresh panel", async () => { + // Creation fails partway through. The in-flight slot has to be cleared even on + // rejection, otherwise every later "Open in editor" receives this same rejected + // promise and the command is permanently broken for the session. + const failure = new Error("ContextProxy unavailable") + ;(ContextProxy.getInstance as Mock).mockRejectedValueOnce(failure) + + await expect( + openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }), + ).rejects.toThrow("ContextProxy unavailable") + + const provider = await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + + const ctor = ClineProvider as unknown as Mock + expect(ctor.mock.instances[0]).toBeDefined() + expect(provider).toBe(ctor.mock.instances[0]) + expect(vscode.window.createWebviewPanel).toHaveBeenCalledTimes(1) + }) + + it("hands the same rejection to every caller sharing a failed creation", async () => { + const failure = new Error("ContextProxy unavailable") + // Once only: the two callers share one creation, and a persistent rejection would + // leak into the next test. + ;(ContextProxy.getInstance as Mock).mockRejectedValueOnce(failure) + + const open = () => + openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + + const results = await Promise.allSettled([open(), open()]) + + // Overlapping callers share one creation, so both observe the original rejection + // rather than one of them silently receiving a later, unrelated result. + expect(results.map((result) => result.status)).toEqual(["rejected", "rejected"]) + expect((results[0] as PromiseRejectedResult).reason).toBe(failure) + expect((results[1] as PromiseRejectedResult).reason).toBe(failure) + expect(vscode.window.createWebviewPanel).not.toHaveBeenCalled() + }) + + it("keeps the replacement panel tracked when a stale panel's disposal fires late", async () => { + // Capture each created panel so the first panel's (stale) dispose + // handler can fire after the replacement is already tracked. + const createdPanels: { onDidDispose: Mock }[] = [] + ;(vscode.window.createWebviewPanel as Mock).mockImplementation(() => { + const panel = { + webview: { postMessage: vi.fn() }, + onDidChangeViewState: vi.fn(), + onDidDispose: vi.fn(), + } + createdPanels.push(panel) + return panel + }) + + // First open creates and tracks panel A. + await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + expect(getPanel()).toBe(createdPanels[0]) + + // Panel A's provider is disposed before the second open, so the + // second open creates the replacement panel B. + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(undefined) + await openClineInNewTab({ + context: mockContext, + outputChannel: mockOutputChannel, + webviewFocusTracker: tabFocusTracker, + }) + expect(vscode.window.createWebviewPanel).toHaveBeenCalledTimes(2) + expect(getPanel()).toBe(createdPanels[1]) + + // Panel A's stale dispose handler fires after the replacement is + // tracked; it must not clobber the replacement's ref. + createdPanels[0].onDidDispose.mock.calls[0][0]() + + expect(getPanel()).toBe(createdPanels[1]) + + // Tab-surface commands still reach the provider that owns the + // replacement panel after the stale disposal. + const replacementProvider = { postMessageToWebview: vi.fn().mockResolvedValue(undefined) } + ;(ClineProvider.getInstanceForView as Mock).mockReturnValue(replacementProvider) + const commandHandlers: Record unknown> = {} + ;(vscode.commands.registerCommand as Mock).mockImplementation( + (id: string, cb: (...args: unknown[]) => unknown) => { + commandHandlers[id] = cb + return { dispose: vi.fn() } + }, + ) + registerCommands({ + context: mockContext, + outputChannel: mockOutputChannel, + provider: {} as ClineProvider, + }) + await commandHandlers["zoo-code.historyButtonClickedInTab"]() + expect(replacementProvider.postMessageToWebview).toHaveBeenCalledWith({ + type: "action", + action: "historyButtonClicked", + }) + }) }) diff --git a/src/activate/registerCommands.ts b/src/activate/registerCommands.ts index 9ee3443f9a..e7e38cd765 100644 --- a/src/activate/registerCommands.ts +++ b/src/activate/registerCommands.ts @@ -1,7 +1,7 @@ import * as vscode from "vscode" import delay from "delay" -import type { CommandId } from "@roo-code/types" +import type { CommandId, ExtensionMessage } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" import { getCommand } from "../utils/commands" @@ -30,16 +30,28 @@ export function getVisibleProviderOrLog(outputChannel: vscode.OutputChannel): Cl let sidebarPanel: vscode.WebviewView | undefined = undefined let tabPanel: vscode.WebviewPanel | undefined = undefined +// In-flight "open in editor" creation shared by overlapping calls: a +// double-click starts before the first call tracks its new panel, so +// concurrent callers must share one creation instead of racing to create +// two tab panels. +let pendingTabPanelCreation: Promise | undefined + /** * Get the currently active panel * @returns WebviewPanel或WebviewView + * @internal Test-only seam: the extension host no longer reads this directly. */ export function getPanel(): vscode.WebviewPanel | vscode.WebviewView | undefined { return tabPanel || sidebarPanel } /** - * Set panel references + * Set panel references. + * + * The two refs are independent: each surface keeps its own ref for its whole + * lifetime, so resolving the sidebar view never wipes a live tab panel (and + * vice versa). Callers pass `undefined` only when the surface itself is + * disposed (see the `onDidDispose` wiring in `openClineInNewTab`). */ export function setPanel( newPanel: vscode.WebviewPanel | vscode.WebviewView | undefined, @@ -47,13 +59,35 @@ export function setPanel( ): void { if (type === "sidebar") { sidebarPanel = newPanel as vscode.WebviewView - tabPanel = undefined } else { tabPanel = newPanel as vscode.WebviewPanel + } +} + +/** + * Clears the tracked sidebar view only when it is still the disposed one. + * + * A sidebar view can be resolved again while the previous view is being + * disposed; the late disposal of the replaced view must not clear the + * replacement, or focus/commands would lose the live sidebar. + */ +export function clearSidebarPanel(view: vscode.WebviewView | vscode.WebviewPanel): void { + if (sidebarPanel === view) { sidebarPanel = undefined } } +/** + * The instance that owns the tracked tab panel, if it is still alive. + * + * Title-bar commands on the editor-tab surface use this instead of the + * visible-instance heuristic, so a click on the tab's title bar always + * targets that tab even when the sidebar is visible side-by-side. + */ +function getTabProvider(): ClineProvider | undefined { + return tabPanel ? ClineProvider.getInstanceForView(tabPanel) : undefined +} + export type RegisterCommandOptions = { context: vscode.ExtensionContext outputChannel: vscode.OutputChannel @@ -83,27 +117,58 @@ export const registerCommands = (options: RegisterCommandOptions) => { // `filePath?: string`, others take none) and VS Code dispatches positional // args dynamically. type CommandCallback = (...args: any[]) => unknown + +// Posts each action in order to the target instance. Failures are logged +// (not thrown) with the handler-specific prefix so a failed post stays +// attributable in the output channel. +const postActions = ( + outputChannel: vscode.OutputChannel, + target: ClineProvider, + actions: readonly NonNullable[], + logPrefix: string, +) => { + for (const action of actions) { + void target + .postMessageToWebview({ type: "action", action }) + .catch((error) => outputChannel.appendLine(`[${logPrefix}] postMessageToWebview failed: ${error}`)) + } +} + const getCommandsMap = ({ context, outputChannel, provider, }: RegisterCommandOptions): Record, CommandCallback> => ({ activationCompleted: () => {}, + // The `view/title` menu is scoped to the sidebar view, so the click + // origin of these handlers is the sidebar provider wired in at + // activation (`provider`). Target it directly instead of the + // visible-instance heuristic, which would follow the user's focus to a + // tab instance when both surfaces are open side-by-side. The `*InTab` + // variants serve the `editor/title` menu and target the tab instance + // through `getTabProvider()` instead. plusButtonClicked: async () => { - const visibleProvider = getVisibleProviderOrLog(outputChannel) + TelemetryService.instance.captureTitleButtonClicked("plus") - if (!visibleProvider) { + await provider.evictCurrentTask() + await provider.refreshWorkspace() + await provider.postMessageToWebview({ type: "action", action: "chatButtonClicked" }) + // Send focusInput action immediately after chatButtonClicked + // This ensures the focus happens after the view has switched + await provider.postMessageToWebview({ type: "action", action: "focusInput" }) + }, + plusButtonClickedInTab: async () => { + const tabProvider = getTabProvider() + if (!tabProvider) { return } TelemetryService.instance.captureTitleButtonClicked("plus") - await visibleProvider.evictCurrentTask() - await visibleProvider.refreshWorkspace() - await visibleProvider.postMessageToWebview({ type: "action", action: "chatButtonClicked" }) - // Send focusInput action immediately after chatButtonClicked - // This ensures the focus happens after the view has switched - await visibleProvider.postMessageToWebview({ type: "action", action: "focusInput" }) + await tabProvider.evictCurrentTask() + await tabProvider.refreshWorkspace() + await tabProvider.postMessageToWebview({ type: "action", action: "chatButtonClicked" }) + await tabProvider.postMessageToWebview({ type: "action", action: "focusInput" }) }, popoutButtonClicked: () => { TelemetryService.instance.captureTitleButtonClicked("popout") @@ -113,43 +178,50 @@ const getCommandsMap = ({ openInNewTab: () => openClineInNewTab({ context, outputChannel, webviewFocusTracker: provider.webviewFocusTracker }), settingsButtonClicked: () => { - const visibleProvider = getVisibleProviderOrLog(outputChannel) + TelemetryService.instance.captureTitleButtonClicked("settings") - if (!visibleProvider) { + // Also explicitly post the visibility message to trigger scroll reliably. + postActions(outputChannel, provider, ["settingsButtonClicked", "didBecomeVisible"], "settingsButtonClicked") + }, + settingsButtonClickedInTab: () => { + const tabProvider = getTabProvider() + if (!tabProvider) { return } TelemetryService.instance.captureTitleButtonClicked("settings") - void visibleProvider - .postMessageToWebview({ type: "action", action: "settingsButtonClicked" }) - .catch((error) => outputChannel.appendLine(`[settingsButtonClicked] postMessageToWebview failed: ${error}`)) - // Also explicitly post the visibility message to trigger scroll reliably - void visibleProvider - .postMessageToWebview({ type: "action", action: "didBecomeVisible" }) - .catch((error) => outputChannel.appendLine(`[settingsButtonClicked] postMessageToWebview failed: ${error}`)) + postActions( + outputChannel, + tabProvider, + ["settingsButtonClicked", "didBecomeVisible"], + "settingsButtonClickedInTab", + ) }, historyButtonClicked: () => { - const visibleProvider = getVisibleProviderOrLog(outputChannel) + TelemetryService.instance.captureTitleButtonClicked("history") - if (!visibleProvider) { + postActions(outputChannel, provider, ["historyButtonClicked"], "historyButtonClicked") + }, + historyButtonClickedInTab: () => { + const tabProvider = getTabProvider() + if (!tabProvider) { return } TelemetryService.instance.captureTitleButtonClicked("history") - void visibleProvider - .postMessageToWebview({ type: "action", action: "historyButtonClicked" }) - .catch((error) => outputChannel.appendLine(`[historyButtonClicked] postMessageToWebview failed: ${error}`)) + postActions(outputChannel, tabProvider, ["historyButtonClicked"], "historyButtonClickedInTab") }, marketplaceButtonClicked: () => { - const visibleProvider = getVisibleProviderOrLog(outputChannel) - if (!visibleProvider) return - void visibleProvider - .postMessageToWebview({ type: "action", action: "marketplaceButtonClicked" }) - .catch((error) => - outputChannel.appendLine(`[marketplaceButtonClicked] postMessageToWebview failed: ${error}`), - ) + postActions(outputChannel, provider, ["marketplaceButtonClicked"], "marketplaceButtonClicked") + }, + marketplaceButtonClickedInTab: () => { + const tabProvider = getTabProvider() + if (!tabProvider) { + return + } + postActions(outputChannel, tabProvider, ["marketplaceButtonClicked"], "marketplaceButtonClickedInTab") }, newTask: (params: { prompt?: string } | null | undefined) => handleNewTask(params, provider.webviewFocusTracker), setCustomStoragePath: async () => { @@ -174,19 +246,33 @@ const getCommandsMap = ({ }, focusInput: async () => { try { - await focusPanel(tabPanel, sidebarPanel) - - // Send focus input message only for sidebar panels - if (sidebarPanel && getPanel() === sidebarPanel) { - await provider.postMessageToWebview({ type: "action", action: "focusInput" }) - } + // Only a currently tracked, live sidebar is a focus candidate: a tracked ref + // whose view was disposed or replaced must not receive the input focus. + const liveSidebarPanel = + sidebarPanel && ClineProvider.getInstanceForView(sidebarPanel) ? sidebarPanel : undefined + await focusPanel(tabPanel, liveSidebarPanel) + + // focusPanel selects the TAB whenever a tab panel is tracked, so the input + // focus has to go to whichever provider focusPanel actually focused: the tab's + // when one is tracked, otherwise the sidebar's, and nothing when neither is + // available. The previous guard skipped the post as soon as a tab existed, so + // in the both-tracked state the tab became visible without focusing its input. + const focusTarget = tabPanel + ? getTabProvider() + : liveSidebarPanel + ? ClineProvider.getInstanceForView(liveSidebarPanel) + : undefined + await focusTarget?.postMessageToWebview({ type: "action", action: "focusInput" }) } catch (error) { outputChannel.appendLine(`Error focusing input: ${error}`) } }, focusPanel: async () => { try { - await focusPanel(tabPanel, sidebarPanel) + // A stale sidebar ref would make focusPanel reveal a disposed view. + const liveSidebarPanel = + sidebarPanel && ClineProvider.getInstanceForView(sidebarPanel) ? sidebarPanel : undefined + await focusPanel(tabPanel, liveSidebarPanel) } catch (error) { outputChannel.appendLine(`Error focusing panel: ${error}`) } @@ -227,6 +313,53 @@ type OpenClineInNewTabOptions = { } export const openClineInNewTab = async ({ context, outputChannel, webviewFocusTracker }: OpenClineInNewTabOptions) => { + // Serialize overlapping "Open in editor" calls: a double-click starts + // before the first call tracks its new panel, so without a shared + // in-flight creation both calls would race to create two tab panels. + // The shared promise is stored before the creation body awaits + // ContextProxy.getInstance, so every caller started while the creation + // is in flight — openInNewTab and popoutButtonClicked both dispatch + // through here — awaits it instead of creating a second panel: exactly + // one panel is created and every caller receives the same provider. + if (pendingTabPanelCreation) { + return pendingTabPanelCreation + } + + const creation = createTabPanelUnlocked({ context, outputChannel, webviewFocusTracker }) + pendingTabPanelCreation = creation + + try { + return await creation + } finally { + // Clear once settled (success or failure) so the next call starts + // fresh: the reuse path in createTabPanelUnlocked then takes over + // for the tracked panel. Guard the clear so this settlement cannot + // clobber a replacement already stored in the slot. That clobber is + // unreachable in single-threaded settlement order: while the slot + // holds this in-flight creation, every other caller receives that + // same promise (guard above), so no replacement can be stored before + // this finally block runs — the equality check pins the invariant. + // Stryker disable next-line ConditionalExpression: defensive clobber guard, unreachable per the ordering argument above. + if (pendingTabPanelCreation === creation) { + pendingTabPanelCreation = undefined + } + } +} + +// The unserialized tab-creation body. Only openClineInNewTab may call it, +// after it has stored the shared in-flight promise. +const createTabPanelUnlocked = async ({ context, outputChannel, webviewFocusTracker }: OpenClineInNewTabOptions) => { + // Reuse the tracked tab instead of opening a second one: a repeated + // "Open in editor" click reveals the existing tab's panel. + if (tabPanel) { + const existingProvider = ClineProvider.getInstanceForView(tabPanel) + if (existingProvider) { + await tabPanel.reveal() + await existingProvider.postMessageToWebview({ type: "action", action: "didBecomeVisible" }) + return existingProvider + } + } + // (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). @@ -237,8 +370,9 @@ export const openClineInNewTab = async ({ context, outputChannel, webviewFocusTr let mdmService: MdmService | undefined try { mdmService = MdmService.getInstance() - } catch { - // MDM service not initialized, which is fine - extension can work without it + } catch (error) { + // MDM service unavailable: log the fallback and continue without it. + outputChannel.appendLine(`[openClineInNewTab] MDM service unavailable, continuing without it: ${error}`) mdmService = undefined } @@ -269,6 +403,7 @@ export const openClineInNewTab = async ({ context, outputChannel, webviewFocusTr }) // Save as tab type panel. + // Stryker disable next-line StringLiteral: setPanel branches only on type === "sidebar", so any other literal routes to the identical tab-ref assignment setPanel(newPanel, "tab") // TODO: Use better svg icon with light and dark variants (see @@ -292,10 +427,15 @@ export const openClineInNewTab = async ({ context, outputChannel, webviewFocusTr context.subscriptions, // Register listener for disposal ) - // Handle panel closing events. + // Handle panel closing events: clear the tracked ref only if this panel + // is still the tracked one, so a late disposal of an already-replaced + // panel cannot clobber the replacement's ref. newPanel.onDidDispose( () => { - setPanel(undefined, "tab") + if (tabPanel === newPanel) { + // Stryker disable next-line StringLiteral: setPanel branches only on type === "sidebar", so any other literal routes to the identical tab-ref assignment + setPanel(undefined, "tab") + } }, null, context.subscriptions, // Also register dispose listener diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index cdd0975c5d..9628eec401 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -102,7 +102,7 @@ import { getWorkspaceGitInfo } from "../../utils/git" import { getWorkspacePath } from "../../utils/path" import { OrganizationAllowListViolationError } from "../../utils/errors" -import { setPanel } from "../../activate/registerCommands" +import { clearSidebarPanel, setPanel } from "../../activate/registerCommands" import { t } from "../../i18n" @@ -207,6 +207,10 @@ export class ClineProvider private static activeInstances: Set = new Set() private disposables: vscode.Disposable[] = [] private webviewDisposables: vscode.Disposable[] = [] + // Listeners created while resolving one surface. VS Code can resolve a replacement view + // before the previous view's disposal callback runs, so cleanup is scoped to the surface + // that actually went away instead of draining listeners the replacement now owns. + private viewScopedDisposables = new Map() private pendingThemeFixtureProbes = new Map< string, { @@ -800,8 +804,42 @@ export class ClineProvider - https://vscode-docs.readthedocs.io/en/stable/extensions/patterns-and-principles/ - https://github.com/microsoft/vscode-extension-samples/blob/main/webview-sample/src/extension.ts */ + /** Record a disposable against the surface it was created for, in addition to the flat list. */ + private trackViewDisposable(view: vscode.WebviewView | vscode.WebviewPanel, disposable: vscode.Disposable): void { + this.webviewDisposables.push(disposable) + const scoped = this.viewScopedDisposables.get(view) + if (scoped) { + scoped.push(disposable) + } else { + this.viewScopedDisposables.set(view, [disposable]) + } + } + + /** + * Dispose only the listeners created for one surface. A replacement resolved before the old + * view's disposal callback keeps its own listeners. + */ + private clearViewScopedDisposables(view: vscode.WebviewView | vscode.WebviewPanel): void { + const scoped = this.viewScopedDisposables.get(view) + if (!scoped) { + return + } + for (const disposable of scoped) { + if (!disposable) { + continue + } + const index = this.webviewDisposables.indexOf(disposable) + if (index >= 0) { + this.webviewDisposables.splice(index, 1) + } + disposable.dispose() + } + this.viewScopedDisposables.delete(view) + } + private clearWebviewResources() { this.rejectPendingThemeFixtureProbes(new Error("Webview was disposed before the theme fixture probe completed")) + this.viewScopedDisposables.clear() while (this.webviewDisposables.length) { const x = this.webviewDisposables.pop() if (x) { @@ -904,6 +942,23 @@ export class ClineProvider return Array.from(this.activeInstances) } + /** + * Returns the live instance whose current view is the given view or panel, + * if any. Title-bar commands on a specific surface use this to target the + * instance that owns that surface rather than the visible-instance + * heuristic (which picks whichever surface the user last focused). + */ + public static getInstanceForView(view: vscode.WebviewView | vscode.WebviewPanel): ClineProvider | undefined { + // A provider whose disposal has started is still listed in activeInstances until + // dispose() finishes its ASYNCHRONOUS task cleanup (the set removal happens after + // it), so a command queued during that window would otherwise be handed a dying + // provider and re-run task eviction and workspace refresh on top of the disposal + // already performing them. A disposing instance never resolves a view lookup, which + // also means every *InTab handler bails on undefined instead of mutating a disposed + // task. + return Array.from(this.activeInstances).find((instance) => instance.view === view && !instance._disposed) + } + public static async getInstance(): Promise { let visibleProvider = ClineProvider.getVisibleInstance() @@ -1063,8 +1118,8 @@ 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)) + this.setWebviewMessageListener(webviewView.webview, webviewView) + this.trackViewDisposable(webviewView, this.webviewFocusTracker.init(this, webviewView)) // Listen for when the panel becomes visible. // https://github.com/microsoft/vscode-discussions/discussions/840 @@ -1079,7 +1134,7 @@ export class ClineProvider } }) - this.webviewDisposables.push(viewStateDisposable) + this.trackViewDisposable(webviewView, viewStateDisposable) } else if ("onDidChangeVisibility" in webviewView) { // sidebar const visibilityDisposable = webviewView.onDidChangeVisibility(() => { @@ -1090,7 +1145,7 @@ export class ClineProvider } }) - this.webviewDisposables.push(visibilityDisposable) + this.trackViewDisposable(webviewView, visibilityDisposable) } // Listen for when the view is disposed @@ -1102,7 +1157,15 @@ export class ClineProvider await this.dispose() } else { this.log("Clearing webview resources for sidebar view") - this.clearWebviewResources() + // Identity-checked: a view resolved after this one must keep the slot, and only + // this view's listeners are released so the replacement keeps its own. + clearSidebarPanel(webviewView) + if (this.view === webviewView) { + this.rejectPendingThemeFixtureProbes( + new Error("Webview was disposed before the theme fixture probe completed"), + ) + } + this.clearViewScopedDisposables(webviewView) } }, null, @@ -1116,7 +1179,7 @@ export class ClineProvider await this.postMessageToWebview({ type: "theme", text: JSON.stringify(await getTheme()) }) } }) - this.webviewDisposables.push(configDisposable) + this.trackViewDisposable(webviewView, configDisposable) // If the extension is starting a new session, clear previous task state. // But don't clear if there's already an active task (e.g., resumed via IPC/bridge). @@ -1679,12 +1742,12 @@ export class ClineProvider * * @param webview A reference to the extension webview */ - private setWebviewMessageListener(webview: vscode.Webview) { + private setWebviewMessageListener(webview: vscode.Webview, view: vscode.WebviewView | vscode.WebviewPanel) { const onReceiveMessage = async (message: WebviewMessage) => webviewMessageHandler(this, message, this.marketplaceManager) const messageDisposable = webview.onDidReceiveMessage(onReceiveMessage) - this.webviewDisposables.push(messageDisposable) + this.trackViewDisposable(view, messageDisposable) } /** diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 31d658ff52..c5c933f694 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -593,6 +593,94 @@ describe("ClineProvider", () => { expect(ClineProvider.getVisibleInstance()).toBe(provider) }) + test("keeps a replacement sidebar view's listeners when the old view disposes", async () => { + const makeView = () => { + const disposables: Array<{ dispose: ReturnType }> = [] + let disposeCallback: (() => void | Promise) | undefined + const view = { + webview: { + postMessage: vi.fn().mockResolvedValue(true), + html: "", + options: {}, + onDidReceiveMessage: vi.fn(() => { + const disposable = { dispose: vi.fn() } + disposables.push(disposable) + return disposable + }), + asWebviewUri: vi.fn(), + cspSource: "vscode-webview://test-csp-source", + }, + visible: true, + onDidDispose: vi.fn((callback: () => void | Promise) => { + disposeCallback = callback + return { dispose: vi.fn() } + }), + onDidChangeVisibility: vi.fn(() => { + const disposable = { dispose: vi.fn() } + disposables.push(disposable) + return disposable + }), + } as unknown as vscode.WebviewView + return { + view, + disposables, + triggerDispose: async () => { + await disposeCallback?.() + }, + } + } + const first = makeView() + const second = makeView() + provider.getMcpHub = vi.fn().mockReturnValue({ + listTools: vi.fn().mockResolvedValue([]), + callTool: vi.fn(), + listResources: vi.fn(), + readResource: vi.fn(), + getAllServers: vi.fn().mockReturnValue([]), + }) + + await provider.resolveWebviewView(first.view) + // The replacement resolves before the old view's disposal callback runs. + await provider.resolveWebviewView(second.view) + + await first.triggerDispose() + + // Cleanup is scoped to the surface that went away: the replacement keeps every listener. + for (const disposable of second.disposables) { + expect(disposable.dispose).not.toHaveBeenCalled() + } + for (const disposable of first.disposables) { + expect(disposable.dispose).toHaveBeenCalled() + } + await provider.dispose() + }) + + describe("getInstanceForView", () => { + it("returns the instance that owns the given view", async () => { + await provider.resolveWebviewView(mockWebviewView) + + expect(ClineProvider.getInstanceForView(mockWebviewView)).toBe(provider) + }) + + it("returns undefined when no live instance owns the view", () => { + expect(ClineProvider.getInstanceForView({} as vscode.WebviewView)).toBeUndefined() + }) + + it("excludes an instance whose disposal has started", async () => { + await provider.resolveWebviewView(mockWebviewView) + expect(ClineProvider.getInstanceForView(mockWebviewView)).toBe(provider) + + // dispose() marks the instance disposed synchronously but only removes it from + // activeInstances once its ASYNCHRONOUS task cleanup finishes. A tab command + // queued inside that window must not be handed this provider: it would re-run task + // eviction and workspace refresh on top of the disposal already doing them. + const disposal = provider.dispose() + expect(ClineProvider.getInstanceForView(mockWebviewView)).toBeUndefined() + + await expect(disposal).resolves.toBeUndefined() + }) + }) + test("reports an unresolved webview as not visible", () => { expect(provider.isViewVisible).toBe(false) }) diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index f1b2590f7f..b5498d196c 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -64,11 +64,6 @@ "count": 14 } }, - "activate/__tests__/registerCommands.spec.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 2 - } - }, "activate/registerCodeActions.ts": { "@typescript-eslint/no-explicit-any": { "count": 2 diff --git a/src/package.json b/src/package.json index 561b137318..1a1d99d2ef 100644 --- a/src/package.json +++ b/src/package.json @@ -95,6 +95,26 @@ "title": "%command.settings.title%", "icon": "$(settings-gear)" }, + { + "command": "zoo-code.plusButtonClickedInTab", + "title": "%command.newTask.title%", + "icon": "$(edit)" + }, + { + "command": "zoo-code.settingsButtonClickedInTab", + "title": "%command.settings.title%", + "icon": "$(settings-gear)" + }, + { + "command": "zoo-code.marketplaceButtonClickedInTab", + "title": "%command.marketplace.title%", + "icon": "$(extensions)" + }, + { + "command": "zoo-code.historyButtonClickedInTab", + "title": "%command.history.title%", + "icon": "$(history)" + }, { "command": "zoo-code.openInNewTab", "title": "%command.openInNewTab.title%", @@ -241,22 +261,22 @@ ], "editor/title": [ { - "command": "zoo-code.plusButtonClicked", + "command": "zoo-code.plusButtonClickedInTab", "group": "navigation@1", "when": "activeWebviewPanelId == zoo-code.TabPanelProvider" }, { - "command": "zoo-code.settingsButtonClicked", + "command": "zoo-code.settingsButtonClickedInTab", "group": "navigation@2", "when": "activeWebviewPanelId == zoo-code.TabPanelProvider" }, { - "command": "zoo-code.marketplaceButtonClicked", + "command": "zoo-code.marketplaceButtonClickedInTab", "group": "navigation@3", "when": "activeWebviewPanelId == zoo-code.TabPanelProvider" }, { - "command": "zoo-code.historyButtonClicked", + "command": "zoo-code.historyButtonClickedInTab", "group": "overflow@1", "when": "activeWebviewPanelId == zoo-code.TabPanelProvider" }, @@ -265,6 +285,40 @@ "group": "overflow@2", "when": "activeWebviewPanelId == zoo-code.TabPanelProvider" } + ], + "commandPalette": [ + { + "command": "zoo-code.plusButtonClickedInTab", + "when": "activeWebviewPanelId == zoo-code.TabPanelProvider" + }, + { + "command": "zoo-code.settingsButtonClickedInTab", + "when": "activeWebviewPanelId == zoo-code.TabPanelProvider" + }, + { + "command": "zoo-code.marketplaceButtonClickedInTab", + "when": "activeWebviewPanelId == zoo-code.TabPanelProvider" + }, + { + "command": "zoo-code.historyButtonClickedInTab", + "when": "activeWebviewPanelId == zoo-code.TabPanelProvider" + }, + { + "command": "zoo-code.plusButtonClicked", + "when": "activeWebviewPanelId != zoo-code.TabPanelProvider" + }, + { + "command": "zoo-code.settingsButtonClicked", + "when": "activeWebviewPanelId != zoo-code.TabPanelProvider" + }, + { + "command": "zoo-code.marketplaceButtonClicked", + "when": "activeWebviewPanelId != zoo-code.TabPanelProvider" + }, + { + "command": "zoo-code.historyButtonClicked", + "when": "activeWebviewPanelId != zoo-code.TabPanelProvider" + } ] }, "keybindings": [