Skip to content

fix: handle empty workspace folders array to prevent path type errors #7696

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

Closed
wants to merge 2 commits into from
Closed

fix: handle empty workspace folders array to prevent path type errors #7696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db095b7
fix: handle empty workspaceFolders array to prevent path type error
roomote Sep 5, 2025
f276e68
fix: handle empty workspace folders array to prevent path type errors
roomote Sep 5, 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
4 changes: 3 additions & 1 deletion src/core/context-tracking/FileContextTracker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ export class FileContextTracker {

// Gets the current working directory or returns undefined if it cannot be determined
private getCwd(): string | undefined {
const cwd = vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0)
// Ensure we handle empty arrays properly - check length before accessing
Copy link
Contributor Author

@roomote roomote bot Sep 5, 2025

Choose a reason for hiding this comment

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

The comment "Ensure we handle empty arrays properly" could be more specific. Consider: "Check array length to avoid undefined when accessing first element" to make the intent clearer.

const workspaceFolders = vscode.workspace.workspaceFolders
const cwd = workspaceFolders && workspaceFolders.length > 0 ? workspaceFolders[0].uri.fsPath : undefined
if (!cwd) {
console.info("No workspace folder available - cannot determine current working directory")
}
Expand Down
4 changes: 3 additions & 1 deletion src/integrations/claude-code/run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import { CLAUDE_CODE_DEFAULT_MAX_OUTPUT_TOKENS } from "@roo-code/types"
import * as os from "os"
import { t } from "../../i18n"

const cwd = vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0)
// Ensure we handle empty arrays properly - check length before accessing
const workspaceFolders = vscode.workspace.workspaceFolders
const cwd = workspaceFolders && workspaceFolders.length > 0 ? workspaceFolders[0].uri.fsPath : undefined
Copy link
Contributor Author

@roomote roomote bot Sep 5, 2025

Choose a reason for hiding this comment

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

Since this same pattern appears in 4 different files, would it make sense to create a shared utility function like getFirstWorkspaceFolder() to centralize this logic? This could help prevent similar regressions in the future.


// Claude Code installation URL - can be easily updated if needed
const CLAUDE_CODE_INSTALLATION_URL = "https://docs.anthropic.com/en/docs/claude-code/setup"
Expand Down
5 changes: 4 additions & 1 deletion src/services/mcp/McpHub.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ const createServerTypeSchema = () => {
type: z.enum(["stdio"]).optional(),
command: z.string().min(1, "Command cannot be empty"),
args: z.array(z.string()).optional(),
cwd: z.string().default(() => vscode.workspace.workspaceFolders?.at(0)?.uri.fsPath ?? process.cwd()),
cwd: z.string().default(() => {
const workspaceFolders = vscode.workspace.workspaceFolders
return workspaceFolders && workspaceFolders.length > 0 ? workspaceFolders[0].uri.fsPath : process.cwd()
Copy link
Contributor Author

@roomote roomote bot Sep 5, 2025

Choose a reason for hiding this comment

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

Good defensive programming here with the fallback to process.cwd(). Though I wonder if we should add a test case that specifically validates the behavior when vscode.workspace.workspaceFolders is empty or undefined? The PR mentions tests pass but doesn't add new ones for this edge case.

}),
env: z.record(z.string()).optional(),
// Ensure no SSE fields are present
url: z.undefined().optional(),
Expand Down
4 changes: 3 additions & 1 deletion src/utils/path.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ export const toRelativePath = (filePath: string, cwd: string) => {
}

export const getWorkspacePath = (defaultCwdPath = "") => {
const cwdPath = vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0) || defaultCwdPath
// Ensure we handle empty arrays properly - check length before mapping
const workspaceFolders = vscode.workspace.workspaceFolders
const cwdPath = workspaceFolders && workspaceFolders.length > 0 ? workspaceFolders[0].uri.fsPath : defaultCwdPath
Copy link
Contributor Author

@roomote roomote bot Sep 5, 2025

Choose a reason for hiding this comment

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

I notice this implementation differs from the other three files - here I removed the .map() call entirely, while in the others I store workspaceFolders in a variable first. Would it be better to use a consistent pattern across all four files for maintainability?

const currentFileUri = vscode.window.activeTextEditor?.document.uri
if (currentFileUri) {
const workspaceFolder = vscode.workspace.getWorkspaceFolder(currentFileUri)
Expand Down
Loading