mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 08:03:20 +00:00
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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(
|
||||
<TooltipProvider>
|
||||
<PaletteOpenTabPrimaryLine
|
||||
title="Terminal"
|
||||
titleRanges={[]}
|
||||
secondaryText="src/app.ts"
|
||||
secondaryRanges={[]}
|
||||
secondaryMatches={secondaryMatches}
|
||||
worktreeName="Workspace"
|
||||
worktreeRanges={[]}
|
||||
/>
|
||||
</TooltipProvider>
|
||||
)
|
||||
}
|
||||
|
||||
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()
|
||||
})
|
||||
@@ -124,27 +124,35 @@ export function PaletteOpenTabPrimaryLine({
|
||||
</>
|
||||
) : null}
|
||||
{additionalSecondaryMatches.length ? (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<span
|
||||
aria-label={additionalSecondaryMatches.map((match) => 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}
|
||||
</span>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="top" sideOffset={4} align="start" className="max-w-96 space-y-1">
|
||||
{additionalSecondaryMatches.map((match) => (
|
||||
<div className="break-all" key={match.text}>
|
||||
<HighlightedText
|
||||
text={match.text}
|
||||
matchRanges={match.ranges}
|
||||
highlightClassName="font-semibold text-inherit"
|
||||
/>
|
||||
</div>
|
||||
))}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
<>
|
||||
{/* 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. */}
|
||||
<span className="sr-only" data-slot="palette-open-tab-extra-matches">
|
||||
{additionalSecondaryMatches.map((match) => match.text).join(', ')}
|
||||
</span>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<span
|
||||
aria-hidden
|
||||
tabIndex={-1}
|
||||
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}
|
||||
</span>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="top" sideOffset={4} align="start" className="max-w-96 space-y-1">
|
||||
{additionalSecondaryMatches.map((match) => (
|
||||
<div className="break-all" key={match.text}>
|
||||
<HighlightedText
|
||||
text={match.text}
|
||||
matchRanges={match.ranges}
|
||||
highlightClassName="font-semibold text-inherit"
|
||||
/>
|
||||
</div>
|
||||
))}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
</>
|
||||
) : null}
|
||||
{showWorktree ? (
|
||||
<>
|
||||
|
||||
@@ -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<Worktree> & Pick<Worktree, 'id'>): 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<string, Worktree[]>, 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)
|
||||
})
|
||||
@@ -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
|
||||
|
||||
@@ -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<Worktree> & Pick<Worktree, 'id'>): 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' })
|
||||
})
|
||||
|
||||
@@ -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'
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user