diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 1d63cf97deb..9219733ef0c 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -134,17 +134,14 @@ import { } from '../project-runtime-git-options' import { getBranchNameOverrideCandidate, - getGeneratedWorktreeCreateCandidate, - getWorktreeCreateCandidate, - isGeneratedWorktreeCreateName, WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS } from '../worktree-create-candidates' +import { planWorktreeCreateNames } from '../worktree-create-name-plan' import { failedWorktreeCreationNeedsRetirement, getRetiredNameRegistryForRepo, retireGeneratedWorktreeName } from '../worktree-name-retirement' -import { createRetiredNameLookup } from '../../shared/worktree/retired-name-registry' const SSH_WORKTREE_CREATE_FETCH_FRESHNESS_MS = 30_000 const SSH_WORKTREE_CREATE_FETCH_CACHE_MAX = 512 @@ -1589,29 +1586,20 @@ export async function createRemoteWorktree( let selectedExistingLocalBranchName: string | null = null let lastBranchConflictKind: 'local' | 'remote' | null = null let remotePathResolved = false - const shouldRetireGeneratedName = - args.nameWasGenerated === true && isGeneratedWorktreeCreateName(sanitizedName) - const retiredNameRegistry = shouldRetireGeneratedName - ? await getRetiredNameRegistryForRepo(store, repo, store.getRepos(), settings) - : null - const isRetiredName = retiredNameRegistry ? createRetiredNameLookup(retiredNameRegistry) : null + const namePlan = await planWorktreeCreateNames({ + sanitizedName, + requestedName: args.name, + nameWasGenerated: args.nameWasGenerated, + loadRetiredNames: () => getRetiredNameRegistryForRepo(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, attempts = 0; attempts < WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS; suffix += 1) { - effectiveSanitizedName = shouldRetireGeneratedName - ? getGeneratedWorktreeCreateCandidate( - sanitizedName, - suffix, - retiredNameRegistry?.exhaustedTiers - ) - : getWorktreeCreateCandidate(sanitizedName, suffix) - effectiveRequestedName = shouldRetireGeneratedName - ? effectiveSanitizedName - : args.name.trim() - ? getWorktreeCreateCandidate(args.name, suffix) - : effectiveSanitizedName - if (isRetiredName?.(effectiveSanitizedName)) { + const nameCandidate = namePlan.candidateAt(suffix) + if (!nameCandidate) { continue } + effectiveSanitizedName = nameCandidate.sanitizedName + effectiveRequestedName = nameCandidate.requestedName attempts += 1 branchName = await resolveCreateBranchNameSsh( provider, @@ -1813,7 +1801,7 @@ export async function createRemoteWorktree( } catch (rollbackError) { console.warn('[worktree-create] Failed to roll back remote sparse worktree:', rollbackError) } - if (!rollbackSucceeded && shouldRetireGeneratedName) { + if (!rollbackSucceeded && namePlan.retiresCreatedName) { await retireGeneratedWorktreeName(store, repo, settings, effectiveSanitizedName) } throw err @@ -1821,7 +1809,7 @@ export async function createRemoteWorktree( } // Why: fallible metadata work after creation must not leave a real workspace name reusable. - if (shouldRetireGeneratedName) { + if (namePlan.retiresCreatedName) { await retireGeneratedWorktreeName(store, repo, settings, effectiveSanitizedName) } @@ -2165,29 +2153,20 @@ export async function createLocalWorktree( let lastBranchConflictKind: 'local' | 'remote' | null = null let lastExistingPR: Awaited> | null = null let lastExistingReviewNumber: number | null = null - const shouldRetireGeneratedName = - args.nameWasGenerated === true && isGeneratedWorktreeCreateName(sanitizedName) - const retiredNameRegistry = shouldRetireGeneratedName - ? await getRetiredNameRegistryForRepo(store, repo, store.getRepos(), settings) - : null - const isRetiredName = retiredNameRegistry ? createRetiredNameLookup(retiredNameRegistry) : null + const namePlan = await planWorktreeCreateNames({ + sanitizedName, + requestedName, + nameWasGenerated: args.nameWasGenerated, + loadRetiredNames: () => getRetiredNameRegistryForRepo(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, attempts = 0; attempts < WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS; suffix += 1) { - effectiveSanitizedName = shouldRetireGeneratedName - ? getGeneratedWorktreeCreateCandidate( - sanitizedName, - suffix, - retiredNameRegistry?.exhaustedTiers - ) - : getWorktreeCreateCandidate(sanitizedName, suffix) - effectiveRequestedName = shouldRetireGeneratedName - ? effectiveSanitizedName - : requestedName.trim() - ? getWorktreeCreateCandidate(requestedName, suffix) - : effectiveSanitizedName - if (isRetiredName?.(effectiveSanitizedName)) { + const nameCandidate = namePlan.candidateAt(suffix) + if (!nameCandidate) { continue } + effectiveSanitizedName = nameCandidate.sanitizedName + effectiveRequestedName = nameCandidate.requestedName attempts += 1 lastExistingReviewNumber = null @@ -2450,14 +2429,14 @@ export async function createLocalWorktree( ) })) ?? {} } catch (error) { - if (shouldRetireGeneratedName && failedWorktreeCreationNeedsRetirement(error)) { + if (namePlan.retiresCreatedName && failedWorktreeCreationNeedsRetirement(error)) { await retireGeneratedWorktreeName(store, repo, settings, effectiveSanitizedName) } throw error } // Why: fallible metadata work after creation must not leave a real workspace name reusable. - if (shouldRetireGeneratedName) { + if (namePlan.retiresCreatedName) { await retireGeneratedWorktreeName(store, repo, settings, effectiveSanitizedName) } diff --git a/src/main/ipc/worktrees-ssh-pr-head-fetch.test.ts b/src/main/ipc/worktrees-ssh-pr-head-fetch.test.ts index 109dca653d3..43c48b01f51 100644 --- a/src/main/ipc/worktrees-ssh-pr-head-fetch.test.ts +++ b/src/main/ipc/worktrees-ssh-pr-head-fetch.test.ts @@ -464,8 +464,10 @@ describe('registerWorktreeHandlers', () => { }) }) - it('leaves a user-typed name reusable on the SSH create path', async () => { - // Why: `nautilus` is retired here, yet the user typed it — it must neither be skipped nor burned. + it('keeps a typed name on its retired cwd yet still records it as spent on the SSH create path', async () => { + // Why: a name the user typed is a request, so `nautilus` stays `nautilus` even where this host + // spent that cwd. The write half is unconditional — otherwise a client that predates the + // provenance bit leaves the registry blind and the next generated create collides here. const repo = { id: 'repo-ssh', path: '/remote/repo', @@ -517,9 +519,70 @@ describe('registerWorktreeHandlers', () => { name: 'nautilus' }) - expect(store.addRetiredWorktreeName).not.toHaveBeenCalled() + expect(store.getRetiredWorktreeNameRegistry).not.toHaveBeenCalled() + expect(store.addRetiredWorktreeName).toHaveBeenCalledWith('repo-ssh', 'nautilus') expect(result).toMatchObject({ worktree: expect.objectContaining({ path: '/remote/repo-nautilus' }) }) }) + + it('never consults the retirement registry for a name outside the creature pool', async () => { + // Why: the pool holds ordinary words, so only pool-shaped names may be redirected. Everything + // else must skip the registry entirely — including its backfill scan. + const repo = { + id: 'repo-ssh', + path: '/remote/repo', + displayName: 'ssh', + badgeColor: '#000', + addedAt: 0, + connectionId: 'conn-1', + worktreeBaseRef: 'origin/main' + } + const provider = { + exec: vi.fn().mockImplementation(async (args: string[]) => { + if (args[0] === 'remote') { + return { stdout: 'origin\n', stderr: '' } + } + return { stdout: '', stderr: '' } + }), + fetchRemoteTrackingRef: vi.fn().mockResolvedValue(undefined), + addWorktree: vi.fn().mockResolvedValue(undefined), + listWorktrees: vi.fn().mockResolvedValue([ + { + path: '/remote/repo', + head: 'base123', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + }, + { + path: '/remote/repo-improve-dashboard', + head: 'abc123', + branch: 'refs/heads/improve-dashboard', + isBare: false, + isMainWorktree: false + } + ]), + worktreeIsClean: vi.fn().mockResolvedValue({ clean: true }) + } + store.getRepos.mockReturnValue([repo]) + store.getRepo.mockReturnValue(repo) + getSshGitProviderMock.mockReturnValue(provider) + getActiveMultiplexerMock.mockReturnValue({ + request: vi.fn().mockResolvedValue(undefined), + notify: vi.fn() + }) + store.setWorktreeMeta.mockImplementation((_worktreeId, meta) => meta) + + const result = await handlers['worktrees:create'](null, { + repoId: 'repo-ssh', + name: 'improve-dashboard' + }) + + expect(store.getRetiredWorktreeNameRegistry).not.toHaveBeenCalled() + expect(store.addRetiredWorktreeName).not.toHaveBeenCalled() + expect(result).toMatchObject({ + worktree: expect.objectContaining({ path: '/remote/repo-improve-dashboard' }) + }) + }) }) diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts index d4b3dde71dc..9bf52d34b2b 100644 --- a/src/main/ipc/worktrees-windows.test.ts +++ b/src/main/ipc/worktrees-windows.test.ts @@ -358,9 +358,10 @@ describe('registerWorktreeHandlers – Windows path handling', () => { expect(store.addRetiredWorktreeName).toHaveBeenCalledWith('repo-1', 'nautilus-2') }) - it('leaves a user-typed name reusable even when the same name is retired', async () => { - // Why: the creature pool contains ordinary words ("orca", "runner", "molly"). Silently - // renaming a deliberate `nautilus` to `nautilus-2` — and burning it — is the wrong trade. + it('keeps a typed name on its retired cwd yet still records it as spent', async () => { + // Why: a name the user typed is a request, so `nautilus` stays `nautilus` even where this host + // spent that cwd. The write half is unconditional — otherwise a client that predates the + // provenance bit leaves the registry blind and the next generated create collides here. store.getRetiredWorktreeNameRegistry.mockReturnValue({ exhaustedTiers: 0, names: ['nautilus'] }) computeWorktreePathMock.mockReturnValue('C:\\workspaces\\nautilus') ensurePathWithinWorkspaceMock.mockReturnValue('C:\\workspaces\\nautilus') @@ -383,6 +384,28 @@ describe('registerWorktreeHandlers – Windows path handling', () => { 'origin/main', false ) + expect(store.getRetiredWorktreeNameRegistry).not.toHaveBeenCalled() + expect(store.addRetiredWorktreeName).toHaveBeenCalledWith('repo-1', 'nautilus') + }) + + it('never consults the retirement registry for a name outside the creature pool', async () => { + // Why: the pool holds ordinary words, so only pool-shaped names may be redirected. Everything + // else must skip the registry entirely — including its backfill scan. + computeWorktreePathMock.mockReturnValue('C:\\workspaces\\fix-login') + ensurePathWithinWorkspaceMock.mockReturnValue('C:\\workspaces\\fix-login') + listWorktreesMock.mockResolvedValue([ + { + path: 'C:/workspaces/fix-login', + head: 'abc123', + branch: 'refs/heads/fix-login', + isBare: false, + isMainWorktree: false + } + ]) + + await handlers['worktrees:create'](null, { repoId: 'repo-1', name: 'fix-login' }) + + expect(store.getRetiredWorktreeNameRegistry).not.toHaveBeenCalled() expect(store.addRetiredWorktreeName).not.toHaveBeenCalled() }) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 3688fec9c39..50d2b720180 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -4592,13 +4592,61 @@ describe('OrcaRuntimeService', () => { expect(addRetiredWorktreeName).toHaveBeenCalledWith(TEST_REPO_ID, 'nautilus') }) - it('neither skips nor retires a name the user typed', async () => { - // Why: the pool contains ordinary words. Retirement only ever applies to generated names. + it('retires a pool-shaped name a failed create left behind even with no provenance bit', async () => { + // Why: the failure paths are where a blind registry bites hardest — the directory survives the + // rollback, so a later generated create that reuses it inherits the previous agent history. + // Recording is shape-decided, so it must fire here for a client that never sent the bit. const addRetiredWorktreeName = vi.fn() + const runtime = new OrcaRuntimeService({ ...store, addRetiredWorktreeName }) + computeWorktreePathMock.mockReturnValue('/tmp/workspaces/nautilus') + ensurePathWithinWorkspaceMock.mockReturnValue('/tmp/workspaces/nautilus') + vi.mocked(addSparseWorktree).mockRejectedValueOnce( + Object.assign(new Error('sparse setup failed'), { cleanupFailed: true }) + ) + + await expect( + runtime.createManagedWorktree({ + repoSelector: 'id:repo-1', + name: 'nautilus', + sparseCheckout: { directories: ['packages/web'] } + }) + ).rejects.toThrow('sparse setup failed') + + expect(addRetiredWorktreeName).toHaveBeenCalledWith(TEST_REPO_ID, 'nautilus') + }) + + it('leaves a coined name unrecorded when a create fails', async () => { + // Why: the shape gate has to hold on the failure paths too, or a failed `fix-login` burns a + // name the generator could never have produced. + const addRetiredWorktreeName = vi.fn() + const runtime = new OrcaRuntimeService({ ...store, addRetiredWorktreeName }) + computeWorktreePathMock.mockReturnValue('/tmp/workspaces/fix-login') + ensurePathWithinWorkspaceMock.mockReturnValue('/tmp/workspaces/fix-login') + vi.mocked(addSparseWorktree).mockRejectedValueOnce( + Object.assign(new Error('sparse setup failed'), { cleanupFailed: true }) + ) + + await expect( + runtime.createManagedWorktree({ + repoSelector: 'id:repo-1', + name: 'fix-login', + sparseCheckout: { directories: ['packages/web'] } + }) + ).rejects.toThrow('sparse setup failed') + + expect(addRetiredWorktreeName).not.toHaveBeenCalled() + }) + + it('keeps a typed name on its retired cwd yet still records it as spent', async () => { + // Why: a name the user typed is a request, so `nautilus` stays `nautilus` even where this host + // spent that cwd. The write half is unconditional — otherwise a client that predates the + // provenance bit leaves the registry blind and the next generated create collides here. + const addRetiredWorktreeName = vi.fn() + const getRetiredWorktreeNameRegistry = vi.fn(() => ({ exhaustedTiers: 0, names: ['nautilus'] })) const runtime = new OrcaRuntimeService({ ...store, addRetiredWorktreeName, - getRetiredWorktreeNameRegistry: () => ({ exhaustedTiers: 0, names: ['nautilus'] }) + getRetiredWorktreeNameRegistry }) const createdWorktree = { path: '/tmp/workspaces/nautilus', @@ -4619,6 +4667,43 @@ describe('OrcaRuntimeService', () => { }) expect(result.worktree.path).toBe(createdWorktree.path) + expect(computeWorktreePathMock).toHaveBeenCalledWith( + 'nautilus', + expect.anything(), + expect.anything() + ) + expect(getRetiredWorktreeNameRegistry).not.toHaveBeenCalled() + expect(addRetiredWorktreeName).toHaveBeenCalledWith(TEST_REPO_ID, 'nautilus') + }) + + it('never consults the retirement registry for a name outside the creature pool', async () => { + // Why: the pool contains ordinary words, so only pool-shaped names may be redirected. A name + // the user coined must not even pay for the registry read, which can hit disk. + const addRetiredWorktreeName = vi.fn() + const getRetiredWorktreeNameRegistry = vi.fn(() => ({ exhaustedTiers: 0, names: ['nautilus'] })) + const runtime = new OrcaRuntimeService({ + ...store, + addRetiredWorktreeName, + getRetiredWorktreeNameRegistry + }) + const createdWorktree = { + path: '/tmp/workspaces/fix-login', + head: 'def', + branch: 'fix-login', + isBare: false, + isMainWorktree: false + } + computeWorktreePathMock.mockReturnValue(createdWorktree.path) + ensurePathWithinWorkspaceMock.mockReturnValue(createdWorktree.path) + vi.mocked(listWorktrees).mockResolvedValue([...MOCK_GIT_WORKTREES, createdWorktree]) + + const result = await runtime.createManagedWorktree({ + repoSelector: 'id:repo-1', + name: 'fix-login' + }) + + expect(result.worktree.path).toBe(createdWorktree.path) + expect(getRetiredWorktreeNameRegistry).not.toHaveBeenCalled() expect(addRetiredWorktreeName).not.toHaveBeenCalled() }) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index aac73f5e7f4..daa25089e09 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -1081,21 +1081,16 @@ import { } from '../ipc/worktree-remote' import { getBranchNameOverrideCandidate, - getGeneratedWorktreeCreateCandidate, - getWorktreeCreateCandidate, - isGeneratedWorktreeCreateName, WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS } from '../worktree-create-candidates' import { WORKTREE_CREATE_DEDUPE_TTL_MS } from '../../shared/new-workspace/worktree-create-retry-policy' +import { planWorktreeCreateNames } from '../worktree-create-name-plan' import { failedWorktreeCreationNeedsRetirement, getRetiredNameRegistryForRepo, retireGeneratedWorktreeName } from '../worktree-name-retirement' -import { - createRetiredNameLookup, - type RetiredNameRegistry -} from '../../shared/worktree/retired-name-registry' +import type { RetiredNameRegistry } from '../../shared/worktree/retired-name-registry' import { normalizeSparseDirectories } from '../ipc/sparse-checkout-directories' import type { PtyBindingSourceExpectation, Store } from '../persistence' import { collectLayoutLeafIdsInOrder } from '../persistence/restoring-sessions/terminal-layout-normalization' @@ -24576,12 +24571,16 @@ export class OrcaRuntimeService { let branchConflictKind: 'local' | 'remote' | null = null let worktreePath = '' let worktreePathResolved = false - const shouldRetireGeneratedName = - args.nameWasGenerated === true && isGeneratedWorktreeCreateName(sanitizedName) - const retiredNameRegistry = shouldRetireGeneratedName - ? await getRetiredNameRegistryForRepo(this.store, repo, this.store.getRepos(), settings) - : null - const isRetiredName = retiredNameRegistry ? createRetiredNameLookup(retiredNameRegistry) : null + // Why the alias: `this.store` is nullable, and narrowing does not survive into the closure + // below — capturing it once here is what keeps `loadRetiredNames` type-safe. + const retirementStore = this.store + const namePlan = await planWorktreeCreateNames({ + sanitizedName, + requestedName: args.name, + nameWasGenerated: args.nameWasGenerated, + loadRetiredNames: () => + getRetiredNameRegistryForRepo(retirementStore, repo, retirementStore.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 ( @@ -24589,21 +24588,12 @@ export class OrcaRuntimeService { attempts < WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS; suffix += 1 ) { - effectiveSanitizedName = shouldRetireGeneratedName - ? getGeneratedWorktreeCreateCandidate( - sanitizedName, - suffix, - retiredNameRegistry?.exhaustedTiers - ) - : getWorktreeCreateCandidate(sanitizedName, suffix) - effectiveRequestedName = shouldRetireGeneratedName - ? effectiveSanitizedName - : args.name.trim() - ? getWorktreeCreateCandidate(args.name, suffix) - : effectiveSanitizedName - if (isRetiredName?.(effectiveSanitizedName)) { + const nameCandidate = namePlan.candidateAt(suffix) + if (!nameCandidate) { continue } + effectiveSanitizedName = nameCandidate.sanitizedName + effectiveRequestedName = nameCandidate.requestedName attempts += 1 branchName = await resolveCreateBranchName( repo.path, @@ -24906,14 +24896,14 @@ export class OrcaRuntimeService { settings.refreshLocalBaseRefOnWorktreeCreate ))) ?? {} } catch (error) { - if (shouldRetireGeneratedName && failedWorktreeCreationNeedsRetirement(error)) { + if (namePlan.retiresCreatedName && failedWorktreeCreationNeedsRetirement(error)) { await retireGeneratedWorktreeName(this.store, repo, settings, effectiveSanitizedName) } throw error } // Why: fallible metadata work after creation must not leave a real workspace name reusable. - if (shouldRetireGeneratedName) { + if (namePlan.retiresCreatedName) { await retireGeneratedWorktreeName(this.store, repo, settings, effectiveSanitizedName) } diff --git a/src/main/worktree-create-name-plan.test.ts b/src/main/worktree-create-name-plan.test.ts new file mode 100644 index 00000000000..ce09e878b44 --- /dev/null +++ b/src/main/worktree-create-name-plan.test.ts @@ -0,0 +1,140 @@ +import { describe, expect, it, vi } from 'vitest' +import { planWorktreeCreateNames } from './worktree-create-name-plan' +import { + EMPTY_RETIRED_NAME_REGISTRY, + type RetiredNameRegistry +} from '../shared/worktree/retired-name-registry' + +function plan(args: { + sanitizedName: string + requestedName?: string + nameWasGenerated?: boolean + retired?: RetiredNameRegistry +}): ReturnType { + return planWorktreeCreateNames({ + sanitizedName: args.sanitizedName, + requestedName: args.requestedName ?? args.sanitizedName, + nameWasGenerated: args.nameWasGenerated, + loadRetiredNames: async () => args.retired ?? EMPTY_RETIRED_NAME_REGISTRY + }) +} + +/** The candidate ladder, with retired rungs dropped the way the create loops drop them. */ +function walk( + namePlan: Awaited>, + suffixes: number +): string[] { + const taken: string[] = [] + for (let suffix = 1; suffix <= suffixes; suffix += 1) { + const candidate = namePlan.candidateAt(suffix) + if (candidate) { + taken.push(candidate.sanitizedName) + } + } + return taken +} + +describe('planWorktreeCreateNames', () => { + it('steps over a retired cwd for a generated name', async () => { + const namePlan = await plan({ + sanitizedName: 'nautilus', + nameWasGenerated: true, + retired: { exhaustedTiers: 0, names: ['nautilus'] } + }) + + expect(namePlan.candidateAt(1)).toBeNull() + expect(namePlan.candidateAt(2)).toEqual({ + sanitizedName: 'nautilus-2', + requestedName: 'nautilus-2' + }) + }) + + it('hands a typed name the retired cwd it asked for', async () => { + // Why: the pool is ordinary English, and people type those words on purpose. Redirecting a + // deliberate `nautilus` to `nautilus-2` is a worse surprise than inheriting a spent cwd. + const namePlan = await plan({ + sanitizedName: 'nautilus', + nameWasGenerated: false, + retired: { exhaustedTiers: 0, names: ['nautilus'] } + }) + + expect(namePlan.candidateAt(1)).toEqual({ + sanitizedName: 'nautilus', + requestedName: 'nautilus' + }) + }) + + it('treats a client that omits the provenance bit as having typed the name', async () => { + // Why: clients predating `nameWasGenerated` are indistinguishable from a typing user, and + // pre-#14350 they kept the cwd. Guessing "generated" would silently redirect real requests. + const loadRetiredNames = vi.fn(async () => ({ exhaustedTiers: 0, names: ['nautilus'] })) + + const namePlan = await planWorktreeCreateNames({ + sanitizedName: 'nautilus', + requestedName: 'nautilus', + nameWasGenerated: undefined, + loadRetiredNames + }) + + expect(namePlan.candidateAt(1)).toEqual({ + sanitizedName: 'nautilus', + requestedName: 'nautilus' + }) + // The typed path never consults the registry, so it never pays for its backfill scan either. + expect(loadRetiredNames).not.toHaveBeenCalled() + expect(namePlan.retiresCreatedName).toBe(true) + }) + + it('never loads the registry for a name the generator could not have produced', async () => { + const loadRetiredNames = vi.fn(async () => ({ exhaustedTiers: 0, names: ['fix-login'] })) + + const namePlan = await planWorktreeCreateNames({ + sanitizedName: 'fix-login', + requestedName: 'fix-login', + nameWasGenerated: undefined, + loadRetiredNames + }) + + expect(loadRetiredNames).not.toHaveBeenCalled() + expect(namePlan.retiresCreatedName).toBe(false) + expect(walk(namePlan, 2)).toEqual(['fix-login', 'fix-login-2']) + }) + + it('keeps a typed name literal instead of canonicalizing it onto the tier ladder', async () => { + // `nautilus-2-3` is a legacy repeat-suffixed spelling. The generated ladder folds it to + // `nautilus-4`; a name the user typed must keep the directory it asked for. + const typed = await plan({ sanitizedName: 'nautilus-2-3' }) + const generated = await plan({ sanitizedName: 'nautilus-2-3', nameWasGenerated: true }) + + expect(walk(typed, 2)).toEqual(['nautilus-2-3', 'nautilus-2-3-2']) + expect(walk(generated, 2)).toEqual(['nautilus-4', 'nautilus-5']) + }) + + it('suffixes the display name in step with a typed name a path collision pushed along', async () => { + const namePlan = await plan({ sanitizedName: 'nautilus', requestedName: 'Nautilus' }) + + expect(namePlan.candidateAt(2)).toEqual({ + sanitizedName: 'nautilus-2', + requestedName: 'Nautilus-2' + }) + }) + + it('starts a generated name past the compaction watermark rather than walking every spent tier', async () => { + const namePlan = await plan({ + sanitizedName: 'nautilus', + nameWasGenerated: true, + retired: { exhaustedTiers: 3, names: [] } + }) + + expect(walk(namePlan, 2)).toEqual(['nautilus-4', 'nautilus-5']) + }) + + it('records a pool-shaped name as spent however the client labelled it', async () => { + await expect(plan({ sanitizedName: 'nautilus' })).resolves.toMatchObject({ + retiresCreatedName: true + }) + await expect( + plan({ sanitizedName: 'nautilus', nameWasGenerated: true }) + ).resolves.toMatchObject({ retiresCreatedName: true }) + }) +}) diff --git a/src/main/worktree-create-name-plan.ts b/src/main/worktree-create-name-plan.ts new file mode 100644 index 00000000000..b57ab496dc5 --- /dev/null +++ b/src/main/worktree-create-name-plan.ts @@ -0,0 +1,81 @@ +import { + getGeneratedWorktreeCreateCandidate, + getWorktreeCreateCandidate, + isGeneratedWorktreeCreateName +} from './worktree-create-candidates' +import type { RetiredNameRegistry } from '../shared/worktree/retired-name-registry' +import { createRetiredNameLookup } from '../shared/worktree/retired-name-registry' + +export type WorktreeCreateNameCandidate = { + /** Filesystem-safe name; drives the workspace path and branch. */ + sanitizedName: string + /** Display name the workspace is labelled with. */ + requestedName: string +} + +export type WorktreeCreateNamePlan = { + /** True when the created name must be recorded as spent once creation commits. */ + readonly retiresCreatedName: boolean + /** Null when this suffix lands on a retired cwd; the caller skips it without spending an attempt. */ + candidateAt(suffix: number): WorktreeCreateNameCandidate | null +} + +export type WorktreeCreateNamePlanArgs = { + sanitizedName: string + /** The raw name the caller asked for; empty falls back to the sanitized candidate. */ + requestedName: string + /** `true` only from clients that know the name came from the creature-name generator. Every + * producer strips it when false, so absent means typed — from a current client as much as an + * older one — and must be read that way. */ + nameWasGenerated: boolean | undefined + loadRetiredNames: () => Promise +} + +/** Resolves what a create attempt may call itself, shared by the three create paths (local IPC, SSH, + * runtime) so a retirement rule cannot hold on one and not the others. + * + * Two decisions, deliberately split — PR #14350 conflated them, and STA-4471 is the fallout: + * + * - *Which cwds a create may land on* follows the client's `nameWasGenerated` bit. A generated name + * is one of ours: nothing was chosen, so folding it onto the canonical tier ladder and stepping + * over cwds this host already spent costs the user nothing and keeps the previous occupant's + * Claude/Codex history out of the new workspace. A name the user typed is a request — it keeps + * its literal spelling, its plain `-2`, `-3` suffixes, and the cwd it asked for even when that + * cwd is retired. Asking for `nautilus` and silently getting `nautilus-2` is the worse surprise. + * - *Whether the created name is recorded as spent* is decided by the host alone, from the name's + * shape. A client that predates the provenance bit still hands back pool-shaped names, and a + * registry that never learns about them lets the next generated create collide with one. + * `addRetiredWorktreeName` self-gates on pool shape, so a coined name (`fix-login`) is never + * recorded — recording is only ever a write, and never redirects anybody. + * + * Only the generated ladder reads the registry, so a typed name and a name outside the pool alike + * skip the load — and with it `ensureRetiredWorktreeNamesBackfilled`'s scan (STA-4473). */ +export async function planWorktreeCreateNames( + args: WorktreeCreateNamePlanArgs +): Promise { + const isPoolShaped = isGeneratedWorktreeCreateName(args.sanitizedName) + const usesGeneratedLadder = args.nameWasGenerated === true && isPoolShaped + // Only the generated ladder reads the registry, so a typed name skips the load and its backfill. + const registry = usesGeneratedLadder ? await args.loadRetiredNames() : null + const isRetiredName = registry ? createRetiredNameLookup(registry) : null + const trimmedRequestedName = args.requestedName.trim() + return { + retiresCreatedName: isPoolShaped, + candidateAt(suffix) { + // Skipping is pure string math, so a run of retired tiers costs no I/O and no attempt budget. + const sanitizedName = usesGeneratedLadder + ? getGeneratedWorktreeCreateCandidate(args.sanitizedName, suffix, registry?.exhaustedTiers) + : getWorktreeCreateCandidate(args.sanitizedName, suffix) + if (isRetiredName?.(sanitizedName)) { + return null + } + return { + sanitizedName, + requestedName: + !usesGeneratedLadder && trimmedRequestedName + ? getWorktreeCreateCandidate(args.requestedName, suffix) + : sanitizedName + } + } + } +}