Skip to content

fix(app): preserve native path separators in file path helpers #14912

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
Hona merged 2 commits into from
Feb 24, 2026
Merged

fix(app): preserve native path separators in file path helpers #14912

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e57fe7d
fix(app): preserve native path separators in file path helpers
Hona Feb 24, 2026
a852b11
Merge branch 'dev' into fix/path-helpers-native-separators
Hona Feb 24, 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
4 changes: 2 additions & 2 deletions packages/app/src/context/file/path.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ describe("file path helpers", () => {

test("normalizes Windows absolute paths with mixed separators", () => {
const path = createPathHelpers(() => "C:\\repo")
expect(path.normalize("C:\\repo\\src\\app.ts")).toBe("src/app.ts")
expect(path.normalize("C:\\repo\\src\\app.ts")).toBe("src\\app.ts")
expect(path.normalize("C:/repo/src/app.ts")).toBe("src/app.ts")
expect(path.normalize("file://C:/repo/src/app.ts")).toBe("src/app.ts")
expect(path.normalize("c:\\repo\\src\\app.ts")).toBe("src/app.ts")
expect(path.normalize("c:\\repo\\src\\app.ts")).toBe("src\\app.ts")

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

The PR description mentions that Cygwin paths like "/c/Users/..." or "C:/Users/..." should match against native roots like "C:\Users...", but there are no test cases verifying this behavior. Consider adding test cases to ensure Cygwin path compatibility works as intended, for example testing paths starting with "/c/" or "/cygdrive/c/" against a Windows root.

Suggested change
expect(path.normalize("c:\\repo\\src\\app.ts")).toBe("src\\app.ts")
expect(path.normalize("c:\\repo\\src\\app.ts")).toBe("src\\app.ts")
// Cygwin-style paths should also normalize against the Windows root
expect(path.normalize("/c/repo/src/app.ts")).toBe("src/app.ts")
expect(path.normalize("/cygdrive/c/repo/src/app.ts")).toBe("src/app.ts")

Copilot uses AI. Check for mistakes.
})

test("keeps query/hash stripping behavior stable", () => {
Expand Down
24 changes: 11 additions & 13 deletions packages/app/src/context/file/path.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,32 +103,30 @@ export function encodeFilePath(filepath: string): string {

export function createPathHelpers(scope: () => string) {
const normalize = (input: string) => {
const root = scope().replace(/\\/g, "/")
const root = scope()

let path = unquoteGitPath(decodeFilePath(stripQueryAndHash(stripFileProtocol(input)))).replace(/\\/g, "/")
let path = unquoteGitPath(decodeFilePath(stripQueryAndHash(stripFileProtocol(input))))

// Remove initial root prefix, if it's a complete match or followed by /
// (don't want /foo/bar to root of /f).
// For Windows paths, also check for case-insensitive match.
const windows = /^[A-Za-z]:/.test(root)
const canonRoot = windows ? root.toLowerCase() : root
const canonPath = windows ? path.toLowerCase() : path
// Separator-agnostic prefix stripping for Cygwin/native Windows compatibility
// Only case-insensitive on Windows (drive letter or UNC paths)
const windows = /^[A-Za-z]:/.test(root) || root.startsWith("\\\\")
Comment on lines +111 to +112

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

The canonicalization always lowercases both the root and path, even on non-Windows systems. This could cause issues on case-sensitive filesystems (Linux/Mac) where "/repo/Src/app.ts" and "/repo/src/app.ts" are different files. The original code only applied case-insensitive matching on Windows (when root started with a drive letter). Consider adding back the Windows detection to only lowercase on Windows systems.

Copilot uses AI. Check for mistakes.
const canonRoot = windows ? root.replace(/\\/g, "/").toLowerCase() : root.replace(/\\/g, "/")
const canonPath = windows ? path.replace(/\\/g, "/").toLowerCase() : path.replace(/\\/g, "/")
if (
canonPath.startsWith(canonRoot) &&
(canonRoot.endsWith("/") || canonPath === canonRoot || canonPath.startsWith(canonRoot + "/"))
(canonRoot.endsWith("/") || canonPath === canonRoot || canonPath[canonRoot.length] === "/")
) {
// If we match canonRoot + "/", the slash will be removed below.
// Slice from original path to preserve native separators
path = path.slice(root.length)
}

if (path.startsWith("./")) {
if (path.startsWith("./") || path.startsWith(".\\")) {
path = path.slice(2)
}

if (path.startsWith("/")) {
if (path.startsWith("/") || path.startsWith("\\")) {
path = path.slice(1)
}

return path
}

Expand Down
Loading