From f5586673acdfad0eefcebbe947419acbb76cc1fb Mon Sep 17 00:00:00 2001 From: manuaudio Date: Sat, 8 Aug 2026 07:52:20 -0700 Subject: [PATCH] fix(sidebar): scope the worktree-visibility toggle to its own host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dialog resolved its repo with a bare id and called updateRepo without a host. When one repo id exists on several hosts that reads whichever row matches first, so the dialog can show one host's visibility state and write the toggle to another host's row. The menu that opens it already has the full Repo in hand, so carry that host through the modal and into updateRepo — the same identity updateRepo already documents for ids that exist on multiple hosts. The host stays optional: the inbox-recovery caller only has a bare id, and that path keeps its current first-match behaviour rather than opening an empty dialog. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012ZcrKK54K7sRnSkT6a6Jut --- .../src/components/sidebar/WorktreeList.tsx | 13 ++- .../sidebar/WorktreeVisibilityDialog.tsx | 38 +++++--- ...worktree-visibility-host-identity.test.tsx | 90 +++++++++++++++++++ 3 files changed, 127 insertions(+), 14 deletions(-) create mode 100644 src/renderer/src/components/sidebar/worktree-visibility-host-identity.test.tsx diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 422b765f082a..aa83074eca46 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -644,7 +644,7 @@ type VirtualizedWorktreeViewportProps = { collapsedGroups: Set handleCreateForRepo: (projectId: string) => void handleOpenRepoSettings: (projectId: string, sectionId?: string) => void - handleOpenWorktreeVisibility: (projectId: string) => void + handleOpenWorktreeVisibility: (projectId: string, hostId?: ExecutionHostId) => void handleShowImportedWorktrees: (projectId: string) => void handleKeepImportedWorktreesHidden: (projectId: string) => void importedWorktreeCardActionState: ReadonlyMap @@ -4584,7 +4584,10 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp { if (row.repo) { - handleOpenWorktreeVisibility(row.repo.id) + handleOpenWorktreeVisibility( + row.repo.id, + getRepoExecutionHostId(row.repo) + ) } }} > @@ -5978,8 +5981,10 @@ const WorktreeList = React.memo(function WorktreeList({ ) const handleOpenWorktreeVisibility = useCallback( - (projectId: string) => { - openModal('worktree-visibility', { repoId: projectId }) + (projectId: string, hostId?: ExecutionHostId) => { + // Why: the id can exist on several hosts, so carry the row's own host when the caller + // knows it — otherwise the dialog reads and writes whichever row resolves first. + openModal('worktree-visibility', { repoId: projectId, ...(hostId ? { hostId } : {}) }) }, [openModal] ) diff --git a/src/renderer/src/components/sidebar/WorktreeVisibilityDialog.tsx b/src/renderer/src/components/sidebar/WorktreeVisibilityDialog.tsx index 5cad24b5ec79..f5b60e03861b 100644 --- a/src/renderer/src/components/sidebar/WorktreeVisibilityDialog.tsx +++ b/src/renderer/src/components/sidebar/WorktreeVisibilityDialog.tsx @@ -18,6 +18,8 @@ import { effectiveExternalWorktreeVisibility, isLegacyRepoForExternalWorktreeVisibility } from '../../../../shared/worktree-ownership' +import { findRepoForHost } from '@/store/slices/repo-host-identity' +import { normalizeExecutionHostId } from '../../../../shared/execution-host' import { translate } from '@/i18n/i18n' export default function WorktreeVisibilityDialog(): React.JSX.Element | null { @@ -28,9 +30,21 @@ export default function WorktreeVisibilityDialog(): React.JSX.Element | null { const updateRepo = useAppStore((s) => s.updateRepo) const fetchWorktrees = useAppStore((s) => s.fetchWorktrees) const detectedWorktreesByRepo = useAppStore((s) => s.detectedWorktreesByRepo) + const settings = useAppStore((s) => s.settings) const repoId = typeof modalData.repoId === 'string' ? modalData.repoId : '' - const repo = repos.find((candidate) => candidate.id === repoId) ?? null + // Why: a repo id can exist on several hosts, so read and write the row the caller meant + // rather than whichever one matches the id first. + const hostId = + normalizeExecutionHostId(typeof modalData.hostId === 'string' ? modalData.hostId : null) ?? + undefined + // Why: findRepoForHost deliberately returns null when a bare id is ambiguous across hosts. + // Fall back to the previous first-match so a host-less caller still opens the dialog rather + // than silently rendering nothing. + const repo = + findRepoForHost(repos, repoId, { hostId, settings }) ?? + repos.find((candidate) => candidate.id === repoId) ?? + null const detected = repoId ? detectedWorktreesByRepo[repoId] : undefined const showOther = repo ? effectiveExternalWorktreeVisibility(repo, isLegacyRepoForExternalWorktreeVisibility(repo)) === @@ -45,17 +59,21 @@ export default function WorktreeVisibilityDialog(): React.JSX.Element | null { if (!repoId) { return } - await updateRepo(repoId, { - externalWorktreeVisibility: showOther ? 'hide' : 'show', - // Why: showing hidden externals again should re-enable the inbox if the - // user previously opted out of discovery prompts for this repo. - // Why: null is the transport sentinel for clearing on remote runtime paths - // where `undefined` is stripped before persistence. - ...(!showOther ? { externalWorktreeDiscoverySuppressedAt: null } : {}) - }) + await updateRepo( + repoId, + { + externalWorktreeVisibility: showOther ? 'hide' : 'show', + // Why: showing hidden externals again should re-enable the inbox if the + // user previously opted out of discovery prompts for this repo. + // Why: null is the transport sentinel for clearing on remote runtime paths + // where `undefined` is stripped before persistence. + ...(!showOther ? { externalWorktreeDiscoverySuppressedAt: null } : {}) + }, + hostId ? { hostId } : undefined + ) await fetchWorktrees(repoId) closeModal() - }, [closeModal, fetchWorktrees, repoId, showOther, updateRepo]) + }, [closeModal, fetchWorktrees, hostId, repoId, showOther, updateRepo]) if (activeModal !== 'worktree-visibility' || !repo || !isGitRepoKind(repo)) { return null diff --git a/src/renderer/src/components/sidebar/worktree-visibility-host-identity.test.tsx b/src/renderer/src/components/sidebar/worktree-visibility-host-identity.test.tsx new file mode 100644 index 000000000000..aa0fc3076115 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-visibility-host-identity.test.tsx @@ -0,0 +1,90 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../../../shared/types' + +const { storeState } = vi.hoisted(() => ({ + storeState: { + activeModal: 'worktree-visibility' as string, + modalData: {} as Record, + closeModal: vi.fn(), + repos: [] as Repo[], + updateRepo: vi.fn().mockResolvedValue(true), + fetchWorktrees: vi.fn().mockResolvedValue(true), + detectedWorktreesByRepo: {} as Record, + settings: {} as Record + } +})) + +vi.mock('@/store', () => ({ + useAppStore: Object.assign( + (selector: (state: typeof storeState) => unknown) => selector(storeState), + { getState: () => storeState } + ) +})) + +vi.mock('@/i18n/i18n', () => ({ + i18n: { language: 'en' }, + translate: (_key: string, fallback: string) => fallback +})) + +import WorktreeVisibilityDialog from './WorktreeVisibilityDialog' + +/** Same repo id on two hosts; only host-a currently shows its external worktrees. */ +function reposOnTwoHosts(): Repo[] { + const base = { id: 'repo-1', badgeColor: '#000000', addedAt: 0, kind: 'git' as const } + return [ + { + ...base, + path: '/repos/b', + displayName: 'on host B', + connectionId: 'host-b', + externalWorktreeVisibility: 'hide' + }, + { + ...base, + path: '/repos/a', + displayName: 'on host A', + connectionId: 'host-a', + externalWorktreeVisibility: 'show' + } + ] as unknown as Repo[] +} + +afterEach(cleanup) + +describe('WorktreeVisibilityDialog host identity', () => { + // Why: one repo id can exist on several hosts. Resolved by bare id, the dialog reads whichever + // row matches first — so it can show host B's state while the user opened it for host A, then + // write the toggle to the wrong row. + it('writes the toggle to the host the dialog was opened for', async () => { + storeState.repos = reposOnTwoHosts() + storeState.modalData = { repoId: 'repo-1', hostId: 'ssh:host-a' } + storeState.updateRepo = vi.fn().mockResolvedValue(true) + + render() + fireEvent.click(await screen.findByRole('button', { name: /import|hide/i })) + + expect(storeState.updateRepo).toHaveBeenCalledWith( + 'repo-1', + expect.objectContaining({ externalWorktreeVisibility: 'hide' }), + { hostId: 'ssh:host-a' } + ) + }) + + it('omits the host option when the caller could not supply one', async () => { + storeState.repos = reposOnTwoHosts() + storeState.modalData = { repoId: 'repo-1' } + storeState.updateRepo = vi.fn().mockResolvedValue(true) + + render() + fireEvent.click(await screen.findByRole('button', { name: /import|hide/i })) + + expect(storeState.updateRepo).toHaveBeenCalledWith( + 'repo-1', + expect.objectContaining({}), + undefined + ) + }) +})