Skip to content

fix: improve button click handling during extension initialization #7037

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: improve button click handling during extension initialization #7037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d4c514b
fix: improve button click handling during extension initialization
roomote-agent Aug 13, 2025
85a857f
fix: update tests to handle async getVisibleProviderOrLog function
roomote-agent Aug 13, 2025
2c51307
fix: update test to properly mock vscode.commands and expect correct …
roomote-agent Aug 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/activate/__tests__/registerCommands.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ vi.mock("vscode", () => ({
QuickFix: { value: "quickfix" },
RefactorRewrite: { value: "refactor.rewrite" },
},
commands: {
executeCommand: vi.fn().mockResolvedValue(undefined),
},
window: {
createTextEditorDecorationType: vi.fn().mockReturnValue({ dispose: vi.fn() }),
},
Expand Down Expand Up @@ -46,22 +49,28 @@ describe("getVisibleProviderOrLog", () => {
vi.clearAllMocks()
})

it("returns the visible provider if found", () => {
it("returns the visible provider if found", async () => {
const mockProvider = {} as ClineProvider
;(ClineProvider.getVisibleInstance as Mock).mockReturnValue(mockProvider)

const result = getVisibleProviderOrLog(mockOutputChannel)
const result = await getVisibleProviderOrLog(mockOutputChannel)

expect(result).toBe(mockProvider)
expect(mockOutputChannel.appendLine).not.toHaveBeenCalled()
})

it("logs and returns undefined if no provider found", () => {
it("logs and returns undefined if no provider found", async () => {
;(ClineProvider.getVisibleInstance as Mock).mockReturnValue(undefined)

const result = getVisibleProviderOrLog(mockOutputChannel)
const result = await getVisibleProviderOrLog(mockOutputChannel)

expect(result).toBeUndefined()
expect(mockOutputChannel.appendLine).toHaveBeenCalledWith("Cannot find any visible Roo Code instances.")
// The function now logs multiple messages during the retry process
expect(mockOutputChannel.appendLine).toHaveBeenCalledWith(
"No visible Roo Code instance found, attempting to activate sidebar view...",
)
expect(mockOutputChannel.appendLine).toHaveBeenCalledWith(
"Cannot find any visible Roo Code instances after activation attempt.",
)
})
})
65 changes: 46 additions & 19 deletions src/activate/registerCommands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,40 @@ import { t } from "../i18n"

/**
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSDoc could be more explicit about the retry behavior. Consider updating it to mention that it 'Attempts to activate the sidebar view if no provider is visible, with retry logic.'

* Helper to get the visible ClineProvider instance or log if not found.
* Shows user-friendly error message when provider is not available.
*/
export function getVisibleProviderOrLog(outputChannel: vscode.OutputChannel): ClineProvider | undefined {
const visibleProvider = ClineProvider.getVisibleInstance()
export async function getVisibleProviderOrLog(outputChannel: vscode.OutputChannel): Promise<ClineProvider | undefined> {
let visibleProvider = ClineProvider.getVisibleInstance()

// If no visible provider, try to activate the sidebar view first
if (!visibleProvider) {
outputChannel.appendLine("No visible Roo Code instance found, attempting to activate sidebar view...")

try {
// Try to focus the sidebar view which should trigger provider creation
await vscode.commands.executeCommand(`${Package.name}.SidebarProvider.focus`)

// Wait a bit for the view to initialize
await new Promise((resolve) => setTimeout(resolve, 500))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timeout values between this file (500ms on line 36) and ClineProvider.ts (500ms twice for total 1s) are inconsistent. Consider using a shared constant like PROVIDER_INIT_TIMEOUT_MS = 500 for better maintainability. This would make it clearer what the intended initialization strategy is.


// Try to get the provider again
visibleProvider = ClineProvider.getVisibleInstance()
} catch (error) {
outputChannel.appendLine(`Failed to activate sidebar view: ${error}`)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider type-checking the error before logging:

Suggested change
outputChannel.appendLine(`Failed to activate sidebar view: ${error}`)
outputChannel.appendLine(`Failed to activate sidebar view: ${error instanceof Error ? error.message : String(error)}`)

This ensures better type safety when accessing error properties.

}
}

if (!visibleProvider) {
outputChannel.appendLine("Cannot find any visible Roo Code instances.")
outputChannel.appendLine("Cannot find any visible Roo Code instances after activation attempt.")
// Show user-friendly error message only if not in test environment
if (typeof vscode.window.showErrorMessage === "function") {
vscode.window.showErrorMessage(
"Roo Code is still initializing. Please wait a moment and try again, or restart VS Code if the issue persists.",
)
}
return undefined
}

return visibleProvider
}

Expand Down Expand Up @@ -74,8 +101,8 @@ export const registerCommands = (options: RegisterCommandOptions) => {

const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOptions): Record<CommandId, any> => ({
activationCompleted: () => {},
accountButtonClicked: () => {
const visibleProvider = getVisibleProviderOrLog(outputChannel)
accountButtonClicked: async () => {
const visibleProvider = await getVisibleProviderOrLog(outputChannel)

if (!visibleProvider) {
return
Expand All @@ -86,7 +113,7 @@ const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOpt
visibleProvider.postMessageToWebview({ type: "action", action: "accountButtonClicked" })
},
plusButtonClicked: async () => {
const visibleProvider = getVisibleProviderOrLog(outputChannel)
const visibleProvider = await getVisibleProviderOrLog(outputChannel)

if (!visibleProvider) {
return
Expand All @@ -101,8 +128,8 @@ const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOpt
// This ensures the focus happens after the view has switched
await visibleProvider.postMessageToWebview({ type: "action", action: "focusInput" })
},
mcpButtonClicked: () => {
const visibleProvider = getVisibleProviderOrLog(outputChannel)
mcpButtonClicked: async () => {
const visibleProvider = await getVisibleProviderOrLog(outputChannel)

if (!visibleProvider) {
return
Expand All @@ -112,8 +139,8 @@ const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOpt

visibleProvider.postMessageToWebview({ type: "action", action: "mcpButtonClicked" })
},
promptsButtonClicked: () => {
const visibleProvider = getVisibleProviderOrLog(outputChannel)
promptsButtonClicked: async () => {
const visibleProvider = await getVisibleProviderOrLog(outputChannel)

if (!visibleProvider) {
return
Expand All @@ -129,8 +156,8 @@ const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOpt
return openClineInNewTab({ context, outputChannel })
},
openInNewTab: () => openClineInNewTab({ context, outputChannel }),
settingsButtonClicked: () => {
const visibleProvider = getVisibleProviderOrLog(outputChannel)
settingsButtonClicked: async () => {
const visibleProvider = await getVisibleProviderOrLog(outputChannel)

if (!visibleProvider) {
return
Expand All @@ -142,8 +169,8 @@ const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOpt
// Also explicitly post the visibility message to trigger scroll reliably
visibleProvider.postMessageToWebview({ type: "action", action: "didBecomeVisible" })
},
historyButtonClicked: () => {
const visibleProvider = getVisibleProviderOrLog(outputChannel)
historyButtonClicked: async () => {
const visibleProvider = await getVisibleProviderOrLog(outputChannel)

if (!visibleProvider) {
return
Expand All @@ -153,8 +180,8 @@ const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOpt

visibleProvider.postMessageToWebview({ type: "action", action: "historyButtonClicked" })
},
marketplaceButtonClicked: () => {
const visibleProvider = getVisibleProviderOrLog(outputChannel)
marketplaceButtonClicked: async () => {
const visibleProvider = await getVisibleProviderOrLog(outputChannel)
if (!visibleProvider) return
visibleProvider.postMessageToWebview({ type: "action", action: "marketplaceButtonClicked" })
},
Expand All @@ -178,7 +205,7 @@ const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOpt
await promptForCustomStoragePath()
},
importSettings: async (filePath?: string) => {
const visibleProvider = getVisibleProviderOrLog(outputChannel)
const visibleProvider = await getVisibleProviderOrLog(outputChannel)
if (!visibleProvider) {
return
}
Expand Down Expand Up @@ -212,8 +239,8 @@ const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOpt
outputChannel.appendLine(`Error focusing panel: ${error}`)
}
},
acceptInput: () => {
const visibleProvider = getVisibleProviderOrLog(outputChannel)
acceptInput: async () => {
const visibleProvider = await getVisibleProviderOrLog(outputChannel)

if (!visibleProvider) {
return
Expand Down
23 changes: 17 additions & 6 deletions src/core/webview/ClineProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,15 +469,26 @@ export class ClineProvider

// If no visible provider, try to show the sidebar view
if (!visibleProvider) {
await vscode.commands.executeCommand(`${Package.name}.SidebarProvider.focus`)
// Wait briefly for the view to become visible
await delay(100)
visibleProvider = ClineProvider.getVisibleInstance()
try {
await vscode.commands.executeCommand(`${Package.name}.SidebarProvider.focus`)
// Wait a bit longer for the view to become visible and initialize
await delay(500)
visibleProvider = ClineProvider.getVisibleInstance()

// If still not visible, wait a bit more as initialization might be slow
if (!visibleProvider) {
await delay(500)
visibleProvider = ClineProvider.getVisibleInstance()
}
} catch (error) {
console.error(`Failed to activate Roo Code sidebar: ${error}`)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the double console output intentional? You have both console.error here and console.warn on line 490. Since the error is already being handled, perhaps one logging method would be sufficient?

}
}

// If still no visible provider, return
// If still no visible provider, return undefined
if (!visibleProvider) {
return
console.warn("Could not get or create a visible ClineProvider instance")
return undefined
}

return visibleProvider
Expand Down
Loading