From 2d9ec3224cecd4ce4989e1bb9cc5ead61e1b8570 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Thu, 12 Nov 2026 23:37:51 -0800 Subject: [PATCH] fix: keep delete dialog status stable during catalog refreshes (#41864) Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- config/react-compiler-bailout-baseline.txt | 1 - ...-delete-worktree-status-hydration.test.tsx | 118 ++++++++++++++++++ .../use-delete-worktree-status-hydration.ts | 71 +++++++---- 3 files changed, 164 insertions(+), 26 deletions(-) create mode 100644 src/renderer/src/components/sidebar/workspace-actions/deletion/use-delete-worktree-status-hydration.test.tsx diff --git a/config/react-compiler-bailout-baseline.txt b/config/react-compiler-bailout-baseline.txt index ecf0b935257..43049616dfa 100644 --- a/config/react-compiler-bailout-baseline.txt +++ b/config/react-compiler-bailout-baseline.txt @@ -501,7 +501,6 @@ 1 src/renderer/src/components/sidebar/workspace-actions/deletion/DeleteNestedWorktreesDialog.tsx 1 src/renderer/src/components/sidebar/workspace-actions/deletion/RemovalUnsavedFilesNotice.tsx 1 src/renderer/src/components/sidebar/workspace-actions/deletion/WorktreeForceDeleteButton.tsx -1 src/renderer/src/components/sidebar/workspace-actions/deletion/use-delete-worktree-status-hydration.ts 1 src/renderer/src/components/sidebar/workspace-actions/parenting/WorktreeParentPickerPopover.tsx 3 src/renderer/src/components/sidebar/workspace-actions/parenting/use-worktree-parent-picker-transition.ts 7 src/renderer/src/components/sidebar/workspace-board/drag/use-workspace-kanban-card-pointer-drag.ts diff --git a/src/renderer/src/components/sidebar/workspace-actions/deletion/use-delete-worktree-status-hydration.test.tsx b/src/renderer/src/components/sidebar/workspace-actions/deletion/use-delete-worktree-status-hydration.test.tsx new file mode 100644 index 00000000000..6b1812a4519 --- /dev/null +++ b/src/renderer/src/components/sidebar/workspace-actions/deletion/use-delete-worktree-status-hydration.test.tsx @@ -0,0 +1,118 @@ +// @vitest-environment happy-dom + +import { act, cleanup, renderHook, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { makeWorktree } from '@/store/slices/worktrees/worktree.test-fixture' +import type { Repo } from '../../../../../../shared/repo-types' +import type { GitStatusResult } from '../../../../../../shared/git/status/git-status-types' +import { getRuntimeGitStatus } from '@/runtime/source-control/git/runtime-git-status-client' +import { useDeleteWorktreeStatusHydration } from './use-delete-worktree-status-hydration' + +const state = vi.hoisted(() => { + const repos: Repo[] = [] + return { + repos, + activeWorktreeId: null, + activeWorkspaceExecutionHostId: null, + gitStatusByWorktree: {} + } +}) + +vi.mock('@/store', () => ({ + useAppStore: Object.assign((selector: (value: typeof state) => unknown) => selector(state), { + getState: () => state + }) +})) +vi.mock('@/runtime/source-control/git/runtime-git-status-client', () => ({ + getRuntimeGitStatus: vi.fn() +})) + +afterEach(cleanup) +beforeEach(() => { + vi.resetAllMocks() + state.repos = [] +}) + +describe('delete status snapshot continuity', () => { + it('keeps loaded paths when catalog objects refresh for the same workspace', async () => { + vi.mocked(getRuntimeGitStatus) + .mockResolvedValueOnce({ + entries: [{ path: 'keep-visible.ts', status: 'modified', area: 'unstaged' }], + conflictOperation: 'unknown' + }) + .mockImplementation(() => new Promise(() => {})) + const target = makeWorktree({ id: 'feature', repoId: 'repo', hostId: 'local' }) + const targets = [target] + const view = renderHook(useDeleteWorktreeStatusHydration, { + initialProps: { + isOpen: true, + deleteTargets: targets, + visibleTargets: targets, + repoMap: new Map() + } + }) + await waitFor(() => + expect(view.result.current.get('local|feature')).toEqual([ + { path: 'keep-visible.ts', status: 'modified', area: 'unstaged' } + ]) + ) + + state.repos = [...state.repos] + view.rerender({ + isOpen: true, + deleteTargets: [{ ...target, lastActivityAt: 42 }], + visibleTargets: [{ ...target }], + repoMap: new Map() + }) + + expect(view.result.current.get('local|feature')).toEqual([ + { path: 'keep-visible.ts', status: 'modified', area: 'unstaged' } + ]) + }) + + it('replaces the snapshot when the same host changes its SSH connection', async () => { + const repo: Repo = { + id: 'repo', + path: '/repo', + displayName: 'repo', + badgeColor: '', + addedAt: 1, + executionHostId: 'runtime:fixture', + connectionId: 'first' + } + state.repos = [repo] + const target = makeWorktree({ id: 'feature', repoId: 'repo', hostId: 'runtime:fixture' }) + let finish: (status: GitStatusResult) => void = () => { + throw new Error('Status read has not started') + } + vi.mocked(getRuntimeGitStatus) + .mockResolvedValueOnce({ entries: [], conflictOperation: 'unknown' }) + .mockImplementationOnce( + () => + new Promise((resolve) => { + finish = resolve + }) + ) + const props = { + isOpen: true, + deleteTargets: [target], + visibleTargets: [target], + repoMap: new Map([['repo', repo]]) + } + const view = renderHook(useDeleteWorktreeStatusHydration, { initialProps: props }) + await waitFor(() => expect(view.result.current.get('runtime:fixture|feature')).toEqual([])) + + state.repos = [{ ...repo, connectionId: 'second' }] + view.rerender({ ...props, repoMap: new Map([['repo', state.repos[0]!]]) }) + expect(view.result.current.get('runtime:fixture|feature')).toBeUndefined() + await act(async () => { + finish({ + entries: [{ path: 'second-host.ts', status: 'added', area: 'staged' }], + conflictOperation: 'unknown' + }) + }) + expect(view.result.current.get('runtime:fixture|feature')).toEqual([ + { path: 'second-host.ts', status: 'added', area: 'staged' } + ]) + }) +}) diff --git a/src/renderer/src/components/sidebar/workspace-actions/deletion/use-delete-worktree-status-hydration.ts b/src/renderer/src/components/sidebar/workspace-actions/deletion/use-delete-worktree-status-hydration.ts index 278f9e55013..0c992e9a6ef 100644 --- a/src/renderer/src/components/sidebar/workspace-actions/deletion/use-delete-worktree-status-hydration.ts +++ b/src/renderer/src/components/sidebar/workspace-actions/deletion/use-delete-worktree-status-hydration.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react' +import { useEffect, useEffectEvent, useState } from 'react' import { useAppStore } from '@/store' import { getConnectionId } from '@/lib/workspace/ownership/connection-context' import { runtimeTargetForWorkspaceOwner } from '@/lib/workspace/ownership/resolve-owner' @@ -51,20 +51,24 @@ export function useDeleteWorktreeStatusHydration({ repoMap: ReadonlyMap }): ReadonlyMap { const repos = useAppStore((state) => state.repos) - const generation = isOpen ? deleteTargets.map(getWorktreeHostIdentity).join('\n') : '' - const generationRef = useRef(generation) - const [statusByIdentity, setStatusByIdentity] = useState< - Map - >(() => new Map()) - const currentStatusByIdentity = - generationRef.current === generation ? statusByIdentity : EMPTY_STATUS_BY_IDENTITY - - useEffect(() => { - generationRef.current = generation - setStatusByIdentity(new Map()) - if (!isOpen) { - return - } + const generation = isOpen + ? deleteTargets + .map((target) => + JSON.stringify({ + identity: getWorktreeHostIdentity(target), + isMainWorktree: target.isMainWorktree, + isFolderWorkspace: isFolderWorkspaceDelete(repoMap, target), + context: resolveDeleteTargetGitContext(target, repos) + }) + ) + .sort() + .join('\n') + : '' + const [result, setResult] = useState<{ + generation: string + statuses: Map + }>({ generation: '', statuses: new Map() }) + const startChecks = useEffectEvent((signal: AbortSignal) => { const gitStatusByWorktree = useAppStore.getState().gitStatusByWorktree const currentState = useAppStore.getState() const targets = orderDeleteWorktreeStatusHydrationTargets({ @@ -75,39 +79,56 @@ export function useDeleteWorktreeStatusHydration({ activeWorktreeId: currentState.activeWorktreeId, activeExecutionHostId: currentState.activeWorkspaceExecutionHostId }) - const controller = new AbortController() for (const target of targets) { const identity = getWorktreeHostIdentity(target) + const publish = (entries: GitStatusResult['entries'] | null): void => + setResult((current) => ({ + generation, + statuses: new Map(current.generation === generation ? current.statuses : []).set( + identity, + entries + ) + })) const existingStatus = target.hostId ? undefined : gitStatusByWorktree[target.id] if (existingStatus) { - setStatusByIdentity((current) => new Map(current).set(identity, existingStatus)) + publish(existingStatus) continue } const gitContext = resolveDeleteTargetGitContext(target, repos) if (!gitContext) { - setStatusByIdentity((current) => new Map(current).set(identity, null)) + publish(null) continue } void getRuntimeGitStatus(gitContext, { admissionTier: 'background', includeLineStats: false, - signal: controller.signal + signal }) .then((status) => { - if (!controller.signal.aborted && generationRef.current === generation) { - setStatusByIdentity((current) => new Map(current).set(identity, status.entries)) + if (!signal.aborted) { + publish(status.entries) } }) .catch(() => { - if (!controller.signal.aborted && generationRef.current === generation) { - setStatusByIdentity((current) => new Map(current).set(identity, null)) + if (!signal.aborted) { + publish(null) } }) } + }) + + // Catalog refreshes replace arrays; restart only when targets or their read routes change. + useEffect(() => { + setResult({ generation, statuses: new Map() }) + if (!isOpen) { + return + } + const controller = new AbortController() + startChecks(controller.signal) return () => { controller.abort() } - }, [deleteTargets, generation, isOpen, repoMap, repos, visibleTargets]) + }, [generation, isOpen]) - return currentStatusByIdentity + return result.generation === generation ? result.statuses : EMPTY_STATUS_BY_IDENTITY }