fix(worktree): key retirement by repo id and prune it with the repo

Reverts the collision-key storage key. It was a function of workspaceDir,
nestWorkspaces, worktreeBasePath and repo.path, so toggling any one of those
orphaned every retirement for every affected repo at once — trading a rare
churn (remove/re-add) for a common one. The read path already unions by cwd
namespace at query time, so cross-repo sharing never depended on the storage
key.

Instead, address the growth and orphaning directly:
- Drop the registry in removeProject, and in removeProjectForHost once the last
  host's copy of the repo id is gone, alongside the sparse-preset deletes that
  already follow this convention.
- Bound each repo's registry. The cap sits far above the 552-name pool because
  evicting inside it would reissue a name whose agent state is still on disk;
  only -2/-3 tier accumulation can ever reach it.
- Carry retirements through profile transfer, re-keyed to the destination repo
  id and dropped from the source, mirroring sparsePresetsByRepo.

Separately, fix the backfill merge: the scan promise is cached per cwd
namespace, but it closed over the first repo id that triggered it, so a second
repo in the same namespace received nothing. The scan stays shared; the merge
moves out of the cached promise and runs for whichever repo asked.

Local repos re-seed on re-add through that backfill. SSH repos do not — the
scan cannot see the execution host — which is now stated in the module.
This commit is contained in:
Brennan Benson
2026-08-13 17:59:00 -07:00
parent 9d8ce5c6f2
commit 9234cb4055
16 changed files with 270 additions and 169 deletions
+5 -8
View File
@@ -135,10 +135,7 @@ import {
getWorktreeCreateCandidate,
WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS
} from '../worktree-create-candidates'
import {
getRetiredWorktreeNamesForRepo,
retireGeneratedWorktreeName
} from '../worktree-name-retirement'
import { getRetiredWorktreeNamesForRepo } from '../worktree-name-retirement'
const SSH_WORKTREE_CREATE_FETCH_FRESHNESS_MS = 30_000
const SSH_WORKTREE_CREATE_FETCH_CACHE_MAX = 512
@@ -1555,7 +1552,7 @@ export async function createRemoteWorktree(
let lastBranchConflictKind: 'local' | 'remote' | null = null
let remotePathResolved = false
const retiredNames = args.nameWasGenerated
? new Set(getRetiredWorktreeNamesForRepo(store, repo, settings))
? new Set(getRetiredWorktreeNamesForRepo(store, repo, store.getRepos(), settings))
: 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) {
@@ -1773,7 +1770,7 @@ 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) {
retireGeneratedWorktreeName(store, repo, settings, effectiveSanitizedName)
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.
@@ -2104,7 +2101,7 @@ export async function createLocalWorktree(
let lastExistingPR: Awaited<ReturnType<typeof getPRForBranch>> | null = null
let lastExistingReviewNumber: number | null = null
const retiredNames = args.nameWasGenerated
? new Set(getRetiredWorktreeNamesForRepo(store, repo, settings))
? new Set(getRetiredWorktreeNamesForRepo(store, repo, store.getRepos(), settings))
: 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) {
@@ -2400,7 +2397,7 @@ 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) {
retireGeneratedWorktreeName(store, repo, settings, effectiveSanitizedName)
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.
+1 -1
View File
@@ -348,7 +348,7 @@ describe('registerWorktreeHandlers – Windows path handling', () => {
nameWasGenerated: true
})
expect(store.addRetiredWorktreeName).toHaveBeenCalledWith(expect.any(String), 'nautilus-2')
expect(store.addRetiredWorktreeName).toHaveBeenCalledWith('repo-1', 'nautilus-2')
})
it('leaves a user-typed name reusable even when the same name is retired', async () => {
+1 -1
View File
@@ -4715,7 +4715,7 @@ describe('registerWorktreeHandlers', () => {
)
expect(provider.listWorktrees).toHaveBeenCalledTimes(1)
expect(provider.worktreeIsClean).not.toHaveBeenCalled()
expect(store.addRetiredWorktreeName).toHaveBeenCalledWith(expect.any(String), 'nautilus-2')
expect(store.addRetiredWorktreeName).toHaveBeenCalledWith('repo-ssh', 'nautilus-2')
expect(store.setWorktreeMeta).toHaveBeenCalledWith(
'repo-ssh::/remote/repo-nautilus-2',
expect.objectContaining({
+1 -1
View File
@@ -1925,7 +1925,7 @@ export function registerWorktreeHandlers(
// 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 getRetiredWorktreeNamesForRepo(store, repo, store.getSettings())
return getRetiredWorktreeNamesForRepo(store, repo, store.getRepos(), store.getSettings())
})
ipcMain.handle('worktrees:list', async (_event, args: { repoId: string }) => {
@@ -18,6 +18,7 @@ export function removeSourceRepo(
...state,
repos: state.repos.filter((repo) => repo.id !== repoId),
sparsePresetsByRepo: { ...state.sparsePresetsByRepo },
retiredWorktreeNamesByRepo: { ...state.retiredWorktreeNamesByRepo },
worktreeMeta: { ...state.worktreeMeta },
worktreeLineageById: { ...state.worktreeLineageById },
workspaceLineageByChildKey: { ...state.workspaceLineageByChildKey },
@@ -38,6 +39,7 @@ export function removeSourceRepo(
}
}
delete next.sparsePresetsByRepo[repoId]
delete next.retiredWorktreeNamesByRepo?.[repoId]
removeRepoWorktreeMetadata(next, repoId)
return rebuildRepoBackedProjectState(next)
}
@@ -45,6 +45,7 @@ export function readProfileState(profileId: string, userDataPath: string): Trans
projectGroups: arrayOrEmpty(parsed.projectGroups),
folderWorkspaces: arrayOrEmpty(parsed.folderWorkspaces),
sparsePresetsByRepo: recordOrEmpty<SparsePreset[]>(parsed.sparsePresetsByRepo),
retiredWorktreeNamesByRepo: recordOrEmpty<string[]>(parsed.retiredWorktreeNamesByRepo),
worktreeMeta: recordOrEmpty(parsed.worktreeMeta),
worktreeLineageById: recordOrEmpty(parsed.worktreeLineageById),
workspaceLineageByChildKey: recordOrEmpty(parsed.workspaceLineageByChildKey),
@@ -20,6 +20,9 @@ import {
export type TransferPayload = {
repo: Repo
sparsePresets: SparsePreset[]
/** Generated names already spent for this repo. Carried so the destination profile does not
* reissue a name whose old directory still holds agent state. */
retiredWorktreeNames: string[]
worktreeMeta: PersistedState['worktreeMeta']
worktreeLineageById: PersistedState['worktreeLineageById']
workspaceLineageByChildKey: PersistedState['workspaceLineageByChildKey']
@@ -214,6 +217,7 @@ export function createTransferPayload(args: {
...structuredClone(preset),
repoId: newRepoId
})),
retiredWorktreeNames: [...(sourceState.retiredWorktreeNamesByRepo?.[oldRepoId] ?? [])],
worktreeMeta: rekeyWorktreeIdRecord(
sourceState.worktreeMeta,
worktreeIds,
@@ -269,6 +273,13 @@ export function applyPayloadToTarget(
...targetState.sparsePresetsByRepo,
...(payload.sparsePresets.length > 0 ? { [payload.repo.id]: payload.sparsePresets } : {})
},
retiredWorktreeNamesByRepo: {
...targetState.retiredWorktreeNamesByRepo,
...(payload.retiredWorktreeNames.length > 0
? { [payload.repo.id]: payload.retiredWorktreeNames }
: {})
},
worktreeMeta: { ...targetState.worktreeMeta, ...payload.worktreeMeta },
worktreeLineageById: { ...targetState.worktreeLineageById, ...payload.worktreeLineageById },
workspaceLineageByChildKey: {
@@ -126,6 +126,7 @@ describe('profile project transfer', () => {
}
]
},
retiredWorktreeNamesByRepo: { 'repo-1': ['nautilus'] },
worktreeMeta: {
[sourceWorktreeId]: makeWorktreeMeta({ projectHostSetupId: 'repo-1' })
},
@@ -177,6 +178,9 @@ describe('profile project transfer', () => {
expect(target.sparsePresetsByRepo[targetRepoId]).toEqual([
expect.objectContaining({ id: 'preset-1', repoId: targetRepoId })
])
// Why: without the re-key the destination profile reissues a name whose old directory may
// still hold the previous occupant's agent conversation.
expect(target.retiredWorktreeNamesByRepo?.[targetRepoId]).toEqual(['nautilus'])
expect(target.workspaceSession.tabsByWorktree).toEqual({})
expect(readProfileState('personal').repos.map((repo) => repo.id)).toEqual(['repo-1'])
})
@@ -3,6 +3,7 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { getDefaultPersistedState } from '../shared/constants'
import { MARINE_CREATURES } from '../shared/marine-creatures'
const testState = { dir: '' }
@@ -13,9 +14,8 @@ vi.mock('electron', () => ({
vi.mock('./telemetry/client', () => ({ track: vi.fn() }))
vi.mock('./telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn() }))
// The registry is keyed by the cwd namespace a repo creates into, not by repo id.
const KEY = 'local:posix:/workspaces/repo-1'
const OTHER_KEY = 'local:posix:/workspaces/repo-2'
const REPO = 'repo-1'
const OTHER_REPO = 'repo-2'
async function createStore(persisted: Record<string, unknown> = {}) {
mkdirSync(testState.dir, { recursive: true })
@@ -41,81 +41,134 @@ afterEach(() => {
describe('worktree name retirement registry', () => {
it('retains a retired name so it is never reissued', async () => {
const store = await createStore()
store.addRetiredWorktreeName(KEY, 'Nautilus')
expect(store.getRetiredWorktreeNames(KEY)).toEqual(['nautilus'])
store.addRetiredWorktreeName(REPO, 'Nautilus')
expect(store.getRetiredWorktreeNames(REPO)).toEqual(['nautilus'])
})
it('scopes retirement per cwd namespace so one workspace root does not burn another', async () => {
it('scopes retirement per repo so one repo does not burn another pool', async () => {
const store = await createStore()
store.addRetiredWorktreeName(KEY, 'nautilus')
expect(store.getRetiredWorktreeNames(OTHER_KEY)).toEqual([])
store.addRetiredWorktreeName(REPO, 'nautilus')
expect(store.getRetiredWorktreeNames(OTHER_REPO)).toEqual([])
})
it('normalizes case and whitespace so a name cannot be retired twice', async () => {
const store = await createStore()
store.addRetiredWorktreeName(KEY, ' NaUtIlUs ')
store.addRetiredWorktreeName(KEY, 'nautilus')
expect(store.getRetiredWorktreeNames(KEY)).toEqual(['nautilus'])
store.addRetiredWorktreeName(REPO, ' NaUtIlUs ')
store.addRetiredWorktreeName(REPO, 'nautilus')
expect(store.getRetiredWorktreeNames(REPO)).toEqual(['nautilus'])
})
it('ignores an empty name or missing workspace key', async () => {
it('ignores an empty name or missing repo id', async () => {
const store = await createStore()
store.addRetiredWorktreeName(KEY, ' ')
store.addRetiredWorktreeName(REPO, ' ')
store.addRetiredWorktreeName('', 'nautilus')
expect(store.getRetiredWorktreeNames(KEY)).toEqual([])
expect(store.getRetiredWorktreeNames(REPO)).toEqual([])
})
it('does not persist arbitrary user and issue-title names', async () => {
const store = await createStore()
store.addRetiredWorktreeName(KEY, 'fix-login-redirect')
store.addRetiredWorktreeName(KEY, 'STA-4189-duplicate-name')
expect(store.getRetiredWorktreeNames(KEY)).toEqual([])
store.addRetiredWorktreeName(REPO, 'fix-login-redirect')
store.addRetiredWorktreeName(REPO, 'STA-4189-duplicate-name')
expect(store.getRetiredWorktreeNames(REPO)).toEqual([])
})
it('returns a copy so callers cannot mutate the registry in place', async () => {
const store = await createStore()
store.addRetiredWorktreeName(KEY, 'nautilus')
store.getRetiredWorktreeNames(KEY).push('seahorse')
expect(store.getRetiredWorktreeNames(KEY)).toEqual(['nautilus'])
store.addRetiredWorktreeName(REPO, 'nautilus')
store.getRetiredWorktreeNames(REPO).push('seahorse')
expect(store.getRetiredWorktreeNames(REPO)).toEqual(['nautilus'])
})
it('merges backfilled names without dropping a concurrent retirement', async () => {
const store = await createStore()
store.addRetiredWorktreeName(KEY, 'nautilus')
expect(store.mergeRetiredWorktreeNames(KEY, ['seahorse', 'starfish'])).toBe(true)
expect(store.getRetiredWorktreeNames(KEY).sort()).toEqual(['nautilus', 'seahorse', 'starfish'])
store.addRetiredWorktreeName(REPO, 'nautilus')
expect(store.mergeRetiredWorktreeNames(REPO, ['seahorse', 'starfish'])).toBe(true)
expect(store.getRetiredWorktreeNames(REPO).sort()).toEqual(['nautilus', 'seahorse', 'starfish'])
})
it('reports no change when a merge adds nothing new', async () => {
const store = await createStore()
store.addRetiredWorktreeName(KEY, 'nautilus')
expect(store.mergeRetiredWorktreeNames(KEY, ['NAUTILUS'])).toBe(false)
store.addRetiredWorktreeName(REPO, 'nautilus')
expect(store.mergeRetiredWorktreeNames(REPO, ['NAUTILUS'])).toBe(false)
})
it('survives a reload so retirement outlives the workspace and the app session', async () => {
const store = await createStore({
retiredWorktreeNamesByWorkspaceKey: { [KEY]: ['nautilus', 'seahorse'] }
retiredWorktreeNamesByRepo: { [REPO]: ['nautilus', 'seahorse'] }
})
expect(store.getRetiredWorktreeNames(KEY).sort()).toEqual(['nautilus', 'seahorse'])
expect(store.getRetiredWorktreeNames(REPO).sort()).toEqual(['nautilus', 'seahorse'])
})
it('degrades to nothing retired when the persisted map is corrupt', async () => {
// Why: a load failure costs the app; over- or under-retiring costs at most a name.
const store = await createStore({ retiredWorktreeNamesByWorkspaceKey: 'not-an-object' })
expect(store.getRetiredWorktreeNames(KEY)).toEqual([])
const store = await createStore({ retiredWorktreeNamesByRepo: 'not-an-object' })
expect(store.getRetiredWorktreeNames(REPO)).toEqual([])
})
it('drops non-string entries but keeps the valid ones', async () => {
const store = await createStore({
retiredWorktreeNamesByWorkspaceKey: { [KEY]: ['nautilus', 42, null, 'Seahorse'] }
retiredWorktreeNamesByRepo: { [REPO]: ['nautilus', 42, null, 'Seahorse'] }
})
expect(store.getRetiredWorktreeNames(KEY).sort()).toEqual(['nautilus', 'seahorse'])
expect(store.getRetiredWorktreeNames(REPO).sort()).toEqual(['nautilus', 'seahorse'])
})
it('drops the registry when the repo is removed, matching the sparse-preset convention', async () => {
// Why: entries are repo-id keyed, so without this they orphan forever — a repo id is never
// reused, and remove/re-add mints a new one.
const store = await createStore()
store.addRepo({ id: REPO, path: '/repos/a', displayName: 'a', badgeColor: '', addedAt: 0 })
store.addRetiredWorktreeName(REPO, 'nautilus')
store.removeProject(REPO)
expect(store.getRetiredWorktreeNames(REPO)).toEqual([])
})
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 })
store.addRepo({
id: REPO,
path: '/repos/a',
displayName: 'a',
badgeColor: '',
addedAt: 0,
executionHostId: 'runtime:env-1'
} as never)
store.addRetiredWorktreeName(REPO, 'nautilus')
store.removeProjectForHost(REPO, 'runtime:env-1')
expect(store.getRetiredWorktreeNames(REPO)).toEqual(['nautilus'])
store.removeProjectForHost(REPO, 'local')
expect(store.getRetiredWorktreeNames(REPO)).toEqual([])
})
it('bounds the per-repo registry above the pool so the cap never reissues a base name', async () => {
// The cap must sit well past the 552-name pool: evicting inside it would hand back a name whose
// agent state is still on disk, which is the bug this registry exists to prevent.
const store = await createStore()
const names = MARINE_CREATURES.map((name) => name.toLowerCase())
for (let tier = 2; tier <= 5; tier += 1) {
store.mergeRetiredWorktreeNames(
REPO,
names.map((name) => `${name}-${tier}`)
)
}
store.mergeRetiredWorktreeNames(REPO, names)
const retained = store.getRetiredWorktreeNames(REPO)
expect(retained.length).toBe(2000)
// Oldest-first eviction, so the most recent merge survives intact.
expect(retained).toEqual(expect.arrayContaining(names))
})
it('drops legacy arbitrary names while loading the persisted map', async () => {
const store = await createStore({
retiredWorktreeNamesByWorkspaceKey: { [KEY]: ['fix-login', 'Nautilus'] }
retiredWorktreeNamesByRepo: { [REPO]: ['fix-login', 'Nautilus'] }
})
expect(store.getRetiredWorktreeNames(KEY)).toEqual(['nautilus'])
expect(store.getRetiredWorktreeNames(REPO)).toEqual(['nautilus'])
})
})
+42 -25
View File
@@ -2335,15 +2335,28 @@ const MAX_CLAUDE_LIVE_PTY_SESSION_IDS = 200
// Why: bound removed-SSH-target history so remove/re-add churn can't grow the file unbounded.
const MAX_REMOVED_SSH_TARGET_TOMBSTONES = 50
// Why: bound per-repo retirement so long-lived repos can't grow the file unbounded. Deliberately
// far above the 552-name pool — a cap below it would start reissuing names and reintroduce the bug
// this registry exists to prevent, so only `-2`/`-3` tier accumulation can ever reach it.
const MAX_RETIRED_WORKTREE_NAMES_PER_REPO = 2000
/** Oldest-first eviction: entries are appended in retirement order, and the oldest paths are the
* ones whose agent state is likeliest to be gone already. */
function boundRetiredWorktreeNames(names: readonly string[]): string[] {
return names.length > MAX_RETIRED_WORKTREE_NAMES_PER_REPO
? names.slice(names.length - MAX_RETIRED_WORKTREE_NAMES_PER_REPO)
: [...names]
}
/** 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 normalizeRetiredWorktreeNamesByWorkspaceKey(value: unknown): Record<string, string[]> {
function normalizeRetiredWorktreeNamesByRepo(value: unknown): Record<string, string[]> {
if (!value || typeof value !== 'object' || Array.isArray(value)) {
return {}
}
const byKey: Record<string, string[]> = {}
for (const [workspaceKey, names] of Object.entries(value as Record<string, unknown>)) {
if (!workspaceKey || !Array.isArray(names)) {
const byRepo: Record<string, string[]> = {}
for (const [repoId, names] of Object.entries(value as Record<string, unknown>)) {
if (!repoId || !Array.isArray(names)) {
continue
}
const seen = new Set<string>()
@@ -2357,10 +2370,10 @@ function normalizeRetiredWorktreeNamesByWorkspaceKey(value: unknown): Record<str
}
}
if (seen.size > 0) {
byKey[workspaceKey] = [...seen]
byRepo[repoId] = boundRetiredWorktreeNames([...seen])
}
}
return byKey
return byRepo
}
function normalizeClaudeLivePtySessionIds(value: unknown): string[] {
@@ -3848,8 +3861,8 @@ export class Store {
(alias): alias is string => typeof alias === 'string'
)
: [],
retiredWorktreeNamesByWorkspaceKey: normalizeRetiredWorktreeNamesByWorkspaceKey(
parsed.retiredWorktreeNamesByWorkspaceKey
retiredWorktreeNamesByRepo: normalizeRetiredWorktreeNamesByRepo(
parsed.retiredWorktreeNamesByRepo
),
sshRemotePtyLeases: (parsed.sshRemotePtyLeases ?? [])
.map(normalizeSshRemotePtyLease)
@@ -4881,6 +4894,8 @@ export class Store {
this.syncProjectHostSetupCompatibilityState()
// Why: presets are repo-scoped and unreachable once the repo is gone, so drop them with it.
delete this.state.sparsePresetsByRepo[id]
// Same convention: retirements are repo-id-keyed, so they would otherwise orphan forever.
delete this.state.retiredWorktreeNamesByRepo?.[id]
this.pruneWorktreeStateForRepo(id, null)
this.state.workspaceSession = removeRepoFromWorkspaceSession(this.state.workspaceSession, id)
this.state.workspaceSessionsByHostId = removeRepoFromHostWorkspaceSessions(
@@ -4896,9 +4911,11 @@ export class Store {
(r) => !(r.id === id && getRepoExecutionHostId(r) === hostId)
)
const idStillPresent = this.state.repos.some((r) => r.id === id)
// Why: presets are repo-id-scoped (not host-scoped); drop them only when the last host's copy is gone.
// Why: presets and retirements are repo-id-scoped (not host-scoped); drop them only when the
// last host's copy is gone.
if (!idStillPresent) {
delete this.state.sparsePresetsByRepo[id]
delete this.state.retiredWorktreeNamesByRepo?.[id]
}
this.syncProjectHostSetupCompatibilityState()
// Why: prune only this host's worktree metas if the id survives elsewhere; otherwise prune everything (matches removeProject).
@@ -7215,32 +7232,32 @@ export class Store {
this.scheduleSave()
}
getRetiredWorktreeNames(workspaceKey: string): string[] {
return [...(this.state.retiredWorktreeNamesByWorkspaceKey?.[workspaceKey] ?? [])]
getRetiredWorktreeNames(repoId: string): string[] {
return [...(this.state.retiredWorktreeNamesByRepo?.[repoId] ?? [])]
}
/** Records a generated workspace name as spent in this cwd namespace. Called with the name main
* actually used, not the one the renderer proposed — create can advance past it on collision. */
addRetiredWorktreeName(workspaceKey: string, name: string): void {
/** 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 {
const normalized = normalizeRetirableGeneratedName(name)
if (!workspaceKey || !normalized) {
if (!repoId || !normalized) {
return
}
this.state.retiredWorktreeNamesByWorkspaceKey ??= {}
const existing = this.state.retiredWorktreeNamesByWorkspaceKey[workspaceKey]
this.state.retiredWorktreeNamesByRepo ??= {}
const existing = this.state.retiredWorktreeNamesByRepo[repoId]
if (existing?.includes(normalized)) {
return
}
this.state.retiredWorktreeNamesByWorkspaceKey[workspaceKey] = existing
? [...existing, normalized]
: [normalized]
this.state.retiredWorktreeNamesByRepo[repoId] = boundRetiredWorktreeNames(
existing ? [...existing, normalized] : [normalized]
)
this.scheduleSave()
}
/** Seeds retirements discovered by the one-time backfill. Merges rather than replaces so a
* concurrent create during backfill is not lost. Returns true when anything was added. */
mergeRetiredWorktreeNames(workspaceKey: string, names: Iterable<string>): boolean {
if (!workspaceKey) {
mergeRetiredWorktreeNames(repoId: string, names: Iterable<string>): boolean {
if (!repoId) {
return false
}
const incoming = new Set<string>()
@@ -7253,8 +7270,8 @@ export class Store {
if (incoming.size === 0) {
return false
}
this.state.retiredWorktreeNamesByWorkspaceKey ??= {}
const existing = this.state.retiredWorktreeNamesByWorkspaceKey[workspaceKey] ?? []
this.state.retiredWorktreeNamesByRepo ??= {}
const existing = this.state.retiredWorktreeNamesByRepo[repoId] ?? []
const merged = new Set(existing)
for (const name of incoming) {
merged.add(name)
@@ -7262,7 +7279,7 @@ export class Store {
if (merged.size === existing.length) {
return false
}
this.state.retiredWorktreeNamesByWorkspaceKey[workspaceKey] = [...merged]
this.state.retiredWorktreeNamesByRepo[repoId] = boundRetiredWorktreeNames([...merged])
this.scheduleSave()
return true
}
+1 -1
View File
@@ -4092,7 +4092,7 @@ describe('OrcaRuntimeService', () => {
worktrees: expect.arrayContaining([expect.objectContaining({ path: createdWorktree.path })])
}
)
expect(addRetiredWorktreeName).toHaveBeenCalledWith(expect.any(String), 'nautilus-2')
expect(addRetiredWorktreeName).toHaveBeenCalledWith(TEST_REPO_ID, 'nautilus-2')
})
it('neither skips nor retires a name the user typed', async () => {
+4 -5
View File
@@ -936,8 +936,7 @@ import {
} from '../worktree-create-candidates'
import {
ensureRetiredWorktreeNamesBackfilled,
getRetiredWorktreeNamesForRepo,
retireGeneratedWorktreeName
getRetiredWorktreeNamesForRepo
} from '../worktree-name-retirement'
import { normalizeSparseDirectories } from '../ipc/sparse-checkout-directories'
import type { PtyBindingSourceExpectation, Store } from '../persistence'
@@ -21180,7 +21179,7 @@ export class OrcaRuntimeService {
} catch (error) {
console.warn(`[runtime] retirement backfill failed for repo ${repo.id}:`, error)
}
return { [repo.id]: getRetiredWorktreeNamesForRepo(store, repo, settings) }
return { [repo.id]: getRetiredWorktreeNamesForRepo(store, repo, store.getRepos(), settings) }
}
async listDetectedManagedWorktrees(
@@ -22374,7 +22373,7 @@ export class OrcaRuntimeService {
let worktreePath = ''
let worktreePathResolved = false
const retiredNames = args.nameWasGenerated
? new Set(getRetiredWorktreeNamesForRepo(this.store, repo, settings))
? new Set(getRetiredWorktreeNamesForRepo(this.store, repo, this.store.getRepos(), settings))
: 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.
@@ -22707,7 +22706,7 @@ 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) {
retireGeneratedWorktreeName(this.store, repo, settings, effectiveSanitizedName)
this.store.addRetiredWorktreeName(repo.id, effectiveSanitizedName)
}
const now = Date.now()
// Why: PR/MR-created worktrees can start from a head ref/SHA while Source
+59 -53
View File
@@ -11,8 +11,7 @@ import {
extractBucketLeafCandidates,
getRetiredWorktreeNamesForRepo,
normalizeRetirableGeneratedName,
resetRetirementCollisionKeyCacheForTests,
retireGeneratedWorktreeName
resetRetirementCollisionKeyCacheForTests
} from './worktree-name-retirement'
const FIRST = MARINE_CREATURES[0].toLowerCase()
@@ -201,62 +200,67 @@ describe('discoverRetiredWorktreeNames', () => {
describe('getRetiredWorktreeNamesForRepo', () => {
const settingsFor = (nestWorkspaces: boolean): GlobalSettings =>
({ workspaceDir: '/workspaces', nestWorkspaces }) as GlobalSettings
const storeOf = (byRepo: Record<string, string[]>) => {
const calls: string[] = []
return {
calls,
getRetiredWorktreeNames: (repoId: string) => {
calls.push(repoId)
return byRepo[repoId] ?? []
}
}
}
it('shares retirements when two repos create into the same cwd namespace', () => {
const byKey = new Map<string, string[]>()
const store = {
getRetiredWorktreeNames: (key: string) => byKey.get(key) ?? [],
addRetiredWorktreeName: (key: string, name: string) =>
byKey.set(key, [...(byKey.get(key) ?? []), name])
}
const settings = settingsFor(false)
const repoA = makeRepo('repo-a', '/repos/a')
const repoB = makeRepo('repo-b', '/repos/b')
const store = storeOf({ 'repo-a': [FIRST], 'repo-b': [SECOND] })
const repos = [makeRepo('repo-a', '/repos/a'), makeRepo('repo-b', '/repos/b')]
retireGeneratedWorktreeName(store, repoA, settings, FIRST)
expect(getRetiredWorktreeNamesForRepo(store, repoB, settings)).toEqual([FIRST])
expect(
getRetiredWorktreeNamesForRepo(store, repos[1], repos, settingsFor(false)).sort()
).toEqual([FIRST, SECOND].sort())
})
it('keeps independent nested repo paths in separate retirement domains', () => {
const byKey = new Map<string, string[]>()
const store = {
getRetiredWorktreeNames: (key: string) => byKey.get(key) ?? [],
addRetiredWorktreeName: (key: string, name: string) =>
byKey.set(key, [...(byKey.get(key) ?? []), name])
}
const settings = settingsFor(true)
const repoA = makeRepo('repo-a', '/repos/a')
const repoB = makeRepo('repo-b', '/repos/b')
const store = storeOf({ 'repo-a': [FIRST], 'repo-b': [SECOND] })
const repos = [makeRepo('repo-a', '/repos/a'), makeRepo('repo-b', '/repos/b')]
retireGeneratedWorktreeName(store, repoA, settings, FIRST)
expect(getRetiredWorktreeNamesForRepo(store, repoB, settings)).toEqual([])
expect(getRetiredWorktreeNamesForRepo(store, repoA, settings)).toEqual([FIRST])
expect(getRetiredWorktreeNamesForRepo(store, repos[1], repos, settingsFor(true))).toEqual([
SECOND
])
})
it('survives a repo being removed and re-added under a new id', () => {
// Why: repo ids are regenerated on re-add. Keying the registry by repo id silently dropped
// every retirement for a path that had not changed at all.
const byKey = new Map<string, string[]>()
const store = {
getRetiredWorktreeNames: (key: string) => byKey.get(key) ?? [],
addRetiredWorktreeName: (key: string, name: string) =>
byKey.set(key, [...(byKey.get(key) ?? []), name])
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.
const pathReads: string[] = []
const spyRepo = (id: string, path: string): Repo => {
const repo = makeRepo(id, path)
return Object.defineProperty(repo, 'path', {
get: () => {
pathReads.push(id)
return path
}
}) as Repo
}
const settings = settingsFor(true)
const store = storeOf({ 'repo-a': [FIRST] })
const repos = [
spyRepo('repo-a', '/repos/a'),
...Array.from({ length: 20 }, (_unused, index) => spyRepo(`peer-${index}`, `/repos/${index}`))
]
retireGeneratedWorktreeName(store, makeRepo('repo-old', '/repos/a'), settings, FIRST)
expect(
getRetiredWorktreeNamesForRepo(store, makeRepo('repo-new', '/repos/a'), settings)
).toEqual([FIRST])
expect(getRetiredWorktreeNamesForRepo(store, repos[0], repos, settingsFor(false))).toEqual([
FIRST
])
expect(pathReads.filter((id) => id.startsWith('peer-'))).toEqual([])
})
it('reports nothing for a folder workspace, which has no generated worktree names', () => {
const store = { getRetiredWorktreeNames: () => [FIRST] }
const store = storeOf({ folder: [FIRST] })
const folderRepo = { ...makeRepo('folder', '/repos/folder'), kind: 'folder' as const }
expect(getRetiredWorktreeNamesForRepo(store, folderRepo, settingsFor(false))).toEqual([])
expect(
getRetiredWorktreeNamesForRepo(store, folderRepo, [folderRepo], settingsFor(false))
).toEqual([])
})
})
@@ -265,24 +269,26 @@ describe('ensureRetiredWorktreeNamesBackfilled', () => {
const root = await mkdtemp(join(tmpdir(), 'orca-retirement-backfill-'))
const workspaceRoot = join(root, 'workspaces')
await mkdir(join(workspaceRoot, FIRST), { recursive: true })
const merged: { key: string; names: string[] }[] = []
const merged: { repoId: string; names: string[] }[] = []
const store = {
mergeRetiredWorktreeNames: (key: string, names: Iterable<string>) => {
merged.push({ key, names: [...names] })
mergeRetiredWorktreeNames: (repoId: string, names: Iterable<string>) => {
merged.push({ repoId, names: [...names] })
return true
}
}
const repo = makeRepo('repo-a', join(root, 'repos', 'a'))
const settings = { workspaceDir: workspaceRoot, nestWorkspaces: false }
try {
await ensureRetiredWorktreeNamesBackfilled(store, repo, settings)
expect(merged).toHaveLength(1)
expect(merged[0].names).toContain(FIRST)
await ensureRetiredWorktreeNamesBackfilled(store, makeRepo('repo-a', '/repos/a'), settings)
expect(merged).toEqual([{ repoId: 'repo-a', names: [FIRST] }])
// A second repo in the same namespace reuses the one in-flight scan.
// Why: the scan is cached per cwd namespace but the registry is per repo, so a second repo in
// the same namespace must still receive the merge rather than silently inheriting nothing.
await ensureRetiredWorktreeNamesBackfilled(store, makeRepo('repo-b', '/repos/b'), settings)
expect(merged).toHaveLength(1)
expect(merged).toEqual([
{ repoId: 'repo-a', names: [FIRST] },
{ repoId: 'repo-b', names: [FIRST] }
])
} finally {
await rm(root, { force: true, recursive: true })
}
@@ -291,7 +297,7 @@ describe('ensureRetiredWorktreeNamesBackfilled', () => {
it('skips repos whose agent state lives on another host', async () => {
const merged: string[] = []
const store = {
mergeRetiredWorktreeNames: (_key: string, names: Iterable<string>) => {
mergeRetiredWorktreeNames: (_repoId: string, names: Iterable<string>) => {
merged.push(...names)
return true
}
+44 -34
View File
@@ -18,16 +18,13 @@ import {
import { worktreePathComparisonKey } from './ipc/worktree-path-comparison'
const RETIREMENT_PROBE_NAME = 'orca-retirement-probe'
const backfillsByStore = new WeakMap<object, Map<string, Promise<void>>>()
const scansByStore = new WeakMap<object, Map<string, Promise<Set<string>>>>()
type RetirementReadStore = {
getRetiredWorktreeNames(collisionKey: string): string[]
}
type RetirementWriteStore = {
addRetiredWorktreeName(collisionKey: string, name: string): void
getRetiredWorktreeNames(repoId: string): string[]
}
type RetirementBackfillStore = {
mergeRetiredWorktreeNames(collisionKey: string, names: Iterable<string>): boolean
mergeRetiredWorktreeNames(repoId: string, names: Iterable<string>): boolean
}
type RetirementPathSettings = Pick<GlobalSettings, 'nestWorkspaces' | 'workspaceDir'>
@@ -121,29 +118,40 @@ export function getRetirementCollisionKey(repo: Repo, settings: RetirementPathSe
return key
}
/** Stored per repo id, which is stable, but read across every repo that creates into the same cwd
* 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 getRetiredWorktreeNamesForRepo(
store: RetirementReadStore,
repo: Repo,
repos: readonly Repo[],
settings: RetirementPathSettings
): string[] {
if (isFolderRepo(repo)) {
return []
}
return store.getRetiredWorktreeNames(getRetirementCollisionKey(repo, settings))
}
/** Records the name main actually used, not the one the client proposed — the create path can
* advance past a taken candidate. Call only for names Orca generated. */
export function retireGeneratedWorktreeName(
store: RetirementWriteStore,
repo: Repo,
settings: RetirementPathSettings,
name: string
): void {
if (isFolderRepo(repo)) {
return
const names = new Set(store.getRetiredWorktreeNames(repo.id))
let collisionKey: string | null = null
for (const candidate of repos) {
if (candidate.id === repo.id || isFolderRepo(candidate)) {
continue
}
const candidateNames = store.getRetiredWorktreeNames(candidate.id)
if (candidateNames.length === 0) {
continue
}
collisionKey ??= getRetirementCollisionKey(repo, settings)
if (getRetirementCollisionKey(candidate, settings) !== collisionKey) {
continue
}
for (const name of candidateNames) {
names.add(name)
}
}
store.addRetiredWorktreeName(getRetirementCollisionKey(repo, settings), name)
return [...names]
}
function parentPath(path: string): string {
@@ -157,27 +165,29 @@ export function ensureRetiredWorktreeNamesBackfilled(
repo: Repo,
settings: RetirementPathSettings
): Promise<void> {
// Remote workspaces keep their agent state on the execution host, which this scan cannot see.
// 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 Promise.resolve()
}
let storeBackfills = backfillsByStore.get(store)
if (!storeBackfills) {
storeBackfills = new Map()
backfillsByStore.set(store, storeBackfills)
let storeScans = scansByStore.get(store)
if (!storeScans) {
storeScans = new Map()
scansByStore.set(store, storeScans)
}
const collisionKey = getRetirementCollisionKey(repo, settings)
const existing = storeBackfills.get(collisionKey)
if (existing) {
return existing
let scan = storeScans.get(collisionKey)
if (!scan) {
scan = discoverRetiredWorktreeNames({
workspaceRoots: [parentPath(getRetirementProbePath(repo, settings))]
})
storeScans.set(collisionKey, scan)
}
const backfill = discoverRetiredWorktreeNames({
workspaceRoots: [parentPath(getRetirementProbePath(repo, settings))]
}).then((names) => {
store.mergeRetiredWorktreeNames(collisionKey, names)
// Why the merge sits outside the cached promise: 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.
return scan.then((names) => {
store.mergeRetiredWorktreeNames(repo.id, names)
})
storeBackfills.set(collisionKey, backfill)
return backfill
}
async function readDirectoryNames(path: string): Promise<string[]> {
+1 -1
View File
@@ -431,7 +431,7 @@ export function getDefaultPersistedState(homedir: string): PersistedState {
projectGroups: [],
folderWorkspaces: [],
sparsePresetsByRepo: {},
retiredWorktreeNamesByWorkspaceKey: {},
retiredWorktreeNamesByRepo: {},
worktreeMeta: {},
worktreeLineageById: {},
workspaceLineageByChildKey: {},
+7 -6
View File
@@ -3742,12 +3742,13 @@ export type PersistedState = {
folderWorkspaceDiffComments?: Record<string, DiffComment[]>
/** Sparse-checkout presets keyed by repoId. */
sparsePresetsByRepo: Record<string, SparsePreset[]>
/** Generated workspace names already issued, keyed by the cwd namespace repos create into
* (execution host + workspace-root path), not by repoId — the collision this prevents is on the
* path, and repo ids change on remove/re-add while the path does not. 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; entries are never removed on workspace deletion. */
retiredWorktreeNamesByWorkspaceKey?: Record<string, string[]>
/** 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. */
retiredWorktreeNamesByRepo?: Record<string, string[]>
/** Per paired device last tab selection by worktree; keeps mobile navigation across host restarts. */
mobileClientTabSelectionsByDeviceId?: PersistedMobileClientTabSelections
worktreeMeta: Record<string, WorktreeMeta>