mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 00:03:46 +00:00
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
This commit is contained in:
@@ -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<string, unknown[]>
|
||||
}))
|
||||
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<string, unknown> = {}): 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.
|
||||
|
||||
@@ -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<HTMLInputElement>(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({
|
||||
/>
|
||||
</div>
|
||||
{/* Above the list, not instead of it: a stale switch target must not wipe
|
||||
the rows the user can still act on. */}
|
||||
{switchError ? (
|
||||
<div className="mt-1 px-1">
|
||||
<EntryStatusRow message={switchError} />
|
||||
</div>
|
||||
) : 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. */}
|
||||
<div role="status">
|
||||
{switchError ? (
|
||||
<div className="mt-1 px-1">
|
||||
<EntryStatusRow message={switchError} />
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
{error || activeOptions.length > 0 || hasQuery ? (
|
||||
<div
|
||||
className="mt-1 space-y-0.5 px-1"
|
||||
|
||||
@@ -77,7 +77,9 @@ export function selectOpenTabSearchEntryState(
|
||||
: undefined) ??
|
||||
repoCandidates[0] ??
|
||||
null
|
||||
const executionHostId = getWorktreeExecutionHostId(worktree, repo ?? undefined)
|
||||
// preferredHostId last: it found this worktree, so it beats the local default
|
||||
// when neither the worktree nor a repo names a host.
|
||||
const executionHostId = getWorktreeExecutionHostId(worktree, repo ?? undefined, preferredHostId)
|
||||
return {
|
||||
activeBrowserTabId: state.activeBrowserTabId,
|
||||
activeFileId: state.activeFileId,
|
||||
|
||||
@@ -172,12 +172,17 @@ describe('activateOpenTabSearchResult', () => {
|
||||
|
||||
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'
|
||||
|
||||
@@ -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([])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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]
|
||||
)
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user