Browse Source

fix(desktop): change detection on Windows, especially Cygwin (#13659)

Co-authored-by: LukeParkerDev <[email protected]>
Erik Demaine 1 month ago
parent
commit
a74fedd23b

+ 8 - 0
packages/app/src/context/file/path.test.ts

@@ -13,6 +13,14 @@ describe("file path helpers", () => {
     expect(path.pathFromTab("other://src/app.ts")).toBeUndefined()
   })
 
+  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("file://C:/repo/src/app.ts")).toBe("src/app.ts")
+    expect(path.normalize("c:\\repo\\src\\app.ts")).toBe("src/app.ts")
+  })
+
   test("keeps query/hash stripping behavior stable", () => {
     expect(stripQueryAndHash("a/b.ts#L12?x=1")).toBe("a/b.ts")
     expect(stripQueryAndHash("a/b.ts?x=1#L12")).toBe("a/b.ts")

+ 14 - 10
packages/app/src/context/file/path.ts

@@ -103,16 +103,20 @@ export function encodeFilePath(filepath: string): string {
 
 export function createPathHelpers(scope: () => string) {
   const normalize = (input: string) => {
-    const root = scope()
-    const prefix = root.endsWith("/") ? root : root + "/"
-
-    let path = unquoteGitPath(decodeFilePath(stripQueryAndHash(stripFileProtocol(input))))
-
-    if (path.startsWith(prefix)) {
-      path = path.slice(prefix.length)
-    }
-
-    if (path.startsWith(root)) {
+    const root = scope().replace(/\\/g, "/")
+
+    let path = unquoteGitPath(decodeFilePath(stripQueryAndHash(stripFileProtocol(input)))).replace(/\\/g, "/")
+
+    // 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
+    if (canonPath.startsWith(canonRoot) &&
+        (canonRoot.endsWith("/") || canonPath === canonRoot ||
+         canonPath.startsWith(canonRoot + "/"))) {
+      // If we match canonRoot + "/", the slash will be removed below.
       path = path.slice(root.length)
     }
 

+ 1 - 0
packages/app/src/i18n/en.ts

@@ -495,6 +495,7 @@ export const dict = {
   "session.review.change.other": "Changes",
   "session.review.loadingChanges": "Loading changes...",
   "session.review.empty": "No changes in this session yet",
+  "session.review.noVcs": "No git VCS detected, so session changes will not be detected",
   "session.review.noChanges": "No changes",
 
   "session.files.selectToOpen": "Select a file to open",

+ 6 - 1
packages/app/src/pages/session.tsx

@@ -274,6 +274,11 @@ export default function Page() {
     if (!hasReview()) return true
     return sync.data.session_diff[id] !== undefined
   })
+  const reviewEmptyKey = createMemo(() => {
+    const project = sync.project
+    if (!project || project.vcs) return "session.review.empty"
+    return "session.review.noVcs"
+  })
 
   let inputRef!: HTMLDivElement
   let promptDock: HTMLDivElement | undefined
@@ -531,7 +536,7 @@ export default function Page() {
             ) : (
               <div class={input.emptyClass}>
                 <Mark class="w-14 opacity-10" />
-                <div class="text-14-regular text-text-weak max-w-56">{language.t("session.review.empty")}</div>
+                <div class="text-14-regular text-text-weak max-w-56">{language.t(reviewEmptyKey())}</div>
               </div>
             )
           }

+ 22 - 21
packages/opencode/src/file/watcher.ts

@@ -46,7 +46,6 @@ export namespace FileWatcher {
 
   const state = Instance.state(
     async () => {
-      if (Instance.project.vcs !== "git") return {}
       log.info("init")
       const cfg = await Config.get()
       const backend = (() => {
@@ -88,26 +87,28 @@ export namespace FileWatcher {
         if (sub) subs.push(sub)
       }
 
-      const vcsDir = await $`git rev-parse --git-dir`
-        .quiet()
-        .nothrow()
-        .cwd(Instance.worktree)
-        .text()
-        .then((x) => path.resolve(Instance.worktree, x.trim()))
-        .catch(() => undefined)
-      if (vcsDir && !cfgIgnores.includes(".git") && !cfgIgnores.includes(vcsDir)) {
-        const gitDirContents = await readdir(vcsDir).catch(() => [])
-        const ignoreList = gitDirContents.filter((entry) => entry !== "HEAD")
-        const pending = w.subscribe(vcsDir, subscribe, {
-          ignore: ignoreList,
-          backend,
-        })
-        const sub = await withTimeout(pending, SUBSCRIBE_TIMEOUT_MS).catch((err) => {
-          log.error("failed to subscribe to vcsDir", { error: err })
-          pending.then((s) => s.unsubscribe()).catch(() => {})
-          return undefined
-        })
-        if (sub) subs.push(sub)
+      if (Instance.project.vcs === "git") {
+        const vcsDir = await $`git rev-parse --git-dir`
+          .quiet()
+          .nothrow()
+          .cwd(Instance.worktree)
+          .text()
+          .then((x) => path.resolve(Instance.worktree, x.trim()))
+          .catch(() => undefined)
+        if (vcsDir && !cfgIgnores.includes(".git") && !cfgIgnores.includes(vcsDir)) {
+          const gitDirContents = await readdir(vcsDir).catch(() => [])
+          const ignoreList = gitDirContents.filter((entry) => entry !== "HEAD")
+          const pending = w.subscribe(vcsDir, subscribe, {
+            ignore: ignoreList,
+            backend,
+          })
+          const sub = await withTimeout(pending, SUBSCRIBE_TIMEOUT_MS).catch((err) => {
+            log.error("failed to subscribe to vcsDir", { error: err })
+            pending.then((s) => s.unsubscribe()).catch(() => {})
+            return undefined
+          })
+          if (sub) subs.push(sub)
+        }
       }
 
       return { subs }

+ 17 - 4
packages/opencode/src/project/project.ts

@@ -17,6 +17,19 @@ import { Glob } from "../util/glob"
 
 export namespace Project {
   const log = Log.create({ service: "project" })
+
+  function gitpath(cwd: string, name: string) {
+    if (!name) return cwd
+    // git output includes trailing newlines; keep path whitespace intact.
+    name = name.replace(/[\r\n]+$/, "")
+    if (!name) return cwd
+
+    name = Filesystem.windowsPath(name)
+
+    if (path.isAbsolute(name)) return path.normalize(name)
+    return path.resolve(cwd, name)
+  }
+
   export const Info = z
     .object({
       id: z.string(),
@@ -141,7 +154,7 @@ export namespace Project {
         const top = await git(["rev-parse", "--show-toplevel"], {
           cwd: sandbox,
         })
-          .then(async (result) => path.resolve(sandbox, (await result.text()).trim()))
+          .then(async (result) => gitpath(sandbox, await result.text()))
           .catch(() => undefined)
 
         if (!top) {
@@ -159,9 +172,9 @@ export namespace Project {
           cwd: sandbox,
         })
           .then(async (result) => {
-            const dirname = path.dirname((await result.text()).trim())
-            if (dirname === ".") return sandbox
-            return dirname
+            const common = gitpath(sandbox, await result.text())
+            // Avoid going to parent of sandbox when git-common-dir is empty.
+            return common === sandbox ? sandbox : path.dirname(common)
           })
           .catch(() => undefined)
 

+ 1 - 4
packages/opencode/src/tool/bash.ts

@@ -124,11 +124,8 @@ export const BashTool = Tool.define("bash", async () => {
               .then((x) => x.trim())
             log.info("resolved path", { arg, resolved })
             if (resolved) {
-              // Git Bash on Windows returns Unix-style paths like /c/Users/...
               const normalized =
-                process.platform === "win32" && resolved.match(/^\/[a-z]\//)
-                  ? resolved.replace(/^\/([a-z])\//, (_, drive) => `${drive.toUpperCase()}:\\`).replace(/\//g, "\\")
-                  : resolved
+                process.platform === "win32" ? Filesystem.windowsPath(resolved).replace(/\//g, "\\") : resolved
               if (!Instance.containsPath(normalized)) {
                 const dir = (await Filesystem.isDir(normalized)) ? normalized : path.dirname(normalized)
                 directories.add(dir)

+ 12 - 0
packages/opencode/src/util/filesystem.ts

@@ -113,6 +113,18 @@ export namespace Filesystem {
     }
   }
 
+  export function windowsPath(p: string): string {
+    if (process.platform !== "win32") return p
+    return (
+      p
+        // Git Bash for Windows paths are typically /<drive>/...
+        .replace(/^\/([a-zA-Z])\//, (_, drive) => `${drive.toUpperCase()}:/`)
+        // Cygwin git paths are typically /cygdrive/<drive>/...
+        .replace(/^\/cygdrive\/([a-zA-Z])\//, (_, drive) => `${drive.toUpperCase()}:/`)
+        // WSL paths are typically /mnt/<drive>/...
+        .replace(/^\/mnt\/([a-zA-Z])\//, (_, drive) => `${drive.toUpperCase()}:/`)
+    )
+  }
   export function overlaps(a: string, b: string) {
     const relA = relative(a, b)
     const relB = relative(b, a)

+ 34 - 0
packages/opencode/test/util/filesystem.test.ts

@@ -286,6 +286,40 @@ describe("filesystem", () => {
     })
   })
 
+  describe("windowsPath()", () => {
+    test("converts Git Bash paths", () => {
+      if (process.platform === "win32") {
+        expect(Filesystem.windowsPath("/c/Users/test")).toBe("C:/Users/test")
+        expect(Filesystem.windowsPath("/d/dev/project")).toBe("D:/dev/project")
+      } else {
+        expect(Filesystem.windowsPath("/c/Users/test")).toBe("/c/Users/test")
+      }
+    })
+
+    test("converts Cygwin paths", () => {
+      if (process.platform === "win32") {
+        expect(Filesystem.windowsPath("/cygdrive/c/Users/test")).toBe("C:/Users/test")
+        expect(Filesystem.windowsPath("/cygdrive/x/dev/project")).toBe("X:/dev/project")
+      } else {
+        expect(Filesystem.windowsPath("/cygdrive/c/Users/test")).toBe("/cygdrive/c/Users/test")
+      }
+    })
+
+    test("converts WSL paths", () => {
+      if (process.platform === "win32") {
+        expect(Filesystem.windowsPath("/mnt/c/Users/test")).toBe("C:/Users/test")
+        expect(Filesystem.windowsPath("/mnt/z/dev/project")).toBe("Z:/dev/project")
+      } else {
+        expect(Filesystem.windowsPath("/mnt/c/Users/test")).toBe("/mnt/c/Users/test")
+      }
+    })
+
+    test("ignores normal Windows paths", () => {
+      expect(Filesystem.windowsPath("C:/Users/test")).toBe("C:/Users/test")
+      expect(Filesystem.windowsPath("D:\\dev\\project")).toBe("D:\\dev\\project")
+    })
+  })
+
   describe("writeStream()", () => {
     test("writes from Web ReadableStream", async () => {
       await using tmp = await tmpdir()