fix(worktree): hold retired names across a refresh instead of blanking

refreshKey changes on every workspace-list mutation, so create-multiple
refetches after each create and the hook returned an empty list until the
refetch landed — precisely the window in which resetForNextCreate clears the
name field and a fresh suggestion is drawn. Keep the previous answer while
revalidating and reset only when the repo changes; a failed refresh keeps what
was already loaded rather than un-retiring everything.

Also makes the returned array referentially stable, so the suggestion memo
downstream stops rerunning on every refetch.
This commit is contained in:
Brennan Benson
2026-08-13 18:24:41 -07:00
parent 348bf5d472
commit 58e8c7d481
2 changed files with 102 additions and 5 deletions
@@ -0,0 +1,85 @@
// @vitest-environment happy-dom
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { renderHook, waitFor } from '@testing-library/react'
import { useRetiredWorktreeNames } from './useRetiredWorktreeNames'
const listRetiredNames = vi.fn()
beforeEach(() => {
listRetiredNames.mockReset()
Object.assign(window, { api: { worktrees: { listRetiredNames } } })
})
describe('useRetiredWorktreeNames', () => {
it('keeps the previous names while a refresh is in flight', async () => {
// Why: refreshKey changes on every workspace-list mutation, so create-multiple refetches after
// each create. Dropping to empty in between would suggest a spent name in exactly that window.
listRetiredNames.mockResolvedValueOnce(['nautilus'])
const { result, rerender } = renderHook(
({ key }: { key: string }) => useRetiredWorktreeNames('repo-1', key),
{ initialProps: { key: 'a' } }
)
await waitFor(() => expect(result.current.names).toEqual(['nautilus']))
let resolveSecond: (names: string[]) => void = () => {}
listRetiredNames.mockReturnValueOnce(
new Promise<string[]>((resolve) => {
resolveSecond = resolve
})
)
rerender({ key: 'b' })
expect(result.current.names).toEqual(['nautilus'])
expect(result.current.loading).toBe(true)
resolveSecond(['nautilus', 'seahorse'])
await waitFor(() => expect(result.current.names).toEqual(['nautilus', 'seahorse']))
})
it('returns a referentially stable array across refreshes that change nothing', async () => {
// The suggestion memo downstream keys on this array; a new identity per refetch reruns it.
const names = ['nautilus']
listRetiredNames.mockResolvedValue(names)
const { result, rerender } = renderHook(
({ key }: { key: string }) => useRetiredWorktreeNames('repo-1', key),
{ initialProps: { key: 'a' } }
)
await waitFor(() => expect(result.current.names).toEqual(['nautilus']))
const first = result.current.names
rerender({ key: 'b' })
expect(result.current.names).toBe(first)
})
it('drops names when the repo changes rather than showing another repo pool', async () => {
listRetiredNames.mockResolvedValueOnce(['nautilus'])
const { result, rerender } = renderHook(
({ repoId }: { repoId: string }) => useRetiredWorktreeNames(repoId, 'key'),
{ initialProps: { repoId: 'repo-1' } }
)
await waitFor(() => expect(result.current.names).toEqual(['nautilus']))
listRetiredNames.mockReturnValueOnce(new Promise<string[]>(() => {}))
rerender({ repoId: 'repo-2' })
expect(result.current.names).toEqual([])
})
it('keeps previously loaded names when a refresh fails', async () => {
listRetiredNames.mockResolvedValueOnce(['nautilus'])
const { result, rerender } = renderHook(
({ key }: { key: string }) => useRetiredWorktreeNames('repo-1', key),
{ initialProps: { key: 'a' } }
)
await waitFor(() => expect(result.current.names).toEqual(['nautilus']))
vi.spyOn(console, 'warn').mockImplementation(() => {})
listRetiredNames.mockRejectedValueOnce(new Error('host unreachable'))
rerender({ key: 'b' })
await waitFor(() => expect(result.current.loading).toBe(false))
expect(result.current.names).toEqual(['nautilus'])
})
})
@@ -1,24 +1,32 @@
import { useEffect, useState } from 'react'
const NO_RETIRED_NAMES: readonly string[] = []
/** Names already spent in a repo, including workspaces that have since been deleted.
*
* Why this is fetched rather than derived from the worktree list: a deleted workspace leaves no
* row behind, but its directory may still hold agent conversation state keyed by that path. Only
* the host-side registry remembers it, so name suggestions must ask for it explicitly.
*
* Stale-while-revalidate across `refreshKey`, reset only when the repo changes. A refresh fires on
* every workspace-list mutation, so create-multiple refetches after each create — dropping to
* empty in between would suggest a spent name in exactly that window, which is when
* `resetForNextCreate` clears the field. Holding the previous answer also keeps the returned array
* referentially stable, so the suggestion memo downstream does not rerun on every refetch.
*
* Failure returns an empty set, which degrades to the pre-existing behavior (a spent name can be
* suggested) rather than blocking workspace creation. */
export function useRetiredWorktreeNames(
repoId: string | null | undefined,
refreshKey: unknown
): {
names: string[]
names: readonly string[]
loading: boolean
} {
const [loaded, setLoaded] = useState<{
repoId: string
refreshKey: unknown
names: string[]
names: readonly string[]
} | null>(null)
useEffect(() => {
@@ -36,7 +44,12 @@ export function useRetiredWorktreeNames(
})
.catch((err) => {
if (!cancelled) {
setLoaded({ repoId, refreshKey, names: [] })
// Keep whatever a previous refresh loaded; a transient failure must not un-retire names.
setLoaded((previous) => ({
repoId,
refreshKey,
names: previous?.repoId === repoId ? previous.names : NO_RETIRED_NAMES
}))
}
console.warn(`Failed to load retired workspace names for repo ${repoId}:`, err)
})
@@ -46,8 +59,7 @@ export function useRetiredWorktreeNames(
}, [refreshKey, repoId])
return {
names:
loaded && loaded.repoId === repoId && loaded.refreshKey === refreshKey ? loaded.names : [],
names: loaded && loaded.repoId === repoId ? loaded.names : NO_RETIRED_NAMES,
loading: Boolean(repoId) && (loaded?.repoId !== repoId || loaded?.refreshKey !== refreshKey)
}
}