-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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.
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
Conversation
- Fix getWorkspacePath() in src/utils/path.ts to check array length before accessing - Fix getCwd() in FileContextTracker to check array length before accessing - Fix cwd assignment in claude-code/run.ts to check array length before accessing This prevents 'path argument must be of type string. Received an instance of Array' error when workspaceFolders is an empty array. Fixes #7695
- Replace .at(0) with safe array access using length checks - Fixes "path argument must be of type string" error when workspace is empty - Addresses regression introduced between v3.25.17 and v3.26.7 Fixes #7695
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like proofreading my own typos - I'll find them all after it's merged.
| 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 |
There was a problem hiding this comment.
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?
| // 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 |
There was a problem hiding this comment.
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 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 |
There was a problem hiding this comment.
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.
| 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() |
There was a problem hiding this comment.
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.
|
Fixed by #7697 |
Summary
This PR fixes the "path argument must be of type string" error that occurs when VSCode workspace folders array is empty or undefined. This regression was introduced between v3.25.17 and v3.26.7.
Problem
The
.at(0)method was being used to access the first element of the workspace folders array. When the array is empty, this returnsundefined, which then gets passed to path functions expecting a string, causing the error.Solution
Replaced all
.at(0)usages with safe array access patterns that check the array length before accessing elements:src/utils/path.ts: ModifiedgetWorkspacePath()src/core/context-tracking/FileContextTracker.ts: ModifiedgetCwd()src/integrations/claude-code/run.ts: Modifiedcwdconstant initializationsrc/services/mcp/McpHub.ts: Modified defaultcwdin Zod schemaTesting
Related Issues
Fixes #7695
Important
Fixes path type error by checking array length before accessing elements in workspace folders.
getWorkspacePath()inpath.ts,getCwd()inFileContextTracker.ts,cwdinitialization inrun.ts, and defaultcwdinMcpHub.ts.This description was created by
for f276e68. You can customize this summary. It will automatically update as commits are pushed.