From 3b082a94709e1693bbf9ed164aa95eb80502ca75 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 13 Aug 2026 15:56:03 -0700 Subject: [PATCH] fix(worktree): enforce retired names at creation --- src/main/ipc/worktree-remote.ts | 13 +++++++++++++ src/main/ipc/worktrees-windows.test.ts | 13 +++++++------ src/main/ipc/worktrees.test.ts | 11 +++++++---- src/main/runtime/orca-runtime.test.ts | 12 ++++++++---- src/main/runtime/orca-runtime.ts | 9 ++++++++- 5 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 5f80538f81c..7c30336427c 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -135,6 +135,7 @@ import { getWorktreeCreateCandidate, WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS } from '../worktree-create-candidates' +import { getRetiredWorktreeNamesForRepo } from '../worktree-name-retirement' const SSH_WORKTREE_CREATE_FETCH_FRESHNESS_MS = 30_000 const SSH_WORKTREE_CREATE_FETCH_CACHE_MAX = 512 @@ -1550,12 +1551,18 @@ export async function createRemoteWorktree( let selectedExistingLocalBranchName: string | null = null let lastBranchConflictKind: 'local' | 'remote' | null = null let remotePathResolved = false + const retiredNames = new Set( + getRetiredWorktreeNamesForRepo(store, repo, store.getRepos(), settings) + ) // Why: duplicate PR/MR checkouts still need a workspace; suffix branch/path while preserving review metadata and push target. for (let suffix = 1; suffix <= WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS; suffix += 1) { effectiveSanitizedName = getWorktreeCreateCandidate(sanitizedName, suffix) effectiveRequestedName = args.name.trim() ? getWorktreeCreateCandidate(args.name, suffix) : effectiveSanitizedName + if (retiredNames.has(effectiveSanitizedName)) { + continue + } branchName = await resolveCreateBranchNameSsh( provider, repo.path, @@ -2091,12 +2098,18 @@ export async function createLocalWorktree( let lastBranchConflictKind: 'local' | 'remote' | null = null let lastExistingPR: Awaited> | null = null let lastExistingReviewNumber: number | null = null + const retiredNames = new Set( + getRetiredWorktreeNamesForRepo(store, repo, store.getRepos(), settings) + ) // Why: a create-from-review branch override may already exist locally; suffix both branch and path instead of blocking the user. for (let suffix = 1; suffix <= WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS; suffix += 1) { effectiveSanitizedName = getWorktreeCreateCandidate(sanitizedName, suffix) effectiveRequestedName = requestedName.trim() ? getWorktreeCreateCandidate(requestedName, suffix) : effectiveSanitizedName + if (retiredNames.has(effectiveSanitizedName)) { + continue + } lastExistingReviewNumber = null branchName = await resolveCreateBranchName( diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts index 4ebefe848ad..32477a81998 100644 --- a/src/main/ipc/worktrees-windows.test.ts +++ b/src/main/ipc/worktrees-windows.test.ts @@ -325,14 +325,15 @@ describe('registerWorktreeHandlers – Windows path handling', () => { }) }) - it('retires the generated name when flat layout decorates the physical leaf', async () => { - computeWorktreePathMock.mockReturnValue('C:\\workspaces\\repo-nautilus') - ensurePathWithinWorkspaceMock.mockReturnValue('C:\\workspaces\\repo-nautilus') + it('skips a retired generated name when the physical leaf is decorated', async () => { + store.getRetiredWorktreeNames.mockReturnValue(['nautilus']) + computeWorktreePathMock.mockReturnValue('C:\\workspaces\\repo-nautilus-2') + ensurePathWithinWorkspaceMock.mockReturnValue('C:\\workspaces\\repo-nautilus-2') listWorktreesMock.mockResolvedValue([ { - path: 'C:/workspaces/repo-nautilus', + path: 'C:/workspaces/repo-nautilus-2', head: 'abc123', - branch: 'refs/heads/nautilus', + branch: 'refs/heads/nautilus-2', isBare: false, isMainWorktree: false } @@ -340,7 +341,7 @@ describe('registerWorktreeHandlers – Windows path handling', () => { await handlers['worktrees:create'](null, { repoId: 'repo-1', name: 'nautilus' }) - expect(store.addRetiredWorktreeName).toHaveBeenCalledWith('repo-1', 'nautilus') + expect(store.addRetiredWorktreeName).toHaveBeenCalledWith('repo-1', 'nautilus-2') }) it('resolves the Git username when the configured prefix consumes it', async () => { diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 565a6c8c5f5..8bf3c05595b 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -4672,9 +4672,9 @@ describe('registerWorktreeHandlers', () => { isMainWorktree: true }, { - path: '/remote/repo-nautilus', + path: '/remote/repo-nautilus-2', head: 'abc123', - branch: 'refs/heads/nautilus', + branch: 'refs/heads/nautilus-2', isBare: false, isMainWorktree: false } @@ -4689,6 +4689,9 @@ describe('registerWorktreeHandlers', () => { store.getRepo.mockReturnValue(repo) getSshGitProviderMock.mockReturnValue(provider) getActiveMultiplexerMock.mockReturnValue(mux) + store.getRetiredWorktreeNames.mockImplementation((repoId) => + repoId === 'repo-ssh' ? ['nautilus'] : [] + ) store.setWorktreeMeta.mockImplementation((_worktreeId, meta) => meta) const result = await handlers['worktrees:create'](null, { @@ -4711,9 +4714,9 @@ describe('registerWorktreeHandlers', () => { ) expect(provider.listWorktrees).toHaveBeenCalledTimes(1) expect(provider.worktreeIsClean).not.toHaveBeenCalled() - expect(store.addRetiredWorktreeName).toHaveBeenCalledWith('repo-ssh', 'nautilus') + expect(store.addRetiredWorktreeName).toHaveBeenCalledWith('repo-ssh', 'nautilus-2') expect(store.setWorktreeMeta).toHaveBeenCalledWith( - 'repo-ssh::/remote/repo-nautilus', + 'repo-ssh::/remote/repo-nautilus-2', expect.objectContaining({ linkedIssue: 123, linkedPR: 456, diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 97ffb515d61..cc23e66d419 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -4052,12 +4052,16 @@ describe('OrcaRuntimeService', () => { it('does not reuse stale in-flight worktree scans after creating a worktree', async () => { const addRetiredWorktreeName = vi.fn() - const runtime = new OrcaRuntimeService({ ...store, addRetiredWorktreeName }) + const runtime = new OrcaRuntimeService({ + ...store, + addRetiredWorktreeName, + getRetiredWorktreeNames: () => ['nautilus'] + }) const staleScan = deferred() const createdWorktree = { - path: '/tmp/workspaces/repo-nautilus', + path: '/tmp/workspaces/repo-nautilus-2', head: 'def', - branch: 'nautilus', + branch: 'nautilus-2', isBare: false, isMainWorktree: false } @@ -4087,7 +4091,7 @@ describe('OrcaRuntimeService', () => { worktrees: expect.arrayContaining([expect.objectContaining({ path: createdWorktree.path })]) } ) - expect(addRetiredWorktreeName).toHaveBeenCalledWith(TEST_REPO_ID, 'nautilus') + expect(addRetiredWorktreeName).toHaveBeenCalledWith(TEST_REPO_ID, 'nautilus-2') }) it('creates additional workspace metadata for folder-mode repos through runtime create', async () => { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 18426159349..822c6ea7050 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -935,7 +935,8 @@ import { } from '../worktree-create-candidates' import { ensureRetiredWorktreeNamesBackfilled, - getRetiredWorktreeNamesByRepo + getRetiredWorktreeNamesByRepo, + getRetiredWorktreeNamesForRepo } from '../worktree-name-retirement' import { normalizeSparseDirectories } from '../ipc/sparse-checkout-directories' import type { PtyBindingSourceExpectation, Store } from '../persistence' @@ -22297,6 +22298,9 @@ export class OrcaRuntimeService { let branchConflictKind: 'local' | 'remote' | null = null let worktreePath = '' let worktreePathResolved = false + const retiredNames = new Set( + getRetiredWorktreeNamesForRepo(this.store, repo, this.store.getRepos(), settings) + ) // Why: runtime/mobile create-from-review callers should get a new workspace // even when the PR branch or review branch name is already in use. for (let suffix = 1; suffix <= WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS; suffix += 1) { @@ -22304,6 +22308,9 @@ export class OrcaRuntimeService { effectiveRequestedName = args.name.trim() ? getWorktreeCreateCandidate(args.name, suffix) : effectiveSanitizedName + if (retiredNames.has(effectiveSanitizedName)) { + continue + } branchName = await resolveCreateBranchName( repo.path, selectedExistingLocalBranchName ??