diff --git a/packages/app/src/components/prompt-input/build-request-parts.test.ts b/packages/app/src/components/prompt-input/build-request-parts.test.ts index b284c3884..b0fd3a050 100644 --- a/packages/app/src/components/prompt-input/build-request-parts.test.ts +++ b/packages/app/src/components/prompt-input/build-request-parts.test.ts @@ -64,4 +64,214 @@ describe("buildRequestParts", () => { expect(fooFiles).toHaveLength(2) expect(synthetic).toHaveLength(1) }) + + test("handles Windows paths correctly (simulated on macOS)", () => { + const prompt: Prompt = [{ type: "file", path: "src\\foo.ts", content: "@src\\foo.ts", start: 0, end: 11 }] + + const result = buildRequestParts({ + prompt, + context: [], + images: [], + text: "@src\\foo.ts", + messageID: "msg_win_1", + sessionID: "ses_win_1", + sessionDirectory: "D:\\projects\\myapp", // Windows path + }) + + // Should create valid file URLs + const filePart = result.requestParts.find((part) => part.type === "file") + expect(filePart).toBeDefined() + if (filePart?.type === "file") { + // URL should be parseable + expect(() => new URL(filePart.url)).not.toThrow() + // Should not have encoded backslashes in wrong place + expect(filePart.url).not.toContain("%5C") + // Should have normalized to forward slashes + expect(filePart.url).toContain("/src/foo.ts") + } + }) + + test("handles Windows absolute path with special characters", () => { + const prompt: Prompt = [{ type: "file", path: "file#name.txt", content: "@file#name.txt", start: 0, end: 14 }] + + const result = buildRequestParts({ + prompt, + context: [], + images: [], + text: "@file#name.txt", + messageID: "msg_win_2", + sessionID: "ses_win_2", + sessionDirectory: "C:\\Users\\test\\Documents", // Windows path + }) + + const filePart = result.requestParts.find((part) => part.type === "file") + expect(filePart).toBeDefined() + if (filePart?.type === "file") { + // URL should be parseable + expect(() => new URL(filePart.url)).not.toThrow() + // Special chars should be encoded + expect(filePart.url).toContain("file%23name.txt") + // Should have Windows drive letter properly encoded + expect(filePart.url).toMatch(/file:\/\/\/[A-Z]%3A/) + } + }) + + test("handles Linux absolute paths correctly", () => { + const prompt: Prompt = [{ type: "file", path: "src/app.ts", content: "@src/app.ts", start: 0, end: 10 }] + + const result = buildRequestParts({ + prompt, + context: [], + images: [], + text: "@src/app.ts", + messageID: "msg_linux_1", + sessionID: "ses_linux_1", + sessionDirectory: "/home/user/project", + }) + + const filePart = result.requestParts.find((part) => part.type === "file") + expect(filePart).toBeDefined() + if (filePart?.type === "file") { + // URL should be parseable + expect(() => new URL(filePart.url)).not.toThrow() + // Should be a normal Unix path + expect(filePart.url).toBe("file:///home/user/project/src/app.ts") + } + }) + + test("handles macOS paths correctly", () => { + const prompt: Prompt = [{ type: "file", path: "README.md", content: "@README.md", start: 0, end: 9 }] + + const result = buildRequestParts({ + prompt, + context: [], + images: [], + text: "@README.md", + messageID: "msg_mac_1", + sessionID: "ses_mac_1", + sessionDirectory: "/Users/kelvin/Projects/opencode", + }) + + const filePart = result.requestParts.find((part) => part.type === "file") + expect(filePart).toBeDefined() + if (filePart?.type === "file") { + // URL should be parseable + expect(() => new URL(filePart.url)).not.toThrow() + // Should be a normal Unix path + expect(filePart.url).toBe("file:///Users/kelvin/Projects/opencode/README.md") + } + }) + + test("handles context files with Windows paths", () => { + const prompt: Prompt = [] + + const result = buildRequestParts({ + prompt, + context: [ + { key: "ctx:1", type: "file", path: "src\\utils\\helper.ts" }, + { key: "ctx:2", type: "file", path: "test\\unit.test.ts", comment: "check tests" }, + ], + images: [], + text: "test", + messageID: "msg_win_ctx", + sessionID: "ses_win_ctx", + sessionDirectory: "D:\\workspace\\app", + }) + + const fileParts = result.requestParts.filter((part) => part.type === "file") + expect(fileParts).toHaveLength(2) + + // All file URLs should be valid + fileParts.forEach((part) => { + if (part.type === "file") { + expect(() => new URL(part.url)).not.toThrow() + expect(part.url).not.toContain("%5C") // No encoded backslashes + } + }) + }) + + test("handles absolute Windows paths (user manually specifies full path)", () => { + const prompt: Prompt = [ + { type: "file", path: "D:\\other\\project\\file.ts", content: "@D:\\other\\project\\file.ts", start: 0, end: 25 }, + ] + + const result = buildRequestParts({ + prompt, + context: [], + images: [], + text: "@D:\\other\\project\\file.ts", + messageID: "msg_abs", + sessionID: "ses_abs", + sessionDirectory: "C:\\current\\project", + }) + + const filePart = result.requestParts.find((part) => part.type === "file") + expect(filePart).toBeDefined() + if (filePart?.type === "file") { + // Should handle absolute path that differs from sessionDirectory + expect(() => new URL(filePart.url)).not.toThrow() + expect(filePart.url).toContain("/D%3A/other/project/file.ts") + } + }) + + test("handles selection with query parameters on Windows", () => { + const prompt: Prompt = [ + { + type: "file", + path: "src\\App.tsx", + content: "@src\\App.tsx", + start: 0, + end: 11, + selection: { startLine: 10, startChar: 0, endLine: 20, endChar: 5 }, + }, + ] + + const result = buildRequestParts({ + prompt, + context: [], + images: [], + text: "@src\\App.tsx", + messageID: "msg_sel", + sessionID: "ses_sel", + sessionDirectory: "C:\\project", + }) + + const filePart = result.requestParts.find((part) => part.type === "file") + expect(filePart).toBeDefined() + if (filePart?.type === "file") { + // Should have query parameters + expect(filePart.url).toContain("?start=10&end=20") + // Should be valid URL + expect(() => new URL(filePart.url)).not.toThrow() + // Query params should parse correctly + const url = new URL(filePart.url) + expect(url.searchParams.get("start")).toBe("10") + expect(url.searchParams.get("end")).toBe("20") + } + }) + + test("handles file paths with dots and special segments on Windows", () => { + const prompt: Prompt = [ + { type: "file", path: "..\\..\\shared\\util.ts", content: "@..\\..\\shared\\util.ts", start: 0, end: 21 }, + ] + + const result = buildRequestParts({ + prompt, + context: [], + images: [], + text: "@..\\..\\shared\\util.ts", + messageID: "msg_dots", + sessionID: "ses_dots", + sessionDirectory: "C:\\projects\\myapp\\src", + }) + + const filePart = result.requestParts.find((part) => part.type === "file") + expect(filePart).toBeDefined() + if (filePart?.type === "file") { + // Should be valid URL + expect(() => new URL(filePart.url)).not.toThrow() + // Should preserve .. segments (backend normalizes) + expect(filePart.url).toContain("/..") + } + }) }) diff --git a/packages/app/src/components/prompt-input/build-request-parts.ts b/packages/app/src/components/prompt-input/build-request-parts.ts index 7010a1fd8..11aec9631 100644 --- a/packages/app/src/components/prompt-input/build-request-parts.ts +++ b/packages/app/src/components/prompt-input/build-request-parts.ts @@ -30,11 +30,21 @@ type BuildRequestPartsInput = { const absolute = (directory: string, path: string) => path.startsWith("/") ? path : (directory + "/" + path).replace("//", "/") -const encodeFilePath = (filepath: string): string => - filepath +const encodeFilePath = (filepath: string): string => { + // Normalize Windows paths: convert backslashes to forward slashes + let normalized = filepath.replace(/\\/g, "/") + + // Handle Windows absolute paths (D:/path -> /D:/path for proper file:// URLs) + if (/^[A-Za-z]:/.test(normalized)) { + normalized = "/" + normalized + } + + // Encode each path segment (preserving forward slashes as path separators) + return normalized .split("/") .map((segment) => encodeURIComponent(segment)) .join("/") +} const fileQuery = (selection: FileSelection | undefined) => selection ? `?start=${selection.startLine}&end=${selection.endLine}` : "" diff --git a/packages/app/src/context/file/path.test.ts b/packages/app/src/context/file/path.test.ts index dba9ae06d..95247c08b 100644 --- a/packages/app/src/context/file/path.test.ts +++ b/packages/app/src/context/file/path.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test" -import { createPathHelpers, stripQueryAndHash, unquoteGitPath } from "./path" +import { createPathHelpers, stripQueryAndHash, unquoteGitPath, encodeFilePath } from "./path" describe("file path helpers", () => { test("normalizes file inputs against workspace root", () => { @@ -25,3 +25,328 @@ describe("file path helpers", () => { expect(unquoteGitPath("a/b/c.ts")).toBe("a/b/c.ts") }) }) + +describe("encodeFilePath", () => { + describe("Linux/Unix paths", () => { + test("should handle Linux absolute path", () => { + const linuxPath = "/home/user/project/README.md" + const result = encodeFilePath(linuxPath) + const fileUrl = `file://${result}` + + // Should create a valid URL + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toBe("/home/user/project/README.md") + + const url = new URL(fileUrl) + expect(url.protocol).toBe("file:") + expect(url.pathname).toBe("/home/user/project/README.md") + }) + + test("should handle Linux path with special characters", () => { + const linuxPath = "/home/user/file#name with spaces.txt" + const result = encodeFilePath(linuxPath) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toBe("/home/user/file%23name%20with%20spaces.txt") + }) + + test("should handle Linux relative path", () => { + const relativePath = "src/components/App.tsx" + const result = encodeFilePath(relativePath) + + expect(result).toBe("src/components/App.tsx") + }) + + test("should handle Linux root directory", () => { + const result = encodeFilePath("/") + expect(result).toBe("/") + }) + + test("should handle Linux path with all special chars", () => { + const path = "/path/to/file#with?special%chars&more.txt" + const result = encodeFilePath(path) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toContain("%23") // # + expect(result).toContain("%3F") // ? + expect(result).toContain("%25") // % + expect(result).toContain("%26") // & + }) + }) + + describe("macOS paths", () => { + test("should handle macOS absolute path", () => { + const macPath = "/Users/kelvin/Projects/opencode/README.md" + const result = encodeFilePath(macPath) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toBe("/Users/kelvin/Projects/opencode/README.md") + }) + + test("should handle macOS path with spaces", () => { + const macPath = "/Users/kelvin/My Documents/file.txt" + const result = encodeFilePath(macPath) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toContain("My%20Documents") + }) + }) + + describe("Windows paths", () => { + test("should handle Windows absolute path with backslashes", () => { + const windowsPath = "D:\\dev\\projects\\opencode\\README.bs.md" + const result = encodeFilePath(windowsPath) + const fileUrl = `file://${result}` + + // Should create a valid, parseable URL + expect(() => new URL(fileUrl)).not.toThrow() + + const url = new URL(fileUrl) + expect(url.protocol).toBe("file:") + expect(url.pathname).toContain("README.bs.md") + expect(result).toBe("/D%3A/dev/projects/opencode/README.bs.md") + }) + + test("should handle mixed separator path (Windows + Unix)", () => { + // This is what happens in build-request-parts.ts when concatenating paths + const mixedPath = "D:\\dev\\projects\\opencode/README.bs.md" + const result = encodeFilePath(mixedPath) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toBe("/D%3A/dev/projects/opencode/README.bs.md") + }) + + test("should handle Windows path with spaces", () => { + const windowsPath = "C:\\Program Files\\MyApp\\file with spaces.txt" + const result = encodeFilePath(windowsPath) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toContain("Program%20Files") + expect(result).toContain("file%20with%20spaces.txt") + }) + + test("should handle Windows path with special chars in filename", () => { + const windowsPath = "D:\\projects\\file#name with ?marks.txt" + const result = encodeFilePath(windowsPath) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toContain("file%23name%20with%20%3Fmarks.txt") + }) + + test("should handle Windows root directory", () => { + const windowsPath = "C:\\" + const result = encodeFilePath(windowsPath) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toBe("/C%3A/") + }) + + test("should handle Windows relative path with backslashes", () => { + const windowsPath = "src\\components\\App.tsx" + const result = encodeFilePath(windowsPath) + + // Relative paths shouldn't get the leading slash + expect(result).toBe("src/components/App.tsx") + }) + + test("should NOT create invalid URL like the bug report", () => { + // This is the exact scenario from bug report by @alexyaroshuk + const windowsPath = "D:\\dev\\projects\\opencode\\README.bs.md" + const result = encodeFilePath(windowsPath) + const fileUrl = `file://${result}` + + // The bug was creating: file://D%3A%5Cdev%5Cprojects%5Copencode/README.bs.md + expect(result).not.toContain("%5C") // Should not have encoded backslashes + expect(result).not.toBe("D%3A%5Cdev%5Cprojects%5Copencode/README.bs.md") + + // Should be valid + expect(() => new URL(fileUrl)).not.toThrow() + }) + + test("should handle lowercase drive letters", () => { + const windowsPath = "c:\\users\\test\\file.txt" + const result = encodeFilePath(windowsPath) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toBe("/c%3A/users/test/file.txt") + }) + }) + + describe("Cross-platform compatibility", () => { + test("should preserve Unix paths unchanged (except encoding)", () => { + const unixPath = "/usr/local/bin/app" + const result = encodeFilePath(unixPath) + expect(result).toBe("/usr/local/bin/app") + }) + + test("should normalize Windows paths for cross-platform use", () => { + const windowsPath = "C:\\Users\\test\\file.txt" + const result = encodeFilePath(windowsPath) + // Should convert to forward slashes and add leading / + expect(result).not.toContain("\\") + expect(result).toMatch(/^\/[A-Za-z]%3A\//) + }) + + test("should handle relative paths the same on all platforms", () => { + const unixRelative = "src/app.ts" + const windowsRelative = "src\\app.ts" + + const unixResult = encodeFilePath(unixRelative) + const windowsResult = encodeFilePath(windowsRelative) + + // Both should normalize to forward slashes + expect(unixResult).toBe("src/app.ts") + expect(windowsResult).toBe("src/app.ts") + }) + }) + + describe("Edge cases", () => { + test("should handle empty path", () => { + const result = encodeFilePath("") + expect(result).toBe("") + }) + + test("should handle path with multiple consecutive slashes", () => { + const result = encodeFilePath("//path//to///file.txt") + // Multiple slashes should be preserved (backend handles normalization) + expect(result).toBe("//path//to///file.txt") + }) + + test("should encode Unicode characters", () => { + const unicodePath = "/home/user/文档/README.md" + const result = encodeFilePath(unicodePath) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + // Unicode should be encoded + expect(result).toContain("%E6%96%87%E6%A1%A3") + }) + + test("should handle already normalized Windows path", () => { + // Path that's already been normalized (has / before drive letter) + const alreadyNormalized = "/D:/path/file.txt" + const result = encodeFilePath(alreadyNormalized) + + // Should not add another leading slash + expect(result).toBe("/D%3A/path/file.txt") + expect(result).not.toContain("//D") + }) + + test("should handle just drive letter", () => { + const justDrive = "D:" + const result = encodeFilePath(justDrive) + const fileUrl = `file://${result}` + + expect(result).toBe("/D%3A") + expect(() => new URL(fileUrl)).not.toThrow() + }) + + test("should handle Windows path with trailing backslash", () => { + const trailingBackslash = "C:\\Users\\test\\" + const result = encodeFilePath(trailingBackslash) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toBe("/C%3A/Users/test/") + }) + + test("should handle very long paths", () => { + const longPath = "C:\\Users\\test\\" + "verylongdirectoryname\\".repeat(20) + "file.txt" + const result = encodeFilePath(longPath) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).not.toContain("\\") + }) + + test("should handle paths with dots", () => { + const pathWithDots = "C:\\Users\\..\\test\\.\\file.txt" + const result = encodeFilePath(pathWithDots) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + // Dots should be preserved (backend normalizes) + expect(result).toContain("..") + expect(result).toContain("/./") + }) + }) + + describe("Regression tests for PR #12424", () => { + test("should handle file with # in name", () => { + const path = "/path/to/file#name.txt" + const result = encodeFilePath(path) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toBe("/path/to/file%23name.txt") + }) + + test("should handle file with ? in name", () => { + const path = "/path/to/file?name.txt" + const result = encodeFilePath(path) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toBe("/path/to/file%3Fname.txt") + }) + + test("should handle file with % in name", () => { + const path = "/path/to/file%name.txt" + const result = encodeFilePath(path) + const fileUrl = `file://${result}` + + expect(() => new URL(fileUrl)).not.toThrow() + expect(result).toBe("/path/to/file%25name.txt") + }) + }) + + describe("Integration with file:// URL construction", () => { + test("should work with query parameters (Linux)", () => { + const path = "/home/user/file.txt" + const encoded = encodeFilePath(path) + const fileUrl = `file://${encoded}?start=10&end=20` + + const url = new URL(fileUrl) + expect(url.searchParams.get("start")).toBe("10") + expect(url.searchParams.get("end")).toBe("20") + expect(url.pathname).toBe("/home/user/file.txt") + }) + + test("should work with query parameters (Windows)", () => { + const path = "C:\\Users\\test\\file.txt" + const encoded = encodeFilePath(path) + const fileUrl = `file://${encoded}?start=10&end=20` + + const url = new URL(fileUrl) + expect(url.searchParams.get("start")).toBe("10") + expect(url.searchParams.get("end")).toBe("20") + }) + + test("should parse correctly in URL constructor (Linux)", () => { + const path = "/var/log/app.log" + const fileUrl = `file://${encodeFilePath(path)}` + const url = new URL(fileUrl) + + expect(url.protocol).toBe("file:") + expect(url.pathname).toBe("/var/log/app.log") + }) + + test("should parse correctly in URL constructor (Windows)", () => { + const path = "D:\\logs\\app.log" + const fileUrl = `file://${encodeFilePath(path)}` + const url = new URL(fileUrl) + + expect(url.protocol).toBe("file:") + expect(url.pathname).toContain("app.log") + }) + }) +}) diff --git a/packages/app/src/context/file/path.ts b/packages/app/src/context/file/path.ts index 155f05aaf..e1d47c644 100644 --- a/packages/app/src/context/file/path.ts +++ b/packages/app/src/context/file/path.ts @@ -81,7 +81,16 @@ export function decodeFilePath(input: string) { } export function encodeFilePath(filepath: string): string { - return filepath + // Normalize Windows paths: convert backslashes to forward slashes + let normalized = filepath.replace(/\\/g, "/") + + // Handle Windows absolute paths (D:/path -> /D:/path for proper file:// URLs) + if (/^[A-Za-z]:/.test(normalized)) { + normalized = "/" + normalized + } + + // Encode each path segment (preserving forward slashes as path separators) + return normalized .split("/") .map((segment) => encodeURIComponent(segment)) .join("/")