mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 08:02:43 +00:00
fix(worktree): let the worktree path alone decide whose home it is
A WSL project is registered under its UNC spelling (`\\wsl.localhost\<distro>\...`) 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/<user>` — 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.
This commit is contained in:
@@ -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<typeof isHomeDirectoryRemovalPath>[2]
|
||||
home: Parameters<typeof isHomeDirectoryRemovalPath>[1]
|
||||
): boolean {
|
||||
const pathOps = getPathOps(worktreePath)
|
||||
return isHomeDirectoryRemovalPath(pathOps.resolve(worktreePath), pathOps, home)
|
||||
return isHomeDirectoryRemovalPath(worktreePath, home)
|
||||
}
|
||||
|
||||
function withProcessPlatform<T>(platform: NodeJS.Platform, callback: () => T): T {
|
||||
|
||||
@@ -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\<distro>\...` while git-in-the-distro answers in Linux paths
|
||||
* (`toWslExecutionSpace`), so the pair picked win32 and every POSIX home shape
|
||||
* went quiet — `/home/<user>` 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
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -75,7 +75,7 @@ export function isDangerousWorktreeRemovalPath(
|
||||
return true
|
||||
}
|
||||
|
||||
return isHomeDirectoryRemovalPath(resolvedWorktreePath, pathOps, home)
|
||||
return isHomeDirectoryRemovalPath(worktreePath, home)
|
||||
}
|
||||
|
||||
export function getRegisteredDeletableWorktree(
|
||||
|
||||
Reference in New Issue
Block a user