From 7cbc96ff92c8c5397653d46ae287f74593438830 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 4 May 2026 16:55:17 +0800 Subject: [PATCH 1/3] fix(windows): canonicalize rooted-but-driveless paths via drive-root probe assertExternalDirectory and read both call AppFileSystem.normalizePath, but on Windows that fell through to path.resolve which silently bound rooted-but- driveless paths (/users/runner/...) to cwd's drive. When the file lived on a different drive (typical on GitHub runners: temp on C: vs workspace on D:) the resulting permission glob used the wrong drive. Probe each known drive root for an existing path before falling back to win32.resolve. Move the helper into core/filesystem and delegate from opencode/util/filesystem so both paths share a single implementation. Closes #427 --- packages/core/src/filesystem.ts | 48 +++++++++++-- packages/opencode/src/util/filesystem.ts | 87 ++---------------------- 2 files changed, 50 insertions(+), 85 deletions(-) diff --git a/packages/core/src/filesystem.ts b/packages/core/src/filesystem.ts index cb40daa91..fae279f03 100644 --- a/packages/core/src/filesystem.ts +++ b/packages/core/src/filesystem.ts @@ -1,6 +1,6 @@ import { NodeFileSystem } from "@effect/platform-node" -import { dirname, join, relative, resolve as pathResolve } from "path" -import { realpathSync } from "fs" +import { dirname, join, relative, resolve as pathResolve, win32 } from "path" +import { existsSync, realpathSync } from "fs" import * as NFS from "fs/promises" import { lookup } from "mime-types" import { Context, Effect, FileSystem, Layer, Schema } from "effect" @@ -188,7 +188,7 @@ export namespace AppFileSystem { export function normalizePath(path: string): string { if (process.platform !== "win32") return path - const resolved = pathResolve(windowsPath(path)) + const resolved = normalizeWindowsAbsolutePath(windowsPath(path)) try { return realpathSync.native(resolved) } catch { @@ -206,7 +206,8 @@ export namespace AppFileSystem { } export function resolve(path: string): string { - const resolved = pathResolve(windowsPath(path)) + const resolved = + process.platform === "win32" ? normalizeWindowsAbsolutePath(windowsPath(path)) : pathResolve(path) try { return normalizePath(realpathSync(resolved)) } catch (error: any) { @@ -224,6 +225,45 @@ export namespace AppFileSystem { .replace(/^\/mnt\/([a-zA-Z])(?:\/|$)/, (_, drive) => `${drive.toUpperCase()}:/`) } + // Rooted but driveless paths (e.g. "/users/runner/...") arrive when callers + // strip the drive letter before handing the path back. path.resolve would + // bind such a path to the cwd's drive, which silently mismatches when the + // file actually lives on a different drive. Probe each known drive root for + // an existing file and fall back to win32.resolve only when none match. + function normalizeWindowsAbsolutePath(p: string): string { + const existing = resolveRootedWindowsVariant(p) + return win32.normalize(existing ?? win32.resolve(p)) + } + + function resolveRootedWindowsVariant(p: string): string | undefined { + if (!/^[\\/](?![\\/])/.test(p)) return + const suffix = p.replace(/^[\\/]+/, "").replaceAll("/", "\\") + for (const root of windowsDriveRoots()) { + const candidate = win32.join(root, suffix) + if (existsSync(candidate)) return candidate + } + } + + function windowsDriveRoots(): string[] { + const result: string[] = [] + const seen = new Set() + const push = (input?: string) => { + if (!input) return + const match = input.match(/^([A-Za-z]:)/) + if (!match) return + const root = match[1].toUpperCase() + if (seen.has(root)) return + seen.add(root) + result.push(root + "\\") + } + push(process.cwd()) + push(process.env.SystemDrive) + for (let code = 65; code <= 90; code++) { + push(String.fromCharCode(code) + ":") + } + return result + } + export function overlaps(a: string, b: string) { const relA = relative(a, b) const relB = relative(b, a) diff --git a/packages/opencode/src/util/filesystem.ts b/packages/opencode/src/util/filesystem.ts index b401f4813..904e7d5ca 100644 --- a/packages/opencode/src/util/filesystem.ts +++ b/packages/opencode/src/util/filesystem.ts @@ -1,10 +1,10 @@ import { chmod, mkdir, readFile, stat as statFile, writeFile } from "fs/promises" import { createWriteStream, existsSync, statSync } from "fs" import { lookup } from "mime-types" -import { realpathSync } from "fs" import { dirname, isAbsolute as pathIsAbsolute, join, relative, resolve as pathResolve, win32 } from "path" import { Readable } from "stream" import { pipeline } from "stream/promises" +import { AppFileSystem } from "@opencode-ai/core/filesystem" import { Glob } from "./glob" export namespace Filesystem { @@ -111,89 +111,14 @@ export namespace Filesystem { * This is needed because Windows paths are case-insensitive but LSP servers * may return paths with different casing than what we send them. */ - export function normalizePath(p: string): string { - if (process.platform !== "win32") return p - const resolved = normalizeWindowsAbsolutePath(windowsPath(p)) - try { - return realpathSync.native(resolved) - } catch { - return resolved - } - } - - export function normalizePathPattern(p: string): string { - if (process.platform !== "win32") return p - if (p === "*") return p - const match = p.match(/^(.*)[\\/]\*$/) - if (!match) return normalizePath(p) - const dir = /^[A-Za-z]:$/.test(match[1]) ? match[1] + "\\" : match[1] - return join(normalizePath(dir), "*") - } - - // We cannot rely on path.resolve() here because git.exe may come from Git Bash, Cygwin, or MSYS2, so we need to translate these paths at the boundary. - // Also resolves symlinks so that callers using the result as a cache key - // always get the same canonical path for a given physical directory. - export function resolve(p: string): string { - const resolved = process.platform === "win32" ? normalizeWindowsAbsolutePath(windowsPath(p)) : pathResolve(p) - try { - return normalizePath(realpathSync(resolved)) - } catch (e) { - if (isEnoent(e)) return normalizePath(resolved) - throw e - } - } - - export function windowsPath(p: string): string { - if (process.platform !== "win32") return p - return ( - p - .replace(/^\/([a-zA-Z]):(?:[\\/]|$)/, (_, drive) => `${drive.toUpperCase()}:/`) - // Git Bash for Windows paths are typically //... - .replace(/^\/([a-zA-Z])(?:\/|$)/, (_, drive) => `${drive.toUpperCase()}:/`) - // Cygwin git paths are typically /cygdrive//... - .replace(/^\/cygdrive\/([a-zA-Z])(?:\/|$)/, (_, drive) => `${drive.toUpperCase()}:/`) - // WSL paths are typically /mnt//... - .replace(/^\/mnt\/([a-zA-Z])(?:\/|$)/, (_, drive) => `${drive.toUpperCase()}:/`) - ) - } - - function normalizeWindowsAbsolutePath(p: string) { - const existing = resolveRootedWindowsVariant(p) - return win32.normalize(existing ?? win32.resolve(p)) - } - - function resolveRootedWindowsVariant(p: string) { - if (!/^[\\/](?![\\/])/.test(p)) return - const suffix = p.replace(/^[\\/]+/, "").replaceAll("/", "\\") - for (const root of windowsDriveRoots()) { - const candidate = win32.join(root, suffix) - if (existsSync(candidate)) return candidate - } - } - - function windowsDriveRoots() { - const result: string[] = [] - const seen = new Set() - const push = (input?: string) => { - if (!input) return - const match = input.match(/^([A-Za-z]:)/) - if (!match) return - const root = match[1].toUpperCase() - if (seen.has(root)) return - seen.add(root) - result.push(root + "\\") - } - push(process.cwd()) - push(process.env.SystemDrive) - for (let code = 65; code <= 90; code++) { - push(String.fromCharCode(code) + ":") - } - return result - } + export const normalizePath = AppFileSystem.normalizePath + export const normalizePathPattern = AppFileSystem.normalizePathPattern + export const resolve = AppFileSystem.resolve + export const windowsPath = AppFileSystem.windowsPath function comparablePath(p: string) { if (process.platform !== "win32") return pathResolve(p) - return win32.normalize(win32.resolve(windowsPath(p))) + return AppFileSystem.normalizePath(p) } function comparableRelative(from: string, to: string) { From 36a7bfc0311ece74c75dc68279b9be7278006a4e Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 4 May 2026 17:21:50 +0800 Subject: [PATCH 2/3] test(core): cover Windows path canonicalization variants Drive probe only repairs existing rooted-driveless paths; surface that contract as a test asserting non-existent targets still bind to cwd's drive. Cover Git Bash /c/, Cygwin /cygdrive/c/, WSL /mnt/c/ entry points plus the bare /users/... shape that motivated the fix. Tests are no-ops on macOS/Linux where normalizePath is the identity. --- packages/core/src/filesystem.ts | 5 + .../test/filesystem/normalize-path.test.ts | 136 ++++++++++++++++++ 2 files changed, 141 insertions(+) create mode 100644 packages/core/test/filesystem/normalize-path.test.ts diff --git a/packages/core/src/filesystem.ts b/packages/core/src/filesystem.ts index fae279f03..a49af2181 100644 --- a/packages/core/src/filesystem.ts +++ b/packages/core/src/filesystem.ts @@ -230,6 +230,11 @@ export namespace AppFileSystem { // bind such a path to the cwd's drive, which silently mismatches when the // file actually lives on a different drive. Probe each known drive root for // an existing file and fall back to win32.resolve only when none match. + // + // Important: this only repairs *existing* rooted-driveless paths. Targets + // that have not been created yet (e.g. write destinations) still fall back + // to the cwd-drive answer that win32.resolve produces. Callers that need + // a stable drive for a future path should pre-resolve via the project root. function normalizeWindowsAbsolutePath(p: string): string { const existing = resolveRootedWindowsVariant(p) return win32.normalize(existing ?? win32.resolve(p)) diff --git a/packages/core/test/filesystem/normalize-path.test.ts b/packages/core/test/filesystem/normalize-path.test.ts new file mode 100644 index 000000000..85f9ca608 --- /dev/null +++ b/packages/core/test/filesystem/normalize-path.test.ts @@ -0,0 +1,136 @@ +import { test, expect } from "bun:test" +import path from "path" +import os from "os" +import fs from "fs" +import { AppFileSystem } from "@opencode-ai/core/filesystem" + +// All Windows variants below collapse to the same canonical form on Windows. +// On macOS/Linux normalizePath is the identity function, so we keep the +// Windows-only guards explicit to avoid accidental cross-platform skew. + +test("normalizePath is identity on non-Windows", () => { + if (process.platform === "win32") return + const p = "/usr/local/bin/foo" + expect(AppFileSystem.normalizePath(p)).toBe(p) +}) + +test.skipIf(process.platform !== "win32")( + "normalizePath probes drive roots for rooted-but-driveless paths to existing files", + () => { + const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), "pawwork-fs-")) + try { + const file = path.join(tmpdir, "marker.txt") + fs.writeFileSync(file, "x") + + const driveless = file.replace(/^[A-Za-z]:/, "").replaceAll("\\", "/").toLowerCase() + const result = AppFileSystem.normalizePath(driveless) + + expect(result.toLowerCase()).toBe(file.toLowerCase()) + } finally { + fs.rmSync(tmpdir, { recursive: true, force: true }) + } + }, +) + +test.skipIf(process.platform !== "win32")( + "normalizePath canonicalizes Git Bash /c/... style paths", + () => { + const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), "pawwork-fs-")) + try { + const file = path.join(tmpdir, "marker.txt") + fs.writeFileSync(file, "x") + + const drive = file.match(/^([A-Za-z]):/)![1].toLowerCase() + const tail = file.slice(2).replaceAll("\\", "/") + const gitBash = `/${drive}${tail}` + const result = AppFileSystem.normalizePath(gitBash) + + expect(result.toLowerCase()).toBe(file.toLowerCase()) + } finally { + fs.rmSync(tmpdir, { recursive: true, force: true }) + } + }, +) + +test.skipIf(process.platform !== "win32")( + "normalizePath canonicalizes Cygwin /cygdrive/c/... paths", + () => { + const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), "pawwork-fs-")) + try { + const file = path.join(tmpdir, "marker.txt") + fs.writeFileSync(file, "x") + + const drive = file.match(/^([A-Za-z]):/)![1].toLowerCase() + const tail = file.slice(2).replaceAll("\\", "/") + const cygwin = `/cygdrive/${drive}${tail}` + const result = AppFileSystem.normalizePath(cygwin) + + expect(result.toLowerCase()).toBe(file.toLowerCase()) + } finally { + fs.rmSync(tmpdir, { recursive: true, force: true }) + } + }, +) + +test.skipIf(process.platform !== "win32")( + "normalizePath canonicalizes WSL /mnt/c/... paths", + () => { + const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), "pawwork-fs-")) + try { + const file = path.join(tmpdir, "marker.txt") + fs.writeFileSync(file, "x") + + const drive = file.match(/^([A-Za-z]):/)![1].toLowerCase() + const tail = file.slice(2).replaceAll("\\", "/") + const wsl = `/mnt/${drive}${tail}` + const result = AppFileSystem.normalizePath(wsl) + + expect(result.toLowerCase()).toBe(file.toLowerCase()) + } finally { + fs.rmSync(tmpdir, { recursive: true, force: true }) + } + }, +) + +test.skipIf(process.platform !== "win32")( + "normalizePath leaves drive-prefixed paths intact for already-canonical input", + () => { + const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), "pawwork-fs-")) + try { + const file = path.join(tmpdir, "marker.txt") + fs.writeFileSync(file, "x") + + const result = AppFileSystem.normalizePath(file) + expect(result.toLowerCase()).toBe(file.toLowerCase()) + } finally { + fs.rmSync(tmpdir, { recursive: true, force: true }) + } + }, +) + +test.skipIf(process.platform !== "win32")( + "normalizePath falls back to cwd-drive resolution for non-existent rooted-driveless paths", + () => { + // The probe only repairs existing paths; documenting behavior so future + // changes don't accidentally start guessing for write targets. + const driveless = "/this/path/should/not/exist/anywhere/marker.txt" + const result = AppFileSystem.normalizePath(driveless) + expect(result).toMatch(/^[A-Za-z]:/) + }, +) + +test.skipIf(process.platform !== "win32")( + "normalizePathPattern preserves trailing /* glob", + () => { + const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), "pawwork-fs-")) + try { + const driveless = tmpdir.replace(/^[A-Za-z]:/, "").replaceAll("\\", "/").toLowerCase() + const pattern = AppFileSystem.normalizePathPattern(`${driveless}/*`) + + expect(pattern.endsWith("\\*") || pattern.endsWith("/*")).toBe(true) + expect(pattern.toLowerCase()).toContain(tmpdir.toLowerCase()) + } finally { + fs.rmSync(tmpdir, { recursive: true, force: true }) + } + }, +) From af8a4b79bf309c4499f0f791c23cc9bac7871e9b Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 4 May 2026 17:39:29 +0800 Subject: [PATCH 3/3] test(core): assert exact cwd-drive fallback for non-existent paths toMatch(/^[A-Za-z]:/) accepted any drive letter, so the fallback could land on the wrong drive without failing the test. Construct the expected cwd-drive answer and assert exact equality. --- packages/core/test/filesystem/normalize-path.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/core/test/filesystem/normalize-path.test.ts b/packages/core/test/filesystem/normalize-path.test.ts index 85f9ca608..d81fb49d6 100644 --- a/packages/core/test/filesystem/normalize-path.test.ts +++ b/packages/core/test/filesystem/normalize-path.test.ts @@ -114,8 +114,11 @@ test.skipIf(process.platform !== "win32")( // The probe only repairs existing paths; documenting behavior so future // changes don't accidentally start guessing for write targets. const driveless = "/this/path/should/not/exist/anywhere/marker.txt" + const cwdDrive = process.cwd().match(/^([A-Za-z]:)/)![1].toUpperCase() + const expected = path.win32.normalize(path.win32.join(`${cwdDrive}\\`, driveless.replaceAll("/", "\\"))) const result = AppFileSystem.normalizePath(driveless) - expect(result).toMatch(/^[A-Za-z]:/) + + expect(result.toUpperCase()).toBe(expected.toUpperCase()) }, )