From 3e079debec65aa958fc69bbf3ffa8979662688ef Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 20 Aug 2026 01:38:28 -0700 Subject: [PATCH] fix(sidebar): host-qualify discovery notice rows on multi-host projects (#15546) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(sidebar): host-qualify discovery notice rows and collapse one checkout's twins A project checked out on several hosts emits one discovery-notice row per checkout, and those rows only named the project. A sidebar with paired remote hosts therefore showed several identical "N hidden worktrees" buttons under one project header, with no way to tell which machine each belonged to — or that one of them was another machine's worktree inbox entirely. Two causes, both fixed here: - Notice rows carried no host context, unlike worktree rows, which have been host-labelled since STA-4343. Both notice rows now take a host label, applied per project (not per rendered section, since a card can land in the pinned fallback) and only when that project spans hosts. The label also lands in the review, expand, and dismiss accessible names, so the actions that write to a specific host's repo record say which host that is. - One machine registered as a direct SSH target *and* paired as a runtime environment gives a single on-disk checkout two repo records with independent hidden-worktree state, so it emitted two rows for one directory. Repos now resolve to a (hostname, path) checkout key, and twins collapse to the record this client persists itself — its visibility state is the user's own and survives the paired runtime going away. The key is deliberately conservative: an unresolved hostname, or a tunnelled environment answering on loopback, yields no key and never collapses anything. Renderer-only; no wire or persistence change. * fix(sidebar): drop the machine-identity collapse, gate notice labels on host ids Replaces this branch's second change after a plan review found it has no precedent and eight concrete failure modes. Deleted: the (hostname, path) checkout key that collapsed two repo records believed to be one machine. Orca models a direct SSH target and a paired runtime environment as different execution hosts everywhere else; that change asserted sameness by resolving strings a user typed in two places. It also dropped rows (a differing count vanished with the shadowed record), flipped with the sidebar host filter, ignored port and user so a host and a container on it could merge, tie-broke on repo-store order, was disabled in the one case Orca can prove (a tunnelled pairing answers on loopback) and fired only on coincidence, and left the visibility dialog showing state the sidebar had hidden. Its module also carried a literal NUL byte, so git classified the file as binary and the diff was unreviewable. Kept, with two corrections: notice rows still carry a host label, but the gate now counts distinct host ids rather than distinct label strings — two hosts sharing one user-facing label is exactly when the rows are hardest to tell apart — and membership is read from the unfiltered repo universe rather than the host-filtered notice candidates, so a label no longer appears and disappears with the filter. Two hosts that share a label still render the same label. Disambiguating that is a shared concern across worktree badges, host headers, and host-filter options, and needs its own design; three verification passes each found a different hole in doing it here. Follow-ups: general host-label collision, and the repo-record duplication that produces the twin rows in the first place. * fix(i18n): catalog notice host scope copy * feat(sidebar): show each notice row's host with the project-on-host glyph Notice rows on a multi-host project already carried a host label, but two hosts can share one user-facing name, and the label truncates first in a narrow sidebar. Each row now also carries its host's glyph. Deliberately the same indicator worktree cards use (worktree-card-header): a Server glyph, ServerOff when a paired runtime has no live status, and a "Project on ..." tooltip naming the host — SSH and runtime keep their distinct tooltip wording. Local hosts draw nothing, as on the cards. The glyph is shrink-0, so unlike the text label it survives the sidebar narrowing, and the row keeps an identifying mark either way. Rows now carry the host id alongside the label, since the label alone cannot select a glyph or its tooltip. Catalog entries for the new copy ship with the change rather than relying on inline fallbacks. * refactor(sidebar): draw notice-row hosts with the shared host glyph Follow-up to the notice-row host indicator: use the one glyph vocabulary the app already has instead of a second copy of it. HostRowIcon — a monitor for this computer, a server for anything remote — was private to the composer's run-target rows. Moved to a shared home and reused, so the sidebar and the composer cannot drift apart. The run-target module re-exports it, leaving its own call sites untouched. Every notice row now gets a glyph, local included, so no row is the odd one out; the tooltip still names the host and says when a paired runtime has no live status. Same size and tone tokens across kinds, so no row reads as decorated relative to its neighbours. * fix(sidebar): make notice host glyphs accessible --- src/renderer/src/components/host-row-icon.tsx | 16 ++ .../new-workspace/RunTargetComboboxRow.tsx | 11 +- .../ImportedWorktreesVisibilityLine.test.tsx | 20 ++ .../ImportedWorktreesVisibilityLine.tsx | 32 ++- .../NewExternalWorktreesInboxLine.test.tsx | 34 +++ .../sidebar/NewExternalWorktreesInboxLine.tsx | 37 ++- .../sidebar/NoticeHostGlyph.test.tsx | 157 +++++++++++++ .../components/sidebar/NoticeHostGlyph.tsx | 86 +++++++ ...ree-list-groups-notice-host-labels.test.ts | 220 ++++++++++++++++++ .../worktree-list/grouping/build-rows.ts | 18 +- .../worktree-list/grouping/group-sections.ts | 17 +- .../worktree-list/grouping/host-labels.ts | 75 +++++- .../grouping/pinned-group-rows.ts | 22 +- .../worktree-list/grouping/row-builders.ts | 17 +- .../worktree-list/grouping/row-types.ts | 6 + .../worktree-list/rows/notice-rows.tsx | 4 + src/renderer/src/i18n/locales/en.json | 9 +- src/renderer/src/i18n/locales/es.json | 6 + src/renderer/src/i18n/locales/ja.json | 6 + src/renderer/src/i18n/locales/ko.json | 6 + src/renderer/src/i18n/locales/zh.json | 6 + .../i18n/worktree-visibility-locales.test.ts | 3 + 22 files changed, 782 insertions(+), 26 deletions(-) create mode 100644 src/renderer/src/components/host-row-icon.tsx create mode 100644 src/renderer/src/components/sidebar/NoticeHostGlyph.test.tsx create mode 100644 src/renderer/src/components/sidebar/NoticeHostGlyph.tsx create mode 100644 src/renderer/src/components/sidebar/worktree-list-groups-notice-host-labels.test.ts diff --git a/src/renderer/src/components/host-row-icon.tsx b/src/renderer/src/components/host-row-icon.tsx new file mode 100644 index 00000000000..7e7d519f60a --- /dev/null +++ b/src/renderer/src/components/host-row-icon.tsx @@ -0,0 +1,16 @@ +import React from 'react' +import { Monitor, Server } from 'lucide-react' + +import { LOCAL_EXECUTION_HOST_ID, type ExecutionHostId } from '../../../shared/execution-host' + +/** The local machine isn't a server — a monitor glyph reads as "this computer". */ +export function HostRowIcon({ + hostId, + className +}: { + hostId: ExecutionHostId + className?: string +}): React.JSX.Element { + const Icon = hostId === LOCAL_EXECUTION_HOST_ID ? Monitor : Server + return +} diff --git a/src/renderer/src/components/new-workspace/RunTargetComboboxRow.tsx b/src/renderer/src/components/new-workspace/RunTargetComboboxRow.tsx index 4d7cb3df50f..dde3ae66b28 100644 --- a/src/renderer/src/components/new-workspace/RunTargetComboboxRow.tsx +++ b/src/renderer/src/components/new-workspace/RunTargetComboboxRow.tsx @@ -1,17 +1,14 @@ import React from 'react' -import { AlertTriangle, ChevronRight, LoaderCircle, Monitor, Server } from 'lucide-react' +import { AlertTriangle, ChevronRight, LoaderCircle } from 'lucide-react' import { Button } from '@/components/ui/button' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { cn } from '@/lib/utils' -import { LOCAL_EXECUTION_HOST_ID, type ExecutionHostId } from '../../../../shared/execution-host' +import type { ExecutionHostId } from '../../../../shared/execution-host' import { ProjectOptionDetail } from './ProjectComboboxRow' import { translate } from '@/i18n/i18n' +import { HostRowIcon } from '../host-row-icon' -/** The local machine isn't a server — a monitor glyph reads as "this computer". */ -export function HostRowIcon({ hostId }: { hostId: ExecutionHostId }): React.JSX.Element { - const Icon = hostId === LOCAL_EXECUTION_HOST_ID ? Monitor : Server - return -} +export { HostRowIcon } /** * One run-target row. Shares the Project picker's shape — 32px, label and diff --git a/src/renderer/src/components/sidebar/ImportedWorktreesVisibilityLine.test.tsx b/src/renderer/src/components/sidebar/ImportedWorktreesVisibilityLine.test.tsx index 527c7496a41..0dc3f7c3ce2 100644 --- a/src/renderer/src/components/sidebar/ImportedWorktreesVisibilityLine.test.tsx +++ b/src/renderer/src/components/sidebar/ImportedWorktreesVisibilityLine.test.tsx @@ -73,6 +73,26 @@ describe('ImportedWorktreesVisibilityLine', () => { expect(markup).not.toContain('/worktrees/demo-project') }) + it('names the host when the project is checked out on more than one', () => { + const markup = renderLine({ hostContextLabel: 'openclaw' }) + + expect(markup).toContain('openclaw') + expect(markup).toContain('Expand hidden worktrees for orca on openclaw') + expect(markup).toContain( + 'Keep 4 discovered worktrees hidden for orca on openclaw; recover from the project menu' + ) + }) + + it('folds the host into pinned fallback copy, which already names the repo', () => { + const markup = renderLine({ + hostContextLabel: 'openclaw', + placement: 'pinned-fallback', + onKeepHidden: undefined + }) + + expect(markup).toContain('Hiding 4 discovered worktrees in orca on openclaw') + }) + it('scopes pinned fallback copy to the repo name without a dismiss action', () => { const markup = renderLine({ placement: 'pinned-fallback', onKeepHidden: undefined }) diff --git a/src/renderer/src/components/sidebar/ImportedWorktreesVisibilityLine.tsx b/src/renderer/src/components/sidebar/ImportedWorktreesVisibilityLine.tsx index 3c82858579e..9418b29b5ce 100644 --- a/src/renderer/src/components/sidebar/ImportedWorktreesVisibilityLine.tsx +++ b/src/renderer/src/components/sidebar/ImportedWorktreesVisibilityLine.tsx @@ -4,6 +4,8 @@ import { ChevronRight, X } from 'lucide-react' import { Button } from '@/components/ui/button' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { cn } from '@/lib/utils' +import NoticeHostGlyph from './NoticeHostGlyph' +import type { ExecutionHostId } from '../../../../shared/execution-host' import { getExternalWorktreeParentPath } from '../../../../shared/external-worktree-visibility' import { normalizeRuntimePathForComparison } from '../../../../shared/cross-platform-path' import { translate } from '@/i18n/i18n' @@ -19,6 +21,10 @@ export type ImportedWorktreeVisibilityPreview = { type ImportedWorktreesVisibilityLineProps = { repoDisplayName: string + /** Host this checkout lives on. Set only when the project is checked out on + * more than one host, where the line alone cannot identify the row. */ + hostContextLabel?: string + hostContextHostId?: ExecutionHostId hiddenWorktrees: readonly ImportedWorktreeVisibilityPreview[] placement: ImportedWorktreesVisibilityPlacement pending: boolean @@ -74,6 +80,8 @@ export function groupWorktreesByParentPath( export default function ImportedWorktreesVisibilityLine({ repoDisplayName, + hostContextLabel, + hostContextHostId, hiddenWorktrees, placement, pending, @@ -89,7 +97,11 @@ export default function ImportedWorktreesVisibilityLine({ 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` + // Why: two hosts checking out one project render two identical lines. + const repoScopeLabel = hostContextLabel + ? `${repoDisplayName} on ${hostContextLabel}` + : repoDisplayName + const keepHiddenAriaLabel = `Keep ${hiddenCount} discovered ${worktreeNoun} hidden for ${repoScopeLabel}; recover from the project menu` if (hiddenCount === 0) { return null @@ -97,7 +109,7 @@ export default function ImportedWorktreesVisibilityLine({ const lineText = placement === 'pinned-fallback' - ? `Hiding ${hiddenCount} discovered ${worktreeNoun} in ${repoDisplayName}` + ? `Hiding ${hiddenCount} discovered ${worktreeNoun} in ${repoScopeLabel}` : `Hiding ${hiddenCount} discovered ${worktreeNoun}` const toggleGroupExpanded = (path: string): void => { @@ -133,7 +145,7 @@ export default function ImportedWorktreesVisibilityLine({ aria-label={translate( 'auto.components.sidebar.ImportedWorktreesVisibilityLine.f54f2bec5d', '{{value0}} hidden worktrees for {{value1}}', - { value0: isExpanded ? 'Collapse' : 'Expand', value1: repoDisplayName } + { value0: isExpanded ? 'Collapse' : 'Expand', value1: repoScopeLabel } )} onClick={() => setIsExpanded((value) => !value)} className="shrink-0 rounded-[4px] text-muted-foreground hover:bg-worktree-sidebar-accent hover:text-worktree-sidebar-accent-foreground" @@ -144,6 +156,20 @@ export default function ImportedWorktreesVisibilityLine({ /> {lineText} + {hostContextLabel && placement !== 'pinned-fallback' ? ( + + {hostContextHostId ? ( + + ) : null} + + {hostContextLabel} + + + ) : null} {onKeepHidden ? ( diff --git a/src/renderer/src/components/sidebar/NewExternalWorktreesInboxLine.test.tsx b/src/renderer/src/components/sidebar/NewExternalWorktreesInboxLine.test.tsx index 38554945f12..426f9adf02d 100644 --- a/src/renderer/src/components/sidebar/NewExternalWorktreesInboxLine.test.tsx +++ b/src/renderer/src/components/sidebar/NewExternalWorktreesInboxLine.test.tsx @@ -17,6 +17,7 @@ vi.mock('@/components/ui/tooltip', () => ({ const roots: Root[] = [] type RenderOverrides = { + hostContextLabel?: string inboxCount?: number pending?: boolean error?: string | null @@ -34,6 +35,7 @@ async function renderLine(overrides: RenderOverrides = {}): Promise { ) }) + it('names the host so two checkouts of one project are distinguishable', async () => { + // Both rows read "N hidden worktrees"; only the host tells them apart. + const local = await renderLine({ hostContextLabel: 'Local Mac', inboxCount: 61 }) + const remote = await renderLine({ hostContextLabel: 'openclaw', inboxCount: 134 }) + + expect(local.textContent).toContain('Local Mac') + expect(getReviewButton(local)?.getAttribute('aria-label')).toBe( + 'Review 61 hidden worktrees in orca on Local Mac' + ) + expect(getReviewButton(remote)?.getAttribute('aria-label')).toBe( + 'Review 134 hidden worktrees in orca on openclaw' + ) + }) + + it('host-qualifies the suppress control, which writes to that host alone', async () => { + const container = await renderLine({ hostContextLabel: 'openclaw', onSuppress: vi.fn() }) + + expect( + container.querySelector( + 'button[aria-label="Hide external worktrees permanently for orca on openclaw"]' + ) + ).not.toBeNull() + }) + + it('stays unqualified when the project has a single checkout', async () => { + const container = await renderLine() + + expect(getReviewButton(container)?.getAttribute('aria-label')).toBe( + 'Review 24 hidden worktrees in orca' + ) + }) + it('keeps suppress as a hover-revealed control that does not trigger review', async () => { const onReview = vi.fn() const onSuppress = vi.fn() diff --git a/src/renderer/src/components/sidebar/NewExternalWorktreesInboxLine.tsx b/src/renderer/src/components/sidebar/NewExternalWorktreesInboxLine.tsx index 657b3b254e0..33fe674575f 100644 --- a/src/renderer/src/components/sidebar/NewExternalWorktreesInboxLine.tsx +++ b/src/renderer/src/components/sidebar/NewExternalWorktreesInboxLine.tsx @@ -4,10 +4,16 @@ import { ChevronRight, X } from 'lucide-react' import { Button } from '@/components/ui/button' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { cn } from '@/lib/utils' +import NoticeHostGlyph from './NoticeHostGlyph' +import type { ExecutionHostId } from '../../../../shared/execution-host' import { translate } from '@/i18n/i18n' type NewExternalWorktreesInboxLineProps = { repoDisplayName: string + /** Host this checkout lives on. Set only when the project is checked out on + * more than one host, where the count alone cannot identify the row. */ + hostContextLabel?: string + hostContextHostId?: ExecutionHostId inboxCount: number pending: boolean error: string | null @@ -18,6 +24,8 @@ type NewExternalWorktreesInboxLineProps = { export default function NewExternalWorktreesInboxLine({ repoDisplayName, + hostContextLabel, + hostContextHostId, inboxCount, pending, error, @@ -29,10 +37,19 @@ export default function NewExternalWorktreesInboxLine({ 'auto.components.sidebar.NewExternalWorktreesInboxLine.c3e8a1f4b2', "Don't show again" ) + // Why: the same project on two hosts renders two identical rows, so every + // accessible name has to name the host as well as the project. + const repoScopeLabel = hostContextLabel + ? translate( + 'auto.components.sidebar.NewExternalWorktreesInboxLine.6c07f3a91e', + '{{value0}} on {{value1}}', + { value0: repoDisplayName, value1: hostContextLabel } + ) + : repoDisplayName const suppressAriaLabel = translate( 'auto.components.sidebar.NewExternalWorktreesInboxLine.9f2d4c8b17', 'Hide external worktrees permanently for {{value0}}', - { value0: repoDisplayName } + { value0: repoScopeLabel } ) const isSingular = inboxCount === 1 const countLabel = isSingular @@ -48,12 +65,12 @@ export default function NewExternalWorktreesInboxLine({ ? translate( 'auto.components.sidebar.NewExternalWorktreesInboxLine.7f18c5b0d3', 'Review {{value0}} hidden worktree in {{value1}}', - { value0: inboxCount, value1: repoDisplayName } + { value0: inboxCount, value1: repoScopeLabel } ) : translate( 'auto.components.sidebar.NewExternalWorktreesInboxLine.4e2b7a9c05', 'Review {{value0}} hidden worktrees in {{value1}}', - { value0: inboxCount, value1: repoDisplayName } + { value0: inboxCount, value1: repoScopeLabel } ) if (inboxCount === 0) { @@ -83,6 +100,20 @@ export default function NewExternalWorktreesInboxLine({ {inboxCount} {countLabel} + {hostContextLabel ? ( + + {hostContextHostId ? ( + + ) : null} + + {hostContextLabel} + + + ) : null}