diff --git a/src/renderer/src/components/tab-bar/open-tab-search.test.ts b/src/renderer/src/components/tab-bar/open-tab-search.test.ts index e7aeee8e8cd..d304c37a519 100644 --- a/src/renderer/src/components/tab-bar/open-tab-search.test.ts +++ b/src/renderer/src/components/tab-bar/open-tab-search.test.ts @@ -564,6 +564,51 @@ describe('searchOpenTabs result fields', () => { }) }) + it('keeps editor paths scoped to their host and worktree when tab ids repeat', () => { + const local = makeWorkspaceTab({ + id: 'same-tab', + title: 'Atlas', + contentType: 'editor', + secondaryText: 'local/atlas.ts' + }) + const remote = makeWorkspaceTab({ + id: 'same-tab', + title: 'Atlas', + contentType: 'editor', + secondaryText: 'remote/atlas.ts' + }) + remote.worktree = { ...worktree, hostId: 'ssh:remote' } + remote.tab = { ...remote.tab, executionHostId: 'ssh:remote' } + const sibling = makeWorkspaceTab({ + id: 'same-tab', + title: 'Atlas', + contentType: 'editor', + secondaryText: 'sibling/atlas.ts' + }) + sibling.worktree = { ...worktree, id: 'wt-2' } + sibling.tab = { ...sibling.tab, worktreeId: 'wt-2' } + + expect(search({ query: 'Atlas', workspaceTabs: [local, remote, sibling] })).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + executionHostId: 'local', + worktreeId: 'wt-1', + relativePath: 'local/atlas.ts' + }), + expect.objectContaining({ + executionHostId: 'ssh:remote', + worktreeId: 'wt-1', + relativePath: 'remote/atlas.ts' + }), + expect.objectContaining({ + executionHostId: 'local', + worktreeId: 'wt-2', + relativePath: 'sibling/atlas.ts' + }) + ]) + ) + }) + it('copies a confident occupant agent onto workspace results', () => { const results = search({ query: 'grok', diff --git a/src/renderer/src/components/tab-bar/open-tab-search.ts b/src/renderer/src/components/tab-bar/open-tab-search.ts index a5c7c2e98aa..176dddc0eef 100644 --- a/src/renderer/src/components/tab-bar/open-tab-search.ts +++ b/src/renderer/src/components/tab-bar/open-tab-search.ts @@ -1,6 +1,7 @@ // Merges the three Cmd+J open-tab engines into one ranked list for the new-tab // omnibox. Pure: no store, no React. +import { capPaletteSection } from '../cmd-j/palette-section-render-cap' import { isClipboardTextByteLengthOverLimit } from '../../../../shared/clipboard-text' import type { PaletteDocumentRank } from '@/lib/palette-match/palette-document' import { @@ -21,6 +22,7 @@ import { type SearchableSimulatorTab, type SimulatorPaletteSearchResult } from '@/lib/simulator-palette-search' +import { getUnifiedTabPaletteExecutionHostId } from '@/lib/unified-tab-host-ownership' import type { TuiAgent } from '../../../../shared/tui-agent' import { searchWorkspaceTabs, @@ -170,7 +172,7 @@ function rank( }) } -export function searchOpenTabCandidates({ +function searchOpenTabCandidates({ workspaceTabs, browserPages, simulatorTabs, @@ -184,7 +186,16 @@ export function searchOpenTabCandidates({ const context = suppliedContext ?? createPaletteSearchContext(Date.now()) // Why map workspace only: editor relativePath is read from the searchable entry. - const workspaceEntriesByTabId = new Map(workspaceTabs.map((entry) => [entry.tab.id, entry])) + const workspaceEntriesByIdentity = new Map( + workspaceTabs.map((entry) => [ + encodePaletteIdentity([ + getUnifiedTabPaletteExecutionHostId(entry.tab, entry.worktree) ?? LOCAL_EXECUTION_HOST_ID, + entry.worktree.id, + entry.tab.id + ]), + entry + ]) + ) return [ // Why no isCurrentTab filter: Cmd+J lists the tab you are on, and hiding it @@ -204,7 +215,15 @@ export function searchOpenTabCandidates({ tabId: result.tabId, entityId: result.entityId, groupId: result.groupId, - relativePath: getEditorRelativePath(workspaceEntriesByTabId.get(result.tabId)), + relativePath: getEditorRelativePath( + workspaceEntriesByIdentity.get( + encodePaletteIdentity([ + result.executionHostId ?? LOCAL_EXECUTION_HOST_ID, + result.worktreeId, + result.tabId + ]) + ) + ), occupantAgent: result.occupantAgent }) ), @@ -262,23 +281,11 @@ export function searchOpenTabCandidates({ .map((ranked) => ranked.result) } -function retainCappedResult( - candidates: readonly OpenTabSearchResult[], - retainedResultId: string | null | undefined -): OpenTabSearchResult[] { - const top = candidates.slice(0, OPEN_TAB_SEARCH_RESULT_LIMIT) - if (!retainedResultId || top.some((result) => result.id === retainedResultId)) { - return top - } - const retained = candidates.find((result) => result.id === retainedResultId) - if (!retained || OPEN_TAB_SEARCH_RESULT_LIMIT <= 0) { - return top - } - return [...candidates.slice(0, OPEN_TAB_SEARCH_RESULT_LIMIT - 1), retained].sort( - (a, b) => candidates.indexOf(a) - candidates.indexOf(b) - ) -} - export function searchOpenTabs(input: OpenTabSearchInput): OpenTabSearchResult[] { - return retainCappedResult(searchOpenTabCandidates(input), input.retainedResultId) + const capped = capPaletteSection( + searchOpenTabCandidates(input), + OPEN_TAB_SEARCH_RESULT_LIMIT, + (result) => result.id === input.retainedResultId + ) + return [...capped.visible] } diff --git a/src/renderer/src/components/use-worktree-jump-palette-controller.ts b/src/renderer/src/components/use-worktree-jump-palette-controller.ts index 80dbbd211ee..97d3391e011 100644 --- a/src/renderer/src/components/use-worktree-jump-palette-controller.ts +++ b/src/renderer/src/components/use-worktree-jump-palette-controller.ts @@ -163,7 +163,7 @@ export function useWorktreeJumpPaletteController({ ...listEntries, ...selectionLifecycle, ...selectionActions, - paletteNowMs: paletteSearchContext.nowMs, + paletteNowMs: worktrees.hasQuery ? paletteSearchContext.nowMs : storeState.paletteNowMs, emojiInput, ...createAction } diff --git a/src/renderer/src/components/worktree-jump-palette-worktree-row.tsx b/src/renderer/src/components/worktree-jump-palette-worktree-row.tsx index ff6568bd612..80610a700ca 100644 --- a/src/renderer/src/components/worktree-jump-palette-worktree-row.tsx +++ b/src/renderer/src/components/worktree-jump-palette-worktree-row.tsx @@ -51,7 +51,10 @@ export function WorktreeJumpPaletteWorktreeRow({ activeWorktreeId, controller.activeWorkspaceExecutionHostId ) - const sessionAge = formatPaletteSessionAge(entry.match.lastActiveAt, controller.paletteNowMs) + const sessionAge = formatPaletteSessionAge( + controller.hasQuery ? entry.match.lastActiveAt : worktree.lastActivityAt, + controller.paletteNowMs + ) const sshConnectionId = repo?.connectionId && !isRuntimeOwnedSshTargetId(repo.connectionId) ? repo.connectionId : null const sshStatus = sshConnectionId diff --git a/src/renderer/src/hooks/use-palette-search-evaluation-context.test.ts b/src/renderer/src/hooks/use-palette-search-evaluation-context.test.ts new file mode 100644 index 00000000000..984397a9393 --- /dev/null +++ b/src/renderer/src/hooks/use-palette-search-evaluation-context.test.ts @@ -0,0 +1,34 @@ +// @vitest-environment happy-dom + +import { renderHook } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { usePaletteSearchEvaluationContext } from './use-palette-search-evaluation-context' + +afterEach(() => vi.restoreAllMocks()) + +describe('usePaletteSearchEvaluationContext', () => { + it('captures one clock per snapshot without committing a stale ranking pass', () => { + const clock = vi.spyOn(Date, 'now').mockReturnValue(1_000) + const evaluations: number[] = [] + const snapshot = { query: 'atlas' } + const { result, rerender } = renderHook( + ({ snapshot }) => { + const context = usePaletteSearchEvaluationContext(snapshot) + evaluations.push(context.nowMs) + return context + }, + { initialProps: { snapshot } } + ) + expect(evaluations).toEqual([1_000]) + const initial = result.current + + clock.mockReturnValue(2_000) + rerender({ snapshot }) + expect(result.current).toBe(initial) + + evaluations.length = 0 + rerender({ snapshot: { query: 'atlas notes' } }) + expect(evaluations).toEqual([2_000]) + expect(result.current).not.toBe(initial) + }) +}) diff --git a/src/renderer/src/hooks/use-palette-search-evaluation-context.ts b/src/renderer/src/hooks/use-palette-search-evaluation-context.ts index 4a3ea1aa546..5ad43ca0bc5 100644 --- a/src/renderer/src/hooks/use-palette-search-evaluation-context.ts +++ b/src/renderer/src/hooks/use-palette-search-evaluation-context.ts @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react' +import { useMemo } from 'react' import { createPaletteSearchContext, type PaletteSearchContext @@ -6,10 +6,9 @@ import { /** One clock for every source participating in the current search snapshot. */ export function usePaletteSearchEvaluationContext(snapshot: unknown): PaletteSearchContext { - const [context, setContext] = useState(() => createPaletteSearchContext(Date.now())) - useEffect(() => { + return useMemo(() => { void snapshot - setContext(createPaletteSearchContext(Date.now())) + // oxlint-disable-next-line react/purity -- Each changed snapshot starts one synchronous evaluation clock. + return createPaletteSearchContext(Date.now()) }, [snapshot]) - return context } 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 f839684c320..62b99146a60 100644 --- a/src/renderer/src/lib/browser-page-palette-activation.test.ts +++ b/src/renderer/src/lib/browser-page-palette-activation.test.ts @@ -195,7 +195,7 @@ describe('activateBrowserPagePaletteResult', () => { }) }) - it('focuses the owned unified tab when child ids collide across hosts', () => { + it('rejects colliding child ids before mutating either host', () => { seedStore({ worktreesByRepo: { 'repo-1': [makeWorktree({ hostId: 'ssh:host-1' })], @@ -212,10 +212,28 @@ describe('activateBrowserPagePaletteResult', () => { } }) - expect( - activateBrowserPagePaletteResult({ ...target, executionHostId: 'ssh:host-2' }).status - ).toBe('activated') - expect(useAppStore.getState().activeGroupIdByWorktree['wt-1']).toBe('group-host-2') + const before = useAppStore.getState() + expect(activateBrowserPagePaletteResult({ ...target, executionHostId: 'ssh:host-2' })).toEqual({ + status: 'failed', + reason: 'missing-tab' + }) + expect(useAppStore.getState()).toBe(before) + expect(mocks.activateAndRevealWorktree).not.toHaveBeenCalled() + }) + + it('keeps browser workspaces with distinct unified tabs in multiple groups activatable', () => { + seedStore({ + unifiedTabsByWorktree: { + 'wt-1': [makeBrowserTab(), makeBrowserTab({ id: 'second-view', groupId: 'group-2' })] + }, + groupsByWorktree: { + 'wt-1': [ + makeGroup(), + makeGroup({ id: 'group-2', activeTabId: 'second-view', tabOrder: ['second-view'] }) + ] + } + }) + expect(activateBrowserPagePaletteResult(target).status).toBe('activated') }) it('activates pages in remote folder workspaces', () => { diff --git a/src/renderer/src/lib/browser-page-palette-activation.ts b/src/renderer/src/lib/browser-page-palette-activation.ts index e013bd00da6..72f76722cd7 100644 --- a/src/renderer/src/lib/browser-page-palette-activation.ts +++ b/src/renderer/src/lib/browser-page-palette-activation.ts @@ -1,5 +1,8 @@ import { useAppStore } from '@/store' -import { activateBrowserWorkspaceTab } from '@/lib/browser-workspace-tab-activation' +import { + activateBrowserWorkspaceTab, + getActivatableBrowserWorkspaceTab +} from '@/lib/browser-workspace-tab-activation' import type { ExecutionHostId } from '../../../shared/execution-host' import { isBlankBrowserUrl } from './browser-palette-search' import { activateAndRevealWorktree } from './worktree-activation' @@ -55,6 +58,11 @@ export function activateBrowserPagePaletteResult({ : 'webview' const targetHostId = executionHostId ?? worktree.hostId + if ( + !getActivatableBrowserWorkspaceTab({ worktreeId, workspaceId, executionHostId: targetHostId }) + ) { + return { status: 'failed', reason: 'missing-tab' } + } const activated = activateAndRevealWorktree( worktree.id, targetHostId ? { executionHostId: targetHostId } : {} diff --git a/src/renderer/src/lib/browser-workspace-tab-activation.ts b/src/renderer/src/lib/browser-workspace-tab-activation.ts index a4a3b92ed93..3e82d7c0985 100644 --- a/src/renderer/src/lib/browser-workspace-tab-activation.ts +++ b/src/renderer/src/lib/browser-workspace-tab-activation.ts @@ -1,40 +1,54 @@ import { useAppStore } from '@/store' +import type { Tab } from '../../../shared/tab-types' import type { ExecutionHostId } from '../../../shared/execution-host' import { isUnifiedTabOwnedByWorktree } from './unified-tab-host-ownership' -/** - * Bring a browser workspace forward as the surface the reader is in. - * - * Why the unified tab and not just the browser state: the pane renders whatever its group's active - * tab is, so selecting the workspace alone leaves the page live behind a tab that never shows it. - * Returns false when the workspace has no unified tab yet, which is the caller's cue that there is - * nothing to bring forward. - */ -export function activateBrowserWorkspaceTab(params: { +type BrowserWorkspaceTabTarget = { worktreeId: string workspaceId: string pageId?: string executionHostId?: ExecutionHostId -}): boolean { +} + +export function getActivatableBrowserWorkspaceTab(params: BrowserWorkspaceTabTarget): Tab | null { const state = useAppStore.getState() const worktree = params.executionHostId ? state.getKnownWorktreeById(params.worktreeId, params.executionHostId) : undefined - const unifiedTab = (state.unifiedTabsByWorktree[params.worktreeId] ?? []).find( - (candidate) => - candidate.contentType === 'browser' && - candidate.entityId === params.workspaceId && - (!worktree || isUnifiedTabOwnedByWorktree(candidate, worktree, new Set())) + if (params.executionHostId && !worktree) { + return null + } + // setActiveBrowserTab resolves its backing tab globally by workspace ID. + const tabs = Object.values(state.unifiedTabsByWorktree).flat() + const browserTabs = tabs.filter( + (candidate) => candidate.contentType === 'browser' && candidate.entityId === params.workspaceId ) + const unifiedTab = browserTabs[0] + if ( + browserTabs.some( + (tab) => + tab.worktreeId !== params.worktreeId || + (worktree && !isUnifiedTabOwnedByWorktree(tab, worktree, new Set())) + ) || + !unifiedTab || + tabs.filter((candidate) => candidate.id === unifiedTab.id).length !== 1 + ) { + return null + } + return unifiedTab +} + +export function activateBrowserWorkspaceTab(params: BrowserWorkspaceTabTarget): boolean { + const unifiedTab = getActivatableBrowserWorkspaceTab(params) if (!unifiedTab) { return false } - state.activateTab(unifiedTab.id) + const state = useAppStore.getState() + state.focusGroup(params.worktreeId, unifiedTab.groupId) + state.activateTab(unifiedTab.id, { worktreeId: params.worktreeId }) state.setActiveBrowserTab(params.workspaceId) if (params.pageId) { state.setActiveBrowserPage(params.workspaceId, params.pageId) } - // Refocus the host-owned group if stale mirrored state temporarily repeats a UUID. - state.focusGroup(params.worktreeId, unifiedTab.groupId) return true } diff --git a/src/renderer/src/lib/file-preview.test.ts b/src/renderer/src/lib/file-preview.test.ts index 561d92bbc8a..4c770c85d9e 100644 --- a/src/renderer/src/lib/file-preview.test.ts +++ b/src/renderer/src/lib/file-preview.test.ts @@ -179,15 +179,27 @@ describe('openFileInBrowserTab', () => { } mocks.unifiedTabsByWorktree = { 'wt-1': [ - { id: 'tab-terminal', contentType: 'terminal', entityId: 'term-1', groupId: 'group-1' }, - { id: 'tab-doc', contentType: 'browser', entityId: 'browser-9', groupId: 'group-1' } + { + id: 'tab-terminal', + worktreeId: 'wt-1', + contentType: 'terminal', + entityId: 'term-1', + groupId: 'group-1' + }, + { + id: 'tab-doc', + worktreeId: 'wt-1', + contentType: 'browser', + entityId: 'browser-9', + groupId: 'group-1' + } ] } openFileInBrowserTab({ filePath: '/home/alice/report.html', worktreeId: 'wt-1' }) expect(mocks.focusGroup).toHaveBeenCalledWith('wt-1', 'group-1') - expect(mocks.activateTab).toHaveBeenCalledWith('tab-doc') + expect(mocks.activateTab).toHaveBeenCalledWith('tab-doc', { worktreeId: 'wt-1' }) expect(mocks.setActiveBrowserTab).toHaveBeenCalledWith('browser-9') expect(mocks.createBrowserTab).not.toHaveBeenCalled() }) diff --git a/src/renderer/src/lib/palette-match/cmd-j-ranking-contract.test.ts b/src/renderer/src/lib/palette-match/cmd-j-ranking-contract.test.ts index 7e41c4c2e27..48005bb6683 100644 --- a/src/renderer/src/lib/palette-match/cmd-j-ranking-contract.test.ts +++ b/src/renderer/src/lib/palette-match/cmd-j-ranking-contract.test.ts @@ -42,6 +42,24 @@ describe('Cmd+J semantic proof contract', () => { expect(match?.rank).toMatchObject({ recovery: 0, wordMatch: 0, coverage: 1 }) }) + it('uses the stronger secondary proof when another token already requires container coverage', () => { + const match = matchPaletteTabDocument( + buildPaletteTabDocument({ + id: 'tab', + title: 'alphabet', + secondaryTexts: ['/alpha'], + worktreeName: 'beta', + branch: 'main', + repoName: 'repo' + }), + ready('alpha beta') + ) + expect(match?.rank).toMatchObject({ coverage: 2, strength: 0 }) + expect(match?.titleRanges).toEqual([]) + expect(match?.secondaryMatches).toEqual([{ index: 0, ranges: [{ start: 1, end: 6 }] }]) + expect(match?.worktreeRanges).toEqual([{ start: 0, end: 4 }]) + }) + it('restores contained secondary fields and preserves every selected representation', () => { const restored = matchTitleAndPath('foobar', 'bar', 'b') expect(restored?.secondaryMatches[0]?.ranges).toEqual([{ start: 0, end: 1 }]) diff --git a/src/renderer/src/lib/palette-match/palette-assignment-ranking.ts b/src/renderer/src/lib/palette-match/palette-assignment-ranking.ts index 3217e86ea62..bee797d771f 100644 --- a/src/renderer/src/lib/palette-match/palette-assignment-ranking.ts +++ b/src/renderer/src/lib/palette-match/palette-assignment-ranking.ts @@ -74,24 +74,6 @@ export function summarizeCandidates( return [...byMetric.values()] } -function mergeCandidateSummaries( - visible: readonly TokenCandidate[], - evidence: readonly TokenCandidate[], - diagnostics?: PaletteMatchDiagnostics -): TokenCandidate[] { - const byMetric = new Map() - for (const candidate of [...visible, ...evidence]) { - if (diagnostics) { - diagnostics.selectionCandidateVisits += 1 - } - const key = candidateMetricKey(candidate) - if (!byMetric.has(key)) { - byMetric.set(key, candidate) - } - } - return [...byMetric.values()] -} - function assignmentPlacement( document: PaletteDocument, selected: readonly TokenCandidate[], @@ -170,9 +152,8 @@ export function collectScopeAssignments(args: { diagnostics?: PaletteMatchDiagnostics }): RankedAssignment[] { const scopeCandidates = args.visibleSummaries.map((visible, index) => - mergeCandidateSummaries( - visible, - args.evidenceSummaries[index].get(args.evidenceId) ?? [], + summarizeCandidates( + [...visible, ...(args.evidenceSummaries[index].get(args.evidenceId) ?? [])], args.diagnostics ) ) diff --git a/src/renderer/src/lib/palette-match/palette-ranking.ts b/src/renderer/src/lib/palette-match/palette-ranking.ts index bb0bec33063..89a99b07548 100644 --- a/src/renderer/src/lib/palette-match/palette-ranking.ts +++ b/src/renderer/src/lib/palette-match/palette-ranking.ts @@ -63,19 +63,6 @@ export function maxValidPaletteActivityTimestamp( return maximum } -export function comparePaletteActivity(a: PaletteActivityRank, b: PaletteActivityRank): number { - if (a.ageBucket !== b.ageBucket) { - if (a.ageBucket === null) { - return 1 - } - if (b.ageBucket === null) { - return -1 - } - return a.ageBucket - b.ageBucket - } - return b.timestamp - a.timestamp -} - function compareCodeUnits(a: string, b: string): number { return a < b ? -1 : a > b ? 1 : 0 } diff --git a/src/renderer/src/lib/simulator-tab-palette-activation.test.ts b/src/renderer/src/lib/simulator-tab-palette-activation.test.ts index 4c8bb2b42d4..abad93ae838 100644 --- a/src/renderer/src/lib/simulator-tab-palette-activation.test.ts +++ b/src/renderer/src/lib/simulator-tab-palette-activation.test.ts @@ -132,7 +132,7 @@ describe('activateSimulatorTabPaletteResult', () => { }) }) - it('picks the host that owns the row when the worktree id exists on two hosts', () => { + it('rejects colliding child ids before mutating either host', () => { seedStore({ worktreesByRepo: { 'repo-1': [makeWorktree({ hostId: 'ssh:host-1' })], @@ -149,13 +149,12 @@ describe('activateSimulatorTabPaletteResult', () => { } }) - expect( - activateSimulatorTabPaletteResult({ ...target, executionHostId: 'ssh:host-2' }).status - ).toBe('activated') - expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith('wt-1', { - executionHostId: 'ssh:host-2' - }) - expect(useAppStore.getState().activeGroupIdByWorktree['wt-1']).toBe('group-host-2') + const before = useAppStore.getState() + expect(activateSimulatorTabPaletteResult({ ...target, executionHostId: 'ssh:host-2' })).toEqual( + { status: 'failed', reason: 'missing-tab' } + ) + expect(useAppStore.getState()).toBe(before) + expect(mocks.activateAndRevealWorktree).not.toHaveBeenCalled() }) it('reports an unknown worktree without activating', () => { diff --git a/src/renderer/src/lib/simulator-tab-palette-activation.ts b/src/renderer/src/lib/simulator-tab-palette-activation.ts index 0ad96e4d36f..388b57d126a 100644 --- a/src/renderer/src/lib/simulator-tab-palette-activation.ts +++ b/src/renderer/src/lib/simulator-tab-palette-activation.ts @@ -25,13 +25,15 @@ export function activateSimulatorTabPaletteResult({ if (!worktree) { return { status: 'failed', reason: 'missing-worktree' } } - const tab = (initialState.unifiedTabsByWorktree[worktreeId] ?? []).find( - (candidate) => - candidate.id === tabId && - candidate.contentType === 'simulator' && - isUnifiedTabOwnedByWorktree(candidate, worktree, new Set()) + const tabs = (initialState.unifiedTabsByWorktree[worktreeId] ?? []).filter( + (candidate) => candidate.id === tabId ) - if (!tab) { + const tab = tabs[0] + if ( + tabs.length !== 1 || + tab.contentType !== 'simulator' || + !isUnifiedTabOwnedByWorktree(tab, worktree, new Set()) + ) { return { status: 'failed', reason: 'missing-tab' } } @@ -45,9 +47,8 @@ export function activateSimulatorTabPaletteResult({ } const state = useAppStore.getState() - state.activateTab(tab.id) - // Refocus the host-owned group if stale mirrored state temporarily repeats a UUID. state.focusGroup(worktreeId, tab.groupId) + state.activateTab(tab.id, { worktreeId }) state.setActiveTab(tab.id) state.setActiveTabType('simulator') return { status: 'activated', tabId: tab.id } diff --git a/src/renderer/src/lib/workspace-tab-palette-activation.store.test.ts b/src/renderer/src/lib/workspace-tab-palette-activation.store.test.ts new file mode 100644 index 00000000000..70672e256b6 --- /dev/null +++ b/src/renderer/src/lib/workspace-tab-palette-activation.store.test.ts @@ -0,0 +1,83 @@ +// @vitest-environment happy-dom + +import { afterEach, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import type { Tab } from '../../../shared/tab-types' +import type { Worktree } from '../../../shared/worktree/types' + +vi.mock('./worktree-activation', () => ({ activateAndRevealWorktree: () => true })) + +import { activateWorkspaceTabPaletteResult } from './workspace-tab-palette-activation' + +const initialState = useAppStore.getInitialState() +afterEach(() => useAppStore.setState(initialState, true)) + +it('keeps the selected diff active when an editor for the same file shares its group', () => { + const worktree: Worktree = { + id: 'wt', + repoId: 'repo', + path: '/workspace', + head: '', + branch: 'main', + isBare: false, + isMainWorktree: true, + displayName: 'Workspace', + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0 + } + const editor: Tab = { + id: 'editor', + entityId: 'file', + groupId: 'group', + worktreeId: 'wt', + contentType: 'editor', + label: 'app.ts', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0 + } + useAppStore.setState( + { + ...initialState, + worktreesByRepo: { repo: [worktree] }, + activeWorktreeId: 'wt', + groupsByWorktree: { + wt: [{ id: 'group', worktreeId: 'wt', activeTabId: 'editor', tabOrder: ['editor', 'diff'] }] + }, + activeGroupIdByWorktree: { wt: 'group' }, + unifiedTabsByWorktree: { wt: [editor, { ...editor, id: 'diff', contentType: 'diff' }] }, + openFiles: [ + { + id: 'file', + worktreeId: 'wt', + filePath: '/workspace/app.ts', + relativePath: 'app.ts', + language: 'typescript', + isDirty: false, + mode: 'edit' + } + ] + }, + true + ) + + expect( + activateWorkspaceTabPaletteResult({ + worktreeId: 'wt', + groupId: 'group', + tabId: 'diff', + entityId: 'file', + contentType: 'diff' + }) + ).toEqual({ status: 'activated' }) + expect(useAppStore.getState().groupsByWorktree.wt[0].activeTabId).toBe('diff') + expect(useAppStore.getState().activeFileId).toBe('file') +}) diff --git a/src/renderer/src/lib/workspace-tab-palette-activation.test.ts b/src/renderer/src/lib/workspace-tab-palette-activation.test.ts index aeb1d479a0a..5e28c3736f7 100644 --- a/src/renderer/src/lib/workspace-tab-palette-activation.test.ts +++ b/src/renderer/src/lib/workspace-tab-palette-activation.test.ts @@ -6,7 +6,7 @@ const mocks = vi.hoisted(() => { worktreesByRepo: Record groupsByWorktree: Record[]> unifiedTabsByWorktree: Record[]> - openFiles: { id: string; worktreeId: string }[] + openFiles: { id: string; worktreeId: string; externalSshTargetId?: string }[] repos: unknown[] settings: Record activeGroupIdByWorktree: Record @@ -178,7 +178,9 @@ describe('activateWorkspaceTabPaletteResult', () => { expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith('wt-1') expect(mocks.store.focusGroup).toHaveBeenCalledWith('wt-1', 'group-1') - expect(mocks.store.activateTab).toHaveBeenCalledWith('unified-terminal-1') + expect(mocks.store.activateTab).toHaveBeenCalledWith('unified-terminal-1', { + worktreeId: 'wt-1' + }) expect(mocks.store.setActiveTab).toHaveBeenCalledWith('terminal-1') expect(mocks.store.setActiveTabType).toHaveBeenCalledWith('terminal') expect(mocks.focusTerminalTabSurface).toHaveBeenCalledWith('terminal-1') @@ -202,7 +204,7 @@ describe('activateWorkspaceTabPaletteResult', () => { expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith('wt-1', { executionHostId }) }) - it('activates the owned tab when child ids collide across hosts', () => { + it('rejects colliding child ids before mutating either host', () => { const executionHostId = 'runtime:host-1' as const mocks.store.getKnownWorktreeById.mockImplementation((_worktreeId, hostId) => hostId === executionHostId @@ -220,9 +222,11 @@ describe('activateWorkspaceTabPaletteResult', () => { ] expect(activateWorkspaceTabPaletteResult({ ...makeResult(), executionHostId })).toEqual({ - status: 'activated' + status: 'failed', + reason: 'missing-tab' }) - expect(mocks.activateAndRevealWorktree).toHaveBeenCalledWith('wt-1', { executionHostId }) + expect(mocks.activateAndRevealWorktree).not.toHaveBeenCalled() + expect(mocks.store.activateTab).not.toHaveBeenCalled() }) it('activates tabs in known folder or detected workspaces', () => { @@ -287,7 +291,7 @@ describe('activateWorkspaceTabPaletteResult', () => { expect(mocks.store.focusGroup).toHaveBeenCalledWith('wt-1', 'group-2') expect(mocks.store.setActiveFile).toHaveBeenCalledWith('/tmp/wt-1/src/app.ts') - expect(mocks.store.activateTab).toHaveBeenLastCalledWith('diff-tab-1') + expect(mocks.store.activateTab).toHaveBeenLastCalledWith('diff-tab-1', { worktreeId: 'wt-1' }) expect(mocks.store.setActiveTabType).toHaveBeenCalledWith('editor') expect(mocks.focusTerminalTabSurface).not.toHaveBeenCalled() }) @@ -338,7 +342,7 @@ describe('activateWorkspaceTabPaletteResult', () => { expect(mocks.store.focusGroup).toHaveBeenCalledWith('wt-1', 'group-2') expect(mocks.store.setActiveFile).toHaveBeenCalledWith(entityId) - expect(mocks.store.activateTab).toHaveBeenLastCalledWith(tabId) + expect(mocks.store.activateTab).toHaveBeenLastCalledWith(tabId, { worktreeId: 'wt-1' }) expect(mocks.store.setActiveTabType).toHaveBeenCalledWith('editor') }) @@ -361,6 +365,18 @@ describe('activateWorkspaceTabPaletteResult', () => { expect(mocks.store.focusGroup).not.toHaveBeenCalled() }) + it('rejects a sole backing file whose explicit owner differs from the target', () => { + mocks.store.unifiedTabsByWorktree['wt-1'][0].contentType = 'editor' + mocks.store.openFiles = [ + { id: 'terminal-1', worktreeId: 'wt-1', externalSshTargetId: 'other-host' } + ] + expect(activateWorkspaceTabPaletteResult(makeResult({ contentType: 'editor' }))).toEqual({ + status: 'failed', + reason: 'missing-file' + }) + expect(mocks.activateAndRevealWorktree).not.toHaveBeenCalled() + }) + it('treats missing editor backing files and worktrees as stale', () => { mocks.store.unifiedTabsByWorktree = { 'wt-1': [ diff --git a/src/renderer/src/lib/workspace-tab-palette-activation.ts b/src/renderer/src/lib/workspace-tab-palette-activation.ts index 28c4edac759..4e52d8077f9 100644 --- a/src/renderer/src/lib/workspace-tab-palette-activation.ts +++ b/src/renderer/src/lib/workspace-tab-palette-activation.ts @@ -57,31 +57,31 @@ function validateTarget( if (!group) { return 'missing-group' } - const tab = (state.unifiedTabsByWorktree[result.worktreeId] ?? []).find( + const tabs = (state.unifiedTabsByWorktree[result.worktreeId] ?? []).filter( + (candidate) => candidate.id === result.tabId + ) + const tab = tabs.find( (candidate) => - candidate.id === result.tabId && candidate.entityId === result.entityId && candidate.groupId === result.groupId && candidate.worktreeId === result.worktreeId && candidate.contentType === result.contentType && isUnifiedTabOwnedByWorktree(candidate, worktree, new Set()) ) - if (!tab) { + if (tabs.length !== 1 || !tab) { return 'missing-tab' } - if ( - result.contentType !== 'terminal' && - (() => { - const files = state.openFiles.filter( - (file) => file.id === result.entityId && file.worktreeId === result.worktreeId - ) - return ( - files.length === 0 || - (files.length > 1 && !files.some((file) => isOpenFileOwnedByWorktree(file, worktree))) - ) - })() - ) { - return 'missing-file' + if (result.contentType !== 'terminal') { + const files = state.openFiles.filter((file) => file.id === result.entityId) + if (files.length !== 1 || files[0].worktreeId !== result.worktreeId) { + return 'missing-file' + } + const file = files[0] + const hasExplicitHost = + file.operationProvenance || file.externalSshTargetId || file.runtimeEnvironmentId + if (hasExplicitHost && !isOpenFileOwnedByWorktree(file, worktree)) { + return 'missing-file' + } } return null } @@ -112,7 +112,7 @@ export function activateWorkspaceTabPaletteResult( const runtimeEnvironmentId = getRuntimeEnvironmentIdForWorktree(state, result.worktreeId) state.focusGroup(result.worktreeId, result.groupId) - state.activateTab(result.tabId) + state.activateTab(result.tabId, { worktreeId: result.worktreeId }) if (result.contentType === 'terminal') { if (isWebRuntimeSessionActive(runtimeEnvironmentId)) { @@ -129,6 +129,8 @@ export function activateWorkspaceTabPaletteResult( } state.setActiveFile(result.entityId) + // setActiveFile may pick an editor tab for the same entity instead of this diff. + state.activateTab(result.tabId, { worktreeId: result.worktreeId }) state.setActiveTabType('editor') return { status: 'activated' } }