diff --git a/mobile/src/components/NewWorktreeModal.tsx b/mobile/src/components/NewWorktreeModal.tsx index 0da42f35f21..f10a96f17c0 100644 --- a/mobile/src/components/NewWorktreeModal.tsx +++ b/mobile/src/components/NewWorktreeModal.tsx @@ -189,8 +189,16 @@ function NewWorktreeModalContent({ const [selectedRepo, setSelectedRepo] = useState(null) // Why: a deleted workspace's directory can still hold agent conversation state keyed by cwd, so // its name must never be suggested again. Fetched per selected repo while the sheet is open. - // Create is never gated on it: the host skips retired candidates before doing any git work. - const { names: retiredWorktreeNames } = useRetiredWorktreeNames(client, selectedRepo?.id) + // Keyed on the path set rather than the array so a poll that changes nothing does not refetch. + const retiredNamesRefreshKey = useMemo( + () => [...(existingWorktreePaths ?? [])].sort().join('\0'), + [existingWorktreePaths] + ) + const retiredWorktreeNames = useRetiredWorktreeNames( + client, + selectedRepo?.id, + retiredNamesRefreshKey + ) const { drawerView, formSheetVisible, formSheetInteractive, transitionDrawer, openSourceDrawer } = useNewWorktreeDrawerNavigation(visible) const createInFlightRef = useRef(false) @@ -662,6 +670,8 @@ function NewWorktreeModalContent({ client, repoId: selectedRepo.id, baseName, + // `baseName` is the suggestion exactly when the user typed nothing, so no identity check + // is needed — desktop's seeded composer needs one (useComposerState `nameWasGenerated`). nameWasGenerated: !trimmedName, createdWithAgentId, comment: trimmedNote, diff --git a/mobile/src/worktree/use-retired-worktree-names.test.tsx b/mobile/src/worktree/use-retired-worktree-names.test.tsx new file mode 100644 index 00000000000..34fbe9a6506 --- /dev/null +++ b/mobile/src/worktree/use-retired-worktree-names.test.tsx @@ -0,0 +1,116 @@ +import { createElement } from 'react' +import { act, create } from 'react-test-renderer' +import { describe, expect, it } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import { useRetiredWorktreeNames } from './use-retired-worktree-names' + +type Pending = { resolve: (response: unknown) => void; reject: (err: Error) => void } + +/** Renders the hook against a deferred RPC client so a test decides exactly when each + * `worktree.listRetiredNames` lands, and for which repo. */ +function mountNames() { + const pending: Pending[] = [] + const requests: { method: string; params: unknown }[] = [] + let latest: readonly string[] = [] + + const client = { + sendRequest: (method: string, params: unknown) => { + requests.push({ method, params }) + return new Promise((resolve, reject) => pending.push({ resolve, reject })) + } + } as unknown as RpcClient + + function Probe({ repoId, refreshKey }: { repoId: string | null; refreshKey: string }) { + latest = useRetiredWorktreeNames(client, repoId, refreshKey) + return null + } + + let renderer!: ReturnType + act(() => { + renderer = create(createElement(Probe, { repoId: 'repo-1', refreshKey: 'a' })) + }) + return { + get names(): readonly string[] { + return latest + }, + requests, + rerender(props: { repoId: string | null; refreshKey: string }) { + act(() => { + renderer.update(createElement(Probe, props)) + }) + }, + async settle(index: number, retiredNamesByRepo: Record) { + await act(async () => { + pending[index]!.resolve({ result: { retiredNamesByRepo } }) + await Promise.resolve() + }) + }, + async fail(index: number) { + await act(async () => { + pending[index]!.reject(new Error('host unreachable')) + await Promise.resolve() + }) + } + } +} + +describe('useRetiredWorktreeNames', () => { + it('reads the selected repo out of the response envelope', async () => { + const probe = mountNames() + expect(probe.requests[0]).toEqual({ + method: 'worktree.listRetiredNames', + params: { repo: 'id:repo-1' } + }) + await probe.settle(0, { 'repo-1': ['nautilus'], 'repo-2': ['seahorse'] }) + expect(probe.names).toEqual(['nautilus']) + }) + + it('answers empty for a host that omits the field', async () => { + const probe = mountNames() + await probe.settle(0, {}) + expect(probe.names).toEqual([]) + }) + + // Regression: this hook used to reset to [] on any error, un-retiring every name for the rest of + // the sheet session. Desktop always held the previous answer; both now share that rule. + it('keeps previously loaded names when a refresh fails', async () => { + const probe = mountNames() + await probe.settle(0, { 'repo-1': ['nautilus'] }) + expect(probe.names).toEqual(['nautilus']) + + probe.rerender({ repoId: 'repo-1', refreshKey: 'b' }) + await probe.fail(1) + + expect(probe.names).toEqual(['nautilus']) + }) + + // Regression: the effect used to depend on [client, repoId] only, so a workspace created while + // the sheet was open never made it into the pool. + it('refetches when the workspace list changes', async () => { + const probe = mountNames() + await probe.settle(0, { 'repo-1': ['nautilus'] }) + + probe.rerender({ repoId: 'repo-1', refreshKey: 'b' }) + expect(probe.requests).toHaveLength(2) + // The previous answer stays visible while the refetch is in flight. + expect(probe.names).toEqual(['nautilus']) + + await probe.settle(1, { 'repo-1': ['nautilus', 'seahorse'] }) + expect(probe.names).toEqual(['nautilus', 'seahorse']) + }) + + it('drops names when the repo changes rather than showing another repo pool', async () => { + const probe = mountNames() + await probe.settle(0, { 'repo-1': ['nautilus'] }) + + probe.rerender({ repoId: 'repo-2', refreshKey: 'a' }) + expect(probe.names).toEqual([]) + }) + + it('reports nothing without a selected repo, and asks the host nothing', () => { + const probe = mountNames() + probe.rerender({ repoId: null, refreshKey: 'a' }) + expect(probe.names).toEqual([]) + expect(probe.requests).toHaveLength(1) + }) +}) diff --git a/mobile/src/worktree/use-retired-worktree-names.ts b/mobile/src/worktree/use-retired-worktree-names.ts index a641b3f9d4b..ee3872a6948 100644 --- a/mobile/src/worktree/use-retired-worktree-names.ts +++ b/mobile/src/worktree/use-retired-worktree-names.ts @@ -1,57 +1,50 @@ import { useEffect, useState } from 'react' +import { + readRetiredNamesForRepo, + retiredNamesAfterRefresh, + selectRetiredNames, + type RetiredNamesLoad +} from '../../../src/shared/worktree/retired-name-cache' import type { RpcClient } from '../transport/rpc-client' /** Names already spent in a repo, including workspaces that have since been deleted. * * Why a targeted request rather than the workspace catalog: the catalog is served by `worktree.ps`, * which carries rows only. Retired names are needed just while the create sheet is open and only - * for one repo, so this mirrors the desktop hook and asks for exactly that. + * for one repo, so this asks for exactly that. * - * Hosts predating the field omit it, and any failure yields an empty list, so this degrades to - * deduping against live workspaces alone — the behavior before retirement existed. */ + * `refreshKey` must change on every workspace-list mutation. Caching rules live in + * `retired-name-cache` so this and the desktop hook cannot drift on what a failure means, and no + * loading state is reported because create is never gated on this fetch. */ export function useRetiredWorktreeNames( client: RpcClient | null | undefined, - repoId: string | null | undefined -): { names: readonly string[]; loading: boolean } { - const [loaded, setLoaded] = useState<{ - repoId: string - names: readonly string[] - } | null>(null) + repoId: string | null | undefined, + refreshKey: unknown +): readonly string[] { + const [loaded, setLoaded] = useState(null) + const activeRepoId = client && repoId ? repoId : null useEffect(() => { - if (!client || !repoId) { + if (!client || !activeRepoId) { setLoaded(null) return } let cancelled = false + const settle = (names: readonly string[] | null): void => { + if (!cancelled) { + setLoaded((previous) => retiredNamesAfterRefresh(previous, activeRepoId, names)) + } + } void client - .sendRequest('worktree.listRetiredNames', { repo: `id:${repoId}` }) - .then((response) => { - if (cancelled) { - return - } - const result = (response as { result?: unknown }).result as - | { retiredNamesByRepo?: Record } - | undefined - const names = result?.retiredNamesByRepo?.[repoId] - setLoaded({ - repoId, - // A host can answer with a malformed row; a non-string would throw when normalized. - names: Array.isArray(names) ? names.filter((name) => typeof name === 'string') : [] - }) - }) - .catch(() => { - if (!cancelled) { - setLoaded({ repoId, names: [] }) - } - }) + .sendRequest('worktree.listRetiredNames', { repo: `id:${activeRepoId}` }) + .then((response) => + settle(readRetiredNamesForRepo((response as { result?: unknown }).result, activeRepoId)) + ) + .catch(() => settle(null)) return () => { cancelled = true } - }, [client, repoId]) + }, [activeRepoId, client, refreshKey]) - return { - names: loaded && loaded.repoId === repoId ? loaded.names : [], - loading: Boolean(client && repoId) && loaded?.repoId !== repoId - } + return selectRetiredNames(loaded, activeRepoId) } diff --git a/src/renderer/src/components/sidebar/worktree-name-suggestions.ts b/src/renderer/src/components/sidebar/worktree-name-suggestions.ts index bf1ebebe5fa..64827392bb7 100644 --- a/src/renderer/src/components/sidebar/worktree-name-suggestions.ts +++ b/src/renderer/src/components/sidebar/worktree-name-suggestions.ts @@ -1,7 +1,7 @@ -import { basename } from '@/lib/path' import { normalizeSuggestedName, - selectSuggestedCreatureName + selectSuggestedCreatureName, + suggestionPathBasename } from '../../../../shared/worktree-name-suggestion' type WorktreePathLike = { @@ -14,7 +14,9 @@ function collectUsedNames(worktreesByRepo: Record): const usedNames = new Set() for (const worktrees of Object.values(worktreesByRepo)) { for (const worktree of worktrees) { - usedNames.add(normalizeSuggestedName(basename(worktree.path))) + // Shared basename, not `@/lib/path`'s: mobile keys collisions the same way, and two copies of + // the key could drift into suggesting a name that is in fact taken. + usedNames.add(normalizeSuggestedName(suggestionPathBasename(worktree.path))) } } return usedNames diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index 8f9f56b5573..8356f379a84 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -1601,9 +1601,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS .join('\0'), [repoId, worktreesByRepo] ) - // Create is never gated on this load: the host skips retired candidates before any git work, so - // a pending fetch can only cost a suggestion, never correctness. - const { names: retiredWorktreeNames } = useRetiredWorktreeNames(repoId, retiredNamesRefreshKey) + const retiredWorktreeNames = useRetiredWorktreeNames(repoId, retiredNamesRefreshKey) const fallbackCreatureName = useMemo( () => getSuggestedCreatureName(worktreesByRepo, undefined, retiredWorktreeNames), [worktreesByRepo, retiredWorktreeNames] @@ -3650,6 +3648,9 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS } // Why: only a name Orca generated may be retired — the creature pool contains ordinary words // ("orca", "runner", "molly") a user can type deliberately and expect to reuse. + // The identity check is what a linked PR/issue seed makes necessary here; mobile's blank-create + // path (NewWorktreeModal, `nameWasGenerated: !trimmedName`) has no other seed, so it can't + // share this expression. Same rule, two submit paths — change both together. const nameWasGenerated = !name.trim() && workspaceName === fallbackCreatureName const submitBaseBranch = smartGitHubResolution.kind === 'pr-start-point' diff --git a/src/renderer/src/hooks/useRetiredWorktreeNames.test.ts b/src/renderer/src/hooks/useRetiredWorktreeNames.test.ts index 452771caf14..2c795a7c536 100644 --- a/src/renderer/src/hooks/useRetiredWorktreeNames.test.ts +++ b/src/renderer/src/hooks/useRetiredWorktreeNames.test.ts @@ -20,7 +20,7 @@ describe('useRetiredWorktreeNames', () => { ({ key }: { key: string }) => useRetiredWorktreeNames('repo-1', key), { initialProps: { key: 'a' } } ) - await waitFor(() => expect(result.current.names).toEqual(['nautilus'])) + await waitFor(() => expect(result.current).toEqual(['nautilus'])) let resolveSecond: (names: string[]) => void = () => {} listRetiredNames.mockReturnValueOnce( @@ -30,11 +30,10 @@ describe('useRetiredWorktreeNames', () => { ) rerender({ key: 'b' }) - expect(result.current.names).toEqual(['nautilus']) - expect(result.current.loading).toBe(true) + expect(result.current).toEqual(['nautilus']) resolveSecond(['nautilus', 'seahorse']) - await waitFor(() => expect(result.current.names).toEqual(['nautilus', 'seahorse'])) + await waitFor(() => expect(result.current).toEqual(['nautilus', 'seahorse'])) }) it('returns a referentially stable array across refreshes that change nothing', async () => { @@ -45,12 +44,12 @@ describe('useRetiredWorktreeNames', () => { ({ key }: { key: string }) => useRetiredWorktreeNames('repo-1', key), { initialProps: { key: 'a' } } ) - await waitFor(() => expect(result.current.names).toEqual(['nautilus'])) - const first = result.current.names + await waitFor(() => expect(result.current).toEqual(['nautilus'])) + const first = result.current rerender({ key: 'b' }) - expect(result.current.names).toBe(first) + expect(result.current).toBe(first) }) it('drops names when the repo changes rather than showing another repo pool', async () => { @@ -59,12 +58,12 @@ describe('useRetiredWorktreeNames', () => { ({ repoId }: { repoId: string }) => useRetiredWorktreeNames(repoId, 'key'), { initialProps: { repoId: 'repo-1' } } ) - await waitFor(() => expect(result.current.names).toEqual(['nautilus'])) + await waitFor(() => expect(result.current).toEqual(['nautilus'])) listRetiredNames.mockReturnValueOnce(new Promise(() => {})) rerender({ repoId: 'repo-2' }) - expect(result.current.names).toEqual([]) + expect(result.current).toEqual([]) }) it('keeps previously loaded names when a refresh fails', async () => { @@ -73,13 +72,25 @@ describe('useRetiredWorktreeNames', () => { ({ key }: { key: string }) => useRetiredWorktreeNames('repo-1', key), { initialProps: { key: 'a' } } ) - await waitFor(() => expect(result.current.names).toEqual(['nautilus'])) + await waitFor(() => expect(result.current).toEqual(['nautilus'])) - vi.spyOn(console, 'warn').mockImplementation(() => {}) + const warn = 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']) + await waitFor(() => expect(warn).toHaveBeenCalled()) + expect(result.current).toEqual(['nautilus']) + }) + + it('refetches when the refresh key changes', async () => { + listRetiredNames.mockResolvedValue(['nautilus']) + const { rerender } = renderHook( + ({ key }: { key: string }) => useRetiredWorktreeNames('repo-1', key), + { initialProps: { key: 'a' } } + ) + await waitFor(() => expect(listRetiredNames).toHaveBeenCalledTimes(1)) + + rerender({ key: 'b' }) + await waitFor(() => expect(listRetiredNames).toHaveBeenCalledTimes(2)) }) }) diff --git a/src/renderer/src/hooks/useRetiredWorktreeNames.ts b/src/renderer/src/hooks/useRetiredWorktreeNames.ts index f68c5c91e1b..c4a7e948842 100644 --- a/src/renderer/src/hooks/useRetiredWorktreeNames.ts +++ b/src/renderer/src/hooks/useRetiredWorktreeNames.ts @@ -1,6 +1,9 @@ import { useEffect, useState } from 'react' - -const NO_RETIRED_NAMES: readonly string[] = [] +import { + retiredNamesAfterRefresh, + selectRetiredNames, + type RetiredNamesLoad +} from '../../../shared/worktree/retired-name-cache' /** Names already spent in a repo, including workspaces that have since been deleted. * @@ -8,26 +11,17 @@ const NO_RETIRED_NAMES: readonly string[] = [] * 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. + * `refreshKey` must change on every workspace-list mutation, so create-multiple refetches after + * each create. Caching rules live in `retired-name-cache` because the mobile hook obeys the same + * ones over a different transport. * - * Failure returns an empty set, which degrades to the pre-existing behavior (a spent name can be - * suggested) rather than blocking workspace creation. */ + * Deliberately reports no loading state: create is never gated on this fetch — the host skips + * retired candidates before any git work, so a pending fetch can only cost a suggestion. */ export function useRetiredWorktreeNames( repoId: string | null | undefined, refreshKey: unknown -): { - names: readonly string[] - loading: boolean -} { - const [loaded, setLoaded] = useState<{ - repoId: string - refreshKey: unknown - names: readonly string[] - } | null>(null) +): readonly string[] { + const [loaded, setLoaded] = useState(null) useEffect(() => { if (!repoId) { @@ -35,22 +29,16 @@ export function useRetiredWorktreeNames( return } let cancelled = false + const settle = (names: readonly string[] | null): void => { + if (!cancelled) { + setLoaded((previous) => retiredNamesAfterRefresh(previous, repoId, names)) + } + } void window.api.worktrees .listRetiredNames({ repoId }) - .then((names) => { - if (!cancelled) { - setLoaded({ repoId, refreshKey, names }) - } - }) + .then(settle) .catch((err) => { - if (!cancelled) { - // 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 - })) - } + settle(null) console.warn(`Failed to load retired workspace names for repo ${repoId}:`, err) }) return () => { @@ -58,8 +46,5 @@ export function useRetiredWorktreeNames( } }, [refreshKey, repoId]) - return { - names: loaded && loaded.repoId === repoId ? loaded.names : NO_RETIRED_NAMES, - loading: Boolean(repoId) && (loaded?.repoId !== repoId || loaded?.refreshKey !== refreshKey) - } + return selectRetiredNames(loaded, repoId) } diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 8bd5e98c7b6..2b243aafac3 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -63,6 +63,7 @@ import { legacyBaseRefSearchResult } from '../../../shared/base-ref-search-resul import { EMPTY_PTY_MAIN_DELIVERY_DIAGNOSTICS } from '../../../shared/pty-delivery-diagnostics' import { createE2EConfig } from '../../../shared/e2e-config' import { relativePathInsideRoot } from '../../../shared/cross-platform-path' +import { readRetiredNamesForRepo } from '../../../shared/worktree/retired-name-cache' import { applyPRBotAuthorOverride, normalizePRBotAuthorOverrides @@ -1776,12 +1777,10 @@ function createWorktreesApi(): NonNullable['worktrees']> { // degrades the suggestion to the pre-existing behavior rather than blocking workspace create. listRetiredNames: async ({ repoId }) => { try { - const result = await callRuntimeResult<{ retiredNamesByRepo?: Record }>( - 'worktree.listRetiredNames', - { repo: repoId } + return readRetiredNamesForRepo( + await callRuntimeResult('worktree.listRetiredNames', { repo: repoId }), + repoId ) - const names = result.retiredNamesByRepo?.[repoId] - return Array.isArray(names) ? names.filter((name) => typeof name === 'string') : [] } catch { return [] } diff --git a/src/shared/worktree/retired-name-cache.test.ts b/src/shared/worktree/retired-name-cache.test.ts new file mode 100644 index 00000000000..dacc3f0a58b --- /dev/null +++ b/src/shared/worktree/retired-name-cache.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from 'vitest' +import { + NO_RETIRED_NAMES, + readRetiredNamesForRepo, + retiredNamesAfterRefresh, + selectRetiredNames +} from './retired-name-cache' + +describe('readRetiredNamesForRepo', () => { + it('reads the requested repo only', () => { + const result = { retiredNamesByRepo: { 'repo-1': ['nautilus'], 'repo-2': ['seahorse'] } } + expect(readRetiredNamesForRepo(result, 'repo-1')).toEqual(['nautilus']) + }) + + it.each([ + ['a host predating the method', {}], + ['a repo with no entry', { retiredNamesByRepo: { other: ['nautilus'] } }], + ['a non-array row', { retiredNamesByRepo: { 'repo-1': 'nautilus' } }], + ['a null result', null], + ['a non-object result', 'nope'] + ])('answers empty for %s', (_label, result) => { + expect(readRetiredNamesForRepo(result, 'repo-1')).toEqual([]) + }) + + it('drops non-string elements, which would throw when normalized', () => { + const result = { retiredNamesByRepo: { 'repo-1': ['nautilus', 42, null, 'seahorse'] } } + expect(readRetiredNamesForRepo(result, 'repo-1')).toEqual(['nautilus', 'seahorse']) + }) +}) + +describe('retiredNamesAfterRefresh', () => { + it('takes the new names on success', () => { + const previous = { repoId: 'repo-1', names: ['nautilus'] } + expect(retiredNamesAfterRefresh(previous, 'repo-1', ['nautilus', 'seahorse'])).toEqual({ + repoId: 'repo-1', + names: ['nautilus', 'seahorse'] + }) + }) + + // The divergence this module exists to close: mobile used to reset to [] here, which un-retires + // every name in the window where the create form is asking for a suggestion. + it('holds the previous names when the refresh fails', () => { + const previous = { repoId: 'repo-1', names: ['nautilus'] } + expect(retiredNamesAfterRefresh(previous, 'repo-1', null)).toEqual({ + repoId: 'repo-1', + names: ['nautilus'] + }) + }) + + it('does not carry another repo forward through a failure', () => { + const previous = { repoId: 'repo-1', names: ['nautilus'] } + expect(retiredNamesAfterRefresh(previous, 'repo-2', null)).toEqual({ + repoId: 'repo-2', + names: NO_RETIRED_NAMES + }) + }) + + it('answers empty when the first fetch for a repo fails', () => { + expect(retiredNamesAfterRefresh(null, 'repo-1', null).names).toEqual([]) + }) +}) + +describe('selectRetiredNames', () => { + it('serves a load only to the repo it answered for', () => { + const loaded = { repoId: 'repo-1', names: ['nautilus'] } + expect(selectRetiredNames(loaded, 'repo-1')).toEqual(['nautilus']) + expect(selectRetiredNames(loaded, 'repo-2')).toEqual([]) + expect(selectRetiredNames(loaded, null)).toEqual([]) + expect(selectRetiredNames(null, 'repo-1')).toEqual([]) + }) + + it('returns the stored array itself so downstream memos do not rerun', () => { + const names = ['nautilus'] + expect(selectRetiredNames({ repoId: 'repo-1', names }, 'repo-1')).toBe(names) + }) +}) diff --git a/src/shared/worktree/retired-name-cache.ts b/src/shared/worktree/retired-name-cache.ts new file mode 100644 index 00000000000..aafdc65d268 --- /dev/null +++ b/src/shared/worktree/retired-name-cache.ts @@ -0,0 +1,54 @@ +/** Client-side caching rules for retired workspace names, shared by the desktop hook (IPC) and the + * mobile hook (RPC). The transports differ, but what a refresh and a failure *mean* must not: the + * two hooks drifted, and mobile's copy blanked the list on any error — which un-retires names and + * is the one outcome retirement exists to prevent. + * + * Pure by construction: `src/shared` is on the main process's import graph, so no React here. */ + +export const NO_RETIRED_NAMES: readonly string[] = [] + +/** The last answer, tagged with the repo it answered for. */ +export type RetiredNamesLoad = { + repoId: string + names: readonly string[] +} + +/** Reads one repo's names out of a `worktree.listRetiredNames` result. A host predating the method + * omits the field, and any host can answer with a malformed row that would throw when normalized. */ +export function readRetiredNamesForRepo(result: unknown, repoId: string): string[] { + const names = (result as { retiredNamesByRepo?: Record } | null | undefined) + ?.retiredNamesByRepo?.[repoId] + return Array.isArray(names) + ? names.filter((name): name is string => typeof name === 'string') + : [] +} + +/** Next state once a refresh settles; `names === null` means it failed. Belongs in a `setState` + * updater rather than being applied at call time, so overlapping refreshes fold onto whatever + * actually landed last. + * + * A failure holds the same repo's previous answer: a transient failure must not un-retire names, + * and a refresh fires on every workspace-list mutation, so blanking would suggest a spent name in + * exactly the window where the create form is asking for one. */ +export function retiredNamesAfterRefresh( + previous: RetiredNamesLoad | null, + repoId: string, + names: readonly string[] | null +): RetiredNamesLoad { + return { + repoId, + names: names ?? (previous?.repoId === repoId ? previous.names : NO_RETIRED_NAMES) + } +} + +/** Stale-while-revalidate view: a load answers only for its own repo, so names never leak across a + * repo switch, and a refetch in flight keeps serving the previous answer rather than emptying. + * + * Returns the stored array itself, not a copy, so it stays referentially stable and the suggestion + * memo downstream does not rerun on every refetch. */ +export function selectRetiredNames( + loaded: RetiredNamesLoad | null, + repoId: string | null | undefined +): readonly string[] { + return loaded && loaded.repoId === repoId ? loaded.names : NO_RETIRED_NAMES +}