Improve preserved branch cleanup after squash merges (#6014)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Brennan Benson
GitHub
parent
2ed4346c2d
commit
8228155e4d
|
|
@ -398,6 +398,90 @@ branch refs/heads/main
|
|||
expect(calls).toContain('git config --remove-section branch.feature/test')
|
||||
})
|
||||
|
||||
it('deletes a squash-merged branch with branch-only merge commits via expected head', async () => {
|
||||
mockGitCommands({
|
||||
'git worktree list --porcelain -z': {
|
||||
stdout: `worktree /repo
|
||||
HEAD abc123
|
||||
branch refs/heads/main
|
||||
|
||||
worktree /repo-feature
|
||||
HEAD def456
|
||||
branch refs/heads/feature/test
|
||||
`
|
||||
},
|
||||
'git worktree list --porcelain -z#2': {
|
||||
stdout: `worktree /repo
|
||||
HEAD abc123
|
||||
branch refs/heads/main
|
||||
`
|
||||
},
|
||||
'git worktree list --porcelain': {
|
||||
stdout: `worktree /repo
|
||||
HEAD abc123
|
||||
branch refs/heads/main
|
||||
`
|
||||
},
|
||||
'git branch -d -- feature/test': {
|
||||
error: new Error('branch delete failed'),
|
||||
stderr: 'error: the branch feature/test is not fully merged'
|
||||
},
|
||||
'git config --get branch.feature/test.base': {
|
||||
stdout: 'refs/remotes/origin/main\n'
|
||||
},
|
||||
'git rev-parse --verify --quiet refs/remotes/origin/main^{commit}': {
|
||||
stdout: 'target123\n'
|
||||
},
|
||||
'git merge-tree --write-tree target123 refs/heads/feature/test': {
|
||||
stdout: 'merged-tree\n'
|
||||
},
|
||||
'git rev-parse --verify --quiet target123^{tree}': {
|
||||
stdout: 'target-tree\n'
|
||||
},
|
||||
'git rev-list --right-only --merges --count target123...refs/heads/feature/test': {
|
||||
stdout: '1\n'
|
||||
},
|
||||
'git merge-base target123 refs/heads/feature/test': {
|
||||
stdout: 'base123\n'
|
||||
},
|
||||
'git diff base123 refs/heads/feature/test': {
|
||||
stdout: 'branch net diff\n'
|
||||
},
|
||||
'git patch-id --stable#1': {
|
||||
stdout: 'patch123 0000000000000000000000000000000000000000\n'
|
||||
},
|
||||
'git rev-list --ancestry-path --max-count=201 base123..target123': {
|
||||
stdout: 'squash123\n'
|
||||
},
|
||||
'git show --format= squash123': {
|
||||
stdout: 'squash diff\n'
|
||||
},
|
||||
'git patch-id --stable#2': {
|
||||
stdout: 'patch123 squash123\n'
|
||||
},
|
||||
'git merge-tree --write-tree squash123 refs/heads/feature/test': {
|
||||
stdout: 'squash-tree\n'
|
||||
},
|
||||
'git rev-parse --verify --quiet squash123^{tree}': {
|
||||
stdout: 'squash-tree\n'
|
||||
}
|
||||
})
|
||||
|
||||
await expect(removeWorktree('/repo', '/repo-feature')).resolves.toEqual({})
|
||||
|
||||
const calls = getGitCalls()
|
||||
expect(calls).toContain('git update-ref -d refs/heads/feature/test def456')
|
||||
expect(calls).toContain('git config --remove-section branch.feature/test')
|
||||
expect(gitExecFileAsyncMock.mock.calls).toContainEqual([
|
||||
['patch-id', '--stable'],
|
||||
{ cwd: '/repo', stdin: 'branch net diff\n' }
|
||||
])
|
||||
expect(gitExecFileAsyncMock.mock.calls).toContainEqual([
|
||||
['patch-id', '--stable'],
|
||||
{ cwd: '/repo', stdin: 'squash diff\n' }
|
||||
])
|
||||
})
|
||||
|
||||
it('refreshes the saved remote base before deleting a safe-delete-rejected branch', async () => {
|
||||
mockGitCommands({
|
||||
'git worktree list --porcelain -z': {
|
||||
|
|
|
|||
|
|
@ -250,6 +250,7 @@ type GitExecOptions = {
|
|||
encoding?: BufferEncoding | 'buffer'
|
||||
maxBuffer?: number
|
||||
timeout?: number
|
||||
stdin?: string
|
||||
env?: NodeJS.ProcessEnv
|
||||
signal?: AbortSignal
|
||||
wslDistro?: string
|
||||
|
|
@ -313,6 +314,7 @@ function killSpawnedCommandTree(child: ChildProcess): void {
|
|||
|
||||
type ExecFileCaptureOptions = Omit<ExecFileOptions, 'timeout'> & {
|
||||
timeout?: number
|
||||
stdin?: string
|
||||
}
|
||||
|
||||
function emptyExecFileOutput(options: ExecFileCaptureOptions): string | Buffer {
|
||||
|
|
@ -402,6 +404,12 @@ function execFileCapture(
|
|||
return
|
||||
}
|
||||
|
||||
child.once('error', (error) => finish(error))
|
||||
|
||||
if (options.stdin !== undefined) {
|
||||
child.stdin?.end(options.stdin)
|
||||
}
|
||||
|
||||
// Why: Node's native execFile timeout waits for the child to exit after
|
||||
// signaling it. Some CLIs ignore that signal, so reject the UI operation
|
||||
// on our own timer and kill the child only as best effort.
|
||||
|
|
@ -740,6 +748,7 @@ export async function gitExecFileAsync(
|
|||
encoding: (options.encoding ?? 'utf-8') as BufferEncoding,
|
||||
maxBuffer: options.maxBuffer,
|
||||
timeout: options.timeout,
|
||||
stdin: options.stdin,
|
||||
// Why: never let a git read-path call block on an interactive prompt
|
||||
// (issue #5308) — fail fast instead of hanging the runtime.
|
||||
env: policy.env,
|
||||
|
|
|
|||
|
|
@ -944,7 +944,11 @@ async function deleteAlreadyMergedBranchAfterSafeDeleteFailure(
|
|||
branchHead: string,
|
||||
options: GitWorktreeExecOptions = {}
|
||||
): Promise<boolean> {
|
||||
const runGit = (args: string[]) => gitExecFileAsync(args, gitExecOptions(repoPath, options))
|
||||
const runGit = (args: string[], execOptions?: { stdin?: string }) =>
|
||||
gitExecFileAsync(args, {
|
||||
...gitExecOptions(repoPath, options),
|
||||
...(execOptions?.stdin !== undefined ? { stdin: execOptions.stdin } : {})
|
||||
})
|
||||
const targetRefs = await getBranchCleanupTargetRefs(runGit, branchName)
|
||||
await refreshBranchCleanupTargetRefs(runGit, targetRefs)
|
||||
// Why: squash merges rewrite commit IDs, so `branch -d` can reject a branch
|
||||
|
|
|
|||
|
|
@ -77,6 +77,90 @@ describe('removeWorktreeOp branch cleanup', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('deletes a squash-merged SSH branch with branch-only merge commits via expected head', async () => {
|
||||
let zListCount = 0
|
||||
const git = vi.fn<GitExec>(async (args, _cwd, opts) => {
|
||||
if (args[0] === 'rev-parse' && args[1] === '--git-common-dir') {
|
||||
return { stdout: '/repo/.git\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'worktree' && args[1] === 'list' && args.includes('-z')) {
|
||||
zListCount += 1
|
||||
return {
|
||||
stdout:
|
||||
zListCount === 1
|
||||
? worktreeList(
|
||||
{ path: '/repo', branch: 'main' },
|
||||
{ path: '/repo-feature', branch: 'feature/test' }
|
||||
)
|
||||
: worktreeList({ path: '/repo', branch: 'main' }),
|
||||
stderr: ''
|
||||
}
|
||||
}
|
||||
if (args[0] === 'worktree' && args[1] === 'list') {
|
||||
return { stdout: worktreeList({ path: '/repo', branch: 'main' }), stderr: '' }
|
||||
}
|
||||
if (args[0] === 'branch' && args[1] === '-d') {
|
||||
throw new Error('error: the branch feature/test is not fully merged')
|
||||
}
|
||||
if (args[0] === 'config' && args[1] === '--get') {
|
||||
return { stdout: 'refs/remotes/origin/main\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'rev-parse' && args.includes('refs/remotes/origin/main^{commit}')) {
|
||||
return { stdout: 'target123\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'merge-tree') {
|
||||
return {
|
||||
stdout: args[2] === 'squash123' ? 'squash-tree\n' : 'merged-tree\n',
|
||||
stderr: ''
|
||||
}
|
||||
}
|
||||
if (args[0] === 'rev-parse' && args.includes('target123^{tree}')) {
|
||||
return { stdout: 'target-tree\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'rev-parse' && args.includes('squash123^{tree}')) {
|
||||
return { stdout: 'squash-tree\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'rev-list' && args.includes('--right-only')) {
|
||||
return { stdout: '1\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'merge-base') {
|
||||
return { stdout: 'base123\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'diff') {
|
||||
return { stdout: 'branch net diff\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'rev-list' && args.includes('--ancestry-path')) {
|
||||
return { stdout: 'squash123\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'show') {
|
||||
return { stdout: 'squash diff\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'patch-id' && opts?.stdin === 'branch net diff\n') {
|
||||
return {
|
||||
stdout: 'patch123 0000000000000000000000000000000000000000\n',
|
||||
stderr: ''
|
||||
}
|
||||
}
|
||||
if (args[0] === 'patch-id' && opts?.stdin === 'squash diff\n') {
|
||||
return { stdout: 'patch123 squash123\n', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
await expect(removeWorktreeOp(git, { worktreePath: '/repo-feature' })).resolves.toEqual({})
|
||||
|
||||
expect(git).toHaveBeenCalledWith(
|
||||
['update-ref', '-d', 'refs/heads/feature/test', '1'],
|
||||
expect.any(String)
|
||||
)
|
||||
expect(git).toHaveBeenCalledWith(['patch-id', '--stable'], expect.any(String), {
|
||||
stdin: 'branch net diff\n'
|
||||
})
|
||||
expect(git).toHaveBeenCalledWith(['patch-id', '--stable'], expect.any(String), {
|
||||
stdin: 'squash diff\n'
|
||||
})
|
||||
})
|
||||
|
||||
it('refreshes the saved remote base before deleting a safe-delete-rejected SSH branch', async () => {
|
||||
const calls: { args: string[]; cwd: string }[] = []
|
||||
let zListCount = 0
|
||||
|
|
|
|||
|
|
@ -12,7 +12,8 @@ export async function deleteAlreadyMergedRelayBranchAfterSafeDeleteFailure(
|
|||
branchName: string,
|
||||
branchHead: string
|
||||
): Promise<boolean> {
|
||||
const runGit = (args: string[]) => git(args, repoPath)
|
||||
const runGit = (args: string[], options?: { stdin?: string }) =>
|
||||
options ? git(args, repoPath, options) : git(args, repoPath)
|
||||
const targetRefs = await getBranchCleanupTargetRefs(runGit, branchName)
|
||||
await refreshBranchCleanupTargetRefs(runGit, targetRefs)
|
||||
// Why: SSH worktrees hit the same squash-merge shape as local worktrees.
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ import { readWorkingDiffFile } from './git-working-file-read'
|
|||
export type GitExec = (
|
||||
args: string[],
|
||||
cwd: string,
|
||||
opts?: { maxBuffer?: number; disableOptionalLocks?: boolean }
|
||||
opts?: { maxBuffer?: number; disableOptionalLocks?: boolean; stdin?: string }
|
||||
) => Promise<{ stdout: string; stderr: string }>
|
||||
|
||||
export type GitBufferExec = (args: string[], cwd: string) => Promise<Buffer>
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
/* eslint-disable max-lines -- Why: this relay handler centralizes the git RPC
|
||||
protocol surface so local and SSH git behavior stay in one dispatch table. */
|
||||
import { execFile, spawn } from 'child_process'
|
||||
import { execFile, spawn, type ExecFileOptions } from 'child_process'
|
||||
import { promisify } from 'util'
|
||||
import * as path from 'path'
|
||||
import type { RelayDispatcher, RequestContext } from './dispatcher'
|
||||
|
|
@ -51,6 +51,41 @@ const execFileAsync = promisify(execFile)
|
|||
const MAX_GIT_BUFFER = 10 * 1024 * 1024
|
||||
const BULK_CHUNK_SIZE = 100
|
||||
|
||||
function execFileWithStdin(
|
||||
command: string,
|
||||
args: string[],
|
||||
options: ExecFileOptions,
|
||||
stdin: string
|
||||
): Promise<{ stdout: string; stderr: string }> {
|
||||
return new Promise((resolve, reject) => {
|
||||
let settled = false
|
||||
const finish = (
|
||||
error: Error | null,
|
||||
stdout: string | Buffer = '',
|
||||
stderr: string | Buffer = ''
|
||||
): void => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
settled = true
|
||||
if (error) {
|
||||
reject(Object.assign(error, { stdout, stderr }))
|
||||
return
|
||||
}
|
||||
resolve({ stdout: String(stdout), stderr: String(stderr) })
|
||||
}
|
||||
const child = execFile(command, args, options, (error, stdout, stderr) => {
|
||||
if (error) {
|
||||
finish(error, stdout, stderr)
|
||||
return
|
||||
}
|
||||
finish(null, stdout, stderr)
|
||||
})
|
||||
child.once('error', (error) => finish(error))
|
||||
child.stdin?.end(stdin)
|
||||
})
|
||||
}
|
||||
|
||||
export class GitHandler {
|
||||
private dispatcher: RelayDispatcher
|
||||
|
||||
|
|
@ -114,6 +149,7 @@ export class GitHandler {
|
|||
disableOptionalLocks?: boolean
|
||||
signal?: AbortSignal
|
||||
nonInteractive?: boolean
|
||||
stdin?: string
|
||||
}
|
||||
): Promise<{ stdout: string; stderr: string }> {
|
||||
const env = buildRelayCommandEnv()
|
||||
|
|
@ -126,13 +162,18 @@ export class GitHandler {
|
|||
env.SSH_ASKPASS = ''
|
||||
env.GIT_SSH_COMMAND ??= 'ssh -o BatchMode=yes'
|
||||
}
|
||||
return execFileAsync('git', args, {
|
||||
const execOptions = {
|
||||
cwd: expandTilde(cwd),
|
||||
env,
|
||||
encoding: 'utf-8',
|
||||
maxBuffer: opts?.maxBuffer ?? MAX_GIT_BUFFER,
|
||||
signal: opts?.signal
|
||||
})
|
||||
} satisfies ExecFileOptions
|
||||
if (opts?.stdin !== undefined) {
|
||||
return execFileWithStdin('git', args, execOptions, opts.stdin)
|
||||
}
|
||||
const { stdout, stderr } = await execFileAsync('git', args, execOptions)
|
||||
return { stdout: String(stdout), stderr: String(stderr) }
|
||||
}
|
||||
|
||||
private async gitBuffer(args: string[], cwd: string): Promise<Buffer> {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,105 @@
|
|||
// @vitest-environment happy-dom
|
||||
|
||||
import { act } from 'react'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { toast } from 'sonner'
|
||||
import type { RemoveWorktreeResult } from '../../../../shared/types'
|
||||
import { showPreservedBranchToast } from './preserved-branch-toast'
|
||||
|
||||
vi.mock('sonner', () => ({
|
||||
toast: {
|
||||
warning: vi.fn(),
|
||||
dismiss: vi.fn()
|
||||
}
|
||||
}))
|
||||
|
||||
const mountedRoots: Root[] = []
|
||||
|
||||
function renderToastBody(): HTMLElement {
|
||||
const description = vi.mocked(toast.warning).mock.calls.at(-1)?.[1]
|
||||
?.description as React.ReactElement
|
||||
const container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
const root = createRoot(container)
|
||||
mountedRoots.push(root)
|
||||
act(() => {
|
||||
root.render(description)
|
||||
})
|
||||
return container
|
||||
}
|
||||
|
||||
function clickButton(container: HTMLElement, label: string): void {
|
||||
const button = [...container.querySelectorAll('button')].find(
|
||||
(el) => el.textContent?.trim() === label
|
||||
)
|
||||
if (!button) {
|
||||
throw new Error(`button "${label}" not found`)
|
||||
}
|
||||
act(() => {
|
||||
button.dispatchEvent(new MouseEvent('click', { bubbles: true }))
|
||||
})
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
mountedRoots.splice(0).forEach((root) => act(() => root.unmount()))
|
||||
document.body.innerHTML = ''
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
describe('showPreservedBranchToast', () => {
|
||||
it('renders the branch recovery action below the long description', () => {
|
||||
const onForceDelete = vi.fn()
|
||||
const result: RemoveWorktreeResult = {
|
||||
preservedBranch: {
|
||||
branchName: 'feat/notes-send-any-running-agent',
|
||||
head: 'abc123'
|
||||
}
|
||||
}
|
||||
|
||||
showPreservedBranchToast(
|
||||
result,
|
||||
{
|
||||
displayName: 'Send review notes to any running agent of a worktree',
|
||||
isMainWorktree: false
|
||||
},
|
||||
onForceDelete
|
||||
)
|
||||
const body = renderToastBody()
|
||||
|
||||
expect(toast.warning).toHaveBeenCalledWith(
|
||||
'Worktree deleted, branch kept',
|
||||
expect.objectContaining({
|
||||
id: 'preserved-branch:feat/notes-send-any-running-agent:abc123',
|
||||
dismissible: true,
|
||||
duration: Infinity
|
||||
})
|
||||
)
|
||||
expect(body.textContent).toContain('feat/notes-send-any-running-agent')
|
||||
expect(body.textContent).toContain('Send review notes to any running agent of a worktree')
|
||||
|
||||
clickButton(body, 'Force Delete Branch')
|
||||
|
||||
expect(onForceDelete).toHaveBeenCalledWith('feat/notes-send-any-running-agent', 'abc123')
|
||||
expect(toast.dismiss).toHaveBeenCalledWith(
|
||||
'preserved-branch:feat/notes-send-any-running-agent:abc123'
|
||||
)
|
||||
})
|
||||
|
||||
it('does not show the force-delete action without the preserved head', () => {
|
||||
const result: RemoveWorktreeResult = {
|
||||
preservedBranch: {
|
||||
branchName: 'feature/test'
|
||||
}
|
||||
}
|
||||
|
||||
showPreservedBranchToast(result, undefined, vi.fn())
|
||||
const body = renderToastBody()
|
||||
|
||||
expect(body.textContent).not.toContain('Force Delete Branch')
|
||||
expect(toast.warning).toHaveBeenCalledWith(
|
||||
'Worktree deleted, branch kept',
|
||||
expect.not.objectContaining({ duration: Infinity })
|
||||
)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,118 @@
|
|||
import { Trash2 } from 'lucide-react'
|
||||
import { toast } from 'sonner'
|
||||
import type { RemoveWorktreeResult, Worktree } from '../../../../shared/types'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { Button } from '../ui/button'
|
||||
|
||||
type PreservedBranchWorktree = Pick<Worktree, 'displayName' | 'isMainWorktree'>
|
||||
|
||||
type PreservedBranchToastBodyProps = {
|
||||
description: string
|
||||
forceDeleteLabel: string | undefined
|
||||
onForceDelete: (() => void) | undefined
|
||||
}
|
||||
|
||||
function preservedBranchToastId(branchName: string, expectedHead: string | undefined): string {
|
||||
return `preserved-branch:${branchName}:${expectedHead ?? 'unknown'}`
|
||||
}
|
||||
|
||||
function getPreservedBranchTitle(isWorkspace: boolean): string {
|
||||
return isWorkspace
|
||||
? translate('auto.store.slices.worktrees.5366d13eec', 'Workspace deleted, branch kept')
|
||||
: translate('auto.store.slices.worktrees.2e17f825d4', 'Worktree deleted, branch kept')
|
||||
}
|
||||
|
||||
function getPreservedBranchDescription(
|
||||
branch: string,
|
||||
targetName: string | undefined,
|
||||
isWorkspace: boolean
|
||||
): string {
|
||||
if (!targetName) {
|
||||
return translate(
|
||||
'auto.store.slices.worktrees.78e08cd877',
|
||||
'Git could not safely delete branch "{{value0}}", so Orca kept it to avoid losing local commits.',
|
||||
{ value0: branch }
|
||||
)
|
||||
}
|
||||
return isWorkspace
|
||||
? translate(
|
||||
'auto.store.slices.worktrees.3b57982bf6',
|
||||
'Git could not safely delete branch "{{value0}}" after deleting workspace "{{value1}}", so Orca kept it to avoid losing local commits.',
|
||||
{ value0: branch, value1: targetName }
|
||||
)
|
||||
: translate(
|
||||
'auto.store.slices.worktrees.81f13f48d2',
|
||||
'Git could not safely delete branch "{{value0}}" after deleting worktree "{{value1}}", so Orca kept it to avoid losing local commits.',
|
||||
{ value0: branch, value1: targetName }
|
||||
)
|
||||
}
|
||||
|
||||
// Why: Sonner's native action row pinches long branch/worktree names into a
|
||||
// narrow column. Keep the native toast frame, but give the body its own footer.
|
||||
function PreservedBranchToastBody({
|
||||
description,
|
||||
forceDeleteLabel,
|
||||
onForceDelete
|
||||
}: PreservedBranchToastBodyProps): React.JSX.Element {
|
||||
return (
|
||||
<div className="flex w-[300px] max-w-[calc(100vw-96px)] flex-col gap-3">
|
||||
<p className="min-w-0 break-words text-sm leading-5 text-popover-foreground/80">
|
||||
{description}
|
||||
</p>
|
||||
{forceDeleteLabel && onForceDelete ? (
|
||||
<div className="flex min-w-0 overflow-hidden">
|
||||
<Button
|
||||
type="button"
|
||||
variant="default"
|
||||
size="sm"
|
||||
className="w-full min-w-0"
|
||||
onClick={onForceDelete}
|
||||
>
|
||||
<Trash2 className="size-3.5" />
|
||||
<span className="truncate">{forceDeleteLabel}</span>
|
||||
</Button>
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
export function showPreservedBranchToast(
|
||||
result: RemoveWorktreeResult | undefined,
|
||||
worktree: PreservedBranchWorktree | undefined,
|
||||
onForceDelete: (branchName: string, expectedHead: string) => void
|
||||
): void {
|
||||
const preservedBranch = result?.preservedBranch
|
||||
const branch = preservedBranch?.branchName
|
||||
if (!branch) {
|
||||
return
|
||||
}
|
||||
|
||||
const isWorkspace = worktree?.isMainWorktree === true
|
||||
const targetName = worktree?.displayName?.trim()
|
||||
const expectedHead = preservedBranch.head
|
||||
const toastId = preservedBranchToastId(branch, expectedHead)
|
||||
const forceDeleteLabel = expectedHead
|
||||
? translate('auto.store.slices.worktrees.e50495aae6', 'Force Delete Branch')
|
||||
: undefined
|
||||
const description = getPreservedBranchDescription(branch, targetName, isWorkspace)
|
||||
const forceDelete = expectedHead
|
||||
? (): void => {
|
||||
onForceDelete(branch, expectedHead)
|
||||
toast.dismiss(toastId)
|
||||
}
|
||||
: undefined
|
||||
|
||||
toast.warning(getPreservedBranchTitle(isWorkspace), {
|
||||
id: toastId,
|
||||
description: (
|
||||
<PreservedBranchToastBody
|
||||
description={description}
|
||||
forceDeleteLabel={forceDeleteLabel}
|
||||
onForceDelete={forceDelete}
|
||||
/>
|
||||
),
|
||||
dismissible: true,
|
||||
...(expectedHead ? { duration: Infinity } : {})
|
||||
})
|
||||
}
|
||||
|
|
@ -199,27 +199,10 @@ describe('removeWorktree cascade', () => {
|
|||
preservedBranch: { branchName: 'feature/test', head: 'def456' }
|
||||
})
|
||||
expect(toast.warning).toHaveBeenCalledWith('Worktree deleted, branch kept', {
|
||||
description:
|
||||
'Git could not safely delete branch "feature/test" after deleting worktree "Review cleanup", so Orca kept it to avoid losing local commits.',
|
||||
action: {
|
||||
label: 'Force Delete Branch',
|
||||
onClick: expect.any(Function)
|
||||
}
|
||||
})
|
||||
|
||||
const action = vi.mocked(toast.warning).mock.calls.at(-1)?.[1]?.action as
|
||||
| { onClick?: () => void }
|
||||
| undefined
|
||||
action?.onClick?.()
|
||||
await vi.waitFor(() => {
|
||||
expect(mockApi.worktrees.forceDeletePreservedBranch).toHaveBeenCalledWith({
|
||||
worktreeId,
|
||||
branchName: 'feature/test',
|
||||
expectedHead: 'def456'
|
||||
})
|
||||
})
|
||||
expect(toast.success).toHaveBeenCalledWith('Local branch deleted', {
|
||||
description: 'Deleted "feature/test".'
|
||||
id: 'preserved-branch:feature/test:def456',
|
||||
description: expect.anything(),
|
||||
dismissible: true,
|
||||
duration: Infinity
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -40,6 +40,7 @@ import { requestVirtualizedScrollAnchorRecord } from '@/hooks/requestVirtualized
|
|||
import { branchName } from '@/lib/git-utils'
|
||||
import { markInputQuietSchedulerInput, scheduleAfterInputQuiet } from '@/lib/input-quiet-scheduler'
|
||||
import { showLocalBaseRefUpdateSuggestionToast } from '@/components/sidebar/local-base-ref-suggestion-toast'
|
||||
import { showPreservedBranchToast } from '@/components/sidebar/preserved-branch-toast'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import {
|
||||
getRepoExecutionHostId,
|
||||
|
|
@ -149,42 +150,6 @@ function showLocalBaseRefRefreshToast(result: LocalBaseRefRefreshResult | undefi
|
|||
)
|
||||
}
|
||||
|
||||
function showPreservedBranchToast(
|
||||
result: RemoveWorktreeResult | undefined,
|
||||
worktree: Pick<Worktree, 'displayName' | 'isMainWorktree'> | undefined,
|
||||
onForceDelete: (branchName: string, expectedHead: string) => void
|
||||
): void {
|
||||
const preservedBranch = result?.preservedBranch
|
||||
const branch = preservedBranch?.branchName
|
||||
if (!branch) {
|
||||
return
|
||||
}
|
||||
const targetTitle = worktree?.isMainWorktree ? 'Workspace' : 'Worktree'
|
||||
const targetLabel = targetTitle.toLowerCase()
|
||||
const targetName = worktree?.displayName?.trim()
|
||||
const deletedTarget = targetName ? ` after deleting ${targetLabel} "${targetName}"` : ''
|
||||
const expectedHead = preservedBranch.head
|
||||
const action = expectedHead
|
||||
? {
|
||||
label: translate('auto.store.slices.worktrees.e50495aae6', 'Force Delete Branch'),
|
||||
onClick: () => onForceDelete(branch, expectedHead)
|
||||
}
|
||||
: undefined
|
||||
toast.warning(
|
||||
translate('auto.store.slices.worktrees.4e6496f3d2', '{{value0}} deleted, branch kept', {
|
||||
value0: targetTitle
|
||||
}),
|
||||
{
|
||||
description: translate(
|
||||
'auto.store.slices.worktrees.d1d78a7baa',
|
||||
'Git could not safely delete branch "{{value0}}"{{value1}}, so Orca kept it to avoid losing local commits.',
|
||||
{ value0: branch, value1: deletedTarget }
|
||||
),
|
||||
...(action ? { action } : {})
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
function arraysShallowEqual(a: string[] | undefined, b: string[] | undefined): boolean {
|
||||
if (a === b) {
|
||||
return true
|
||||
|
|
|
|||
|
|
@ -1,5 +1,50 @@
|
|||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { refreshBranchCleanupTargetRefs, type GitBranchCleanupExec } from './git-branch-cleanup'
|
||||
import {
|
||||
branchHasNoUnmergedChangesOnAnyTarget,
|
||||
refreshBranchCleanupTargetRefs,
|
||||
type GitBranchCleanupExec
|
||||
} from './git-branch-cleanup'
|
||||
|
||||
function baseProofResponses(
|
||||
responses: Partial<Record<string, string | Error>> = {}
|
||||
): GitBranchCleanupExec {
|
||||
return vi.fn<GitBranchCleanupExec>(async (args, options) => {
|
||||
if (args[0] === 'patch-id' && options?.stdin === 'branch-diff') {
|
||||
const response = responses.branchPatchId ?? 'branch-patch 0000000\n'
|
||||
if (response instanceof Error) {
|
||||
throw response
|
||||
}
|
||||
return { stdout: response }
|
||||
}
|
||||
if (args[0] === 'patch-id' && options?.stdin === 'squash-diff') {
|
||||
const response = responses.squashPatchId ?? 'branch-patch squash\n'
|
||||
if (response instanceof Error) {
|
||||
throw response
|
||||
}
|
||||
return { stdout: response }
|
||||
}
|
||||
const key = args.join(' ')
|
||||
const response =
|
||||
responses[key] ??
|
||||
{
|
||||
'rev-parse --verify --quiet refs/remotes/origin/main^{commit}': 'target\n',
|
||||
'merge-tree --write-tree target refs/heads/feature/test': 'merged-tree\n',
|
||||
'rev-parse --verify --quiet target^{tree}': 'target-tree\n',
|
||||
'rev-list --right-only --merges --count target...refs/heads/feature/test': '1\n',
|
||||
'merge-base target refs/heads/feature/test': 'base\n',
|
||||
'diff base refs/heads/feature/test': 'branch-diff',
|
||||
'rev-list --ancestry-path --max-count=201 base..target': 'squash\n',
|
||||
'show --format= squash': 'squash-diff',
|
||||
'merge-tree --write-tree squash refs/heads/feature/test': 'squash-tree\n',
|
||||
'rev-parse --verify --quiet squash^{tree}': 'squash-tree\n'
|
||||
}[key] ??
|
||||
''
|
||||
if (response instanceof Error) {
|
||||
throw response
|
||||
}
|
||||
return { stdout: response }
|
||||
})
|
||||
}
|
||||
|
||||
describe('refreshBranchCleanupTargetRefs', () => {
|
||||
it('fetches each remote-tracking target remote once and prefers slashed remote names', async () => {
|
||||
|
|
@ -43,3 +88,56 @@ describe('refreshBranchCleanupTargetRefs', () => {
|
|||
).resolves.toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('branchHasNoUnmergedChangesOnAnyTarget', () => {
|
||||
it('accepts a branch with merge commits when a target squash commit matches its net patch', async () => {
|
||||
const runGit = baseProofResponses()
|
||||
|
||||
await expect(
|
||||
branchHasNoUnmergedChangesOnAnyTarget(runGit, 'feature/test', ['refs/remotes/origin/main'])
|
||||
).resolves.toBe(true)
|
||||
|
||||
expect(runGit).toHaveBeenCalledWith(['patch-id', '--stable'], { stdin: 'branch-diff' })
|
||||
expect(runGit).toHaveBeenCalledWith(['patch-id', '--stable'], { stdin: 'squash-diff' })
|
||||
expect(runGit).not.toHaveBeenCalledWith(['cherry', '-v', 'target', 'refs/heads/feature/test'])
|
||||
})
|
||||
|
||||
it('preserves a branch with merge commits when no target squash commit matches', async () => {
|
||||
const runGit = baseProofResponses({ squashPatchId: 'other-patch squash\n' })
|
||||
|
||||
await expect(
|
||||
branchHasNoUnmergedChangesOnAnyTarget(runGit, 'feature/test', ['refs/remotes/origin/main'])
|
||||
).resolves.toBe(false)
|
||||
})
|
||||
|
||||
it('preserves when a matching squash candidate still changes after merging the branch', async () => {
|
||||
const runGit = baseProofResponses({
|
||||
'merge-tree --write-tree squash refs/heads/feature/test': 'different-tree\n'
|
||||
})
|
||||
|
||||
await expect(
|
||||
branchHasNoUnmergedChangesOnAnyTarget(runGit, 'feature/test', ['refs/remotes/origin/main'])
|
||||
).resolves.toBe(false)
|
||||
})
|
||||
|
||||
it('preserves when the target squash scan exceeds the cap', async () => {
|
||||
const commits = Array.from({ length: 201 }, (_, index) => `commit-${index}`).join('\n')
|
||||
const runGit = baseProofResponses({
|
||||
'rev-list --ancestry-path --max-count=201 base..target': `${commits}\n`
|
||||
})
|
||||
|
||||
await expect(
|
||||
branchHasNoUnmergedChangesOnAnyTarget(runGit, 'feature/test', ['refs/remotes/origin/main'])
|
||||
).resolves.toBe(false)
|
||||
|
||||
expect(runGit).not.toHaveBeenCalledWith(['show', '--format=', 'commit-0'])
|
||||
})
|
||||
|
||||
it('preserves when patch-id cannot be computed', async () => {
|
||||
const runGit = baseProofResponses({ branchPatchId: new Error('patch-id failed') })
|
||||
|
||||
await expect(
|
||||
branchHasNoUnmergedChangesOnAnyTarget(runGit, 'feature/test', ['refs/remotes/origin/main'])
|
||||
).resolves.toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,12 +1,30 @@
|
|||
export type GitBranchCleanupExec = (argv: string[]) => Promise<{ stdout: string }>
|
||||
export type GitBranchCleanupExec = (
|
||||
argv: string[],
|
||||
options?: { stdin?: string }
|
||||
) => Promise<{ stdout: string }>
|
||||
|
||||
const SQUASH_PATCH_SCAN_LIMIT = 200
|
||||
|
||||
async function readOptionalGitStdout(
|
||||
runGit: GitBranchCleanupExec,
|
||||
argv: string[],
|
||||
options?: { stdin?: string }
|
||||
): Promise<string | null> {
|
||||
try {
|
||||
const { stdout } = await runGit(argv, options)
|
||||
return stdout.trim() || null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
async function readOptionalGitRawStdout(
|
||||
runGit: GitBranchCleanupExec,
|
||||
argv: string[]
|
||||
): Promise<string | null> {
|
||||
try {
|
||||
const { stdout } = await runGit(argv)
|
||||
return stdout.trim() || null
|
||||
return stdout || null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
|
|
@ -117,6 +135,77 @@ async function branchOnlyCommitsArePatchEquivalent(
|
|||
return lines.every((line) => line.startsWith('-'))
|
||||
}
|
||||
|
||||
function parsePatchId(stdout: string | null): string | null {
|
||||
const line = stdout
|
||||
?.split(/\r?\n/)
|
||||
.map((candidate) => candidate.trim())
|
||||
.find(Boolean)
|
||||
const patchId = line?.split(/\s+/)[0]
|
||||
return patchId || null
|
||||
}
|
||||
|
||||
async function computeStablePatchId(
|
||||
runGit: GitBranchCleanupExec,
|
||||
patchText: string | null
|
||||
): Promise<string | null> {
|
||||
if (!patchText) {
|
||||
return null
|
||||
}
|
||||
return parsePatchId(
|
||||
await readOptionalGitStdout(runGit, ['patch-id', '--stable'], { stdin: patchText })
|
||||
)
|
||||
}
|
||||
|
||||
async function branchNetPatchMatchesTargetSquashCommit(
|
||||
runGit: GitBranchCleanupExec,
|
||||
targetOid: string,
|
||||
branchRef: string
|
||||
): Promise<boolean> {
|
||||
const mergeBase = await readOptionalGitStdout(runGit, ['merge-base', targetOid, branchRef])
|
||||
if (!mergeBase) {
|
||||
return false
|
||||
}
|
||||
|
||||
const branchPatchId = await computeStablePatchId(
|
||||
runGit,
|
||||
await readOptionalGitRawStdout(runGit, ['diff', mergeBase, branchRef])
|
||||
)
|
||||
if (!branchPatchId) {
|
||||
return false
|
||||
}
|
||||
|
||||
const commits = (
|
||||
await readOptionalGitStdout(runGit, [
|
||||
'rev-list',
|
||||
'--ancestry-path',
|
||||
`--max-count=${SQUASH_PATCH_SCAN_LIMIT + 1}`,
|
||||
`${mergeBase}..${targetOid}`
|
||||
])
|
||||
)
|
||||
?.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean)
|
||||
if (!commits?.length || commits.length > SQUASH_PATCH_SCAN_LIMIT) {
|
||||
return false
|
||||
}
|
||||
|
||||
for (const commitOid of commits) {
|
||||
const commitPatchId = await computeStablePatchId(
|
||||
runGit,
|
||||
await readOptionalGitRawStdout(runGit, ['show', '--format=', commitOid])
|
||||
)
|
||||
// Why: a matching patch-id identifies a possible squash commit, but the
|
||||
// tree merge proves the branch contributes no additional changes there.
|
||||
if (
|
||||
commitPatchId === branchPatchId &&
|
||||
(await branchMergesWithoutTreeChanges(runGit, commitOid, branchRef))
|
||||
) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
export async function branchHasNoUnmergedChangesOnAnyTarget(
|
||||
runGit: GitBranchCleanupExec,
|
||||
branchName: string,
|
||||
|
|
@ -133,6 +222,9 @@ export async function branchHasNoUnmergedChangesOnAnyTarget(
|
|||
return true
|
||||
}
|
||||
if (await hasBranchOnlyMergeCommits(runGit, targetOid, branchRef)) {
|
||||
if (await branchNetPatchMatchesTargetSquashCommit(runGit, targetOid, branchRef)) {
|
||||
return true
|
||||
}
|
||||
continue
|
||||
}
|
||||
if (await branchOnlyCommitsArePatchEquivalent(runGit, targetOid, branchRef)) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue