From 56cd69a1e223b8b449c446a2da87bace96278150 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 7 Aug 2026 20:33:59 -0700 Subject: [PATCH] Prevent stale tab results when user keeps typing ahead of deferred searc - useOpenTabSearch now returns {query, results} to track which query the results describe - Gate tab results on query match so stale results don't appear on user's screen - Add live region (role=status) for accessibility of tab switch error messages - Distinguish missing-worktree from missing-page errors in browser page activation - Improve host resolution to prefer active host when worktree and repo don't specify one --- .../TabBarCreateEntry.tab-results.test.tsx | 45 +++++++++++++--- .../components/tab-bar/TabBarCreateEntry.tsx | 22 +++++--- .../tab-bar/open-tab-search-entries.ts | 4 +- .../open-tab-selection-routing.test.ts | 5 ++ .../tab-bar/use-open-tab-search.test.ts | 54 +++++++++++++------ .../components/tab-bar/use-open-tab-search.ts | 13 ++++- .../browser-page-palette-activation.test.ts | 11 ++-- .../lib/browser-page-palette-activation.ts | 5 +- 8 files changed, 121 insertions(+), 38 deletions(-) diff --git a/src/renderer/src/components/tab-bar/TabBarCreateEntry.tab-results.test.tsx b/src/renderer/src/components/tab-bar/TabBarCreateEntry.tab-results.test.tsx index bbf59640523..6ab3f625f2d 100644 --- a/src/renderer/src/components/tab-bar/TabBarCreateEntry.tab-results.test.tsx +++ b/src/renderer/src/components/tab-bar/TabBarCreateEntry.tab-results.test.tsx @@ -25,14 +25,20 @@ vi.mock('@/lib/agent-catalog', () => ({ AgentIcon: () => null })) -// `hold` stands in for the hook's deferred query: rows the user has not seen yet. +// `hold` stands in for the hook's deferred query: it pins the results to the +// query they were built from, so later keystrokes leave them stale. const tabSearchMock = vi.hoisted(() => ({ - hold: false, + hold: null as string | null, resultsByQuery: {} as Record })) vi.mock('./use-open-tab-search', () => ({ - useOpenTabSearch: ({ enabled, query }: { enabled: boolean; query: string }) => - enabled && !tabSearchMock.hold ? (tabSearchMock.resultsByQuery[query.trim()] ?? []) : [] + useOpenTabSearch: ({ enabled, query }: { enabled: boolean; query: string }) => { + const resolved = tabSearchMock.hold ?? query + return { + query: enabled ? resolved : query, + results: enabled ? (tabSearchMock.resultsByQuery[resolved.trim()] ?? []) : [] + } + } })) // Selection routing itself stays real, so the focus handoff and failure messages @@ -159,7 +165,7 @@ function renderEntry(props: Record = {}): void { beforeEach(() => { vi.clearAllMocks() entryOptionsMock.options = [] - tabSearchMock.hold = false + tabSearchMock.hold = null tabSearchMock.resultsByQuery = {} activationMocks.workspace.mockReturnValue({ status: 'activated' }) activationMocks.browser.mockReturnValue({ @@ -230,14 +236,14 @@ describe('TabBarCreateEntry tab results', () => { it('keeps Enter on the row the user saw when a tab row arrives a render later', () => { entryOptionsMock.options = [newFileOption] tabSearchMock.resultsByQuery['add tab'] = [terminalResult()] - tabSearchMock.hold = true + tabSearchMock.hold = '' const onOpenEntry = vi.fn().mockResolvedValue(undefined) renderEntry({ onOpenEntry }) setQuery('add tab') expect(rowTexts()).toHaveLength(1) - tabSearchMock.hold = false + tabSearchMock.hold = null renderEntry({ onOpenEntry }) expect(rowTexts()[0]).toContain('Switch to tab') submitForm() @@ -246,6 +252,26 @@ describe('TabBarCreateEntry tab results', () => { expect(activationMocks.workspace).not.toHaveBeenCalled() }) + it('drops tab rows built for an earlier query until the search catches up', () => { + entryOptionsMock.options = [newFileOption] + tabSearchMock.resultsByQuery['add tab'] = [terminalResult()] + const onOpenEntry = vi.fn().mockResolvedValue(undefined) + renderEntry({ onOpenEntry }) + + setQuery('add tab') + expect(rowTexts()[0]).toContain('Switch to tab') + + // The user keeps typing; the deferred search still describes 'add tab'. + tabSearchMock.hold = 'add tab' + setQuery('add tabs') + + expect(rowTexts().some((row) => row.includes('Switch to tab'))).toBe(false) + submitForm() + + expect(activationMocks.workspace).not.toHaveBeenCalled() + expect(onOpenEntry).toHaveBeenCalledTimes(1) + }) + it('activates a clicked tab row and closes the menu', () => { tabSearchMock.resultsByQuery['add tab'] = [terminalResult()] const onDidOpenEntry = vi.fn() @@ -294,7 +320,10 @@ describe('TabBarCreateEntry tab results', () => { submitForm() expect(onDidOpenEntry).not.toHaveBeenCalled() - expect(container.textContent).toContain('Tab no longer exists') + // Announced, not just drawn: the failure lands inside the live region. + expect(container.querySelector('[role="status"]')?.textContent).toContain( + 'Tab no longer exists' + ) expect(rowTexts()).toHaveLength(3) // The other tab row and the create row below it still act. diff --git a/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx b/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx index f55a7e15f9e..750924bf4fb 100644 --- a/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx +++ b/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx @@ -28,6 +28,7 @@ import { } from './TabBarCreateEntryRow' import { dropFileEntriesCoveredByTabResults } from './open-tab-entry-dedupe' import { activateOpenTabSearchResult } from './open-tab-selection-routing' +import type { OpenTabSearchResult } from './open-tab-search' import { useOpenTabSearch } from './use-open-tab-search' import type { TuiAgent } from '../../../../shared/types' import { translate } from '@/i18n/i18n' @@ -45,6 +46,7 @@ function omniboxPlaceholder(): string { const EMPTY_AGENT_OPTIONS: readonly TabAgentLaunchOption[] = [] const EMPTY_MENU_OPTIONS: readonly TabCreateMenuOption[] = [] +const EMPTY_TAB_RESULTS: readonly OpenTabSearchResult[] = [] type TabBarCreateEntryProps = { agentOptions?: readonly TabAgentLaunchOption[] @@ -86,7 +88,10 @@ export default function TabBarCreateEntry({ const inputRef = useRef(null) const imeEnter = useImeEnterGestureOwnership() const fileList = useRuntimeFileListForWorktree({ enabled: menuOpen, worktreeId }) - const tabResults = useOpenTabSearch({ enabled: menuOpen, query, worktreeId }) + const tabSearch = useOpenTabSearch({ enabled: menuOpen, query, worktreeId }) + // Why gate on the query: the search defers, so its rows can still describe an + // earlier query — Enter must never submit a tab the current query never matched. + const tabResults = tabSearch.query === query ? tabSearch.results : EMPTY_TAB_RESULTS const shouldResolveAbsolutePaths = menuOpen && isTabEntryAbsolutePathLike(query.trim()) const allowAbsolutePathsSelector = useMemo( () => @@ -339,12 +344,15 @@ export default function TabBarCreateEntry({ /> {/* Above the list, not instead of it: a stale switch target must not wipe - the rows the user can still act on. */} - {switchError ? ( -
- -
- ) : null} + the rows the user can still act on. The live region stays mounted so a + screen reader announces the failure instead of missing the insertion. */} +
+ {switchError ? ( +
+ +
+ ) : null} +
{error || activeOptions.length > 0 || hasQuery ? (
{ it('reports a missing worktree distinguishably from a stale tab', () => { mocks.activateWorkspaceTab.mockReturnValue({ status: 'failed', reason: 'missing-worktree' }) + mocks.activateBrowserPage.mockReturnValue({ status: 'failed', reason: 'missing-worktree' }) mocks.activateSimulatorTab.mockReturnValue({ status: 'failed', reason: 'missing-worktree' }) expect(activateOpenTabSearchResult(terminalResult)).toEqual({ status: 'failed', message: 'Workspace no longer exists' }) + expect(activateOpenTabSearchResult(browserResult)).toEqual({ + status: 'failed', + message: 'Workspace no longer exists' + }) expect(activateOpenTabSearchResult(simulatorResult)).toEqual({ status: 'failed', message: 'Workspace no longer exists' diff --git a/src/renderer/src/components/tab-bar/use-open-tab-search.test.ts b/src/renderer/src/components/tab-bar/use-open-tab-search.test.ts index ce5dbff19eb..43a92cbf1f4 100644 --- a/src/renderer/src/components/tab-bar/use-open-tab-search.test.ts +++ b/src/renderer/src/components/tab-bar/use-open-tab-search.test.ts @@ -227,14 +227,14 @@ describe('useOpenTabSearch', () => { it('returns no results while disabled', () => { const { result } = renderSearch({ enabled: false }) - expect(result.current).toEqual([]) + expect(result.current.results).toEqual([]) }) it('returns only tabs from the requested worktree', () => { const { result } = renderSearch() - expect(result.current.map((entry) => entry.title)).not.toContain('zebra delta') - expect(result.current.every((entry) => entry.worktreeId === 'wt-1')).toBe(true) + expect(result.current.results.map((entry) => entry.title)).not.toContain('zebra delta') + expect(result.current.results.every((entry) => entry.worktreeId === 'wt-1')).toBe(true) }) it('keeps the active runtime host when worktree ids collide', () => { @@ -256,8 +256,10 @@ describe('useOpenTabSearch', () => { const { result } = renderSearch() - expect(result.current).not.toHaveLength(0) - expect(result.current.every((entry) => entry.executionHostId === runtimeHost)).toBe(true) + expect(result.current.results).not.toHaveLength(0) + expect(result.current.results.every((entry) => entry.executionHostId === runtimeHost)).toBe( + true + ) }) it('resolves a hosted worktree when the active host is unknown', () => { @@ -279,8 +281,10 @@ describe('useOpenTabSearch', () => { const { result } = renderSearch() - expect(result.current).not.toHaveLength(0) - expect(result.current.every((entry) => entry.executionHostId === runtimeHost)).toBe(true) + expect(result.current.results).not.toHaveLength(0) + expect(result.current.results.every((entry) => entry.executionHostId === runtimeHost)).toBe( + true + ) }) it('returns tabs for a remote-only worktree when the active host is unknown', () => { @@ -298,20 +302,38 @@ describe('useOpenTabSearch', () => { const { result } = renderSearch() - expect(result.current).not.toHaveLength(0) - expect(result.current.every((entry) => entry.executionHostId === sshHost)).toBe(true) + expect(result.current.results).not.toHaveLength(0) + expect(result.current.results.every((entry) => entry.executionHostId === sshHost)).toBe(true) + }) + + it('falls back to the active host when neither the worktree nor a repo names one', () => { + const runtimeHost = 'runtime:env-1' as const + seedStore({ + activeWorkspaceExecutionHostId: runtimeHost, + repos: [], + worktreesByRepo: { + 'repo-1': [{ ...makeWorktree('wt-1', 'Runtime'), runtimeOwnerEnvironmentId: 'env-1' }] + } + }) + + const { result } = renderSearch() + + expect(result.current.results).not.toHaveLength(0) + expect(result.current.results.every((entry) => entry.executionHostId === runtimeHost)).toBe( + true + ) }) it('does not rebuild results for agent-status heartbeats while open', () => { const { result } = renderSearch() - const initialResults = result.current + const initialResults = result.current.results const state = useAppStore.getState() act(() => { useAppStore.setState({ agentStatusByPaneKey: { ...state.agentStatusByPaneKey } }) }) - expect(result.current).toBe(initialResults) + expect(result.current.results).toBe(initialResults) }) it('includes tabs from every column of the worktree, not just the focused one', () => { @@ -319,7 +341,7 @@ describe('useOpenTabSearch', () => { // zebra alpha is the focused tab and still listed, ranked first by the // engine's current-tab bonus, the way Cmd+J lists the tab you are on. - expect(result.current.map((entry) => entry.title)).toEqual([ + expect(result.current.results.map((entry) => entry.title)).toEqual([ 'zebra alpha', 'zebra beta', 'zebra gamma', @@ -329,7 +351,7 @@ describe('useOpenTabSearch', () => { it('reflects tab changes while open', () => { const { result } = renderSearch({ query: 'epsilon' }) - expect(result.current).toEqual([]) + expect(result.current.results).toEqual([]) const state = useAppStore.getState() act(() => { @@ -358,7 +380,7 @@ describe('useOpenTabSearch', () => { }) }) - expect(result.current.map((entry) => entry.title)).toEqual(['zebra epsilon']) + expect(result.current.results.map((entry) => entry.title)).toEqual(['zebra epsilon']) }) it('reflects the generated-titles setting in matched titles', () => { @@ -373,7 +395,7 @@ describe('useOpenTabSearch', () => { }) const { result } = renderSearch({ query: 'generated' }) - expect(result.current.map((entry) => entry.title)).toEqual(['zebra generated']) + expect(result.current.results.map((entry) => entry.title)).toEqual(['zebra generated']) seedStore({ tabsByWorktree: { @@ -381,6 +403,6 @@ describe('useOpenTabSearch', () => { } }) const disabled = renderSearch({ query: 'generated' }) - expect(disabled.result.current).toEqual([]) + expect(disabled.result.current.results).toEqual([]) }) }) diff --git a/src/renderer/src/components/tab-bar/use-open-tab-search.ts b/src/renderer/src/components/tab-bar/use-open-tab-search.ts index 9e4b660f847..9374efca953 100644 --- a/src/renderer/src/components/tab-bar/use-open-tab-search.ts +++ b/src/renderer/src/components/tab-bar/use-open-tab-search.ts @@ -18,11 +18,17 @@ export type UseOpenTabSearchOptions = { worktreeId: string } +export type OpenTabSearchSnapshot = { + /** The query `results` describe; lags the requested query while deferred. */ + query: string + results: OpenTabSearchResult[] +} + export function useOpenTabSearch({ enabled, query, worktreeId -}: UseOpenTabSearchOptions): OpenTabSearchResult[] { +}: UseOpenTabSearchOptions): OpenTabSearchSnapshot { // Why null while disabled: a closed menu stays stable across store churn. const state = useAppStore( useShallow((store) => (enabled ? selectOpenTabSearchEntryState(store, worktreeId) : null)) @@ -41,7 +47,10 @@ export function useOpenTabSearch({ const deferredQuery = useDeferredValue(query) return useMemo( - () => (entries ? searchOpenTabs({ ...entries, query: deferredQuery }) : EMPTY_RESULTS), + () => ({ + query: deferredQuery, + results: entries ? searchOpenTabs({ ...entries, query: deferredQuery }) : EMPTY_RESULTS + }), [deferredQuery, entries] ) } diff --git a/src/renderer/src/lib/browser-page-palette-activation.test.ts b/src/renderer/src/lib/browser-page-palette-activation.test.ts index 71c3f9922b3..b983590b192 100644 --- a/src/renderer/src/lib/browser-page-palette-activation.test.ts +++ b/src/renderer/src/lib/browser-page-palette-activation.test.ts @@ -215,7 +215,7 @@ describe('activateBrowserPagePaletteResult', () => { }) }) - it('reports a missing page, workspace or worktree as a stale target', () => { + it('reports a missing page or workspace as a stale target', () => { seedStore({ browserPagesByWorkspace: {} }) expect(activateBrowserPagePaletteResult(target)).toEqual({ status: 'failed', @@ -228,12 +228,17 @@ describe('activateBrowserPagePaletteResult', () => { reason: 'missing-page' }) + expect(mocks.activateAndRevealWorktree).not.toHaveBeenCalled() + }) + + // A live page in a dead workspace is a different story than a dead page. + it('reports an absent worktree as a missing workspace', () => { seedStore({ worktreesByRepo: {} }) + expect(activateBrowserPagePaletteResult(target)).toEqual({ status: 'failed', - reason: 'missing-page' + reason: 'missing-worktree' }) - expect(mocks.activateAndRevealWorktree).not.toHaveBeenCalled() }) diff --git a/src/renderer/src/lib/browser-page-palette-activation.ts b/src/renderer/src/lib/browser-page-palette-activation.ts index 935f6069aa5..e6e7fdf4a51 100644 --- a/src/renderer/src/lib/browser-page-palette-activation.ts +++ b/src/renderer/src/lib/browser-page-palette-activation.ts @@ -32,9 +32,12 @@ export function activateBrowserPagePaletteResult({ (candidate) => candidate.id === workspaceId ) const worktree = initialState.getKnownWorktreeById(worktreeId, executionHostId) - if (!page || !workspace || !worktree) { + if (!page || !workspace) { return { status: 'failed', reason: 'missing-page' } } + if (!worktree) { + return { status: 'failed', reason: 'missing-worktree' } + } // Why: activateAndRevealWorktree mutates store state, so a later page lookup // is unreliable — resolve the focus target from the URL captured here.