diff --git a/src/main/git/remove-worktree.test.ts b/src/main/git/remove-worktree.test.ts index 691ed2cf7..c0c29e3b8 100644 --- a/src/main/git/remove-worktree.test.ts +++ b/src/main/git/remove-worktree.test.ts @@ -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': { diff --git a/src/main/git/runner.ts b/src/main/git/runner.ts index 23b57d863..beef7c5e6 100644 --- a/src/main/git/runner.ts +++ b/src/main/git/runner.ts @@ -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 & { 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, diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index 6b5489578..714d31898 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -944,7 +944,11 @@ async function deleteAlreadyMergedBranchAfterSafeDeleteFailure( branchHead: string, options: GitWorktreeExecOptions = {} ): Promise { - 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 diff --git a/src/relay/git-handler-branch-cleanup.test.ts b/src/relay/git-handler-branch-cleanup.test.ts index 90dcdf003..9da1925e9 100644 --- a/src/relay/git-handler-branch-cleanup.test.ts +++ b/src/relay/git-handler-branch-cleanup.test.ts @@ -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(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 diff --git a/src/relay/git-handler-branch-cleanup.ts b/src/relay/git-handler-branch-cleanup.ts index 936178a8d..7dfa4ed3f 100644 --- a/src/relay/git-handler-branch-cleanup.ts +++ b/src/relay/git-handler-branch-cleanup.ts @@ -12,7 +12,8 @@ export async function deleteAlreadyMergedRelayBranchAfterSafeDeleteFailure( branchName: string, branchHead: string ): Promise { - 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. diff --git a/src/relay/git-handler-ops.ts b/src/relay/git-handler-ops.ts index 6241e7192..ca6150a11 100644 --- a/src/relay/git-handler-ops.ts +++ b/src/relay/git-handler-ops.ts @@ -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 diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index cbe91ce3b..46dd750f2 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -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 { diff --git a/src/renderer/src/components/sidebar/preserved-branch-toast.test.tsx b/src/renderer/src/components/sidebar/preserved-branch-toast.test.tsx new file mode 100644 index 000000000..24f768af4 --- /dev/null +++ b/src/renderer/src/components/sidebar/preserved-branch-toast.test.tsx @@ -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 }) + ) + }) +}) diff --git a/src/renderer/src/components/sidebar/preserved-branch-toast.tsx b/src/renderer/src/components/sidebar/preserved-branch-toast.tsx new file mode 100644 index 000000000..b7b343aac --- /dev/null +++ b/src/renderer/src/components/sidebar/preserved-branch-toast.tsx @@ -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 + +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 ( +
+

+ {description} +

+ {forceDeleteLabel && onForceDelete ? ( +
+ +
+ ) : null} +
+ ) +} + +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: ( + + ), + dismissible: true, + ...(expectedHead ? { duration: Infinity } : {}) + }) +} diff --git a/src/renderer/src/store/slices/store-cascades.test.ts b/src/renderer/src/store/slices/store-cascades.test.ts index b258ac0f3..663b4df97 100644 --- a/src/renderer/src/store/slices/store-cascades.test.ts +++ b/src/renderer/src/store/slices/store-cascades.test.ts @@ -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 }) }) diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 2153ee068..bb4e9031f 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -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 | 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 diff --git a/src/shared/git-branch-cleanup.test.ts b/src/shared/git-branch-cleanup.test.ts index 6d5415ee9..6728bf897 100644 --- a/src/shared/git-branch-cleanup.test.ts +++ b/src/shared/git-branch-cleanup.test.ts @@ -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> = {} +): GitBranchCleanupExec { + return vi.fn(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) + }) +}) diff --git a/src/shared/git-branch-cleanup.ts b/src/shared/git-branch-cleanup.ts index da77f053c..3e177bc6c 100644 --- a/src/shared/git-branch-cleanup.ts +++ b/src/shared/git-branch-cleanup.ts @@ -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 { + try { + const { stdout } = await runGit(argv, options) + return stdout.trim() || null + } catch { + return null + } +} + +async function readOptionalGitRawStdout( runGit: GitBranchCleanupExec, argv: string[] ): Promise { 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 { + if (!patchText) { + return null + } + return parsePatchId( + await readOptionalGitStdout(runGit, ['patch-id', '--stable'], { stdin: patchText }) + ) +} + +async function branchNetPatchMatchesTargetSquashCommit( + runGit: GitBranchCleanupExec, + targetOid: string, + branchRef: string +): Promise { + 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)) {