Refine hidden imported worktrees row (#4390)

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Brennan Benson 2026-06-01 05:13:09 -07:00 committed by GitHub
parent d3e2ff8b3c
commit 9b5bfbafb7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 243 additions and 99 deletions

View File

@ -54,19 +54,21 @@ function renderLine(
}
describe('ImportedWorktreesVisibilityLine', () => {
it('renders the compact repo-group line with inline show and dismiss actions', () => {
it('renders the compact repo-group line with expand and dismiss actions', () => {
const markup = renderLine()
expect(markup).toContain('Hiding 4 discovered worktrees')
expect(markup).toContain('Show all')
expect(markup).toContain('Show all 4 discovered worktrees for orca')
expect(markup).toContain('Expand hidden worktrees for orca')
expect(markup).toContain(
'Keep 4 discovered worktrees hidden for orca; recover from the repo menu'
'Keep 4 discovered worktrees hidden for orca; recover from the project menu'
)
expect(markup).toContain('aria-expanded="false"')
expect(markup).not.toContain('Imported 4 existing worktrees')
expect(markup).not.toContain('Orca found 4 worktrees')
expect(markup).not.toContain('repo options')
expect(markup).not.toContain('Reveal')
expect(markup).not.toContain('Always show')
expect(markup).not.toContain('Hidden worktrees by location')
expect(markup).not.toContain('payments-refactor')
expect(markup).not.toContain('/worktrees/demo-project')
})
@ -75,11 +77,11 @@ describe('ImportedWorktreesVisibilityLine', () => {
const markup = renderLine({ placement: 'pinned-fallback', onKeepHidden: undefined })
expect(markup).toContain('Hiding 4 discovered worktrees in orca')
expect(markup).toContain('Show all')
expect(markup).not.toContain('Keep hidden - recover from the repo menu')
expect(markup).not.toContain('Review')
expect(markup).not.toContain('Keep hidden - recover from the project menu')
})
it('preserves Windows parent path separators in preview groups', () => {
it('normalizes Windows parent path separators in preview groups', () => {
const groups = groupWorktreesByParentPath([
{
id: 'windows-hidden',
@ -88,8 +90,42 @@ describe('ImportedWorktreesVisibilityLine', () => {
}
])
expect(groups).toMatchObject([{ path: 'C:\\Repos\\Orca' }])
expect(groups[0]?.path).not.toBe('C:/Repos/Orca')
expect(groups).toMatchObject([{ path: 'C:/Repos/Orca' }])
expect(groups[0]?.path).not.toBe('C:\\Repos\\Orca')
})
it('keeps Windows drive roots as parent path labels', () => {
const groups = groupWorktreesByParentPath([
{
id: 'windows-root-hidden',
displayName: 'FeatureX',
path: 'C:/'
}
])
expect(groups).toMatchObject([{ path: 'C:/' }])
})
it('keeps UNC share roots as parent path labels', () => {
const groups = groupWorktreesByParentPath([
{
id: 'unc-root-hidden',
displayName: 'ShareRoot',
path: '\\\\server\\share'
},
{
id: 'unc-repo-hidden',
displayName: 'Repo',
path: '\\\\server\\share\\repo'
}
])
expect(groups).toMatchObject([
{
path: '//server/share',
worktrees: [{ id: 'unc-root-hidden' }, { id: 'unc-repo-hidden' }]
}
])
})
it('disables actions while pending and renders inline errors', () => {

View File

@ -3,8 +3,9 @@ import { ChevronRight, EyeOff, X } from 'lucide-react'
import { Button } from '@/components/ui/button'
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
import { dirname } from '@/lib/path'
import { cn } from '@/lib/utils'
import { getExternalWorktreeParentPath } from '../../../../shared/external-worktree-visibility'
import { normalizeRuntimePathForComparison } from '../../../../shared/cross-platform-path'
export type ImportedWorktreesVisibilityPlacement = 'repo-group' | 'pinned-fallback'
@ -21,14 +22,14 @@ type ImportedWorktreesVisibilityLineProps = {
placement: ImportedWorktreesVisibilityPlacement
pending: boolean
error: string | null
onShow: () => void
onShow?: () => void
onKeepHidden?: () => void
className?: string
}
const PREVIEW_LIMIT = 3
const UNKNOWN_LOCATION_LABEL = 'Unknown location'
const KEEP_HIDDEN_LABEL = 'Keep hidden - recover from the repo menu'
const KEEP_HIDDEN_LABEL = 'Keep hidden - recover from the project menu'
const GROUP_LIMIT = 5
type ImportedWorktreePathGroup = {
path: string
@ -48,14 +49,7 @@ function getWorktreeKey(
}
function getParentPath(path: string | undefined): string {
if (!path) {
return UNKNOWN_LOCATION_LABEL
}
const parentPath = dirname(path)
if (!parentPath || parentPath === '.') {
return UNKNOWN_LOCATION_LABEL
}
return parentPath
return getExternalWorktreeParentPath(path)
}
export function groupWorktreesByParentPath(
@ -88,12 +82,13 @@ export default function ImportedWorktreesVisibilityLine({
className
}: ImportedWorktreesVisibilityLineProps): React.JSX.Element | null {
const [isExpanded, setIsExpanded] = useState(false)
const [expandedGroupPathKeys, setExpandedGroupPathKeys] = useState<Set<string>>(new Set())
const hiddenCount = hiddenWorktrees.length
const worktreeNoun = pluralizeWorktree(hiddenCount)
const visibleWorktrees = hiddenWorktrees.slice(0, PREVIEW_LIMIT)
const visibleWorktreeGroups = groupWorktreesByParentPath(visibleWorktrees)
const remainingCount = Math.max(0, hiddenWorktrees.length - visibleWorktrees.length)
const keepHiddenAriaLabel = `Keep ${hiddenCount} discovered ${worktreeNoun} hidden for ${repoDisplayName}; recover from the repo menu`
const worktreeGroups = groupWorktreesByParentPath(hiddenWorktrees)
const visibleWorktreeGroups = worktreeGroups.slice(0, GROUP_LIMIT)
const remainingGroupCount = Math.max(0, worktreeGroups.length - visibleWorktreeGroups.length)
const keepHiddenAriaLabel = `Keep ${hiddenCount} discovered ${worktreeNoun} hidden for ${repoDisplayName}; recover from the project menu`
if (hiddenCount === 0) {
return null
@ -104,10 +99,23 @@ export default function ImportedWorktreesVisibilityLine({
? `Hiding ${hiddenCount} discovered ${worktreeNoun} in ${repoDisplayName}`
: `Hiding ${hiddenCount} discovered ${worktreeNoun}`
const toggleGroupExpanded = (path: string): void => {
const key = normalizeRuntimePathForComparison(path)
setExpandedGroupPathKeys((previous) => {
const next = new Set(previous)
if (next.has(key)) {
next.delete(key)
} else {
next.add(key)
}
return next
})
}
return (
<section
aria-busy={pending}
className={cn('mx-1 my-0.5 ml-5 text-sidebar-foreground', className)}
className={cn('mx-1 my-0.5 ml-3 text-sidebar-foreground', className)}
>
<div
className={cn(
@ -132,17 +140,6 @@ export default function ImportedWorktreesVisibilityLine({
</Button>
<EyeOff className="size-3 shrink-0" aria-hidden="true" />
<span className="min-w-0 flex-1 truncate">{lineText}</span>
<Button
type="button"
variant="ghost"
size="xs"
disabled={pending}
aria-label={`Show all ${hiddenCount} discovered ${worktreeNoun} for ${repoDisplayName}`}
onClick={onShow}
className="h-6 shrink-0 px-1.5 text-[11px] font-medium text-sidebar-primary hover:bg-sidebar-accent hover:text-sidebar-primary"
>
Show all
</Button>
{onKeepHidden ? (
<Tooltip>
<TooltipTrigger asChild>
@ -166,41 +163,106 @@ export default function ImportedWorktreesVisibilityLine({
</div>
{isExpanded ? (
<div className="mt-0.5 grid gap-0.5 pb-1" aria-label="Hidden worktree preview">
<div
className="ml-4 mt-0.5 grid gap-1 border-l border-sidebar-border pb-1 pl-2"
aria-label="Hidden worktree groups"
>
{visibleWorktreeGroups.map((group) => (
<div key={group.path} className="grid min-w-0 gap-0.5">
<Tooltip>
<TooltipTrigger asChild>
<span
tabIndex={0}
className="block min-w-0 truncate py-1 pl-7 pr-2 font-mono text-[10px] leading-4 text-muted-foreground outline-none focus-visible:ring-1 focus-visible:ring-sidebar-ring"
>
<div key={group.path} className="grid min-w-0 gap-0.5 rounded-md px-1.5 py-1">
<div className="flex min-h-7 min-w-0 items-center gap-1.5">
<Tooltip>
<TooltipTrigger asChild>
<span
tabIndex={0}
className="block min-w-0 flex-1 truncate font-mono text-[10px] leading-4 text-muted-foreground outline-none focus-visible:ring-1 focus-visible:ring-sidebar-ring"
>
{group.path}
</span>
</TooltipTrigger>
<TooltipContent side="top" sideOffset={4}>
{group.path}
</span>
</TooltipTrigger>
<TooltipContent side="top" sideOffset={4}>
{group.path}
</TooltipContent>
</Tooltip>
{group.worktrees.map((worktree, index) => (
<div
key={getWorktreeKey(worktree, index, 'preview')}
className="flex min-h-7 min-w-0 items-center gap-2 rounded-md py-0 pl-5 pr-2 text-xs text-muted-foreground hover:bg-sidebar-accent"
>
<span
className="size-2 shrink-0 rounded-full border border-dashed border-muted-foreground/50"
aria-hidden="true"
/>
<span className="min-w-0 truncate font-medium">{worktree.displayName}</span>
</div>
))}
</TooltipContent>
</Tooltip>
<span className="shrink-0 rounded-full border border-sidebar-border px-1.5 py-0.5 text-[10px] leading-none text-muted-foreground">
{group.worktrees.length}
</span>
</div>
<ul
className="list-disc space-y-0.5 py-0 pl-5 pr-2 text-xs text-muted-foreground marker:text-muted-foreground"
aria-label={`${group.path} preview`}
>
{group.worktrees
.slice(
0,
expandedGroupPathKeys.has(normalizeRuntimePathForComparison(group.path))
? group.worktrees.length
: PREVIEW_LIMIT
)
.map((worktree, index) => (
<li
key={getWorktreeKey(worktree, index, 'preview')}
className="min-h-6 min-w-0 py-0.5 pl-0"
>
<span className="block min-w-0 truncate font-medium">
{worktree.displayName}
</span>
</li>
))}
{group.worktrees.length > PREVIEW_LIMIT ? (
<li className="list-none">
<Button
type="button"
variant="ghost"
size="xs"
disabled={pending}
onClick={() => toggleGroupExpanded(group.path)}
className="h-6 justify-start px-0 text-[11px] font-normal text-muted-foreground hover:text-sidebar-accent-foreground"
>
{expandedGroupPathKeys.has(normalizeRuntimePathForComparison(group.path))
? 'Show fewer'
: `Show ${group.worktrees.length - PREVIEW_LIMIT} more`}
</Button>
</li>
) : null}
</ul>
</div>
))}
{remainingCount > 0 ? (
{remainingGroupCount > 0 ? (
<div className="py-1 pl-7 pr-2 text-[11px] leading-4 text-muted-foreground">
+ {remainingCount} more
+ {remainingGroupCount} more locations
</div>
) : null}
<div className="grid gap-1 px-1.5 pb-1 pt-1">
<p className="rounded-md bg-sidebar-accent px-2 py-1 text-[10px] font-medium leading-4 text-sidebar-accent-foreground">
Change this later from the project menu.
</p>
<div className="flex min-w-0 items-center gap-1.5">
{onKeepHidden ? (
<Button
type="button"
variant="outline"
size="xs"
disabled={pending}
onClick={onKeepHidden}
className="h-6 px-2 text-[11px] font-medium"
>
Keep hidden
</Button>
) : null}
{onShow ? (
<Button
type="button"
variant="outline"
size="xs"
disabled={pending}
onClick={onShow}
className="h-6 px-2 text-[11px] font-medium"
>
Show in worktree list
</Button>
) : null}
</div>
</div>
</div>
) : null}

View File

@ -22,25 +22,25 @@ describe('imported worktrees card actions', () => {
fetchWorktrees.mockResolvedValue(true)
})
it('shows imported worktrees only after visibility update and refresh succeed', async () => {
it('shows discovered worktrees after visibility update and refresh succeed', async () => {
await showImportedWorktreesCard({ projectId, updateRepo, fetchWorktrees, setCardState })
expect(updateRepo).toHaveBeenCalledWith(projectId, { externalWorktreeVisibility: 'show' })
expect(fetchWorktrees).toHaveBeenCalledWith(projectId, { requireAuthoritative: true })
expect(setCardState).toHaveBeenNthCalledWith(1, projectId, {
pending: true,
error: null,
forceVisible: true
error: null
})
expect(setCardState).toHaveBeenLastCalledWith(projectId, null)
})
it('leaves the card visible when showing fails before refresh', async () => {
updateRepo.mockResolvedValueOnce(false)
it('rolls visibility back when refresh fails after showing', async () => {
fetchWorktrees.mockResolvedValueOnce(false)
await showImportedWorktreesCard({ projectId, updateRepo, fetchWorktrees, setCardState })
expect(fetchWorktrees).not.toHaveBeenCalled()
expect(updateRepo).toHaveBeenNthCalledWith(1, projectId, { externalWorktreeVisibility: 'show' })
expect(updateRepo).toHaveBeenNthCalledWith(2, projectId, { externalWorktreeVisibility: 'hide' })
expect(setCardState).toHaveBeenLastCalledWith(projectId, {
pending: false,
error: IMPORTED_WORKTREES_SHOW_ERROR
@ -71,19 +71,6 @@ describe('imported worktrees card actions', () => {
})
})
it('rolls visibility back and leaves an error when refresh fails after showing', async () => {
fetchWorktrees.mockResolvedValueOnce(false)
await showImportedWorktreesCard({ projectId, updateRepo, fetchWorktrees, setCardState })
expect(updateRepo).toHaveBeenNthCalledWith(1, projectId, { externalWorktreeVisibility: 'show' })
expect(updateRepo).toHaveBeenNthCalledWith(2, projectId, { externalWorktreeVisibility: 'hide' })
expect(setCardState).toHaveBeenLastCalledWith(projectId, {
pending: false,
error: IMPORTED_WORKTREES_SHOW_ERROR
})
})
it('keeps the card force-visible when rollback fails after a refresh failure', async () => {
fetchWorktrees.mockResolvedValueOnce(false)
updateRepo.mockResolvedValueOnce(true).mockResolvedValueOnce(false)

View File

@ -22,18 +22,19 @@ type ImportedWorktreeCardActionDeps = {
) => Promise<boolean>
}
export const IMPORTED_WORKTREES_SHOW_ERROR = 'Could not show imported worktrees. Try again.'
export const IMPORTED_WORKTREES_SHOW_ERROR = 'Could not show discovered worktrees. Try again.'
export const IMPORTED_WORKTREES_KEEP_HIDDEN_ERROR =
'Could not keep imported worktrees hidden. Try again.'
'Could not keep discovered worktrees hidden. Try again.'
export async function showImportedWorktreesCard(
args: ImportedWorktreeCardActionDeps
): Promise<void> {
const forceVisible = args.forceVisible === true
// Preserve rollback-failure retry state so the visible error/action surface does not disappear.
args.setCardState(args.projectId, {
pending: true,
error: null,
forceVisible: true
...(forceVisible ? { forceVisible: true } : {})
})
const updated = await args.updateRepo(args.projectId, { externalWorktreeVisibility: 'show' })
if (!updated) {

View File

@ -84,12 +84,12 @@ describe('imported worktree virtual rows', () => {
).toEqual([{ key: 'repo:repo-1', worktreeIds: ['main', 'feature'] }])
})
it('suppresses keep-hidden actions for force-visible rollback failure cards', () => {
it('only allows keep-hidden actions for repo-group cards that are not forced visible', () => {
expect(canKeepImportedWorktreesHidden(makeImportedCardRow(), undefined)).toBe(true)
expect(
canKeepImportedWorktreesHidden(makeImportedCardRow(), {
pending: false,
error: 'Could not show imported worktrees.',
error: 'Could not show discovered worktrees.',
forceVisible: true
})
).toBe(false)

View File

@ -17,6 +17,10 @@ const STALE_PANE_KEY = makePaneKey('tab-future', STALE_LEAF_ID)
const ORPHAN_PANE_KEY = makePaneKey('tab-orphan', ORPHAN_LEAF_ID)
const TAB_1_PANE_KEY = makePaneKey('tab-1', TAB_1_LEAF_ID)
function expectWorktreeRouting(worktreeId: string): unknown {
return expect.objectContaining({ worktreeId })
}
function makeTarget(args: { hasXtermClass?: boolean; editorClosest?: boolean }): {
classList: { contains: (token: string) => boolean }
closest: (selector: string) => Element | null
@ -2990,7 +2994,8 @@ describe('useIpcEvents agent status snapshot integration', () => {
FUTURE_PANE_KEY,
expect.objectContaining({ state: 'working', prompt: 'p', agentType: 'claude' }),
'Future Tab',
{ updatedAt: 1_700_000_000_000, stateStartedAt: 1_699_999_999_000 }
{ updatedAt: 1_700_000_000_000, stateStartedAt: 1_699_999_999_000 },
expectWorktreeRouting('wt-1')
)
})
@ -3057,7 +3062,8 @@ describe('useIpcEvents agent status snapshot integration', () => {
lastAssistantMessage: 'inactive completion'
}),
'Inactive Tab',
{ updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 }
{ updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 },
expectWorktreeRouting('wt-1')
)
})
@ -3138,7 +3144,8 @@ describe('useIpcEvents agent status snapshot integration', () => {
lastAssistantMessage: 'cursor completion'
}),
'Cursor ready',
{ updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 }
{ updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 },
expectWorktreeRouting('wt-1')
)
})
@ -3221,7 +3228,8 @@ describe('useIpcEvents agent status snapshot integration', () => {
lastAssistantMessage: 'codex completion'
}),
'Codex ready',
{ updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 }
{ updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 },
expectWorktreeRouting('wt-1')
)
expect(updateTabTitle).toHaveBeenCalledTimes(1)
expect(updateTabTitle).toHaveBeenCalledWith('tab-future', 'Codex ready')
@ -3384,7 +3392,8 @@ describe('useIpcEvents agent status snapshot integration', () => {
agentType: 'openclaude'
}),
'Terminal 2',
{ updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 }
{ updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 },
expectWorktreeRouting('wt-1')
)
})
@ -3458,7 +3467,8 @@ describe('useIpcEvents agent status snapshot integration', () => {
lastAssistantMessage: 'inactive completion'
}),
'Inactive Tab',
{ updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 }
{ updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 },
expectWorktreeRouting('wt-1')
)
})
@ -3560,7 +3570,8 @@ describe('useIpcEvents agent status snapshot integration', () => {
FUTURE_PANE_KEY,
expect.objectContaining({ state: 'working', prompt: 'queued prompt', agentType: 'codex' }),
'Future Tab',
{ updatedAt: 1_700_000_000_100, stateStartedAt: 1_699_999_999_100 }
{ updatedAt: 1_700_000_000_100, stateStartedAt: 1_699_999_999_100 },
expectWorktreeRouting('wt-1')
)
expect(setAgentStatus).toHaveBeenNthCalledWith(
2,
@ -3572,7 +3583,8 @@ describe('useIpcEvents agent status snapshot integration', () => {
lastAssistantMessage: 'queued completion'
}),
'Future Tab',
{ updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 }
{ updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 },
expectWorktreeRouting('wt-1')
)
})
@ -3636,7 +3648,8 @@ describe('useIpcEvents agent status snapshot integration', () => {
FUTURE_PANE_KEY,
expect.objectContaining({ state: 'working', prompt: 'remote p', agentType: 'codex' }),
'SSH Tab',
{ updatedAt: 1_700_000_000_000, stateStartedAt: 1_699_999_999_000 }
{ updatedAt: 1_700_000_000_000, stateStartedAt: 1_699_999_999_000 },
expectWorktreeRouting('wt-1')
)
})
@ -4063,7 +4076,8 @@ describe('useIpcEvents agent status snapshot integration', () => {
TAB_1_PANE_KEY,
expect.objectContaining({ state: 'working' }),
'Terminal 1',
{ updatedAt: 1_700_000_000_100, stateStartedAt: 1_699_999_999_100 }
{ updatedAt: 1_700_000_000_100, stateStartedAt: 1_699_999_999_100 },
expectWorktreeRouting('wt-1')
)
})

View File

@ -0,0 +1,44 @@
import { normalizeRuntimePathSeparators } from './cross-platform-path'
export const UNKNOWN_EXTERNAL_WORKTREE_PARENT_PATH = 'Unknown location'
function trimRuntimePathTrailingSlash(value: string): string {
if (value === '/' || /^[A-Za-z]:\/$/.test(value)) {
return value
}
return value.replace(/\/+$/, '')
}
export function getExternalWorktreeParentPath(worktreePath: string | undefined): string {
if (!worktreePath) {
return UNKNOWN_EXTERNAL_WORKTREE_PARENT_PATH
}
const normalized = trimRuntimePathTrailingSlash(normalizeRuntimePathSeparators(worktreePath))
if (!normalized) {
return UNKNOWN_EXTERNAL_WORKTREE_PARENT_PATH
}
if (normalized.startsWith('//')) {
const parts = normalized.slice(2).split('/').filter(Boolean)
if (parts.length < 2) {
return UNKNOWN_EXTERNAL_WORKTREE_PARENT_PATH
}
if (parts.length === 2) {
return `//${parts[0]}/${parts[1]}`
}
return `//${parts.slice(0, -1).join('/')}`
}
const lastSeparatorIndex = normalized.lastIndexOf('/')
if (lastSeparatorIndex < 0) {
return UNKNOWN_EXTERNAL_WORKTREE_PARENT_PATH
}
if (lastSeparatorIndex === 0) {
return '/'
}
if (/^[A-Za-z]:\/$/.test(normalized)) {
return normalized
}
if (/^[A-Za-z]:\/[^/]+$/.test(normalized)) {
return `${normalized.slice(0, 2)}/`
}
return normalized.slice(0, lastSeparatorIndex)
}