mirror of
https://github.com/stablyai/orca.git
synced 2026-09-26 16:02:43 +00:00
fix(worktrees): recover malformed prunable git-file registrations
Use registration-only cleanup for proven named-branch .git regular-file rows before archive or checkout teardown. Keep all checkout files and the branch; document prune scope, dangling marker, older Git refusal and SSH boundaries. Addresses stablyai/orca#17316.
This commit is contained in:
@@ -0,0 +1,43 @@
|
||||
# Malformed worktree registration removal
|
||||
|
||||
Git can report a linked worktree at `<checkout>/.git` when its administrative
|
||||
`gitdir` backlink incorrectly ends in `.git/.git`. That reproduces #17316's
|
||||
validation error. The reproduction establishes the malformed registration, not
|
||||
which program created it; current OMP uses ordinary `git worktree add`.
|
||||
|
||||
Orca's desktop and runtime removal entry points use registration-only recovery
|
||||
when Git positively marks the row prunable, the row has a named local branch and
|
||||
HEAD, it is neither main nor locked, and the execution filesystem confirms the
|
||||
selected `.git` path is a regular file. Missing or unknown evidence does not
|
||||
permit this recovery. A symlink or directory is not a regular-file proof.
|
||||
|
||||
Recovery reuses `git worktree prune` followed by a strict worktree listing that
|
||||
must confirm the selected registration is gone. It does not delete the selected
|
||||
file, infer a parent path for deletion, or delete the branch. Archive hooks and
|
||||
checkout teardown are skipped because the selected row is not a checkout.
|
||||
|
||||
Two consequences are intentional:
|
||||
|
||||
- Git's prune also clears other stale, unlocked registrations in the repository;
|
||||
it is not a path-scoped command. Live and locked registrations remain Git's
|
||||
responsibility, and Orca verifies that the requested registration disappeared.
|
||||
- The surviving checkout's `.git` file points at removed administrative metadata.
|
||||
Files and its named branch are preserved; recovery removes the broken navigation
|
||||
entry and does not repair or claim to restore that checkout.
|
||||
|
||||
Native and WSL checks use the existing execution-filesystem accessor. WSL prune
|
||||
and verification use the same selected distro. Paired runtimes run the recovery
|
||||
on their owning host. Direct SSH does not enter this local recovery: its current
|
||||
provider has no registration-only removal operation, and a failed remote removal
|
||||
never authorizes a local fallback.
|
||||
|
||||
The Git commands already exist in the 2.25-compatible cleanup path. On an older
|
||||
Git that cannot positively attest this file-shaped registration as prunable, Orca
|
||||
refuses this recovery. Deferred deletion independently rejects non-directory and
|
||||
symlink targets, so force cannot move a `.git` file into deletion trash.
|
||||
|
||||
Regression coverage is in `worktree-prunable-git-file.test.ts`,
|
||||
`worktrees-removal-recovery.test.ts`, and
|
||||
`worktree-deferred-removal-real-git.test.ts`. The latter reproduces the exact
|
||||
malformation against the installed Git binary in a disposable repository and
|
||||
checks surviving file contents, branch HEAD, and removed registration.
|
||||
@@ -7,7 +7,9 @@ import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { promisify } from 'node:util'
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
import { removeWorktree } from './worktree'
|
||||
import { listWorktreesStrict, removeWorktree } from './worktree'
|
||||
import { isPrunableGitFileWorktree } from '../worktree-prunable-git-file'
|
||||
import { removeStaleLocalWorktreeRegistration } from '../local-worktree-removal-recovery'
|
||||
import {
|
||||
getWorktreeTrashRoot,
|
||||
isWorktreeTrashEntryName,
|
||||
@@ -114,6 +116,37 @@ describe('deferred worktree removal against the real Git binary', () => {
|
||||
expect(existsSync(getWorktreeTrashRoot(markerPath))).toBe(false)
|
||||
})
|
||||
|
||||
it('prunes a proven malformed registration while retaining checkout files and its branch', async () => {
|
||||
const markerPath = join(worktreePath, '.git')
|
||||
const marker = await readFile(markerPath, 'utf8')
|
||||
const adminPath = marker.trim().replace(/^gitdir: /, '')
|
||||
await writeFile(join(adminPath, 'gitdir'), `${join(markerPath, '.git')}\n`)
|
||||
await writeFile(join(worktreePath, 'untracked.txt'), 'keep this work\n')
|
||||
const row = (await listWorktreesStrict(repoPath)).find((entry) => entry.path === markerPath)
|
||||
expect(row).toBeDefined()
|
||||
if (!row) {
|
||||
throw new Error('Missing malformed registration')
|
||||
}
|
||||
expect(await isPrunableGitFileWorktree(row)).toBe(true)
|
||||
|
||||
const result = await removeStaleLocalWorktreeRegistration({
|
||||
canonicalWorktreePath: markerPath,
|
||||
repoPath,
|
||||
localWorktreeGitOptions: {},
|
||||
registeredWorktree: row,
|
||||
deleteBranch: true
|
||||
})
|
||||
|
||||
expect(result).toEqual({ preservedBranch: { branchName: 'feature', head: row.head } })
|
||||
expect(await readFile(markerPath, 'utf8')).toBe(marker)
|
||||
expect(await readFile(join(worktreePath, 'untracked.txt'), 'utf8')).toBe('keep this work\n')
|
||||
expect(await git(['rev-parse', 'refs/heads/feature'], repoPath)).toBe(`${row.head}\n`)
|
||||
expect((await listWorktreesStrict(repoPath)).some((entry) => entry.path === markerPath)).toBe(
|
||||
false
|
||||
)
|
||||
expect(existsSync(adminPath)).toBe(false)
|
||||
})
|
||||
|
||||
it('sweeps trash a previous run left behind', async () => {
|
||||
const stalePath = join(
|
||||
workspaceRoot,
|
||||
|
||||
@@ -427,6 +427,31 @@ describe('registerWorktreeHandlers', () => {
|
||||
expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'local')
|
||||
})
|
||||
|
||||
it('cleans a prunable Git-file row before archive or checkout teardown', async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), 'orca-prunable-ipc-'))
|
||||
const markerPath = join(root, '.git')
|
||||
await writeFile(markerPath, 'gitdir: /preserved/admin\n')
|
||||
const worktreeId = `repo-1::${markerPath}`
|
||||
const rows = mockKnownFeatureWorktree(markerPath).map((row) =>
|
||||
row.path === markerPath ? { ...row, branch: 'refs/heads/feature', prunable: true } : row
|
||||
)
|
||||
listWorktreesMock.mockResolvedValueOnce(rows).mockResolvedValue([])
|
||||
try {
|
||||
const result = await handlers['worktrees:remove'](null, { worktreeId })
|
||||
expect(result).toEqual({ preservedBranch: { branchName: 'feature', head: 'feature' } })
|
||||
expect(runHookMock).not.toHaveBeenCalled()
|
||||
expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled()
|
||||
expect(removeWorktreeMock).not.toHaveBeenCalled()
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'prune'], {
|
||||
cwd: '/workspace/repo'
|
||||
})
|
||||
expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'local')
|
||||
expect((await lstat(markerPath)).isFile()).toBe(true)
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('preserves a locked missing registration even with force', async () => {
|
||||
setPlatform('win32')
|
||||
const missingWorktreePath = 'C:\\workspace\\locked-already-removed'
|
||||
|
||||
@@ -8,8 +8,9 @@ import { getLocalProjectWorktreeGitOptions } from '../../../project-runtime-git-
|
||||
import { listWorktreesStrict as listGitWorktreesStrict } from '../../../git/worktree'
|
||||
import { requireSshGitProvider } from '../../../providers/ssh-git-dispatch'
|
||||
import { resolveWorktreeRemovalMetadata } from '../../../worktree-removal-repo-owner'
|
||||
import { isPrunableGitFileWorktree } from '../../../worktree-prunable-git-file'
|
||||
import { findRegisteredDeletableWorktree } from '../../../worktree-removal-safety'
|
||||
import { removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval } from '../../../local-worktree-removal-recovery'
|
||||
import { removeStaleLocalWorktreeRegistration } from '../../../local-worktree-removal-recovery'
|
||||
import { runHook } from '../../../hooks'
|
||||
import { withWorktreeRemoveStageSpan } from '../../../observability/instrumentation'
|
||||
import {
|
||||
@@ -85,13 +86,14 @@ export async function executeWorktreeRemoval(
|
||||
|
||||
if (
|
||||
!repo.connectionId &&
|
||||
args.force === true &&
|
||||
process.platform === 'win32' &&
|
||||
(isWindowsAbsolutePathLike(canonicalWorktreePath) || !!localWorktreeGitOptions.wslDistro) &&
|
||||
removedMeta &&
|
||||
(await isAlreadyRemovedWorktreePath(repo, canonicalWorktreePath, localWorktreeGitOptions))
|
||||
((await isPrunableGitFileWorktree(registeredWorktree, localWorktreeGitOptions)) ||
|
||||
(args.force === true &&
|
||||
process.platform === 'win32' &&
|
||||
(isWindowsAbsolutePathLike(canonicalWorktreePath) || !!localWorktreeGitOptions.wslDistro) &&
|
||||
removedMeta &&
|
||||
(await isAlreadyRemovedWorktreePath(repo, canonicalWorktreePath, localWorktreeGitOptions))))
|
||||
) {
|
||||
const removalResult = await removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({
|
||||
const removalResult = await removeStaleLocalWorktreeRegistration({
|
||||
canonicalWorktreePath,
|
||||
repoPath: repo.path,
|
||||
localWorktreeGitOptions,
|
||||
|
||||
@@ -22,7 +22,7 @@ vi.mock('./git/worktree', () => ({
|
||||
|
||||
import {
|
||||
recoverLocalWindowsWorktreeRemoval,
|
||||
removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval
|
||||
removeStaleLocalWorktreeRegistration
|
||||
} from './local-worktree-removal-recovery'
|
||||
|
||||
async function withPlatform<T>(platform: NodeJS.Platform, fn: () => Promise<T>): Promise<T> {
|
||||
@@ -306,7 +306,7 @@ describe('recoverLocalWindowsWorktreeRemoval', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval', () => {
|
||||
describe('removeStaleLocalWorktreeRegistration', () => {
|
||||
beforeEach(() => {
|
||||
gitExecFileAsyncMock.mockReset()
|
||||
listWorktreesStrictMock.mockReset()
|
||||
@@ -314,9 +314,27 @@ describe('removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval', () => {
|
||||
listWorktreesStrictMock.mockResolvedValue([])
|
||||
})
|
||||
|
||||
it('prunes and strictly verifies on the selected WSL host without deleting files or branches', async () => {
|
||||
const options = { wslDistro: 'Ubuntu' }
|
||||
const result = await removeStaleLocalWorktreeRegistration({
|
||||
canonicalWorktreePath: '/home/dev/feature/.git',
|
||||
repoPath: '/home/dev/repo',
|
||||
localWorktreeGitOptions: options,
|
||||
registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' },
|
||||
deleteBranch: true
|
||||
})
|
||||
expect(result).toEqual({ preservedBranch: { branchName: 'feature', head: 'abc123' } })
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledExactlyOnceWith(['worktree', 'prune'], {
|
||||
cwd: '/home/dev/repo',
|
||||
wslDistro: 'Ubuntu'
|
||||
})
|
||||
expect(listWorktreesStrictMock).toHaveBeenCalledExactlyOnceWith('/home/dev/repo', options)
|
||||
expect(removeLocalWorktreePathMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not override a locked missing registration', async () => {
|
||||
await expect(
|
||||
removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({
|
||||
removeStaleLocalWorktreeRegistration({
|
||||
canonicalWorktreePath: 'C:/workspaces/feature',
|
||||
repoPath: 'C:/repo',
|
||||
localWorktreeGitOptions: {},
|
||||
@@ -345,7 +363,7 @@ describe('removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval', () => {
|
||||
])
|
||||
|
||||
await expect(
|
||||
removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({
|
||||
removeStaleLocalWorktreeRegistration({
|
||||
canonicalWorktreePath: 'C:/workspaces/feature',
|
||||
repoPath: 'C:/repo',
|
||||
localWorktreeGitOptions: {},
|
||||
|
||||
@@ -47,7 +47,7 @@ function staleRegistrationRecoveryError(
|
||||
error,
|
||||
canonicalWorktreePath,
|
||||
force
|
||||
)} The worktree directory was removed, but Git still has stale worktree registration. Retry deletion after resolving the Git registration error.`
|
||||
)} Git still has stale worktree registration. Retry deletion after resolving the Git registration error.`
|
||||
)
|
||||
}
|
||||
|
||||
@@ -151,7 +151,7 @@ async function isRecoverableWindowsFilesystemRemovalFailure(
|
||||
}
|
||||
}
|
||||
|
||||
export async function removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval(
|
||||
export async function removeStaleLocalWorktreeRegistration(
|
||||
args: StaleLocalWorktreeRegistrationArgs
|
||||
): Promise<RemoveWorktreeResult> {
|
||||
return removeRequiredGitWorktreeRegistration(args)
|
||||
|
||||
@@ -13,13 +13,14 @@ import { invalidateAuthorizedRootsCache } from '../ipc/filesystem-auth'
|
||||
import { resolveWorktreeRemovalRoute } from '../worktree-removal-execution-host-route'
|
||||
import { getLocalProjectWorktreeGitOptions } from '../project-runtime-git-options'
|
||||
import { listWorktreesStrict } from '../git/worktree'
|
||||
import { isPrunableGitFileWorktree } from '../worktree-prunable-git-file'
|
||||
import { findRegisteredDeletableWorktree } from '../worktree-removal-safety'
|
||||
import { removeRuntimeUnregisteredWorktree } from './runtime-unregistered-worktree-removal'
|
||||
import { assertWorktreeUnlockedForRemoval } from '../../shared/worktree/removal'
|
||||
import { formatWorktreeRemovalError } from '../ipc/worktree-logic'
|
||||
import { isWindowsAbsolutePathLike } from '../../shared/cross-platform-path'
|
||||
import { isRuntimeWorktreePathMissing } from './runtime-worktree-filesystem'
|
||||
import { removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval } from '../local-worktree-removal-recovery'
|
||||
import { removeStaleLocalWorktreeRegistration } from '../local-worktree-removal-recovery'
|
||||
import { cleanupUnusedWorktreePushTargetRemote } from '../ipc/worktree-remote'
|
||||
import { removeRuntimeRegisteredRemoteWorktree } from './runtime-registered-remote-worktree-removal'
|
||||
import { removeRuntimeRegisteredLocalWorktree } from './runtime-registered-local-worktree-removal'
|
||||
@@ -146,18 +147,19 @@ export class OrcaRuntimeWithRemoveManagedWorktree extends OrcaRuntimeWithCreateM
|
||||
}
|
||||
if (
|
||||
route.kind === 'local' &&
|
||||
force === true &&
|
||||
process.platform === 'win32' &&
|
||||
(isWindowsAbsolutePathLike(canonicalWorktreePath) ||
|
||||
!!localWorktreeGitOptions.wslDistro) &&
|
||||
removedMeta &&
|
||||
(await isRuntimeWorktreePathMissing(
|
||||
route.hostId,
|
||||
canonicalWorktreePath,
|
||||
localWorktreeGitOptions
|
||||
))
|
||||
((await isPrunableGitFileWorktree(registeredWorktree, localWorktreeGitOptions)) ||
|
||||
(force === true &&
|
||||
process.platform === 'win32' &&
|
||||
(isWindowsAbsolutePathLike(canonicalWorktreePath) ||
|
||||
!!localWorktreeGitOptions.wslDistro) &&
|
||||
removedMeta &&
|
||||
(await isRuntimeWorktreePathMissing(
|
||||
route.hostId,
|
||||
canonicalWorktreePath,
|
||||
localWorktreeGitOptions
|
||||
))))
|
||||
) {
|
||||
const removalResult = await removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({
|
||||
const removalResult = await removeStaleLocalWorktreeRegistration({
|
||||
canonicalWorktreePath,
|
||||
repoPath: repo.path,
|
||||
localWorktreeGitOptions,
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { GitWorktreeInfo } from '../shared/worktree/types'
|
||||
import { isPrunableGitFileWorktree } from './worktree-prunable-git-file'
|
||||
|
||||
const { statPath, pathAccess, runtimePath } = vi.hoisted(() => ({
|
||||
statPath: vi.fn(),
|
||||
pathAccess: vi.fn(),
|
||||
runtimePath: vi.fn()
|
||||
}))
|
||||
vi.mock('./local-worktree-filesystem', () => ({
|
||||
getLocalWorktreePathAccess: pathAccess,
|
||||
toLocalWorktreeRuntimePath: runtimePath
|
||||
}))
|
||||
const worktree: GitWorktreeInfo = {
|
||||
path: '/workspaces/feature/.git',
|
||||
branch: 'refs/heads/feature',
|
||||
head: 'a'.repeat(40),
|
||||
isMainWorktree: false,
|
||||
isBare: false,
|
||||
prunable: true
|
||||
}
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks()
|
||||
statPath.mockResolvedValue({ isFile: () => true })
|
||||
pathAccess.mockReturnValue({ statPath })
|
||||
runtimePath.mockImplementation((path) => path)
|
||||
})
|
||||
describe('prunable Git-file registration proof', () => {
|
||||
it('accepts an attested named-branch file without reading or changing its parent', async () => {
|
||||
await expect(isPrunableGitFileWorktree(worktree)).resolves.toBe(true)
|
||||
expect(statPath).toHaveBeenCalledExactlyOnceWith(worktree.path)
|
||||
})
|
||||
it.each([
|
||||
{ prunable: false },
|
||||
{ prunable: undefined },
|
||||
{ isMainWorktree: true },
|
||||
{ isBare: true },
|
||||
{ locked: true },
|
||||
{ branch: '' },
|
||||
{ branch: 'refs/tags/feature' },
|
||||
{ branch: 'refs/heads/' },
|
||||
{ head: '' },
|
||||
{ path: '/workspaces/feature' }
|
||||
])('refuses insufficient registration evidence %j', async (override) => {
|
||||
await expect(isPrunableGitFileWorktree({ ...worktree, ...override })).resolves.toBe(false)
|
||||
expect(statPath).not.toHaveBeenCalled()
|
||||
})
|
||||
it.each([{ isFile: () => false }, { type: 'directory' }, { type: 'symlink' }, {}, null])(
|
||||
'refuses non-file or unknown filesystem evidence %j',
|
||||
async (entry) => {
|
||||
statPath.mockResolvedValue(entry)
|
||||
await expect(isPrunableGitFileWorktree(worktree)).resolves.toBe(false)
|
||||
}
|
||||
)
|
||||
it('does not turn host failure into cleanup permission', async () => {
|
||||
statPath.mockRejectedValue(new Error('host unavailable'))
|
||||
await expect(isPrunableGitFileWorktree(worktree)).rejects.toThrow('host unavailable')
|
||||
})
|
||||
it('uses the selected WSL distro and translated execution path', async () => {
|
||||
const options = { wslDistro: 'Ubuntu' }
|
||||
runtimePath.mockReturnValue('/home/dev/feature/.git')
|
||||
statPath.mockResolvedValue({ type: 'file' })
|
||||
await expect(
|
||||
isPrunableGitFileWorktree({ ...worktree, path: 'C:\\workspaces\\feature\\.git' }, options)
|
||||
).resolves.toBe(true)
|
||||
expect(pathAccess).toHaveBeenCalledExactlyOnceWith(options)
|
||||
expect(runtimePath).toHaveBeenCalledWith('C:\\workspaces\\feature\\.git', options)
|
||||
expect(statPath).toHaveBeenCalledExactlyOnceWith('/home/dev/feature/.git')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,32 @@
|
||||
import type { GitWorktreeInfo } from '../shared/worktree/types'
|
||||
import type { LocalWorktreeFilesystemOptions } from './local-worktree-filesystem'
|
||||
import { getLocalWorktreePathAccess, toLocalWorktreeRuntimePath } from './local-worktree-filesystem'
|
||||
|
||||
/** Registration cleanup must never reinterpret a malformed .git row as its parent checkout. */
|
||||
export async function isPrunableGitFileWorktree(
|
||||
worktree: GitWorktreeInfo,
|
||||
options: LocalWorktreeFilesystemOptions = {}
|
||||
): Promise<boolean> {
|
||||
if (
|
||||
worktree.prunable !== true ||
|
||||
worktree.isMainWorktree ||
|
||||
worktree.isBare ||
|
||||
worktree.locked ||
|
||||
!worktree.branch.startsWith('refs/heads/') ||
|
||||
worktree.branch === 'refs/heads/' ||
|
||||
!worktree.head ||
|
||||
worktree.path.split(/[\\/]/).at(-1) !== '.git'
|
||||
) {
|
||||
return false
|
||||
}
|
||||
const access = getLocalWorktreePathAccess(options)
|
||||
const entry = await access.statPath(toLocalWorktreeRuntimePath(worktree.path, options))
|
||||
if (!entry || typeof entry !== 'object') {
|
||||
return false
|
||||
}
|
||||
// WSL returns the owning guest's lstat-equivalent type; native lstat rejects symlinks too.
|
||||
return (
|
||||
('type' in entry && entry.type === 'file') ||
|
||||
('isFile' in entry && typeof entry.isFile === 'function' && entry.isFile() === true)
|
||||
)
|
||||
}
|
||||
Reference in New Issue
Block a user