-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Use correct path for slug on Windows #3307
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
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.
Pull request overview
Updates Claude session discovery to use the same project slug directory as the Claude SDK on Windows (derived from process.cwd()), instead of the hard-coded - directory, and adjusts unit tests accordingly.
Changes:
- Use
this._computeFolderSlug(URI.file(process.cwd()))for multi-root and no-workspace session directories. - Update
claudeCodeSessionService.spec.tsto expect sessions under theprocess.cwd()-derived slug directory in no-workspace and multi-root scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/extension/agents/claude/node/claudeCodeSessionService.ts | Switches multi-root/no-workspace session scanning from - to a process.cwd()-based slug. |
| src/extension/agents/claude/node/test/claudeCodeSessionService.spec.ts | Updates tests to match the new session directory slug behavior for no-workspace and multi-root cases. |
| @@ -116,7 +116,7 @@ export class ClaudeCodeSessionService implements IClaudeCodeSessionService { | |||
| slugs.push(this._computeFolderSlug(folders[0])); | |||
| } else { | |||
| // Multi-root or no folder - add the no-project slug | |||
Copilot
AI
Jan 30, 2026
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 says "Multi-root or no folder - add the no-project slug", but the code now derives the slug from process.cwd(). Please update the comment to match the current behavior (and/or rename "no-project" wording) to avoid misleading future readers.
| // Multi-root or no folder - add the no-project slug | |
| // Multi-root or no workspace folder - derive a slug from the current working directory |
| } else { | ||
| // Multi-root or no folder - add the no-project slug | ||
| slugs.push('-'); | ||
| slugs.push(this._computeFolderSlug(URI.file(process.cwd()))); |
Copilot
AI
Jan 30, 2026
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.
process.cwd() can throw (e.g., if the current working directory was deleted), which would make getAllSessions fail unexpectedly. Consider wrapping process.cwd() in a try/catch and falling back to a safe default (e.g., '-' or userHome) so session discovery remains resilient.
Fixes microsoft/vscode#291786