Skip to content

Fix workspace tracking when RooCode is moved to new window #7872

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 1 commit into
base: main
Choose a base branch
from
Open

Fix workspace tracking when RooCode is moved to new window #7872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 20 additions & 5 deletions src/core/webview/ClineProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,11 +752,16 @@ export class ClineProvider
// Initialize code index status subscription for the current workspace.
this.updateCodeIndexStatusSubscription()

// Listen for active editor changes to update code index status for the
// current workspace.
const activeEditorSubscription = vscode.window.onDidChangeActiveTextEditor(() => {
// Listen for active editor changes to update code index status and workspace path
const activeEditorSubscription = vscode.window.onDidChangeActiveTextEditor(async () => {
// Update subscription when workspace might have changed.
this.updateCodeIndexStatusSubscription()

// Check if workspace path has changed
const newWorkspacePath = getWorkspacePath()
if (newWorkspacePath !== this.currentWorkspacePath) {
await this.refreshWorkspace()
Copy link
Author

Choose a reason for hiding this comment

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

Good addition of workspace path checking! Consider adding error handling here too:

const activeEditorSubscription = vscode.window.onDidChangeActiveTextEditor(() => {
    // Update subscription when workspace might have changed
    this.updateCodeIndexStatusSubscription()
    
    // Check if workspace path has changed
    try {
        const newWorkspacePath = getWorkspacePath()
        if (newWorkspacePath !== this.currentWorkspacePath) {
            this.refreshWorkspace().catch(error => 
                this.log(`Failed to refresh workspace on editor change: ${error}`)
            )
        }
    } catch (error) {
        this.log(`Error getting workspace path: ${error}`)
    }
})

This ensures that any errors in getWorkspacePath() won't crash the event handler.

}
})
this.webviewDisposables.push(activeEditorSubscription)

Expand All @@ -765,17 +770,21 @@ export class ClineProvider
if ("onDidChangeViewState" in webviewView) {
// WebviewView and WebviewPanel have all the same properties except
// for this visibility listener panel.
const viewStateDisposable = webviewView.onDidChangeViewState(() => {
const viewStateDisposable = webviewView.onDidChangeViewState(async () => {
if (this.view?.visible) {
// Update workspace path when view becomes visible (e.g., when moved to new window)
await this.refreshWorkspace()
this.postMessageToWebview({ type: "action", action: "didBecomeVisible" })
Copy link
Author

Choose a reason for hiding this comment

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

The async wrapper here isn't necessary since we're not awaiting the result. Consider:

const viewStateDisposable = webviewView.onDidChangeViewState(() => {
    if (this.view?.visible) {
        // Fire and forget the refresh, but catch errors
        this.refreshWorkspace().catch(error => 
            this.log(`Failed to refresh workspace on view state change: ${error}`)
        )
        this.postMessageToWebview({ type: "action", action: "didBecomeVisible" })
    }
})

This avoids the overhead of creating an async function when we don't need to wait for completion.

}
})

this.webviewDisposables.push(viewStateDisposable)
} else if ("onDidChangeVisibility" in webviewView) {
// sidebar
const visibilityDisposable = webviewView.onDidChangeVisibility(() => {
const visibilityDisposable = webviewView.onDidChangeVisibility(async () => {
if (this.view?.visible) {
// Update workspace path when view becomes visible (e.g., when moved to new window)
Copy link
Author

Choose a reason for hiding this comment

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

Same async pattern issue here. Also, consider debouncing these refresh calls to prevent rapid successive updates:

const visibilityDisposable = webviewView.onDidChangeVisibility(() => {
    if (this.view?.visible) {
        this.refreshWorkspace().catch(error => 
            this.log(`Failed to refresh workspace on visibility change: ${error}`)
        )
        this.postMessageToWebview({ type: "action", action: "didBecomeVisible" })
    }
})

For debouncing, you could create a debounced version of refreshWorkspace to handle rapid visibility changes.

await this.refreshWorkspace()
this.postMessageToWebview({ type: "action", action: "didBecomeVisible" })
}
})
Expand Down Expand Up @@ -810,6 +819,12 @@ export class ClineProvider
})
this.webviewDisposables.push(configDisposable)

// Listen for workspace folder changes
const workspaceFoldersDisposable = vscode.workspace.onDidChangeWorkspaceFolders(async () => {
await this.refreshWorkspace()
})
this.webviewDisposables.push(workspaceFoldersDisposable)

// If the extension is starting a new session, clear previous task state.
await this.removeClineFromStack()
}
Expand Down
1 change: 1 addition & 0 deletions src/core/webview/__tests__/ClineProvider.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ vi.mock("vscode", () => ({
onDidChangeTextDocument: vi.fn(() => ({ dispose: vi.fn() })),
onDidOpenTextDocument: vi.fn(() => ({ dispose: vi.fn() })),
onDidCloseTextDocument: vi.fn(() => ({ dispose: vi.fn() })),
onDidChangeWorkspaceFolders: vi.fn(() => ({ dispose: vi.fn() })),
Copy link
Author

Choose a reason for hiding this comment

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

Good job updating the mocks! Consider adding actual test cases to verify the behavior:

test("refreshes workspace when workspace folders change", async () => {
    const mockRefreshWorkspace = vi.spyOn(provider, 'refreshWorkspace').mockResolvedValue(undefined)
    await provider.resolveWebviewView(mockWebviewView)
    
    // Get the registered handler
    const onDidChangeWorkspaceFolders = vi.mocked(vscode.workspace.onDidChangeWorkspaceFolders)
    const changeHandler = onDidChangeWorkspaceFolders.mock.calls[0][0]
    
    // Trigger the event
    await changeHandler()
    
    // Verify refresh was called
    expect(mockRefreshWorkspace).toHaveBeenCalled()
})

test("handles errors during workspace refresh", async () => {
    const mockLog = vi.spyOn(provider, 'log')
    vi.spyOn(provider, 'refreshWorkspace').mockRejectedValue(new Error("Refresh failed"))
    
    await provider.resolveWebviewView(mockWebviewView)
    
    // Trigger visibility change
    const onDidChangeVisibility = vi.mocked(webviewView.onDidChangeVisibility)
    const handler = onDidChangeVisibility.mock.calls[0][0]
    await handler()
    
    // Verify error was logged
    expect(mockLog).toHaveBeenCalledWith(expect.stringContaining("Failed to refresh workspace"))
})

This would ensure the fix actually works as intended.

},
env: {
uriScheme: "vscode",
Expand Down
1 change: 1 addition & 0 deletions src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ vi.mock("vscode", () => ({
onDidChangeTextDocument: vi.fn(() => ({ dispose: vi.fn() })),
onDidOpenTextDocument: vi.fn(() => ({ dispose: vi.fn() })),
onDidCloseTextDocument: vi.fn(() => ({ dispose: vi.fn() })),
onDidChangeWorkspaceFolders: vi.fn(() => ({ dispose: vi.fn() })),
},
env: {
uriScheme: "vscode",
Expand Down
Loading