diff --git a/mobile/src/tasks/blank-workspace-create.ts b/mobile/src/tasks/blank-workspace-create.ts index a84ac55a3a1..d81d75d5a69 100644 --- a/mobile/src/tasks/blank-workspace-create.ts +++ b/mobile/src/tasks/blank-workspace-create.ts @@ -24,6 +24,7 @@ export async function createBlankWorkspace(args: { return createWorktreeWithNameRetry({ client: args.client, baseName: args.baseName, + nameWasGenerated: args.nameWasGenerated, supportsIdempotentCutoverRetry: args.supportsIdempotentCutoverRetry, buildParams: (name) => { const params: Record = { diff --git a/mobile/src/tasks/worktree-create-retry.test.ts b/mobile/src/tasks/worktree-create-retry.test.ts index a03be2ad24b..35fa050b620 100644 --- a/mobile/src/tasks/worktree-create-retry.test.ts +++ b/mobile/src/tasks/worktree-create-retry.test.ts @@ -154,6 +154,25 @@ describe('createWorktreeWithNameRetry', () => { expect(attempts[1]!.params.name).toBe('topic-2') }) + it('advances generated retries without nesting suffixes', async () => { + const attempts: Attempt[] = [] + const client = scriptedClient( + [{ errorMessage: 'already exists locally' }, { id: 'wt-generated' }], + attempts + ) + + const result = await createWorktreeWithNameRetry({ + client, + baseName: 'nautilus-2', + nameWasGenerated: true, + buildParams: (name) => ({ repo: 'id:r', name, nameWasGenerated: true }), + supportsIdempotentCutoverRetry: false + }) + + expect(result).toEqual({ worktreeId: 'wt-generated', name: 'nautilus-3' }) + expect(attempts.map((attempt) => attempt.params.name)).toEqual(['nautilus-2', 'nautilus-3']) + }) + it('does not replay an ambiguous cutover when the host lacks idempotency support', async () => { const attempts: Attempt[] = [] const client = scriptedClient([{ throws: new LogicalClientCutoverError() }], attempts) diff --git a/mobile/src/tasks/worktree-create-retry.ts b/mobile/src/tasks/worktree-create-retry.ts index ccc46f2057b..6cee659b89f 100644 --- a/mobile/src/tasks/worktree-create-retry.ts +++ b/mobile/src/tasks/worktree-create-retry.ts @@ -4,6 +4,7 @@ import { isLogicalClientCutoverError } from '../transport/stable-logical-rpc-cli import { CLIENT_WORKTREE_CREATE_MAX_ATTEMPTS, getClientWorktreeCreateCandidate, + getGeneratedWorktreeCreateRetryCandidate, isRetryableWorktreeCreateConflict } from '../../../src/shared/new-workspace/worktree-create-retry-policy' import { WORKTREE_CREATE_TIMEOUT_MS } from './workspace-create-timeout' @@ -26,6 +27,7 @@ const WORKTREE_CREATE_CUTOVER_MAX_RETRIES = 5 export type CreateWorktreeWithNameRetryArgs = { client: RpcClient baseName: string + nameWasGenerated?: boolean buildParams: (name: string) => Record supportsIdempotentCutoverRetry: boolean | Promise maxAttempts?: number @@ -49,7 +51,9 @@ export async function createWorktreeWithNameRetry( const mintMutationId = args.mintMutationId ?? defaultWorktreeCreateMutationId let lastError: string | null = null for (let attempt = 0; attempt < maxAttempts; attempt += 1) { - const candidateName = getClientWorktreeCreateCandidate(baseName, attempt) + const candidateName = args.nameWasGenerated + ? getGeneratedWorktreeCreateRetryCandidate(baseName, attempt) + : getClientWorktreeCreateCandidate(baseName, attempt) const candidateParams = buildParams(candidateName) // Why: older hosts strip unknown fields, so only stamp and replay when the // host advertises idempotency. One key per candidate makes cutover retries diff --git a/src/main/git/remove-worktree.test.ts b/src/main/git/remove-worktree.test.ts index 15e115221ea..8ffac39e5a9 100644 --- a/src/main/git/remove-worktree.test.ts +++ b/src/main/git/remove-worktree.test.ts @@ -1600,4 +1600,46 @@ branch refs/heads/main 'git branch -D -- feature/test' ) }) + + it('marks cleanup failed when sparse rollback cannot clear metadata or remove the worktree', async () => { + mockGitCommands({ + 'git config --get push.autoSetupRemote': { + error: Object.assign(new Error('key unset'), { code: 1 }) + }, + 'git sparse-checkout set -- packages/web': { + error: new Error('sparse setup failed') + }, + 'git config --local --unset-all branch.feature/test.base': { + error: new Error('metadata cleanup failed') + }, + 'git worktree list --porcelain': { + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main + +worktree /repo-feature +HEAD def456 +branch refs/heads/feature/test +` + }, + 'git worktree remove --force /repo-feature': { + error: new Error('worktree cleanup failed') + } + }) + + const error = await addSparseWorktree('/repo', '/repo-feature', 'feature/test', [ + 'packages/web' + ]).catch((caught: unknown) => caught) + + expect(error).toMatchObject({ + cleanupFailed: true, + message: expect.stringContaining('cleanup also failed') + }) + const calls = getGitCalls() + expectGitCallOrder( + calls, + 'git config --local --unset-all branch.feature/test.base', + 'git worktree remove --force /repo-feature' + ) + }) }) diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index 005be57351f..b498ebefc6d 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -1067,7 +1067,14 @@ export async function addSparseWorktree( error instanceof Error ? (error as SparseWorktreeCreateError) : new Error(String(error)) if (created) { if (!options.checkoutExistingBranch) { - await unsetWorktreeCreationBase(worktreePath, branch, options) + try { + await unsetWorktreeCreationBase(worktreePath, branch, options) + } catch (cleanupError) { + console.warn( + `addSparseWorktree: failed to clear creation base for ${worktreePath}`, + cleanupError + ) + } } try { await removeWorktree(repoPath, worktreePath, true, { diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index dd0f2e6d4d4..0896ae0a0da 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -136,10 +136,16 @@ import { } from '../project-runtime-git-options' import { getBranchNameOverrideCandidate, + getGeneratedWorktreeCreateCandidate, getWorktreeCreateCandidate, + isGeneratedWorktreeCreateName, WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS } from '../worktree-create-candidates' -import { getRetiredNameRegistryForRepo } from '../worktree-name-retirement' +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 @@ -1556,20 +1562,30 @@ export async function createRemoteWorktree( let selectedExistingLocalBranchName: string | null = null let lastBranchConflictKind: 'local' | 'remote' | null = null let remotePathResolved = false - const isRetiredName = args.nameWasGenerated - ? createRetiredNameLookup( - getRetiredNameRegistryForRepo(store, repo, store.getRepos(), settings) - ) + const shouldRetireGeneratedName = + args.nameWasGenerated === true && isGeneratedWorktreeCreateName(sanitizedName) + const retiredNameRegistry = shouldRetireGeneratedName + ? await getRetiredNameRegistryForRepo(store, repo, store.getRepos(), settings) : null + const isRetiredName = retiredNameRegistry ? createRetiredNameLookup(retiredNameRegistry) : null // 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 + 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)) { continue } + attempts += 1 branchName = await resolveCreateBranchNameSsh( provider, repo.path, @@ -1749,20 +1765,39 @@ export async function createRemoteWorktree( await provider.exec(['sparse-checkout', 'set', '--', ...sparseDirectories], remotePath) await provider.exec(['checkout', branchName], remotePath) } catch (err) { + let rollbackSucceeded = false if (!checkoutExistingBranch) { - await unsetRemoteWorktreeCreationBase(provider, remotePath, branchName) + try { + await unsetRemoteWorktreeCreationBase(provider, remotePath, branchName) + } catch (cleanupError) { + console.warn( + '[worktree-create] Failed to clear remote sparse creation base:', + cleanupError + ) + } } - await provider - .removeWorktree(remotePath, true, { + try { + await provider.removeWorktree(remotePath, true, { deleteBranch: !checkoutExistingBranch, // Why: sparse setup failed before any work happened, so rollback removes the just-created remote branch. forceBranchDelete: !checkoutExistingBranch }) - .catch(() => undefined) + rollbackSucceeded = true + } catch (rollbackError) { + console.warn('[worktree-create] Failed to roll back remote sparse worktree:', rollbackError) + } + if (!rollbackSucceeded && shouldRetireGeneratedName) { + await retireGeneratedWorktreeName(store, repo, settings, effectiveSanitizedName) + } throw err } } + // Why: fallible metadata work after creation must not leave a real workspace name reusable. + if (shouldRetireGeneratedName) { + await retireGeneratedWorktreeName(store, repo, settings, effectiveSanitizedName) + } + // Re-list to get the created worktree info const gitWorktrees = await timing.time('list_created_worktree', async () => provider.listWorktrees(repo.path) @@ -1775,10 +1810,6 @@ export async function createRemoteWorktree( } const worktreeId = `${repo.id}::${created.path}` - // Why: repo-qualified layouts decorate the physical leaf; retire the name main actually used. - if (args.nameWasGenerated) { - store.addRetiredWorktreeName(repo.id, effectiveSanitizedName) - } const now = Date.now() // Why: PR/MR worktrees start from a head ref/SHA but Source Control must compare against the review target branch. const metadataBaseRef = args.compareBaseRef ?? remoteTrackingBase?.ref ?? baseBranch @@ -2107,20 +2138,30 @@ export async function createLocalWorktree( let lastBranchConflictKind: 'local' | 'remote' | null = null let lastExistingPR: Awaited> | null = null let lastExistingReviewNumber: number | null = null - const isRetiredName = args.nameWasGenerated - ? createRetiredNameLookup( - getRetiredNameRegistryForRepo(store, repo, store.getRepos(), settings) - ) + const shouldRetireGeneratedName = + args.nameWasGenerated === true && isGeneratedWorktreeCreateName(sanitizedName) + const retiredNameRegistry = shouldRetireGeneratedName + ? await getRetiredNameRegistryForRepo(store, repo, store.getRepos(), settings) : null + const isRetiredName = retiredNameRegistry ? createRetiredNameLookup(retiredNameRegistry) : null // 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 + 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)) { continue } + attempts += 1 lastExistingReviewNumber = null branchName = await resolveCreateBranchName( @@ -2292,93 +2333,106 @@ export async function createLocalWorktree( ...remoteTrackingBaseOption, ...(suggestLocalBaseRefUpdate ? { suggestLocalBaseRefUpdate } : {}) } - const addResult: AddWorktreeResult = - (await timing.time('git_worktree_add', async () => { - if (sparseDirectories.length > 0) { - if (checkoutExistingBranch) { - return addSparseWorktree( - repo.path, - worktreePath, - branchName, - sparseDirectories, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate, - addProjectGitOptions(existingBranchOption) - ) - } - if (suggestLocalBaseRefUpdate) { - return addSparseWorktree( - repo.path, - worktreePath, - branchName, - sparseDirectories, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate, - addProjectGitOptions({ ...remoteTrackingBaseOption, suggestLocalBaseRefUpdate }) - ) - } - const sparseOptions = addProjectGitOptions(remoteTrackingBaseOption) - return sparseOptions - ? addSparseWorktree( + let addResult: AddWorktreeResult + try { + addResult = + (await timing.time('git_worktree_add', async () => { + if (sparseDirectories.length > 0) { + if (checkoutExistingBranch) { + return addSparseWorktree( repo.path, worktreePath, branchName, sparseDirectories, baseBranch, settings.refreshLocalBaseRefOnWorktreeCreate, - sparseOptions + addProjectGitOptions(existingBranchOption) ) - : addSparseWorktree( + } + if (suggestLocalBaseRefUpdate) { + return addSparseWorktree( repo.path, worktreePath, branchName, sparseDirectories, baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate + settings.refreshLocalBaseRefOnWorktreeCreate, + addProjectGitOptions({ ...remoteTrackingBaseOption, suggestLocalBaseRefUpdate }) ) - } + } + const sparseOptions = addProjectGitOptions(remoteTrackingBaseOption) + return sparseOptions + ? addSparseWorktree( + repo.path, + worktreePath, + branchName, + sparseDirectories, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate, + sparseOptions + ) + : addSparseWorktree( + repo.path, + worktreePath, + branchName, + sparseDirectories, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate + ) + } - if (checkoutExistingBranch) { - return addWorktree( - repo.path, - worktreePath, - branchName, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate, - false, - addProjectGitOptions(existingBranchOption) - ) - } - if (suggestLocalBaseRefUpdate) { - return addWorktree( - repo.path, - worktreePath, - branchName, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate, - false, - addProjectGitOptions({ ...remoteTrackingBaseOption, suggestLocalBaseRefUpdate }) - ) - } - const worktreeOptions = addProjectGitOptions(remoteTrackingBaseOption) - return worktreeOptions - ? addWorktree( + if (checkoutExistingBranch) { + return addWorktree( repo.path, worktreePath, branchName, baseBranch, settings.refreshLocalBaseRefOnWorktreeCreate, false, - worktreeOptions + addProjectGitOptions(existingBranchOption) ) - : addWorktree( + } + if (suggestLocalBaseRefUpdate) { + return addWorktree( repo.path, worktreePath, branchName, baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate + settings.refreshLocalBaseRefOnWorktreeCreate, + false, + addProjectGitOptions({ ...remoteTrackingBaseOption, suggestLocalBaseRefUpdate }) ) - })) ?? {} + } + const worktreeOptions = addProjectGitOptions(remoteTrackingBaseOption) + return worktreeOptions + ? addWorktree( + repo.path, + worktreePath, + branchName, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate, + false, + worktreeOptions + ) + : addWorktree( + repo.path, + worktreePath, + branchName, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate + ) + })) ?? {} + } catch (error) { + if (shouldRetireGeneratedName && 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) { + await retireGeneratedWorktreeName(store, repo, settings, effectiveSanitizedName) + } let configuredPushTarget: GitPushTarget | undefined if (preparedPushTarget) { @@ -2404,10 +2458,6 @@ export async function createLocalWorktree( } const worktreeId = `${repo.id}::${created.path}` - // Why: repo-qualified layouts decorate the physical leaf; retire the name main actually used. - if (args.nameWasGenerated) { - store.addRetiredWorktreeName(repo.id, effectiveSanitizedName) - } const now = Date.now() // Why: PR/MR worktrees start from a head ref/SHA but Source Control must compare against the review target branch. const metadataBaseRef = args.compareBaseRef ?? remoteTrackingBase?.ref ?? baseBranch diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 2b500118752..b7608eef09a 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -5750,7 +5750,7 @@ describe('registerWorktreeHandlers', () => { ) }) - it('unsets SSH branch base config before removing a sparse worktree after setup failure', async () => { + it('attempts SSH base cleanup and still removes a sparse worktree when that cleanup fails', async () => { const repo = { id: 'repo-ssh', path: '/remote/repo', @@ -5769,6 +5769,9 @@ describe('registerWorktreeHandlers', () => { if (args[0] === 'sparse-checkout' && args[1] === 'init') { throw setupError } + if (args[0] === 'config' && args[2] === '--unset-all') { + throw new Error('metadata cleanup failed') + } return { stdout: '', stderr: '' } }), fetchRemoteTrackingRef: vi.fn().mockResolvedValue(undefined), @@ -8284,6 +8287,23 @@ describe('registerWorktreeHandlers', () => { }) }) + it('retires a generated sparse name when creation rollback also fails', async () => { + addSparseWorktreeMock.mockRejectedValueOnce( + Object.assign(new Error('sparse setup failed'), { cleanupFailed: true }) + ) + + await expect( + handlers['worktrees:create'](null, { + repoId: 'repo-1', + name: 'nautilus', + nameWasGenerated: true, + sparseCheckout: { directories: ['packages/web'] } + }) + ).rejects.toThrow('sparse setup failed') + + expect(store.addRetiredWorktreeName).toHaveBeenCalledWith('repo-1', 'nautilus') + }) + it('clears sparse preset attribution when the preset id does not belong to the repo', async () => { listWorktreesMock.mockResolvedValue([ { diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 34115ce8d80..113d6860572 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -121,10 +121,7 @@ import { isOrphanCompatiblePreflightError, isOrphanedWorktreeError } from './worktree-logic' -import { - ensureRetiredWorktreeNamesBackfilled, - getRetiredNameRegistryForRepo -} from '../worktree-name-retirement' +import { getRetiredNameRegistryForRepo } from '../worktree-name-retirement' import { EMPTY_RETIRED_NAME_REGISTRY } from '../../shared/worktree/retired-name-registry' import { dedupeWorktreesByPath } from './worktree-path-comparison' import { joinWorktreeRelativePath } from '../runtime/runtime-relative-paths' @@ -1944,12 +1941,6 @@ export function registerWorktreeHandlers( if (!repo) { return EMPTY_RETIRED_NAME_REGISTRY } - try { - await ensureRetiredWorktreeNamesBackfilled(store, repo, store.getSettings()) - } catch (err) { - // Best effort: without the seed we only under-retire, which is the pre-existing behavior. - console.warn(`[worktrees] retirement backfill failed for repo ${args.repoId}:`, err) - } return getRetiredNameRegistryForRepo(store, repo, store.getRepos(), store.getSettings()) }) diff --git a/src/main/orca-profiles/profile-project-retired-name-transfer.ts b/src/main/orca-profiles/profile-project-retired-name-transfer.ts new file mode 100644 index 00000000000..7ecb952ce74 --- /dev/null +++ b/src/main/orca-profiles/profile-project-retired-name-transfer.ts @@ -0,0 +1,29 @@ +import type { Repo } from '../../shared/repo-types' +import { + mergeRetiredNameRegistries, + type RetiredNameRegistry +} from '../../shared/worktree/retired-name-registry' +import { getRemoteRetirementNamespaceKey } from '../worktree-name-retirement' +import type { TransferProfileState } from './profile-project-state-file' + +export function extractRetiredNameRegistriesByNamespace( + sourceState: TransferProfileState, + sourceRepo: Repo +): Record { + const namespaceKey = getRemoteRetirementNamespaceKey(sourceRepo, sourceState.settings) + const registry = namespaceKey + ? sourceState.retiredWorktreeNamesByNamespace?.[namespaceKey] + : undefined + return namespaceKey && registry ? { [namespaceKey]: registry } : {} +} + +export function mergeRetiredNameRegistryMaps( + base: Record, + incoming: Record +): Record { + const merged = { ...base } + for (const [key, registry] of Object.entries(incoming)) { + merged[key] = merged[key] ? mergeRetiredNameRegistries(merged[key], registry) : registry + } + return merged +} diff --git a/src/main/orca-profiles/profile-project-state-file.ts b/src/main/orca-profiles/profile-project-state-file.ts index e72babef2f4..5c623f19bad 100644 --- a/src/main/orca-profiles/profile-project-state-file.ts +++ b/src/main/orca-profiles/profile-project-state-file.ts @@ -47,6 +47,9 @@ export function readProfileState(profileId: string, userDataPath: string): Trans retiredWorktreeNamesByRepo: recordOrEmpty( parsed.retiredWorktreeNamesByRepo ), + retiredWorktreeNamesByNamespace: recordOrEmpty( + parsed.retiredWorktreeNamesByNamespace + ), worktreeMeta: recordOrEmpty(parsed.worktreeMeta), worktreeLineageById: recordOrEmpty(parsed.worktreeLineageById), workspaceLineageByChildKey: recordOrEmpty(parsed.workspaceLineageByChildKey), diff --git a/src/main/orca-profiles/profile-project-transfer-payload.ts b/src/main/orca-profiles/profile-project-transfer-payload.ts index a984b7bbf14..b2f74fd3609 100644 --- a/src/main/orca-profiles/profile-project-transfer-payload.ts +++ b/src/main/orca-profiles/profile-project-transfer-payload.ts @@ -7,7 +7,10 @@ import type { PersistedState } from '../../shared/persisted-state-types' import type { Repo } from '../../shared/repo-types' import type { SparsePreset } from '../../shared/worktree/create-types' import type { RetiredNameRegistry } from '../../shared/worktree/retired-name-registry' -import { parseWorkspaceKey } from '../../shared/workspace-scope' +import { + extractRetiredNameRegistriesByNamespace, + mergeRetiredNameRegistryMaps +} from './profile-project-retired-name-transfer' import type { TransferProfileState } from './profile-project-state-file' import { rebuildRepoBackedProjectState } from './profile-project-state-file' import { mergeHostWorkspaceSessions, mergeWorkspaceSessions } from './profile-project-session-state' @@ -15,11 +18,8 @@ import { extractHostSessionsForTransfer, extractSessionForTransfer } from './profile-project-session-transfer' -import { - isRepoWorktreeId, - rekeyWorktreeId, - rekeyWorkspaceKey -} from './profile-project-worktree-identity' +import { collectTransferWorktreeIds } from './profile-project-transfer-worktree-ids' +import { rekeyWorktreeId, rekeyWorkspaceKey } from './profile-project-worktree-identity' export type TransferPayload = { repo: Repo @@ -27,6 +27,7 @@ export type TransferPayload = { /** Generated names already spent for this repo, compacted. Carried so the destination profile * does not reissue a name whose old directory still holds agent state. */ retiredNameRegistry: RetiredNameRegistry | undefined + retiredNameRegistriesByNamespace: Record worktreeMeta: PersistedState['worktreeMeta'] worktreeLineageById: PersistedState['worktreeLineageById'] workspaceLineageByChildKey: PersistedState['workspaceLineageByChildKey'] @@ -64,86 +65,6 @@ function createUniqueRepoId(state: TransferProfileState): string { return candidate } -function collectTransferWorktreeIds(state: TransferProfileState, repoId: string): Set { - const ids = new Set() - const add = (value: string | null | undefined): void => { - if (value && isRepoWorktreeId(repoId, value)) { - ids.add(value) - } - } - Object.keys(state.worktreeMeta).forEach(add) - for (const lineage of Object.values(state.worktreeLineageById)) { - add(lineage.worktreeId) - add(lineage.parentWorktreeId) - } - for (const [key, lineage] of Object.entries(state.workspaceLineageByChildKey)) { - const child = parseWorkspaceKey(key) - const parent = parseWorkspaceKey(lineage.parentWorkspaceKey) - if (child?.type === 'worktree') { - add(child.worktreeId) - } - if (parent?.type === 'worktree') { - add(parent.worktreeId) - } - } - collectSessionWorktreeIds(state.workspaceSession, repoId, ids) - for (const session of Object.values(state.workspaceSessionsByHostId ?? {})) { - collectSessionWorktreeIds(session, repoId, ids) - } - Object.keys(state.ui?.showDotfilesByWorktree ?? {}).forEach(add) - return ids -} - -function collectSessionWorktreeIds( - session: PersistedState['workspaceSession'] | undefined, - repoId: string, - ids: Set -): void { - if (!session) { - return - } - const add = (value: string | null | undefined): void => { - if (value && isRepoWorktreeId(repoId, value)) { - ids.add(value) - } - } - const addOwnerKeys = (record: Record | undefined): void => { - for (const key of Object.keys(record ?? {})) { - if (isRepoWorktreeId(repoId, key)) { - ids.add(key) - } - const parsed = parseWorkspaceKey(key) - if (parsed?.type === 'worktree' && isRepoWorktreeId(repoId, parsed.worktreeId)) { - ids.add(parsed.worktreeId) - } - } - } - addOwnerKeys(session.tabsByWorktree) - addOwnerKeys(session.openFilesByWorktree) - addOwnerKeys(session.browserTabsByWorktree) - addOwnerKeys(session.activeBrowserTabIdByWorktree) - addOwnerKeys(session.activeTabTypeByWorktree) - addOwnerKeys(session.activeTabIdByWorktree) - addOwnerKeys(session.unifiedTabs) - addOwnerKeys(session.tabGroups) - addOwnerKeys(session.tabGroupLayouts) - addOwnerKeys(session.activeGroupIdByWorktree) - addOwnerKeys(session.lastVisitedAtByWorktreeId) - addOwnerKeys(session.defaultTerminalTabsAppliedByWorktreeId) - addOwnerKeys(session.terminalTopologyRevisionByRepoId) - addOwnerKeys(session.activeFileIdByWorktree) - for (const tombstone of Object.values(session.terminalSurfaceTombstonesByPaneKey ?? {})) { - add(tombstone.worktreeId) - } - add(session.activeWorktreeId) - const activeScope = session.activeWorkspaceKey - ? parseWorkspaceKey(session.activeWorkspaceKey) - : null - if (activeScope?.type === 'worktree') { - add(activeScope.worktreeId) - } -} - function rekeyWorktreeIdRecord( record: Record, worktreeIds: ReadonlySet, @@ -222,6 +143,10 @@ export function createTransferPayload(args: { repoId: newRepoId })), retiredNameRegistry: sourceState.retiredWorktreeNamesByRepo?.[oldRepoId], + retiredNameRegistriesByNamespace: extractRetiredNameRegistriesByNamespace( + sourceState, + sourceRepo + ), worktreeMeta: rekeyWorktreeIdRecord( sourceState.worktreeMeta, worktreeIds, @@ -281,6 +206,10 @@ export function applyPayloadToTarget( ...targetState.retiredWorktreeNamesByRepo, ...(payload.retiredNameRegistry ? { [payload.repo.id]: payload.retiredNameRegistry } : {}) }, + retiredWorktreeNamesByNamespace: mergeRetiredNameRegistryMaps( + targetState.retiredWorktreeNamesByNamespace ?? {}, + payload.retiredNameRegistriesByNamespace + ), worktreeMeta: { ...targetState.worktreeMeta, ...payload.worktreeMeta }, worktreeLineageById: { ...targetState.worktreeLineageById, ...payload.worktreeLineageById }, diff --git a/src/main/orca-profiles/profile-project-transfer-worktree-ids.ts b/src/main/orca-profiles/profile-project-transfer-worktree-ids.ts new file mode 100644 index 00000000000..51d87cfc668 --- /dev/null +++ b/src/main/orca-profiles/profile-project-transfer-worktree-ids.ts @@ -0,0 +1,87 @@ +import type { PersistedState } from '../../shared/persisted-state-types' +import { parseWorkspaceKey } from '../../shared/workspace-scope' +import type { TransferProfileState } from './profile-project-state-file' +import { isRepoWorktreeId } from './profile-project-worktree-identity' + +export function collectTransferWorktreeIds( + state: TransferProfileState, + repoId: string +): Set { + const ids = new Set() + const add = (value: string | null | undefined): void => { + if (value && isRepoWorktreeId(repoId, value)) { + ids.add(value) + } + } + Object.keys(state.worktreeMeta).forEach(add) + for (const lineage of Object.values(state.worktreeLineageById)) { + add(lineage.worktreeId) + add(lineage.parentWorktreeId) + } + for (const [key, lineage] of Object.entries(state.workspaceLineageByChildKey)) { + const child = parseWorkspaceKey(key) + const parent = parseWorkspaceKey(lineage.parentWorkspaceKey) + if (child?.type === 'worktree') { + add(child.worktreeId) + } + if (parent?.type === 'worktree') { + add(parent.worktreeId) + } + } + collectSessionWorktreeIds(state.workspaceSession, repoId, ids) + for (const session of Object.values(state.workspaceSessionsByHostId ?? {})) { + collectSessionWorktreeIds(session, repoId, ids) + } + Object.keys(state.ui?.showDotfilesByWorktree ?? {}).forEach(add) + return ids +} + +function collectSessionWorktreeIds( + session: PersistedState['workspaceSession'] | undefined, + repoId: string, + ids: Set +): void { + if (!session) { + return + } + const add = (value: string | null | undefined): void => { + if (value && isRepoWorktreeId(repoId, value)) { + ids.add(value) + } + } + const addOwnerKeys = (record: Record | undefined): void => { + for (const key of Object.keys(record ?? {})) { + if (isRepoWorktreeId(repoId, key)) { + ids.add(key) + } + const parsed = parseWorkspaceKey(key) + if (parsed?.type === 'worktree' && isRepoWorktreeId(repoId, parsed.worktreeId)) { + ids.add(parsed.worktreeId) + } + } + } + addOwnerKeys(session.tabsByWorktree) + addOwnerKeys(session.openFilesByWorktree) + addOwnerKeys(session.browserTabsByWorktree) + addOwnerKeys(session.activeBrowserTabIdByWorktree) + addOwnerKeys(session.activeTabTypeByWorktree) + addOwnerKeys(session.activeTabIdByWorktree) + addOwnerKeys(session.unifiedTabs) + addOwnerKeys(session.tabGroups) + addOwnerKeys(session.tabGroupLayouts) + addOwnerKeys(session.activeGroupIdByWorktree) + addOwnerKeys(session.lastVisitedAtByWorktreeId) + addOwnerKeys(session.defaultTerminalTabsAppliedByWorktreeId) + addOwnerKeys(session.terminalTopologyRevisionByRepoId) + addOwnerKeys(session.activeFileIdByWorktree) + for (const tombstone of Object.values(session.terminalSurfaceTombstonesByPaneKey ?? {})) { + add(tombstone.worktreeId) + } + add(session.activeWorktreeId) + const activeScope = session.activeWorkspaceKey + ? parseWorkspaceKey(session.activeWorkspaceKey) + : null + if (activeScope?.type === 'worktree') { + add(activeScope.worktreeId) + } +} diff --git a/src/main/orca-profiles/profile-project-transfer.test.ts b/src/main/orca-profiles/profile-project-transfer.test.ts index facde389e21..60973c97529 100644 --- a/src/main/orca-profiles/profile-project-transfer.test.ts +++ b/src/main/orca-profiles/profile-project-transfer.test.ts @@ -129,6 +129,9 @@ describe('profile project transfer', () => { ] }, retiredWorktreeNamesByRepo: { 'repo-1': { exhaustedTiers: 0, names: ['nautilus'] } }, + retiredWorktreeNamesByNamespace: { + 'ssh:ssh-1:/workspace/orca-nautilus': { exhaustedTiers: 0, names: ['seahorse'] } + }, worktreeMeta: { [sourceWorktreeId]: makeWorktreeMeta({ projectHostSetupId: 'repo-1' }) }, @@ -186,6 +189,7 @@ describe('profile project transfer', () => { exhaustedTiers: 0, names: ['nautilus'] }) + expect(target.retiredWorktreeNamesByNamespace).toEqual({}) expect(target.workspaceSession.tabsByWorktree).toEqual({}) expect(readProfileState('personal').repos.map((repo) => repo.id)).toEqual(['repo-1']) }) @@ -211,6 +215,12 @@ describe('profile project transfer', () => { }) ], sshTargets: [sshTarget], + retiredWorktreeNamesByNamespace: { + 'ssh:ssh-1:posix:/srv/orca-orca-retirement-probe': { + exhaustedTiers: 0, + names: ['seahorse'] + } + }, worktreeMeta: { [sourceWorktreeId]: makeWorktreeMeta({ projectHostSetupId: 'repo-ssh' }) }, @@ -266,6 +276,12 @@ describe('profile project transfer', () => { executionHostId: 'ssh:ssh-1' }) expect(target.sshTargets).toEqual([sshTarget]) + expect(target.retiredWorktreeNamesByNamespace).toEqual({ + 'ssh:ssh-1:posix:/srv/orca-orca-retirement-probe': { + exhaustedTiers: 0, + names: ['seahorse'] + } + }) expect(target.workspaceSession.browserTabsByWorktree?.[sourceWorktreeId]?.[0]).toMatchObject({ worktreeId: sourceWorktreeId, sessionProfileId: null, diff --git a/src/main/persistence-worktree-name-retirement.test.ts b/src/main/persistence-worktree-name-retirement.test.ts index ec567fefa6c..2554abcd16c 100644 --- a/src/main/persistence-worktree-name-retirement.test.ts +++ b/src/main/persistence-worktree-name-retirement.test.ts @@ -140,6 +140,32 @@ describe('worktree name retirement registry', () => { expect(store.getRetiredWorktreeNameRegistry(REPO).names).toEqual([]) }) + it('reassociates remote retirements after remove and re-add mints a new repo id', async () => { + const oldRepo = { + id: REPO, + path: '/remote/repos/a', + displayName: 'a', + badgeColor: '', + addedAt: 0, + connectionId: 'ssh-1' + } + let store = await createStore() + store.addRepo(oldRepo) + const { getRetiredNameRegistryForRepo, retireGeneratedWorktreeName } = + await import('./worktree-name-retirement') + await retireGeneratedWorktreeName(store, oldRepo, store.getSettings(), 'nautilus') + store.removeProject(REPO) + store.flush() + + store = await reloadStore() + const newRepo = { ...oldRepo, id: OTHER_REPO } + store.addRepo(newRepo) + + await expect( + getRetiredNameRegistryForRepo(store, newRepo, [newRepo], store.getSettings()) + ).resolves.toEqual({ exhaustedTiers: 0, names: ['nautilus'] }) + }) + it('keeps the registry when one host row is removed but the repo id survives elsewhere', async () => { const store = await createStore() store.addRepo({ id: REPO, path: '/repos/a', displayName: 'a', badgeColor: '', addedAt: 0 }) diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 535a1503910..253ea79ef1a 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -2358,7 +2358,7 @@ function normalizeRetiredNameRegistry(row: unknown): RetiredNameRegistry { /** Why: a corrupt or hand-edited map must degrade to "nothing retired" rather than throw during * load — over-retiring costs one name from a 552-entry pool, but a load failure costs the app. */ -function normalizeRetiredWorktreeNamesByRepo(value: unknown): Record { +function normalizeRetiredNameRegistryMap(value: unknown): Record { if (!value || typeof value !== 'object' || Array.isArray(value)) { return {} } @@ -3865,9 +3865,12 @@ export class Store { (alias): alias is string => typeof alias === 'string' ) : [], - retiredWorktreeNamesByRepo: normalizeRetiredWorktreeNamesByRepo( + retiredWorktreeNamesByRepo: normalizeRetiredNameRegistryMap( parsed.retiredWorktreeNamesByRepo ), + retiredWorktreeNamesByNamespace: normalizeRetiredNameRegistryMap( + parsed.retiredWorktreeNamesByNamespace + ), sshRemotePtyLeases: (parsed.sshRemotePtyLeases ?? []) .map(normalizeSshRemotePtyLease) .filter((lease): lease is SshRemotePtyLease => lease !== null), @@ -7265,6 +7268,13 @@ export class Store { : EMPTY_RETIRED_NAME_REGISTRY } + getRetiredWorktreeNameRegistryForNamespace(namespaceKey: string): RetiredNameRegistry { + const stored = this.state.retiredWorktreeNamesByNamespace?.[namespaceKey] + return stored + ? { exhaustedTiers: stored.exhaustedTiers, names: [...stored.names] } + : EMPTY_RETIRED_NAME_REGISTRY + } + /** Records a generated workspace name as spent for this repo. Called with the name main actually * used, not the one the renderer proposed — the create path can advance past it on collision. */ addRetiredWorktreeName(repoId: string, name: string): void { @@ -7291,6 +7301,30 @@ export class Store { return incoming.size > 0 && this.applyRetiredWorktreeNames(repoId, incoming) } + mergeRetiredWorktreeNamesForNamespace(namespaceKey: string, names: Iterable): boolean { + if (!namespaceKey) { + return false + } + const normalized = new Set() + for (const name of names) { + const candidate = normalizeRetirableGeneratedName(name) + if (candidate) { + normalized.add(candidate) + } + } + const next = addRetiredNames( + this.getRetiredWorktreeNameRegistryForNamespace(namespaceKey), + normalized + ) + if (!next) { + return false + } + this.state.retiredWorktreeNamesByNamespace ??= {} + this.state.retiredWorktreeNamesByNamespace[namespaceKey] = next + this.scheduleSave() + return true + } + private applyRetiredWorktreeNames(repoId: string, names: Iterable): boolean { const next = addRetiredNames(this.getRetiredWorktreeNameRegistry(repoId), names) if (!next) { diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 65fdb7feed9..eaa64ec203d 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -28,6 +28,7 @@ const ORIGIN_REMOTE_URL = 'git@example.com:group/repo.git' const ORIGIN_HEAD_COMPONENT = reviewHeadRemoteRefComponent('origin', ORIGIN_REMOTE_URL) import { detectAgentStatusFromTitle, MAX_OSC_TITLE_CHARS } from '../../shared/agent-detection' import { + addSparseWorktree, addWorktree, assertWorktreeCleanForRemoval, listWorktrees, @@ -216,6 +217,7 @@ vi.mock('../ipc/filesystem-watcher', () => ({ const { MOCK_GIT_WORKTREES, + addSparseWorktreeMock, addWorktreeMock, removeWorktreeMock, forceDeleteLocalBranchMock, @@ -312,6 +314,7 @@ const { isMainWorktree: false } ], + addSparseWorktreeMock: vi.fn(), addWorktreeMock: vi.fn(), removeWorktreeMock: vi.fn(), forceDeleteLocalBranchMock: vi.fn(), @@ -412,6 +415,7 @@ vi.mock('../git/worktree', () => ({ listWorktrees: vi.fn().mockResolvedValue(MOCK_GIT_WORKTREES), listWorktreesStrict: vi.fn().mockResolvedValue(MOCK_GIT_WORKTREES), assertWorktreeCleanForRemoval: vi.fn().mockResolvedValue(undefined), + addSparseWorktree: addSparseWorktreeMock, addWorktree: addWorktreeMock, removeWorktree: removeWorktreeMock, forceDeleteLocalBranch: forceDeleteLocalBranchMock @@ -677,6 +681,7 @@ function resetRuntimeTestMocks(): void { } }) vi.mocked(addWorktree).mockReset() + vi.mocked(addSparseWorktree).mockReset() vi.mocked(assertWorktreeCleanForRemoval).mockReset() vi.mocked(assertWorktreeCleanForRemoval).mockResolvedValue(undefined) vi.mocked(removeWorktree).mockReset() @@ -4297,13 +4302,18 @@ describe('OrcaRuntimeService', () => { const runtime = new OrcaRuntimeService({ ...store, addRetiredWorktreeName, - getRetiredWorktreeNameRegistry: () => ({ exhaustedTiers: 0, names: ['nautilus'] }) + getRetiredWorktreeNameRegistry: () => ({ + exhaustedTiers: 0, + names: Array.from({ length: 100 }, (_unused, index) => + index === 0 ? 'nautilus' : `nautilus-${index + 1}` + ) + }) }) const staleScan = deferred() const createdWorktree = { - path: '/tmp/workspaces/repo-nautilus-2', + path: '/tmp/workspaces/repo-nautilus-101', head: 'def', - branch: 'nautilus-2', + branch: 'nautilus-101', isBare: false, isMainWorktree: false } @@ -4334,7 +4344,47 @@ describe('OrcaRuntimeService', () => { worktrees: expect.arrayContaining([expect.objectContaining({ path: createdWorktree.path })]) } ) - expect(addRetiredWorktreeName).toHaveBeenCalledWith(TEST_REPO_ID, 'nautilus-2') + expect(addRetiredWorktreeName).toHaveBeenCalledWith(TEST_REPO_ID, 'nautilus-101') + }) + + it('retires a generated name before a post-create listing failure', async () => { + const addRetiredWorktreeName = vi.fn() + const runtime = new OrcaRuntimeService({ ...store, addRetiredWorktreeName }) + computeWorktreePathMock.mockReturnValue('/tmp/workspaces/nautilus') + ensurePathWithinWorkspaceMock.mockReturnValue('/tmp/workspaces/nautilus') + vi.mocked(listWorktrees).mockRejectedValueOnce(new Error('listing unavailable')) + + await expect( + runtime.createManagedWorktree({ + repoSelector: 'id:repo-1', + name: 'nautilus', + nameWasGenerated: true + }) + ).rejects.toThrow('listing unavailable') + + expect(addWorktree).toHaveBeenCalled() + expect(addRetiredWorktreeName).toHaveBeenCalledWith(TEST_REPO_ID, 'nautilus') + }) + + it('retires a generated sparse name when creation rollback also fails', async () => { + 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', + nameWasGenerated: true, + sparseCheckout: { directories: ['packages/web'] } + }) + ).rejects.toThrow('sparse setup failed') + + expect(addRetiredWorktreeName).toHaveBeenCalledWith(TEST_REPO_ID, 'nautilus') }) it('neither skips nor retires a name the user typed', async () => { @@ -44878,12 +44928,12 @@ describe('OrcaRuntimeService', () => { ) }) - it('detects agents on the SSH host before launching remote startup drafts', async () => { + it('forwards generated-name provenance while launching SSH startup drafts', async () => { detectRemoteAgentsMock.mockResolvedValue(['claude']) const created = { - path: '/remote/mobile-startup-draft', + path: '/remote/repo-nautilus-2', head: 'def', - branch: 'refs/heads/mobile-startup-draft', + branch: 'refs/heads/nautilus-2', isBare: false, isMainWorktree: false } @@ -44913,6 +44963,7 @@ describe('OrcaRuntimeService', () => { defaultTuiAgent: null, agentCmdOverrides: {} }), + getRetiredWorktreeNameRegistry: () => ({ exhaustedTiers: 0, names: ['nautilus'] }), getAllWorktreeMeta: () => metaById, getWorktreeMeta: (worktreeId: string) => metaById[worktreeId], setWorktreeMeta: (worktreeId: string, meta: Partial) => { @@ -44956,15 +45007,22 @@ describe('OrcaRuntimeService', () => { const draftUrl = 'https://github.com/stablyai/orca/pull/456' const result = await runtime.createManagedWorktree({ repoSelector: TEST_REPO_ID, - name: 'mobile-startup-draft', + name: 'nautilus', + nameWasGenerated: true, startupDraft: draftUrl }) expect(detectRemoteAgentsMock).toHaveBeenCalledWith({ connectionId: 'ssh-1' }) expect(detectInstalledAgentsWithShellPathHydrationMock).not.toHaveBeenCalled() + expect(provider.addWorktree).toHaveBeenCalledWith( + '/remote/repo', + 'nautilus-2', + '/remote/repo-nautilus-2', + expect.any(Object) + ) expect(spawn).toHaveBeenCalledWith( expect.objectContaining({ - cwd: '/remote/mobile-startup-draft', + cwd: '/remote/repo-nautilus-2', command: `claude '--dangerously-skip-permissions' --prefill '${draftUrl}'`, connectionId: 'ssh-1', worktreeId: result.worktree.id diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 0767e6b9363..3fd2f0321e9 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -962,12 +962,15 @@ import { } from '../ipc/worktree-remote' import { getBranchNameOverrideCandidate, + getGeneratedWorktreeCreateCandidate, getWorktreeCreateCandidate, + isGeneratedWorktreeCreateName, WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS } from '../worktree-create-candidates' import { - ensureRetiredWorktreeNamesBackfilled, - getRetiredNameRegistryForRepo + failedWorktreeCreationNeedsRetirement, + getRetiredNameRegistryForRepo, + retireGeneratedWorktreeName } from '../worktree-name-retirement' import { createRetiredNameLookup, @@ -21348,12 +21351,7 @@ export class OrcaRuntimeService { } const repo = await this.resolveRepoSelector(repoSelector) const settings = store.getSettings() - try { - await ensureRetiredWorktreeNamesBackfilled(store, repo, settings) - } catch (error) { - console.warn(`[runtime] retirement backfill failed for repo ${repo.id}:`, error) - } - const registry: RetiredNameRegistry = getRetiredNameRegistryForRepo( + const registry: RetiredNameRegistry = await getRetiredNameRegistryForRepo( store, repo, store.getRepos(), @@ -22570,21 +22568,35 @@ export class OrcaRuntimeService { let branchConflictKind: 'local' | 'remote' | null = null let worktreePath = '' let worktreePathResolved = false - const isRetiredName = args.nameWasGenerated - ? createRetiredNameLookup( - getRetiredNameRegistryForRepo(this.store, repo, this.store.getRepos(), settings) - ) + 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: 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) { - effectiveSanitizedName = getWorktreeCreateCandidate(sanitizedName, suffix) - effectiveRequestedName = args.name.trim() - ? getWorktreeCreateCandidate(args.name, suffix) - : effectiveSanitizedName + 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)) { continue } + attempts += 1 branchName = await resolveCreateBranchName( repo.path, selectedExistingLocalBranchName ?? @@ -22786,19 +22798,11 @@ export class OrcaRuntimeService { ...(suggestLocalBaseRefUpdate ? { suggestLocalBaseRefUpdate } : {}) } const defaultAddWorktreeOption = addProjectGitOptions() - const addResult: AddWorktreeResult = - (await (sparseDirectories.length > 0 - ? checkoutExistingBranch - ? addSparseWorktree( - repo.path, - worktreePath, - branchName, - sparseDirectories, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate, - addProjectGitOptions(existingBranchOption) - ) - : suggestLocalBaseRefUpdate + let addResult: AddWorktreeResult + try { + addResult = + (await (sparseDirectories.length > 0 + ? checkoutExistingBranch ? addSparseWorktree( repo.path, worktreePath, @@ -22806,9 +22810,9 @@ export class OrcaRuntimeService { sparseDirectories, baseBranch, settings.refreshLocalBaseRefOnWorktreeCreate, - addProjectGitOptions({ ...remoteTrackingBaseOption, suggestLocalBaseRefUpdate }) + addProjectGitOptions(existingBranchOption) ) - : remoteTrackingBaseOption + : suggestLocalBaseRefUpdate ? addSparseWorktree( repo.path, worktreePath, @@ -22816,9 +22820,9 @@ export class OrcaRuntimeService { sparseDirectories, baseBranch, settings.refreshLocalBaseRefOnWorktreeCreate, - addProjectGitOptions(remoteTrackingBaseOption) + addProjectGitOptions({ ...remoteTrackingBaseOption, suggestLocalBaseRefUpdate }) ) - : defaultAddWorktreeOption + : remoteTrackingBaseOption ? addSparseWorktree( repo.path, worktreePath, @@ -22826,27 +22830,27 @@ export class OrcaRuntimeService { sparseDirectories, baseBranch, settings.refreshLocalBaseRefOnWorktreeCreate, - defaultAddWorktreeOption + addProjectGitOptions(remoteTrackingBaseOption) ) - : addSparseWorktree( - repo.path, - worktreePath, - branchName, - sparseDirectories, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate - ) - : checkoutExistingBranch - ? addWorktree( - repo.path, - worktreePath, - branchName, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate, - false, - addProjectGitOptions(existingBranchOption) - ) - : suggestLocalBaseRefUpdate + : defaultAddWorktreeOption + ? addSparseWorktree( + repo.path, + worktreePath, + branchName, + sparseDirectories, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate, + defaultAddWorktreeOption + ) + : addSparseWorktree( + repo.path, + worktreePath, + branchName, + sparseDirectories, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate + ) + : checkoutExistingBranch ? addWorktree( repo.path, worktreePath, @@ -22854,9 +22858,9 @@ export class OrcaRuntimeService { baseBranch, settings.refreshLocalBaseRefOnWorktreeCreate, false, - addProjectGitOptions({ ...remoteTrackingBaseOption, suggestLocalBaseRefUpdate }) + addProjectGitOptions(existingBranchOption) ) - : remoteTrackingBaseOption + : suggestLocalBaseRefUpdate ? addWorktree( repo.path, worktreePath, @@ -22864,9 +22868,9 @@ export class OrcaRuntimeService { baseBranch, settings.refreshLocalBaseRefOnWorktreeCreate, false, - addProjectGitOptions(remoteTrackingBaseOption) + addProjectGitOptions({ ...remoteTrackingBaseOption, suggestLocalBaseRefUpdate }) ) - : defaultAddWorktreeOption + : remoteTrackingBaseOption ? addWorktree( repo.path, worktreePath, @@ -22874,15 +22878,36 @@ export class OrcaRuntimeService { baseBranch, settings.refreshLocalBaseRefOnWorktreeCreate, false, - defaultAddWorktreeOption + addProjectGitOptions(remoteTrackingBaseOption) ) - : addWorktree( - repo.path, - worktreePath, - branchName, - baseBranch, - settings.refreshLocalBaseRefOnWorktreeCreate - ))) ?? {} + : defaultAddWorktreeOption + ? addWorktree( + repo.path, + worktreePath, + branchName, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate, + false, + defaultAddWorktreeOption + ) + : addWorktree( + repo.path, + worktreePath, + branchName, + baseBranch, + settings.refreshLocalBaseRefOnWorktreeCreate + ))) ?? {} + } catch (error) { + if (shouldRetireGeneratedName && 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) { + await retireGeneratedWorktreeName(this.store, repo, settings, effectiveSanitizedName) + } let configuredPushTarget: GitPushTarget | undefined if (preparedPushTarget) { @@ -22904,10 +22929,6 @@ export class OrcaRuntimeService { } const worktreeId = `${repo.id}::${created.path}` - // Why: repo-qualified layouts decorate the physical leaf; retire the name main actually used. - if (args.nameWasGenerated) { - this.store.addRetiredWorktreeName(repo.id, effectiveSanitizedName) - } const now = Date.now() // Why: PR/MR-created worktrees can start from a head ref/SHA while Source // Control must compare against the review target branch. @@ -23355,6 +23376,7 @@ export class OrcaRuntimeService { repo: Repo, args: { name: string + nameWasGenerated?: boolean baseBranch?: string compareBaseRef?: string branchNameOverride?: string @@ -23406,6 +23428,7 @@ export class OrcaRuntimeService { { repoId: repo.id, name: args.name, + ...(args.nameWasGenerated === true ? { nameWasGenerated: true } : {}), ...(args.displayName ? { displayName: args.displayName } : {}), ...(args.baseBranch ? { baseBranch: args.baseBranch } : {}), ...(args.compareBaseRef ? { compareBaseRef: args.compareBaseRef } : {}), diff --git a/src/main/worktree-create-candidates.test.ts b/src/main/worktree-create-candidates.test.ts new file mode 100644 index 00000000000..f39fe7cefc3 --- /dev/null +++ b/src/main/worktree-create-candidates.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest' +import { + getGeneratedWorktreeCreateCandidate, + isGeneratedWorktreeCreateName +} from './worktree-create-candidates' + +describe('generated worktree create candidates', () => { + it('advances canonical tiers instead of nesting collision suffixes', () => { + expect(getGeneratedWorktreeCreateCandidate('nautilus-2', 1)).toBe('nautilus-2') + expect(getGeneratedWorktreeCreateCandidate('nautilus-2', 2)).toBe('nautilus-3') + expect(getGeneratedWorktreeCreateCandidate('nautilus-2', 99)).toBe('nautilus-100') + expect(getGeneratedWorktreeCreateCandidate('nautilus', 1, 100)).toBe('nautilus-101') + }) + + it('canonicalizes legacy nested generated suffixes', () => { + expect(isGeneratedWorktreeCreateName('nautilus')).toBe(true) + expect(isGeneratedWorktreeCreateName('nautilus-2')).toBe(true) + expect(isGeneratedWorktreeCreateName('nautilus-2-3')).toBe(true) + expect(getGeneratedWorktreeCreateCandidate('nautilus-2-3', 1)).toBe('nautilus-4') + expect(isGeneratedWorktreeCreateName('fix-login')).toBe(false) + }) +}) diff --git a/src/main/worktree-create-candidates.ts b/src/main/worktree-create-candidates.ts index 87be1398465..71536ad6c98 100644 --- a/src/main/worktree-create-candidates.ts +++ b/src/main/worktree-create-candidates.ts @@ -1,9 +1,24 @@ +import { + getGeneratedWorktreeCreateRetryCandidate, + isGeneratedWorktreeCreateName +} from '../shared/new-workspace/worktree-create-retry-policy' + +export { isGeneratedWorktreeCreateName } + export const WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS = 100 export function getWorktreeCreateCandidate(value: string, suffix: number): string { return suffix === 1 ? value : `${value}-${suffix}` } +export function getGeneratedWorktreeCreateCandidate( + value: string, + suffix: number, + exhaustedTiers = 0 +): string { + return getGeneratedWorktreeCreateRetryCandidate(value, suffix - 1, exhaustedTiers + 1) +} + export function getBranchNameOverrideCandidate( branchNameOverride: string | undefined, suffix: number diff --git a/src/main/worktree-name-retirement.test.ts b/src/main/worktree-name-retirement.test.ts index e2ec1ab826f..8a0e8237caf 100644 --- a/src/main/worktree-name-retirement.test.ts +++ b/src/main/worktree-name-retirement.test.ts @@ -35,12 +35,9 @@ describe('normalizeRetirableGeneratedName', () => { expect(normalizeRetirableGeneratedName(`${FIRST}-100`)).toBe(`${FIRST}-100`) }) - it('retires a suffixed name that itself took a collision suffix', () => { - // Once the pool is spent the suggester emits `-2`; a collision on that yields - // `-2-3`. Stripping only one suffix leaves `-2`, which is not a pool name, so - // retirement would no-op at exactly the tier where every base name is already gone. - expect(normalizeRetirableGeneratedName(`${FIRST}-2-3`)).toBe(`${FIRST}-2-3`) - expect(normalizeRetirableGeneratedName(`${FIRST}-2-3-4`)).toBe(`${FIRST}-2-3-4`) + it('rejects legacy repeated suffixes that the canonical collision sequence never emits', () => { + expect(normalizeRetirableGeneratedName(`${FIRST}-2-3`)).toBeNull() + expect(normalizeRetirableGeneratedName(`${FIRST}-2-3-4`)).toBeNull() expect(normalizeRetirableGeneratedName('fix-login-2-3')).toBeNull() }) @@ -122,6 +119,18 @@ describe('discoverRetiredWorktreeNames', () => { }) }) + it('matches an NFC bucket for an NFD workspace root', async () => { + const nfdRoot = '/Users/ada/cafe\u0301' + await withFakeHome([`-Users-ada-caf--${FIRST}`], async (home) => { + const retired = await discoverRetiredWorktreeNames({ + workspaceRoots: [nfdRoot], + home, + env: {} + }) + expect(retired).toEqual(new Set([FIRST])) + }) + }) + it('matches a dot-directory root, where the separator run encodes to two dashes', async () => { await withFakeHome([`-Users-ada--orca-worktrees-${FIRST}`], async (home) => { const retired = await discoverRetiredWorktreeNames({ @@ -214,6 +223,7 @@ describe('getRetiredNameRegistryForRepo', () => { const calls: string[] = [] return { calls, + mergeRetiredWorktreeNames: () => false, getRetiredWorktreeNameRegistry: (repoId: string) => { calls.push(repoId) return { exhaustedTiers: 0, names: byRepo[repoId] ?? [] } @@ -221,28 +231,28 @@ describe('getRetiredNameRegistryForRepo', () => { } } - it('shares retirements when two repos create into the same cwd namespace', () => { + it('shares retirements when two repos create into the same cwd namespace', async () => { const store = storeOf({ 'repo-a': [FIRST], 'repo-b': [SECOND] }) const repos = [makeRepo('repo-a', '/repos/a'), makeRepo('repo-b', '/repos/b')] expect( - [...getRetiredNameRegistryForRepo(store, repos[1], repos, settingsFor(false)).names].sort() + [ + ...(await getRetiredNameRegistryForRepo(store, repos[1], repos, settingsFor(false))).names + ].sort() ).toEqual([FIRST, SECOND].sort()) }) - it('keeps independent nested repo paths in separate retirement domains', () => { + it('keeps independent nested repo paths in separate retirement domains', async () => { const store = storeOf({ 'repo-a': [FIRST], 'repo-b': [SECOND] }) const repos = [makeRepo('repo-a', '/repos/a'), makeRepo('repo-b', '/repos/b')] - expect(getRetiredNameRegistryForRepo(store, repos[1], repos, settingsFor(true))).toEqual({ + expect(await getRetiredNameRegistryForRepo(store, repos[1], repos, settingsFor(true))).toEqual({ exhaustedTiers: 0, names: [SECOND] }) }) - it('never probes a path for peers that hold no retirements', () => { - // Why: deriving a peer's namespace runs computeWorktreePath, which for a WSL repo is a blocking - // wsl.exe call. Reading `path` is exactly what a probe does, so count that. + it('never probes a path for peers that hold no retirements', async () => { const pathReads: string[] = [] const spyRepo = (id: string, path: string): Repo => { const repo = makeRepo(id, path) @@ -259,19 +269,22 @@ describe('getRetiredNameRegistryForRepo', () => { ...Array.from({ length: 20 }, (_unused, index) => spyRepo(`peer-${index}`, `/repos/${index}`)) ] - expect(getRetiredNameRegistryForRepo(store, repos[0], repos, settingsFor(false))).toEqual({ - exhaustedTiers: 0, - names: [FIRST] - }) + expect(await getRetiredNameRegistryForRepo(store, repos[0], repos, settingsFor(false))).toEqual( + { + exhaustedTiers: 0, + names: [FIRST] + } + ) expect(pathReads.filter((id) => id.startsWith('peer-'))).toEqual([]) }) - it('reports nothing for a folder workspace, which has no generated worktree names', () => { + it('reports nothing for a folder workspace, which has no generated worktree names', async () => { const store = storeOf({ folder: [FIRST] }) const folderRepo = { ...makeRepo('folder', '/repos/folder'), kind: 'folder' as const } expect( - getRetiredNameRegistryForRepo(store, folderRepo, [folderRepo], settingsFor(false)).names + (await getRetiredNameRegistryForRepo(store, folderRepo, [folderRepo], settingsFor(false))) + .names ).toEqual([]) }) }) diff --git a/src/main/worktree-name-retirement.ts b/src/main/worktree-name-retirement.ts index ba73ba2525d..53ac593011d 100644 --- a/src/main/worktree-name-retirement.ts +++ b/src/main/worktree-name-retirement.ts @@ -3,7 +3,7 @@ import { homedir } from 'node:os' import { join } from 'node:path' import { getRepoExecutionHostId } from '../shared/execution-host' import { - CREATURE_POOL_NAMES, + creatureNameTier, EMPTY_RETIRED_NAME_REGISTRY, isEmptyRetiredNameRegistry, mergeRetiredNameRegistries, @@ -13,12 +13,11 @@ import { isFolderRepo } from '../shared/repo-kind' import type { GlobalSettings } from '../shared/global-settings-types' import type { Repo } from '../shared/repo-types' import { - encodeClaudeProjectPath, + encodeClaudeProjectPaths, isClaudeProjectDirInScope } from './ai-vault/claude-project-dir-encoding' import { computeRemoteWorktreePath, - computeWorktreePath, computeWorktreePathAsync, getWorktreePathSettings, hasRepoWorktreeBasePath @@ -31,29 +30,28 @@ const scansByStore = new WeakMap>>>() type RetirementReadStore = { getRetiredWorktreeNameRegistry(repoId: string): RetiredNameRegistry + getRetiredWorktreeNameRegistryForNamespace?(namespaceKey: string): RetiredNameRegistry } type RetirementBackfillStore = { mergeRetiredWorktreeNames(repoId: string, names: Iterable): boolean } +type RetirementWriteStore = { + addRetiredWorktreeName(repoId: string, name: string): void + mergeRetiredWorktreeNamesForNamespace?(namespaceKey: string, names: Iterable): boolean +} type RetirementPathSettings = Pick -/** Strips EVERY trailing numeric suffix, not just one. Once the pool is spent the suggester emits - * `nautilus-2`, and a collision on that yields `nautilus-2-3` — which a single strip leaves as - * `nautilus-2`, not a pool name, so retirement would silently no-op on exactly the tier where the - * pool is already exhausted. Deliberately not shared with `isAutoGeneratedCreatureBranchName`: - * auto-rename eligibility is a different question and its single-strip behavior is intended. */ +/** Only canonical generator output is persisted. Collision retries advance canonical tiers, so a + * repeat-suffixed path can never be generated again and needs no permanent registry entry. */ export function normalizeRetirableGeneratedName(name: string): string | null { const normalized = name.trim().toLowerCase() - if (normalized.length > 256) { - return null - } - let base = normalized - let stripped = base.replace(/-\d+$/, '') - while (stripped !== base) { - base = stripped - stripped = base.replace(/-\d+$/, '') - } - return base && CREATURE_POOL_NAMES.has(base) ? normalized : null + return normalized.length <= 256 && creatureNameTier(normalized) !== null ? normalized : null +} + +/** A sparse create error carries this marker only when its rollback also failed, leaving the path + * occupied even though creation rejected. */ +export function failedWorktreeCreationNeedsRetirement(error: unknown): boolean { + return typeof error === 'object' && error !== null && Reflect.get(error, 'cleanupFailed') === true } /** Why: over-retiring costs one name from a 552-entry pool; under-retiring reissues a path whose @@ -95,17 +93,33 @@ export function extractBucketLeafCandidates( return [] } -function getRetirementProbePath(repo: Repo, settings: RetirementPathSettings): string { +async function getRetirementProbePath( + repo: Repo, + settings: RetirementPathSettings +): Promise { const pathSettings = getWorktreePathSettings(repo, settings) return repo.connectionId ? computeRemoteWorktreePath(RETIREMENT_PROBE_NAME, repo.path, pathSettings, { useConfiguredAbsolutePath: hasRepoWorktreeBasePath(repo) }) - : computeWorktreePath(RETIREMENT_PROBE_NAME, repo.path, pathSettings) + : computeWorktreePathAsync(RETIREMENT_PROBE_NAME, repo.path, pathSettings) } -// Why: for a WSL repo the probe path costs a blocking `wsl.exe` call, and the create path plus -// every suggestion refresh asks for the same key. Inputs are all the key derives from. +export function getRemoteRetirementNamespaceKey( + repo: Repo, + settings: RetirementPathSettings +): string | null { + if (!repo.connectionId) { + return null + } + const pathSettings = getWorktreePathSettings(repo, settings) + const probePath = computeRemoteWorktreePath(RETIREMENT_PROBE_NAME, repo.path, pathSettings, { + useConfiguredAbsolutePath: hasRepoWorktreeBasePath(repo) + }) + return `${getRepoExecutionHostId(repo)}:${worktreePathComparisonKey(probePath)}` +} + +// Why: the create path and every suggestion refresh ask for the same namespace identity. const COLLISION_KEY_CACHE_MAX = 512 const collisionKeyCache = new Map() @@ -115,7 +129,10 @@ export function resetRetirementCollisionKeyCacheForTests(): void { /** Identifies the cwd namespace a repo creates workspaces into. Two repos that would place a * workspace of the same name at the same path share retirements; independent paths do not. */ -export function getRetirementCollisionKey(repo: Repo, settings: RetirementPathSettings): string { +async function getRetirementCollisionKey( + repo: Repo, + settings: RetirementPathSettings +): Promise { const hostId = getRepoExecutionHostId(repo) const cacheKey = [ hostId, @@ -128,7 +145,9 @@ export function getRetirementCollisionKey(repo: Repo, settings: RetirementPathSe if (cached !== undefined) { return cached } - const key = `${hostId}:${worktreePathComparisonKey(getRetirementProbePath(repo, settings))}` + const key = + getRemoteRetirementNamespaceKey(repo, settings) ?? + `${hostId}:${worktreePathComparisonKey(await getRetirementProbePath(repo, settings))}` // Why not always cache: only the WSL home *success* path is cached upstream, so a stopped distro // yields a fallback namespace. Memoizing that would strand the repo there for the whole session. const wsl = parseWslPath(repo.path) @@ -146,19 +165,30 @@ export function getRetirementCollisionKey(repo: Repo, settings: RetirementPathSe * namespace — that is where a reissued name would actually collide. * * The peer scan is deliberately lazy: a repo's own retirements never touch a path, and a peer's - * namespace is derived only once that peer is known to hold retirements. Deriving one costs a - * blocking `wsl.exe` call for a WSL repo, so it must not be paid per repo on every create. */ -export function getRetiredNameRegistryForRepo( - store: RetirementReadStore, + * namespace is derived only once that peer is known to hold retirements. */ +export async function getRetiredNameRegistryForRepo( + store: RetirementReadStore & RetirementBackfillStore, repo: Repo, repos: readonly Repo[], settings: RetirementPathSettings -): RetiredNameRegistry { +): Promise { if (isFolderRepo(repo)) { return EMPTY_RETIRED_NAME_REGISTRY } - let registry = store.getRetiredWorktreeNameRegistry(repo.id) let collisionKey: string | null = null + try { + collisionKey = await ensureRetiredWorktreeNamesBackfilled(store, repo, settings) + } catch (error) { + console.warn(`[worktrees] retirement backfill failed for repo ${repo.id}:`, error) + } + let registry = store.getRetiredWorktreeNameRegistry(repo.id) + if (repo.connectionId && store.getRetiredWorktreeNameRegistryForNamespace) { + collisionKey ??= await getRetirementCollisionKey(repo, settings) + registry = mergeRetiredNameRegistries( + registry, + store.getRetiredWorktreeNameRegistryForNamespace(collisionKey) + ) + } for (const candidate of repos) { if (candidate.id === repo.id || isFolderRepo(candidate)) { continue @@ -167,8 +197,8 @@ export function getRetiredNameRegistryForRepo( if (isEmptyRetiredNameRegistry(candidateRegistry)) { continue } - collisionKey ??= getRetirementCollisionKey(repo, settings) - if (getRetirementCollisionKey(candidate, settings) !== collisionKey) { + collisionKey ??= await getRetirementCollisionKey(repo, settings) + if ((await getRetirementCollisionKey(candidate, settings)) !== collisionKey) { continue } registry = mergeRetiredNameRegistries(registry, candidateRegistry) @@ -176,6 +206,24 @@ export function getRetiredNameRegistryForRepo( return registry } +export async function retireGeneratedWorktreeName( + store: RetirementWriteStore, + repo: Repo, + settings: RetirementPathSettings, + name: string +): Promise { + store.addRetiredWorktreeName(repo.id, name) + if (!repo.connectionId || !store.mergeRetiredWorktreeNamesForNamespace) { + return + } + try { + const namespaceKey = await getRetirementCollisionKey(repo, settings) + store.mergeRetiredWorktreeNamesForNamespace(namespaceKey, [name]) + } catch (error) { + console.warn(`[worktrees] failed to persist remote retirement namespace for ${repo.id}:`, error) + } +} + function parentPath(path: string): string { const trimmed = path.replace(/[/\\]+$/, '') const separatorIndex = Math.max(trimmed.lastIndexOf('/'), trimmed.lastIndexOf('\\')) @@ -189,11 +237,11 @@ export async function ensureRetiredWorktreeNamesBackfilled( store: RetirementBackfillStore, repo: Repo, settings: RetirementPathSettings -): Promise { +): Promise { // Remote workspaces keep their agent state on the execution host, which this scan cannot see, so // a re-added SSH repo does not recover its retirements the way a local one does. if (isFolderRepo(repo) || repo.connectionId) { - return + return null } const probePath = await computeWorktreePathAsync( RETIREMENT_PROBE_NAME, @@ -214,6 +262,7 @@ export async function ensureRetiredWorktreeNamesBackfilled( // Why the merge sits outside the cached scan: the scan is per cwd namespace but the registry is // per repo, so every repo that asks must receive it — not only the one that triggered it. store.mergeRetiredWorktreeNames(repo.id, await scan) + return scanKey } async function readDirectoryNames(path: string): Promise { @@ -257,7 +306,7 @@ export async function discoverRetiredWorktreeNames(args: { // filesystems, and a missed bucket reissues a live path while a spurious one costs one name. const encodedParents = args.workspaceRoots .filter((root) => root.length > 0) - .map((root) => encodeClaudeProjectPath(root).toLowerCase()) + .flatMap((root) => encodeClaudeProjectPaths(root).map((path) => path.toLowerCase())) if (encodedParents.length > 0) { const projectsDir = getClaudeProjectsDir(args.home ?? homedir(), args.env ?? process.env) for (const bucket of await readDirectoryNames(projectsDir)) { diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 068cf561e45..a7c43c8d596 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -6244,6 +6244,33 @@ describe('worktree remote runtime mutations', () => { expect(store.getState().worktreesByRepo.repo1).toEqual([wt]) }) + it('forwards generated-name provenance through paired-runtime create', async () => { + const store = createTestStore() + const wt = makeWorktree({ id: 'repo1::/path/nautilus', repoId: 'repo1' }) + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-create', + ok: true, + result: { worktree: wt }, + _meta: { runtimeId: 'runtime-remote' } + }) + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' } as never, + worktreesByRepo: { repo1: [] } + } as Partial) + const createWorktree = store.getState().createWorktree + const args: Parameters = ['repo1', 'nautilus'] + args[25] = { nameWasGenerated: true } + + await createWorktree(...args) + + expect(runtimeEnvironmentCall).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'worktree.create', + params: expect.objectContaining({ nameWasGenerated: true }) + }) + ) + }) + it('persists Jira item and source context through paired-runtime create', async () => { const store = createTestStore() const wt = makeWorktree({ diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 7443b816008..8ba5b2881ba 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -109,6 +109,7 @@ import { folderWorkspaceToWorktree } from '../../../../shared/folder-workspace-w import { CLIENT_WORKTREE_CREATE_MAX_ATTEMPTS, getClientWorktreeCreateCandidate, + getGeneratedWorktreeCreateRetryCandidate, isRetryableWorktreeCreateConflict } from '../../../../shared/new-workspace/worktree-create-retry-policy' import { @@ -3941,7 +3942,9 @@ export const createWorktreeSlice: StateCreator const provisionedRoot = options?.provisionedRoot try { for (let attempt = 0; attempt < CLIENT_WORKTREE_CREATE_MAX_ATTEMPTS; attempt += 1) { - const candidateName = getClientWorktreeCreateCandidate(name, attempt) + const candidateName = options?.nameWasGenerated + ? getGeneratedWorktreeCreateRetryCandidate(name, attempt) + : getClientWorktreeCreateCandidate(name, attempt) // Why: older runtimes reject exact PR branch overrides on collision, so retry both branch and worktree names. const candidateBranchNameOverride = branchNameOverride ? getClientWorktreeCreateCandidate(branchNameOverride, attempt) @@ -4020,6 +4023,7 @@ export const createWorktreeSlice: StateCreator { repo: repoId, name: candidateName, + ...(options?.nameWasGenerated ? { nameWasGenerated: true } : {}), baseBranch, ...(compareBaseRef ? { compareBaseRef } : {}), ...(candidateBranchNameOverride diff --git a/src/shared/constants.ts b/src/shared/constants.ts index 03370a86fe6..21a093f43bd 100644 --- a/src/shared/constants.ts +++ b/src/shared/constants.ts @@ -430,6 +430,7 @@ export function getDefaultPersistedState(homedir: string): PersistedState { folderWorkspaces: [], sparsePresetsByRepo: {}, retiredWorktreeNamesByRepo: {}, + retiredWorktreeNamesByNamespace: {}, worktreeMeta: {}, worktreeLineageById: {}, workspaceLineageByChildKey: {}, diff --git a/src/shared/new-workspace/worktree-create-retry-policy.test.ts b/src/shared/new-workspace/worktree-create-retry-policy.test.ts index 47cc5e79ee6..e10ba3e9ddf 100644 --- a/src/shared/new-workspace/worktree-create-retry-policy.test.ts +++ b/src/shared/new-workspace/worktree-create-retry-policy.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { getClientWorktreeCreateCandidate, + getGeneratedWorktreeCreateRetryCandidate, isRetryableWorktreeCreateConflict } from './worktree-create-retry-policy' @@ -10,6 +11,12 @@ describe('client worktree create retry policy', () => { expect(getClientWorktreeCreateCandidate('feature', 1)).toBe('feature-2') }) + it('keeps generated retries on canonical tiers', () => { + expect(getGeneratedWorktreeCreateRetryCandidate('nautilus-2', 0)).toBe('nautilus-2') + expect(getGeneratedWorktreeCreateRetryCandidate('nautilus-2', 1)).toBe('nautilus-3') + expect(getGeneratedWorktreeCreateRetryCandidate('nautilus-2-3', 0)).toBe('nautilus-4') + }) + it('retries only known branch and review conflicts', () => { expect(isRetryableWorktreeCreateConflict('Branch already exists locally')).toBe(true) expect(isRetryableWorktreeCreateConflict('Branch "x" already exists.')).toBe(true) diff --git a/src/shared/new-workspace/worktree-create-retry-policy.ts b/src/shared/new-workspace/worktree-create-retry-policy.ts index 295dd3c7839..e45da0d5530 100644 --- a/src/shared/new-workspace/worktree-create-retry-policy.ts +++ b/src/shared/new-workspace/worktree-create-retry-policy.ts @@ -1,3 +1,9 @@ +import { + CREATURE_POOL_NAMES, + creatureNameAtTier, + creatureNameTier +} from '../worktree/retired-name-registry' + export const CLIENT_WORKTREE_CREATE_MAX_ATTEMPTS = 25 // Why: mixed-version runtimes can still return these legacy conflicts instead @@ -13,6 +19,49 @@ export function getClientWorktreeCreateCandidate(value: string, attempt: number) return attempt === 0 ? value : `${value}-${attempt + 1}` } +function generatedNameIdentity(value: string): { poolName: string; tier: number } | null { + const normalized = value.trim().toLowerCase() + const canonicalTier = creatureNameTier(normalized) + if (canonicalTier !== null) { + const poolName = + canonicalTier === 1 ? normalized : normalized.slice(0, normalized.lastIndexOf('-')) + return { poolName, tier: canonicalTier } + } + let poolName = normalized + let tier = 1 + let suffixCount = 0 + for (;;) { + const match = /-(\d{1,6})$/.exec(poolName) + if (!match) { + break + } + const suffix = Number(match[1]) + if (suffix < 2 || tier + suffix - 1 > 999_999) { + return null + } + tier += suffix - 1 + suffixCount += 1 + poolName = poolName.slice(0, match.index) + } + return suffixCount > 1 && CREATURE_POOL_NAMES.has(poolName) ? { poolName, tier } : null +} + +export function isGeneratedWorktreeCreateName(value: string): boolean { + return generatedNameIdentity(value) !== null +} + +export function getGeneratedWorktreeCreateRetryCandidate( + value: string, + attempt: number, + minimumTier = 1 +): string { + const identity = generatedNameIdentity(value) + if (!identity) { + return getClientWorktreeCreateCandidate(value, attempt) + } + return creatureNameAtTier(identity.poolName, Math.max(identity.tier, minimumTier) + attempt) +} + export function isRetryableWorktreeCreateConflict(message: string): boolean { return RETRYABLE_WORKTREE_CREATE_CONFLICT_PATTERNS.some((pattern) => pattern.test(message)) } diff --git a/src/shared/persisted-state-types.ts b/src/shared/persisted-state-types.ts index c66c335188c..ee6d1f97fa7 100644 --- a/src/shared/persisted-state-types.ts +++ b/src/shared/persisted-state-types.ts @@ -67,12 +67,13 @@ export type PersistedState = { /** Generated workspace names already issued, keyed by repoId. Suppresses reissuing a name so a * recreated workspace never lands on a prior occupant's path and inherits agent state keyed to * that cwd. Grows monotonically within a repo — entries are never removed on workspace deletion - * — and is dropped wholesale when the repo is removed. Readers union across every repo that - * creates into the same cwd namespace, so the storage key stays the stable repo id while the - * guarantee follows the path. + * — and the repo-keyed copy is dropped when the repo is removed. Readers union across repos + * that create into the same cwd namespace; remote namespaces also retain a compact tombstone. * * Stored compacted, not as a flat list: a fully spent tier collapses into the row's watermark. */ retiredWorktreeNamesByRepo?: Record + /** Compacted remote cwd tombstones outlive repo ids, so remove/re-add cannot reissue a path. */ + retiredWorktreeNamesByNamespace?: Record /** Per paired device last tab selection by worktree; keeps mobile navigation across host restarts. */ mobileClientTabSelectionsByDeviceId?: PersistedMobileClientTabSelections worktreeMeta: Record