Files
orca/src/relay/git-handler-worktree-remove.ts
T
Neil 53adf5e2e6 fix(git): share one failed-command error-text reader between local and the SSH relay (#18398)
* fix(git): share one error-text reader between the local and relay branch-delete fallbacks

The relay and the desktop each carried their own `getErrorText`, and they had
drifted: the relay read `message` + `stderr` + `stdout`, the desktop only
`message` + `stderr`. A `git branch -d` refusal arriving on `stdout` therefore
routed the SSH removal through prune-and-retry while the local removal gave up
and preserved the branch.

Against a real binary the two agree, because Git prints the refusal through
`error()` on every supported version — verified on 2.25.1, 2.38.1, 2.49.1 and
2.55.0, none of which put a byte of it on stdout. What the desktop copy actually
missed is that Orca classifies errors it built itself, with the Git output on
`.stdout`: `worktree remove`'s submodule retry attaches `git status --porcelain`
that way on both paths. The stdout-reading form is also already the shared
spelling — `isSubmoduleWorktreeRemovalRefusal` uses it for both hosts — so this
converges on it rather than on the shorter one.

Move the reader to src/shared/git-command-failure-text.ts and the predicate it
feeds to src/shared/git-branch-delete-refusal.ts, and delete all three copies.
The predicate carries both refusal wordings live in the supported range: Git
through 2.40 says "checked out at", 2.43+ says "used by worktree at".

The real-binary contract now pins that boundary: the refusal is recognized, it
lands on stderr, and stdout stays empty on every Git in the matrix.

* fix(test): consolidate the duplicate worktree import in the parity test
2026-09-03 14:42:54 -07:00

208 lines
7.0 KiB
TypeScript

import * as path from 'node:path'
import type { RemoveWorktreeResult } from '../shared/worktree/create-types'
import { isBranchCheckedOutInWorktreeError } from '../shared/git-branch-delete-refusal'
import { assertWorktreeUnlockedForRemoval } from '../shared/worktree/removal'
import { isSubmoduleWorktreeRemovalRefusal } from '../shared/worktree/submodule-removal'
import { deleteAlreadyMergedRelayBranchAfterSafeDeleteFailure } from './git-handler-branch-cleanup'
import type { GitExec } from './git-handler-ops'
import type { GitCapabilityCache } from '../shared/git-capability-cache'
import { readRelayWorktreeList } from './git-handler-worktree-list'
function normalizeLocalBranchRef(branch: string): string {
return branch.replace(/^refs\/heads\//, '')
}
function isPosixAbsolutePath(value: string): boolean {
return value.startsWith('/')
}
function isWindowsAbsolutePath(value: string): boolean {
return /^[A-Za-z]:[\\/]/.test(value) || value.startsWith('\\\\')
}
function resolveRelayRepoPath(worktreePath: string, commonDir: string): string {
if (isPosixAbsolutePath(worktreePath) || isPosixAbsolutePath(commonDir)) {
// Why: tests can run on Windows while the relay operates on SSH/POSIX
// paths; the default path API would reinterpret "/repo" as "G:\repo".
return path.posix.resolve(worktreePath, commonDir, '..')
}
if (isWindowsAbsolutePath(worktreePath) || isWindowsAbsolutePath(commonDir)) {
return path.win32.resolve(worktreePath, commonDir, '..')
}
return path.resolve(worktreePath, commonDir, '..')
}
function normalizeRelayWorktreePathForCompare(value: string): string {
if (isPosixAbsolutePath(value)) {
return path.posix.normalize(path.posix.resolve(value))
}
if (isWindowsAbsolutePath(value)) {
return path.win32.normalize(path.win32.resolve(value))
}
return path.normalize(path.resolve(value))
}
function areRelayWorktreePathsEqual(leftPath: string, rightPath: string): boolean {
const left = normalizeRelayWorktreePathForCompare(leftPath)
const right = normalizeRelayWorktreePathForCompare(rightPath)
const compareCaseInsensitive = isWindowsAbsolutePath(leftPath) && isWindowsAbsolutePath(rightPath)
return compareCaseInsensitive ? left.toLowerCase() === right.toLowerCase() : left === right
}
async function listRelayWorktreesForRemoval(
git: GitExec,
repoPath: string,
capabilities: GitCapabilityCache
) {
try {
return await readRelayWorktreeList(git, repoPath, capabilities)
} catch {
return []
}
}
async function deleteRelayBranchAfterWorktreeRemoval(
git: GitExec,
repoPath: string,
branchName: string,
forceBranchDelete: boolean
): Promise<'deleted' | 'checked-out'> {
const deleteFlag = forceBranchDelete ? '-D' : '-d'
try {
await git(['branch', deleteFlag, '--', branchName], repoPath)
return 'deleted'
} catch (error) {
if (!isBranchCheckedOutInWorktreeError(error)) {
throw error
}
}
try {
// Why: branch deletion is the cheap live-checkout guard. Only prune when
// Git reports a checked-out branch, which may be stale worktree metadata.
await git(['worktree', 'prune'], repoPath)
} catch (error) {
console.warn(
`relay removeWorktree: failed to prune worktrees before deleting branch "${branchName}"`,
error
)
return 'checked-out'
}
try {
await git(['branch', deleteFlag, '--', branchName], repoPath)
return 'deleted'
} catch (error) {
if (isBranchCheckedOutInWorktreeError(error)) {
return 'checked-out'
}
throw error
}
}
export async function removeWorktreeOp(
git: GitExec,
params: Record<string, unknown>,
capabilities: GitCapabilityCache
): Promise<RemoveWorktreeResult> {
const worktreePath = params.worktreePath as string
const force = params.force as boolean | undefined
const deleteBranch = params.deleteBranch !== false
const forceBranchDelete = params.forceBranchDelete === true
let repoPath = worktreePath
try {
const { stdout } = await git(['rev-parse', '--git-common-dir'], worktreePath)
const commonDir = stdout.trim()
if (commonDir && commonDir !== '.git') {
repoPath = resolveRelayRepoPath(worktreePath, commonDir)
}
} catch {
// fall through with worktreePath as repo
}
const worktreesBeforeRemoval = await listRelayWorktreesForRemoval(git, repoPath, capabilities)
const removedWorktree = worktreesBeforeRemoval.find((worktree) =>
areRelayWorktreePathsEqual(worktree.path, worktreePath)
)
const branchName = normalizeLocalBranchRef(removedWorktree?.branch ?? '')
const branchHead = removedWorktree?.head ?? ''
assertWorktreeUnlockedForRemoval(removedWorktree)
const args = ['worktree', 'remove']
if (force) {
args.push('--force')
}
args.push(worktreePath)
try {
await git(args, repoPath)
} catch (error) {
if (force || !isSubmoduleWorktreeRemovalRefusal(error)) {
throw error
}
// Why: Git refuses non-force removal of any worktree with an initialised
// submodule even when everything is clean. Re-prove cleanliness (parent
// status reports dirty submodule content as ` M <sub>`), then --force.
const { stdout } = await git(['status', '--porcelain', '--untracked-files=all'], worktreePath)
if (stdout.trim()) {
const dirtyError = new Error('Worktree has uncommitted or untracked changes.')
;(dirtyError as Error & { stdout?: string }).stdout = stdout
throw dirtyError
}
await git(['worktree', 'remove', '--force', worktreePath], repoPath)
}
if (!branchName) {
return {}
}
if (!deleteBranch) {
return {}
}
// Why: SSH worktree deletion should mirror local deletion. Dropping the
// branch also removes its upstream config, which lets fork-remotes cleanup
// after the last PR review worktree is gone.
try {
// Why: use `-d` (not `-D`) to mirror the local removeWorktree fix.
const branchDeleteResult = await deleteRelayBranchAfterWorktreeRemoval(
git,
repoPath,
branchName,
forceBranchDelete
)
if (branchDeleteResult === 'checked-out') {
return {}
}
return {}
} catch (error) {
if (!forceBranchDelete && branchHead) {
try {
if (
await deleteAlreadyMergedRelayBranchAfterSafeDeleteFailure(
git,
repoPath,
branchName,
branchHead,
capabilities
)
) {
return {}
}
} catch (alreadyMergedDeleteError) {
// Why: worktree is gone; preserve branch recovery on cleanup races.
console.warn(
`relay removeWorktree: failed to delete already-merged local branch "${branchName}" after removing worktree`,
alreadyMergedDeleteError
)
}
}
// Expected when the branch still has unmerged/unpublished commits: keep it.
console.warn(
`relay removeWorktree: preserved local branch "${branchName}" after removing worktree (not fully merged)`,
error
)
return { preservedBranch: { branchName, ...(branchHead ? { head: branchHead } : {}) } }
}
}