From be3bce2f5bb66f660745e9d28d11ef3110a50d89 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:24:47 -0700 Subject: [PATCH] fix(worktree): let the worktree path alone decide whose home it is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A WSL project is registered under its UNC spelling (`\\wsl.localhost\\...`) while its worktree list is read by running git *inside the distro*, which answers in Linux paths and is handed back untranslated (`toWslExecutionSpace`, `worktree-list-reader.ts`). So the pair the removal guard actually receives is `( /home/neil , \\wsl.localhost\... )` — the normal WSL case, not a corrupted row. `isDangerousWorktreeRemovalPath` derived its path ops from that pair (`getPathOps(worktreePath, repoPath)` uses `.some(isWindowsAbsolutePathLike)`), so one Windows-shaped *repo* path picked win32 and every POSIX home-shape rule for the *worktree* path went quiet. `/home/` — the canonical Linux home — read back as an ordinary deletable directory, on the last guard standing in front of the recursive delete. Measured: `/home/neil` under `\\wsl.localhost\Ubuntu\opt\repo` and under `C:/repo` both returned `dangerous=false`. Whose home a path is, is a property of that path and nothing else, so the home guard now re-derives its ops from `worktreePath` alone. The repo-containment check keeps the pair, which genuinely compares two paths. Why no existing test saw it: the guard's unit-test helper called the single-argument form of the path-ops resolver, so those tests never exercised the two-argument call its only production caller makes. A test helper that paraphrases the caller instead of using it misses exactly the bugs that live in the difference. Pre-existing on main (the `pathOps !== posix` gate predates the rewrite) rather than a regression introduced by the surrounding stack, but it lives inside the function that stack rewrote and its new Windows/WSL rules inherit it. --- src/main/worktree-removal-home-guard.test.ts | 7 +++--- src/main/worktree-removal-home-guard.ts | 14 ++++++++--- src/main/worktree-removal-safety.test.ts | 26 ++++++++++++++++++++ src/main/worktree-removal-safety.ts | 2 +- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/main/worktree-removal-home-guard.test.ts b/src/main/worktree-removal-home-guard.test.ts index a562bb3c448..28dfdd56040 100644 --- a/src/main/worktree-removal-home-guard.test.ts +++ b/src/main/worktree-removal-home-guard.test.ts @@ -8,15 +8,14 @@ vi.mock('node:os', async (importOriginal) => { return { ...actual, homedir: homedirMock } }) -const { CLIENT_REMOVAL_HOME, executionHostRemovalHome, getPathOps, isHomeDirectoryRemovalPath } = +const { CLIENT_REMOVAL_HOME, executionHostRemovalHome, isHomeDirectoryRemovalPath } = await import('./worktree-removal-home-guard') function isHome( worktreePath: string, - home: Parameters[2] + home: Parameters[1] ): boolean { - const pathOps = getPathOps(worktreePath) - return isHomeDirectoryRemovalPath(pathOps.resolve(worktreePath), pathOps, home) + return isHomeDirectoryRemovalPath(worktreePath, home) } function withProcessPlatform(platform: NodeJS.Platform, callback: () => T): T { diff --git a/src/main/worktree-removal-home-guard.ts b/src/main/worktree-removal-home-guard.ts index 01031aa0b7c..f3982f6e8de 100644 --- a/src/main/worktree-removal-home-guard.ts +++ b/src/main/worktree-removal-home-guard.ts @@ -54,16 +54,24 @@ export function containsPath(parentPath: string, childPath: string, pathOps: Pat } /** - * Whether removing `resolvedWorktreePath` would take a home directory with it. + * Whether removing `worktreePath` would take a home directory with it. * * True when the path is, or contains, the home of the machine that executes the * removal, or when its shape is a home directory on the filesystem it names. + * + * Why the ops are re-derived from `worktreePath` alone: whose home a path is, is + * a property of that path and nothing else. The caller's `getPathOps(worktreePath, + * repoPath)` lets the *repo* spelling vote, and a WSL project is registered as + * `\\wsl.localhost\\...` while git-in-the-distro answers in Linux paths + * (`toWslExecutionSpace`), so the pair picked win32 and every POSIX home shape + * went quiet — `/home/` read back as an ordinary deletable directory. */ export function isHomeDirectoryRemovalPath( - resolvedWorktreePath: string, - pathOps: PathOps, + worktreePath: string, home: WorktreeRemovalHomeAuthority ): boolean { + const pathOps = getPathOps(worktreePath) + const resolvedWorktreePath = pathOps.resolve(worktreePath) const homePath = resolveGuardHomePath(home, pathOps) if (!!homePath && containsPath(resolvedWorktreePath, pathOps.resolve(homePath), pathOps)) { return true diff --git a/src/main/worktree-removal-safety.test.ts b/src/main/worktree-removal-safety.test.ts index 91859a3b423..122bd31e538 100644 --- a/src/main/worktree-removal-safety.test.ts +++ b/src/main/worktree-removal-safety.test.ts @@ -570,6 +570,32 @@ describe('isDangerousWorktreeRemovalPath on an execution host', () => { ) ).toBe(true) }) + + // A WSL project is registered under its UNC spelling while git-in-the-distro + // answers in Linux paths, so this exact pair reaches the guard. The repo path + // must not get a vote in whose home the *worktree* path is: pairing them made + // `getPathOps` pick win32 and every POSIX home shape went quiet. + it.each([ + ['/home/alice', '\\\\wsl.localhost\\Ubuntu\\srv\\repo'], + ['/home/alice', '//wsl.localhost/Ubuntu/srv/repo'], + ['/root', 'C:\\src\\repo'], + ['/Users/alice', 'C:/src/repo'] + ])('still recognises %s when the repo path is spelled %s', (worktreePath, repoPath) => { + expect( + isDangerousWorktreeRemovalPath(worktreePath, repoPath, executionHostRemovalHome(null)) + ).toBe(true) + expect(isDangerousWorktreeRemovalPath(worktreePath, repoPath, CLIENT_REMOVAL_HOME)).toBe(true) + }) + + it('keeps a linked WSL worktree deletable when the repo path is a UNC spelling', () => { + expect( + isDangerousWorktreeRemovalPath( + '/home/alice/workspaces/feature', + '\\\\wsl.localhost\\Ubuntu\\srv\\repo', + CLIENT_REMOVAL_HOME + ) + ).toBe(false) + }) }) describe('canSafelyRemoveOrphanedWorktreeDirectory on an execution host', () => { diff --git a/src/main/worktree-removal-safety.ts b/src/main/worktree-removal-safety.ts index 129a44cfbef..0d84e3af354 100644 --- a/src/main/worktree-removal-safety.ts +++ b/src/main/worktree-removal-safety.ts @@ -75,7 +75,7 @@ export function isDangerousWorktreeRemovalPath( return true } - return isHomeDirectoryRemovalPath(resolvedWorktreePath, pathOps, home) + return isHomeDirectoryRemovalPath(worktreePath, home) } export function getRegisteredDeletableWorktree(