From bb0e2fd31f309183b347b5cf607cd16e83153bab Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 5 Sep 2026 21:17:02 -0700 Subject: [PATCH] Reject hostless tabs when worktree IDs are ambiguous When worktree IDs collide across hosts, hostless tabs cannot be safely attributed. Refuse activation to prevent accidental host switching. Improve badge accessibility by keeping it out of tab order and exposing secondary matches through screen reader text only. --- .gitignore | 1 - .../worktree-jump-palette-primitives.test.tsx | 45 ++++++ .../worktree-jump-palette-primitives.tsx | 50 ++++--- .../browser-workspace-tab-activation.test.ts | 82 +++++++++++ .../lib/browser-workspace-tab-activation.ts | 9 +- ...space-tab-palette-activation.store.test.ts | 129 ++++++++++++++++++ .../lib/workspace-tab-palette-activation.ts | 16 ++- 7 files changed, 305 insertions(+), 27 deletions(-) create mode 100644 src/renderer/src/components/worktree-jump-palette-primitives.test.tsx create mode 100644 src/renderer/src/lib/browser-workspace-tab-activation.test.ts diff --git a/.gitignore b/.gitignore index a469ca2b20a..6722fc5ae54 100644 --- a/.gitignore +++ b/.gitignore @@ -103,7 +103,6 @@ docs/** !docs/agent-skill-sharing-implementation-checklist.md !docs/mobile-terminal-shortcut-bar.md !docs/reference/ -!docs/reference/cmd-j-ranking.md !docs/reference/git-compatibility.md !docs/reference/headless-linux-server.md !docs/reference/ime-regression-checklist.md diff --git a/src/renderer/src/components/worktree-jump-palette-primitives.test.tsx b/src/renderer/src/components/worktree-jump-palette-primitives.test.tsx new file mode 100644 index 00000000000..1d39ab09749 --- /dev/null +++ b/src/renderer/src/components/worktree-jump-palette-primitives.test.tsx @@ -0,0 +1,45 @@ +// @vitest-environment happy-dom + +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, expect, it } from 'vitest' +import { TooltipProvider } from '@/components/ui/tooltip' +import { PaletteOpenTabPrimaryLine } from './worktree-jump-palette-primitives' + +afterEach(() => cleanup()) + +function renderPrimaryLine( + secondaryMatches: readonly { text: string; ranges: readonly never[] }[] +): void { + render( + + + + ) +} + +it('exposes the extra secondary matches without adding a palette tab stop', () => { + renderPrimaryLine([ + { text: 'src/app.ts', ranges: [] }, + { text: 'src/deep/nested.ts', ranges: [] }, + { text: 'docs/readme.md', ranges: [] } + ]) + + expect(screen.getByText('src/deep/nested.ts, docs/readme.md')).toBeTruthy() + const badge = screen.getByText('+2') + expect(badge.getAttribute('aria-hidden')).toBe('true') + expect(badge.tabIndex).toBe(-1) +}) + +it('renders no badge when every secondary match is already shown', () => { + renderPrimaryLine([{ text: 'src/app.ts', ranges: [] }]) + + expect(screen.queryByText(/^\+\d+$/)).toBeNull() +}) diff --git a/src/renderer/src/components/worktree-jump-palette-primitives.tsx b/src/renderer/src/components/worktree-jump-palette-primitives.tsx index ffdcff0740a..85e8ab04f29 100644 --- a/src/renderer/src/components/worktree-jump-palette-primitives.tsx +++ b/src/renderer/src/components/worktree-jump-palette-primitives.tsx @@ -124,27 +124,35 @@ export function PaletteOpenTabPrimaryLine({ ) : null} {additionalSecondaryMatches.length ? ( - - - match.text).join(', ')} - className="shrink-0 rounded-md border border-border/60 bg-background/45 px-1.5 py-px text-[9px] font-medium text-muted-foreground/88" - > - +{additionalSecondaryMatches.length} - - - - {additionalSecondaryMatches.map((match) => ( -
- -
- ))} - - + <> + {/* Tab selects the palette filter, so the badge stays out of the tab order and + reads its matches through the row's own accessible name instead. */} + + {additionalSecondaryMatches.map((match) => match.text).join(', ')} + + + + + +{additionalSecondaryMatches.length} + + + + {additionalSecondaryMatches.map((match) => ( +
+ +
+ ))} + + + ) : null} {showWorktree ? ( <> diff --git a/src/renderer/src/lib/browser-workspace-tab-activation.test.ts b/src/renderer/src/lib/browser-workspace-tab-activation.test.ts new file mode 100644 index 00000000000..f8e351e1170 --- /dev/null +++ b/src/renderer/src/lib/browser-workspace-tab-activation.test.ts @@ -0,0 +1,82 @@ +// @vitest-environment happy-dom + +import { afterEach, expect, it } from 'vitest' +import { useAppStore } from '@/store' +import type { Tab } from '../../../shared/tab-types' +import type { Worktree } from '../../../shared/worktree/types' +import { getActivatableBrowserWorkspaceTab } from './browser-workspace-tab-activation' + +const initialState = useAppStore.getInitialState() +afterEach(() => useAppStore.setState(initialState, true)) + +function makeWorktree(overrides: Partial & Pick): Worktree { + return { + 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, + ...overrides + } +} + +const browserTab: Tab = { + id: 'unified-browser', + entityId: 'workspace', + groupId: 'group', + worktreeId: 'wt', + contentType: 'browser', + label: 'Browser', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0 +} + +function seedState(worktreesByRepo: Record, tab: Tab): void { + useAppStore.setState( + { ...initialState, worktreesByRepo, unifiedTabsByWorktree: { wt: [tab] } }, + true + ) +} + +it('refuses a hostless browser tab for a remote worktree whose ID also exists locally', () => { + seedState( + { + local: [makeWorktree({ id: 'wt' })], + remote: [makeWorktree({ id: 'wt', repoId: 'repo-remote', hostId: 'ssh:remote' })] + }, + browserTab + ) + + expect( + getActivatableBrowserWorkspaceTab({ + worktreeId: 'wt', + workspaceId: 'workspace', + executionHostId: 'ssh:remote' + }) + ).toBeNull() +}) + +it('accepts a hostless browser tab when the worktree ID is unambiguous', () => { + seedState({ remote: [makeWorktree({ id: 'wt', hostId: 'ssh:remote' })] }, browserTab) + + expect( + getActivatableBrowserWorkspaceTab({ + worktreeId: 'wt', + workspaceId: 'workspace', + executionHostId: 'ssh:remote' + }) + ).toEqual(browserTab) +}) diff --git a/src/renderer/src/lib/browser-workspace-tab-activation.ts b/src/renderer/src/lib/browser-workspace-tab-activation.ts index 3e82d7c0985..3f5146877b4 100644 --- a/src/renderer/src/lib/browser-workspace-tab-activation.ts +++ b/src/renderer/src/lib/browser-workspace-tab-activation.ts @@ -1,7 +1,8 @@ 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' +import { getIndexedAllWorktrees } from '@/store/worktree-repo-index' +import { findAmbiguousWorktreeIds, isUnifiedTabOwnedByWorktree } from './unified-tab-host-ownership' type BrowserWorkspaceTabTarget = { worktreeId: string @@ -18,6 +19,10 @@ export function getActivatableBrowserWorkspaceTab(params: BrowserWorkspaceTabTar if (params.executionHostId && !worktree) { return null } + // A hostless tab cannot be attributed when the same worktree ID exists on several hosts. + const ambiguousWorktreeIds = findAmbiguousWorktreeIds( + getIndexedAllWorktrees(state.worktreesByRepo) + ) // setActiveBrowserTab resolves its backing tab globally by workspace ID. const tabs = Object.values(state.unifiedTabsByWorktree).flat() const browserTabs = tabs.filter( @@ -28,7 +33,7 @@ export function getActivatableBrowserWorkspaceTab(params: BrowserWorkspaceTabTar browserTabs.some( (tab) => tab.worktreeId !== params.worktreeId || - (worktree && !isUnifiedTabOwnedByWorktree(tab, worktree, new Set())) + (worktree && !isUnifiedTabOwnedByWorktree(tab, worktree, ambiguousWorktreeIds)) ) || !unifiedTab || tabs.filter((candidate) => candidate.id === unifiedTab.id).length !== 1 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 index 70672e256b6..60b10097f86 100644 --- a/src/renderer/src/lib/workspace-tab-palette-activation.store.test.ts +++ b/src/renderer/src/lib/workspace-tab-palette-activation.store.test.ts @@ -81,3 +81,132 @@ it('keeps the selected diff active when an editor for the same file shares its g expect(useAppStore.getState().groupsByWorktree.wt[0].activeTabId).toBe('diff') expect(useAppStore.getState().activeFileId).toBe('file') }) + +function makeWorktree(overrides: Partial & Pick): Worktree { + return { + 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, + ...overrides + } +} + +const COLLIDING_WORKTREES = { + local: [makeWorktree({ id: 'wt' })], + remote: [makeWorktree({ id: 'wt', repoId: 'repo-remote', hostId: 'ssh:remote' })] +} + +it('refuses a hostless tab for a remote target whose worktree ID also exists locally', () => { + const terminal: Tab = { + id: 'unified-terminal', + entityId: 'terminal', + groupId: 'group', + worktreeId: 'wt', + contentType: 'terminal', + label: 'Terminal', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0 + } + useAppStore.setState( + { + ...initialState, + worktreesByRepo: COLLIDING_WORKTREES, + groupsByWorktree: { + wt: [ + { + id: 'group', + worktreeId: 'wt', + activeTabId: 'unified-terminal', + tabOrder: ['unified-terminal'] + } + ] + }, + activeGroupIdByWorktree: { wt: 'group' }, + unifiedTabsByWorktree: { wt: [terminal] } + }, + true + ) + + expect( + activateWorkspaceTabPaletteResult({ + worktreeId: 'wt', + groupId: 'group', + tabId: 'unified-terminal', + entityId: 'terminal', + contentType: 'terminal', + executionHostId: 'ssh:remote' + }) + ).toEqual({ status: 'failed', reason: 'missing-tab' }) +}) + +it('refuses a hostless backing file for a remote target whose worktree ID also exists locally', () => { + const editor: Tab = { + id: 'unified-editor', + entityId: 'file', + groupId: 'group', + worktreeId: 'wt', + contentType: 'editor', + executionHostId: 'ssh:remote', + label: 'app.ts', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0 + } + useAppStore.setState( + { + ...initialState, + worktreesByRepo: COLLIDING_WORKTREES, + groupsByWorktree: { + wt: [ + { + id: 'group', + worktreeId: 'wt', + activeTabId: 'unified-editor', + tabOrder: ['unified-editor'] + } + ] + }, + activeGroupIdByWorktree: { wt: 'group' }, + unifiedTabsByWorktree: { wt: [editor] }, + 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: 'unified-editor', + entityId: 'file', + contentType: 'editor', + executionHostId: 'ssh:remote' + }) + ).toEqual({ status: 'failed', reason: 'missing-file' }) +}) diff --git a/src/renderer/src/lib/workspace-tab-palette-activation.ts b/src/renderer/src/lib/workspace-tab-palette-activation.ts index 4e52d8077f9..810e5f13d4c 100644 --- a/src/renderer/src/lib/workspace-tab-palette-activation.ts +++ b/src/renderer/src/lib/workspace-tab-palette-activation.ts @@ -8,7 +8,9 @@ import { useAppStore } from '@/store' import type { AppState } from '@/store/types' import type { ExecutionHostId } from '../../../shared/execution-host' import { activateAndRevealWorktree } from './worktree-activation' +import { getIndexedAllWorktrees } from '@/store/worktree-repo-index' import { + findAmbiguousWorktreeIds, isOpenFileOwnedByWorktree, isUnifiedTabOwnedByWorktree } from './unified-tab-host-ownership' @@ -41,6 +43,7 @@ type WorkspaceTabPaletteActivationState = Pick< | 'setActiveTab' | 'setActiveTabType' | 'unifiedTabsByWorktree' + | 'worktreesByRepo' > function validateTarget( @@ -51,6 +54,10 @@ function validateTarget( if (!worktree) { return 'missing-worktree' } + // A hostless record cannot be attributed when the same worktree ID exists on several hosts. + const ambiguousWorktreeIds = findAmbiguousWorktreeIds( + getIndexedAllWorktrees(state.worktreesByRepo) + ) const group = (state.groupsByWorktree[result.worktreeId] ?? []).find( (candidate) => candidate.id === result.groupId ) @@ -66,7 +73,7 @@ function validateTarget( candidate.groupId === result.groupId && candidate.worktreeId === result.worktreeId && candidate.contentType === result.contentType && - isUnifiedTabOwnedByWorktree(candidate, worktree, new Set()) + isUnifiedTabOwnedByWorktree(candidate, worktree, ambiguousWorktreeIds) ) if (tabs.length !== 1 || !tab) { return 'missing-tab' @@ -77,9 +84,12 @@ function validateTarget( return 'missing-file' } const file = files[0] - const hasExplicitHost = + const hasExplicitHost = Boolean( file.operationProvenance || file.externalSshTargetId || file.runtimeEnvironmentId - if (hasExplicitHost && !isOpenFileOwnedByWorktree(file, worktree)) { + ) + // A hostless file falls back to local ownership, which only decides the match when IDs collide. + const requiresOwnershipCheck = hasExplicitHost || ambiguousWorktreeIds.has(worktree.id) + if (requiresOwnershipCheck && !isOpenFileOwnedByWorktree(file, worktree)) { return 'missing-file' } }