From 672a0935d5cb77ce0cfd10e8933336e8b38fd8bd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:18:31 -0700 Subject: [PATCH] fix(sidebar): confirm filter reset before revealing active workspace --- .../WorktreeList.card-memo-stability.test.tsx | 4 + ....lineage-agent-expansion-coupling.test.tsx | 4 + ...ktreeList.lineage-child-real-card.test.tsx | 4 + ...treeList.status-lane-lineage-drop.test.tsx | 4 + .../navigation/use-reveal-requests.test.tsx | 191 ++++++++++++++++++ .../navigation/use-reveal-requests.ts | 47 ++++- src/renderer/src/i18n/locales/en.json | 8 + 7 files changed, 256 insertions(+), 6 deletions(-) create mode 100644 src/renderer/src/components/sidebar/worktree-list/navigation/use-reveal-requests.test.tsx diff --git a/src/renderer/src/components/sidebar/WorktreeList.card-memo-stability.test.tsx b/src/renderer/src/components/sidebar/WorktreeList.card-memo-stability.test.tsx index 8278eb3482b..978ce0e180e 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.card-memo-stability.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.card-memo-stability.test.tsx @@ -1,5 +1,9 @@ // @vitest-environment happy-dom +vi.mock('@/components/confirmation-dialog-context', () => ({ + useConfirmationDialog: () => vi.fn().mockResolvedValue(false) +})) + import { act, type ReactNode } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' diff --git a/src/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsx b/src/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsx index ab4d4a605d4..a68908e5618 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsx @@ -1,5 +1,9 @@ // @vitest-environment happy-dom +vi.mock('@/components/confirmation-dialog-context', () => ({ + useConfirmationDialog: () => vi.fn().mockResolvedValue(false) +})) + // Regression test for the child-worktrees <-> agent-list expansion coupling: // in a worktree card that shows BOTH inline agent rows (with orchestration // lineage) AND a "N children" child-worktrees chip, toggling the child-worktrees diff --git a/src/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsx b/src/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsx index a14d69a7267..a3f7e14b818 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsx @@ -1,5 +1,9 @@ // @vitest-environment happy-dom +vi.mock('@/components/confirmation-dialog-context', () => ({ + useConfirmationDialog: () => vi.fn().mockResolvedValue(false) +})) + import { act, type ReactNode } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' diff --git a/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx b/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx index 4e726dca60c..35919acae1d 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx @@ -1,5 +1,9 @@ // @vitest-environment happy-dom +vi.mock('@/components/confirmation-dialog-context', () => ({ + useConfirmationDialog: () => vi.fn().mockResolvedValue(false) +})) + import { act, type ReactNode } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' diff --git a/src/renderer/src/components/sidebar/worktree-list/navigation/use-reveal-requests.test.tsx b/src/renderer/src/components/sidebar/worktree-list/navigation/use-reveal-requests.test.tsx new file mode 100644 index 00000000000..7f78d3d011a --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-list/navigation/use-reveal-requests.test.tsx @@ -0,0 +1,191 @@ +// @vitest-environment happy-dom +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { ConfirmationDialogProvider } from '@/components/confirmation-dialog' +import { + requestScrollToCurrentWorkspaceReveal, + requestScrollToCurrentWorkspaceRevealAndRename +} from '@/lib/scroll-to-current-workspace-status' +import { folderWorkspaceKey } from '../../../../../../shared/workspace-scope' +import { useSidebarRevealRequests } from './use-reveal-requests' +import type { Worktree } from '../../../../../../shared/worktree/types' + +globalThis.IS_REACT_ACT_ENVIRONMENT = true + +const state = vi.hoisted(() => ({ + setGroupBy: vi.fn(), + pendingRevealSidebarRow: null, + revealSidebarRow: vi.fn(), + revealWorktreeInSidebar: vi.fn(), + setContextualToursBlockingSurfaceVisible: vi.fn() +})) +vi.mock('@/store', () => ({ + useAppStore: (selector: (value: typeof state) => unknown) => selector(state) +})) + +type Args = Parameters[0] +function Host({ args }: { args: Args }): null { + useSidebarRevealRequests(args) + return null +} + +let root: Root +let container: HTMLDivElement +let args: Args + +async function render(): Promise { + await act(async () => { + root.render( + + + + ) + }) +} + +async function click(label: string): Promise { + const button = Array.from(document.querySelectorAll('button')).find( + (candidate) => candidate.textContent === label + ) + expect(button).toBeDefined() + await act(async () => button!.click()) +} + +beforeEach(() => { + vi.clearAllMocks() + container = document.createElement('div') + document.body.append(container) + root = createRoot(container) + const worktree: Worktree = { + id: 'wt-1', + hostId: 'ssh:dev', + repoId: 'repo-1', + path: '/repo/feature', + displayName: 'Feature', + branch: 'feature', + head: 'abc123', + isBare: false, + isMainWorktree: false, + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + linkedGitLabMR: null, + linkedGitLabIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 1, + lastActivityAt: 1 + } + args = { + groupBy: 'repo', + renderedSidebarRowKeys: new Set(), + renderedWorktreeIdentities: [], + currentSidebarWorktreeId: worktree.id, + currentSidebarExecutionHostId: 'ssh:dev', + worktreeMap: new Map([[worktree.id, worktree]]), + worktrees: [worktree], + folderWorkspaces: [], + hasFilters: true, + clearFilters: vi.fn() + } +}) + +afterEach(async () => { + await act(async () => root.unmount()) + container.remove() +}) + +describe('revealing a filtered workspace', () => { + it('explains the filter reset and leaves filters intact when dismissed', async () => { + await render() + await act(async () => requestScrollToCurrentWorkspaceReveal()) + expect(document.body.textContent).toContain('Revealing it will clear your sidebar filters.') + expect(args.clearFilters).not.toHaveBeenCalled() + expect(state.revealWorktreeInSidebar).not.toHaveBeenCalled() + await click('Keep filters') + expect(args.clearFilters).not.toHaveBeenCalled() + expect(state.revealWorktreeInSidebar).not.toHaveBeenCalled() + }) + + it('clears filters and reveals on the original execution host only after confirmation', async () => { + await render() + await act(async () => { + requestScrollToCurrentWorkspaceReveal() + requestScrollToCurrentWorkspaceReveal() + }) + await click('Clear filters and reveal') + expect(args.clearFilters).toHaveBeenCalledTimes(1) + expect(state.revealWorktreeInSidebar).toHaveBeenCalledWith('wt-1', { + behavior: 'smooth', + highlight: true, + beginRename: false, + executionHostId: 'ssh:dev' + }) + expect(document.querySelector('[role="dialog"]')).toBeNull() + }) + + it.each([true, false])( + 'reveals immediately when clearing filters is unnecessary (%s)', + async (visible) => { + args = { + ...args, + hasFilters: visible, + renderedWorktreeIdentities: visible ? ['ssh:dev|wt-1'] : [] + } + await render() + await act(async () => requestScrollToCurrentWorkspaceReveal()) + expect(document.querySelector('[role="dialog"]')).toBeNull() + expect(args.clearFilters).not.toHaveBeenCalled() + expect(state.revealWorktreeInSidebar).toHaveBeenCalledTimes(1) + } + ) + + it('does not apply a stale confirmation after switching workspaces', async () => { + await render() + await act(async () => requestScrollToCurrentWorkspaceReveal()) + args = { ...args, currentSidebarWorktreeId: 'wt-2' } + await render() + await click('Clear filters and reveal') + expect(args.clearFilters).not.toHaveBeenCalled() + expect(state.revealWorktreeInSidebar).not.toHaveBeenCalled() + }) + + it('confirms filtered folder workspaces and preserves the rename request', async () => { + args = { + ...args, + currentSidebarWorktreeId: folderWorkspaceKey('folder-1'), + currentSidebarExecutionHostId: null, + folderWorkspaces: [ + { + id: 'folder-1', + projectGroupId: 'project-1', + name: 'Notes', + folderPath: '/notes', + linkedTask: null, + comment: '', + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 1, + lastActivityAt: 1, + createdAt: 1, + updatedAt: 1 + } + ] + } + await render() + await act(async () => requestScrollToCurrentWorkspaceRevealAndRename()) + expect(args.clearFilters).not.toHaveBeenCalled() + await click('Clear filters and reveal') + expect(args.clearFilters).toHaveBeenCalledTimes(1) + expect(state.revealWorktreeInSidebar).toHaveBeenCalledWith(folderWorkspaceKey('folder-1'), { + behavior: 'smooth', + highlight: true, + beginRename: true, + executionHostId: undefined + }) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-list/navigation/use-reveal-requests.ts b/src/renderer/src/components/sidebar/worktree-list/navigation/use-reveal-requests.ts index bbe9d57e0cb..a779efccfce 100644 --- a/src/renderer/src/components/sidebar/worktree-list/navigation/use-reveal-requests.ts +++ b/src/renderer/src/components/sidebar/worktree-list/navigation/use-reveal-requests.ts @@ -1,4 +1,6 @@ -import { useCallback, useEffect } from 'react' +import { useCallback, useEffect, useRef } from 'react' +import { useConfirmationDialog } from '@/components/confirmation-dialog-context' +import { translate } from '@/i18n/i18n' import { useAppStore } from '@/store' import { SCROLL_TO_CURRENT_WORKSPACE_REVEAL_REQUEST_EVENT, @@ -41,6 +43,10 @@ export function useSidebarRevealRequests(args: { const pendingRevealSidebarRow = useAppStore((s) => s.pendingRevealSidebarRow) const revealSidebarRow = useAppStore((s) => s.revealSidebarRow) const revealWorktreeInSidebar = useAppStore((s) => s.revealWorktreeInSidebar) + const confirm = useConfirmationDialog() + const confirmationPending = useRef(false) + const latestArgs = useRef(args) + latestArgs.current = args useEffect(() => { if (!pendingRevealSidebarRow) { @@ -68,7 +74,7 @@ export function useSidebarRevealRequests(args: { ]) const handleRevealCurrentWorkspaceRequest = useCallback( - (event: Event) => { + async (event: Event) => { const detail = event instanceof CustomEvent ? (event.detail as ScrollToCurrentWorkspaceRevealRequestDetail | undefined) @@ -101,9 +107,37 @@ export function useSidebarRevealRequests(args: { currentSidebarExecutionHostId ?? undefined, currentSidebarWorktreeId ) - if (!renderedWorktreeIdentities.includes(currentIdentity)) { - // Why: the reveal action must show the current workspace, so relax filters that hide it first. - clearFilters() + if (hasFilters && !renderedWorktreeIdentities.includes(currentIdentity)) { + if (confirmationPending.current) { + return + } + confirmationPending.current = true + let confirmed: boolean + try { + confirmed = await confirm({ + title: translate('sidebar.revealFiltered.title', 'Reveal hidden workspace?'), + description: translate( + 'sidebar.revealFiltered.description', + 'The active workspace is hidden in the sidebar. Revealing it will clear your sidebar filters.' + ), + confirmLabel: translate('sidebar.revealFiltered.confirm', 'Clear filters and reveal'), + cancelLabel: translate('sidebar.revealFiltered.cancel', 'Keep filters') + }) + } finally { + confirmationPending.current = false + } + const latest = latestArgs.current + // A workspace switch while the dialog is open must not clear filters for a stale target. + if ( + !confirmed || + latest.currentSidebarWorktreeId !== currentSidebarWorktreeId || + latest.currentSidebarExecutionHostId !== currentSidebarExecutionHostId + ) { + return + } + if (latest.hasFilters && !latest.renderedWorktreeIdentities.includes(currentIdentity)) { + latest.clearFilters() + } } revealWorktreeInSidebar(currentSidebarWorktreeId, { behavior: 'smooth', @@ -113,7 +147,8 @@ export function useSidebarRevealRequests(args: { }) }, [ - clearFilters, + confirm, + hasFilters, currentSidebarWorktreeId, currentSidebarExecutionHostId, folderWorkspaces, diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index f2da58bffb3..be6d715fd8a 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -1,4 +1,12 @@ { + "sidebar": { + "revealFiltered": { + "title": "Reveal hidden workspace?", + "description": "The active workspace is hidden in the sidebar. Revealing it will clear your sidebar filters.", + "confirm": "Clear filters and reveal", + "cancel": "Keep filters" + } + }, "app": { "recoverableError": { "rootTitle": "Orca hit a renderer error.",