Skip to content

fix: normalize Windows drive letter casing in extractCwd #126

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

Merged
matt1398 merged 2 commits into from
Mar 23, 2026
+13 −2
Merged

fix: normalize Windows drive letter casing in extractCwd #126

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
7f5fbda
fix: normalize Windows drive letter casing in extractCwd
matt1398 Mar 23, 2026
c5d3372
fix: add retry to temp dir cleanup for Windows CI
matt1398 Mar 23, 2026
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
13 changes: 12 additions & 1 deletion src/main/utils/metadataExtraction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ import type { FileSystemProvider } from '../services/infrastructure/FileSystemPr

const logger = createLogger('Util:metadataExtraction');

/**
* Normalize Windows drive letter to uppercase for consistent path comparison.
* CLI uses uppercase (C:\...) while VS Code extension uses lowercase (c:\...).
*/
function normalizeDriveLetter(p: string): string {
if (p.length >= 2 && p[1] === ':') {
Copy link

@gemini-code-assist gemini-code-assist bot Mar 23, 2026

Choose a reason for hiding this comment

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

medium

This logic correctly normalizes Windows drive letters, but it will also run on other platforms like macOS and Linux. This could lead to unintended behavior if a path on a non-Windows system happens to match the pattern (e.g., c:foo/bar). Since this normalization is specific to Windows pathing conventions, it's safer to guard this logic to only execute on Windows by checking process.platform === 'win32'. The process object is globally available in Node.js, so no import is needed.

Suggested change
if (p.length >= 2 && p[1] === ':') {
if (process.platform === 'win32' && p.length >= 2 && p[1] === ':') {

Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

normalizeDriveLetter() currently uppercases the first character of any string whose second character is ':'. This will also match non-Windows strings like 'a:b' (valid relative path on POSIX) and could mutate a real cwd on case-sensitive filesystems. Consider tightening the check to only treat Windows drive prefixes (e.g., /^[A-Za-z]:/ and optionally requiring a following path separator).

Suggested change
if (p.length >= 2 && p[1] === ':') {
if (process.platform === 'win32' && p.length >= 2 && /^[A-Za-z]:/.test(p)) {

Copilot uses AI. Check for mistakes.
return p[0].toUpperCase() + p.slice(1);
}
return p;
}

const defaultProvider = new LocalFileSystemProvider();

interface MessagePreview {
Expand Down Expand Up @@ -48,7 +59,7 @@ export async function extractCwd(
if ('cwd' in entry && entry.cwd) {
rl.close();
fileStream.destroy();
return entry.cwd;
return normalizeDriveLetter(entry.cwd);
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 23, 2026

Choose a reason for hiding this comment

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

P2 Badge Add migration for persisted project IDs after cwd normalization

For Windows users upgrading from a build that already indexed the same repo as separate c:\.../C:\... subprojects, returning a normalized cwd here changes the derived projectId on the next scan (often from composite IDs back to the plain encoded dir, or at least to a different hash). That breaks any state keyed by the old ID: pinned/hidden sessions are stored under the exact projectId in src/main/services/infrastructure/ConfigManager.ts:749-858, and restored selection/tabs are dropped when contextSlice no longer finds the saved ID in src/renderer/store/slices/contextSlice.ts:82-101. Without an alias/migration, affected users silently lose pins/hidden state and reopened tabs after upgrading.

Useful? React with 👍 / 👎.

}
Comment on lines 59 to 63
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

This change alters extractCwd() behavior on Windows-style paths by normalizing the drive letter casing, but there is no test asserting that sessions with 'c:\...' and 'C:\...' are grouped together (and not split) during discovery. Adding a focused unit test (e.g., in ProjectScanner.cwdSplit.test.ts and/or ProjectPathResolver.test.ts) would prevent regressions.

Copilot uses AI. Check for mistakes.
}
} catch (error) {
Expand Down
2 changes: 1 addition & 1 deletion test/main/services/discovery/SessionSearcher.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('SessionSearcher', () => {

afterEach(() => {
for (const dir of tempDirs) {
fs.rmSync(dir, { recursive: true, force: true });
fs.rmSync(dir, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 });
}
tempDirs.length = 0;
});
Expand Down
Loading