From 955dce5a5a1f1fbc58ecb7e9505d7a6e3158828d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 4 Oct 2026 16:49:12 -0700 Subject: [PATCH] Keep workspace deletion dialogs steady while changes load (#25321) * Keep workspace deletion warnings from shifting the dialog * Tighten spacing in workspace deletion confirmations * Address deletion dialog review and synchronize localization catalogs --- .../sidebar/DeleteWorktreeDialog.test.tsx | 4 +- .../sidebar/DeleteWorktreeDialog.tsx | 11 +- .../DeleteWorktreeDialogDescription.tsx | 12 +- .../sidebar/DeleteWorktreeDirtyChangeHint.tsx | 187 ++++++++++------- .../sidebar/DeleteWorktreeLineageNotice.tsx | 12 +- .../DeleteWorktreeTargetPreview.test.tsx | 14 +- .../sidebar/DeleteWorktreeTargetPreview.tsx | 15 +- ...elete-worktree-dirty-change-counts.test.ts | 47 +++++ .../delete-worktree-dirty-change-counts.ts | 62 +++++- ...e-worktree-dirty-change-hydration.test.tsx | 25 +++ .../use-delete-worktree-status-hydration.ts | 14 +- .../src/i18n/en-runtime-required.json | 5 +- src/renderer/src/i18n/locales/en.json | 10 + .../e2e/worktree-delete-dialog-layout.spec.ts | 194 ++++++++++++++++++ 14 files changed, 504 insertions(+), 108 deletions(-) create mode 100644 tests/e2e/worktree-delete-dialog-layout.spec.ts diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx index d1f3e8121de..7b4989a9c5a 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx @@ -359,7 +359,9 @@ describe('DeleteWorktreeDialog lineage copy', () => { const markup = renderToStaticMarkup() expect(markup).toContain('2 uncommitted or untracked changes') - expect(markup).toContain('Deleting this workspace permanently removes these changes from disk.') + expect(markup).toContain( + 'Any uncommitted or untracked changes in Git workspaces will be permanently deleted.' + ) expect(markup).not.toContain('Also delete local branch') }) diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx index d1b0dfea5c8..f8f7731e3cb 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx @@ -23,6 +23,7 @@ import { DeleteWorktreeTargetPreview } from './DeleteWorktreeTargetPreview' import { DeleteWorktreeWarningPanels } from './DeleteWorktreeWarningPanels' import { persistDeleteWorktreeConfirmSkipPreference } from './delete-worktree-preference-toast' import { + getDeleteWorktreeChangeCheckStates, getDeleteWorktreeDirtyChangeCounts, getDeleteWorktreeDirtyChangePreviews } from './delete-worktree-dirty-change-counts' @@ -117,15 +118,11 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { const repoMap = useMemo(() => new Map(repos.map((repo) => [repo.id, repo])), [repos]) const isBatchDelete = worktreeIds.length > 1 const isFolderWorkspaceDelete = !isBatchDelete && getIsFolderWorkspaceDelete(repoMap, worktree) - const folderWorkspaceDeleteCount = useMemo( - () => countFolderWorkspaceDeletes(repoMap, worktrees), - [repoMap, worktrees] - ) const deleteCopy = getDeleteWorktreeDialogCopy({ isBatchDelete, worktree, worktreeCount: worktrees.length, - folderWorkspaceDeleteCount, + folderWorkspaceDeleteCount: countFolderWorkspaceDeletes(repoMap, worktrees), isFolderWorkspaceDelete }) const deleteStateByWorktreeId = useAppStore((s) => s.deleteStateByWorktreeId) @@ -183,6 +180,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { const dirtyChanges = useMemo(() => { const statusInput = { deleteTargets, gitStatusByWorktree, gitStatusByWorktreeIdentity, repoMap } return { + checkStates: getDeleteWorktreeChangeCheckStates(statusInput), counts: getDeleteWorktreeDirtyChangeCounts({ ...statusInput, deleteStateByWorktreeId }), previews: getDeleteWorktreeDirtyChangePreviews(statusInput) } @@ -365,6 +363,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { )} 0} targetClassName={deleteCopy.targetClassName} targetLabel={deleteCopy.targetLabel} canDeleteAllLineage={canDeleteAllLineage} @@ -385,6 +384,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { collisionWorktrees={allWorktrees} hostLabelById={hostLabelById} deleteStateByWorktreeId={deleteStateByWorktreeId} + changeCheckStatesByWorktreeId={dirtyChanges.checkStates} dirtyChangeCountsByWorktreeId={dirtyChanges.counts} dirtyChangePreviewsByWorktreeId={dirtyChanges.previews} /> @@ -392,6 +392,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { {hasLineageChildren && ( diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialogDescription.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDialogDescription.tsx index 60beeda6759..5543348a8d7 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeDialogDescription.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialogDescription.tsx @@ -6,13 +6,15 @@ export function DeleteWorktreeDialogDescription({ targetLabel, canDeleteAllLineage, childTargetLabel, - descriptionSuffix + descriptionSuffix, + showChangeLossWarning }: { targetClassName: string targetLabel: string | undefined canDeleteAllLineage: boolean childTargetLabel: string descriptionSuffix: string + showChangeLossWarning?: boolean }): React.JSX.Element { return ( @@ -28,6 +30,14 @@ export function DeleteWorktreeDialogDescription({ ) : ( <> {descriptionSuffix} )} + {showChangeLossWarning && ( + + {translate( + 'components.workspace.delete.changes.permanentLoss', + 'Any uncommitted or untracked changes in Git workspaces will be permanently deleted.' + )} + + )} ) } diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDirtyChangeHint.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDirtyChangeHint.tsx index 18fa0360c13..75deea5df63 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeDirtyChangeHint.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeDirtyChangeHint.tsx @@ -1,19 +1,43 @@ import type { JSX } from 'react' import { AlertTriangle, ChevronRight } from 'lucide-react' -import { Collapsible, CollapsibleContent, CollapsibleTrigger } from '@/components/ui/collapsible' +import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' import { translate } from '@/i18n/i18n' import { STATUS_COLORS, STATUS_LABELS } from '../right-sidebar/status-display' -import type { DeleteWorktreeDirtyChangePreview } from './delete-worktree-dirty-change-counts' +import type { + DeleteWorktreeChangeCheckState, + DeleteWorktreeDirtyChangePreview +} from './delete-worktree-dirty-change-counts' export function DeleteWorktreeDirtyChangeHint({ changeCount, + checkState, preview }: { changeCount: number | undefined + checkState?: DeleteWorktreeChangeCheckState preview?: DeleteWorktreeDirtyChangePreview }): JSX.Element | null { if (changeCount === undefined) { - return null + if (!checkState) { + return null + } + const statusLabel = + checkState === 'complete' + ? translate( + 'components.workspace.delete.changes.clean', + 'No uncommitted or untracked changes' + ) + : checkState === 'unavailable' + ? translate( + 'components.workspace.delete.changes.unavailable', + 'Changes could not be checked' + ) + : translate('components.workspace.delete.changes.checking', 'Checking for changes…') + return ( +
+ {statusLabel} +
+ ) } const label = @@ -21,10 +45,6 @@ export function DeleteWorktreeDirtyChangeHint({ ? `${changeCount} uncommitted or untracked ${changeCount === 1 ? 'change' : 'changes'}` : 'Uncommitted or untracked changes' - const warning = translate( - 'auto.components.sidebar.DeleteWorktreeDirtyChangeHint.8e2994ce28', - 'Deleting this workspace permanently removes these changes from disk.' - ) const warningLabel = ( <> @@ -33,82 +53,107 @@ export function DeleteWorktreeDirtyChangeHint({ ) if (!preview?.files.length) { + const detailsLabel = + checkState === 'checking' + ? translate('components.workspace.delete.changes.checkingDetails', 'Checking…') + : checkState === 'unavailable' + ? translate( + 'components.workspace.delete.changes.unavailableDetails', + 'Details unavailable' + ) + : null return ( -
+
{warningLabel} + {detailsLabel && · {detailsLabel}}
-

{warning}

) } return ( - - - - -

{warning}

- -
-

- {translate( - 'auto.components.sidebar.DeleteWorktreeDirtyChangeHint.loadedPathsNotice', - 'Loaded paths may be incomplete or out of date.' - )} -

-
+ + +
- {preview.remainingPathCount > 0 ? ( -

+ {warningLabel} + + + + +

+

{translate( - 'auto.components.sidebar.DeleteWorktreeDirtyChangeHint.moreLoadedPaths', - 'and {{value0}} more loaded paths', - { value0: preview.remainingPathCount } + 'auto.components.sidebar.DeleteWorktreeDirtyChangeHint.loadedChangedPaths', + 'Loaded changed paths' )}

- ) : null} -
- - +

+ {translate( + 'auto.components.sidebar.DeleteWorktreeDirtyChangeHint.loadedPathsNotice', + 'Loaded paths may be incomplete or out of date.' + )} +

+
+
    + {preview.files.map((file) => ( +
  • + + {STATUS_LABELS[file.status]} + +
    + {file.path} + {file.hasUnresolvedConflict ? ( + + {translate( + 'auto.components.sidebar.DeleteWorktreeDirtyChangeHint.unresolvedConflict', + 'Unresolved conflict' + )} + + ) : null} +
    +
    • + ))} +
+
+ {preview.remainingPathCount > 0 ? ( +

+ {translate( + 'auto.components.sidebar.DeleteWorktreeDirtyChangeHint.moreLoadedPaths', + 'and {{value0}} more loaded paths', + { value0: preview.remainingPathCount } + )} +

+ ) : null} +
+ + +
) } diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeLineageNotice.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeLineageNotice.tsx index 3338be2ace8..153feee65b8 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeLineageNotice.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeLineageNotice.tsx @@ -3,19 +3,24 @@ import type { JSX } from 'react' import type { Worktree } from '../../../../shared/worktree/types' import { getWorktreeHostIdentity } from '../../../../shared/worktree/host-qualified-identity' import { DeleteWorktreeDirtyChangeHint } from './DeleteWorktreeDirtyChangeHint' -import type { DeleteWorktreeDirtyChangePreview } from './delete-worktree-dirty-change-counts' +import type { + DeleteWorktreeChangeCheckState, + DeleteWorktreeDirtyChangePreview +} from './delete-worktree-dirty-change-counts' import { translate } from '@/i18n/i18n' type DeleteWorktreeLineageNoticeProps = { descendants: readonly Worktree[] dirtyChangeCountsByWorktreeId: ReadonlyMap dirtyChangePreviewsByWorktreeId: ReadonlyMap + changeCheckStatesByWorktreeId?: ReadonlyMap } export function DeleteWorktreeLineageNotice({ descendants, dirtyChangeCountsByWorktreeId, - dirtyChangePreviewsByWorktreeId + dirtyChangePreviewsByWorktreeId, + changeCheckStatesByWorktreeId }: DeleteWorktreeLineageNoticeProps): JSX.Element | null { const childWorkspaceCount = descendants.length if (childWorkspaceCount === 0) { @@ -53,6 +58,9 @@ export function DeleteWorktreeLineageNotice({
{child.displayName}
{child.path}
{ }) expect(trigger).toHaveAttribute('aria-expanded', 'false') expect(screen.queryByText('src/app.ts')).not.toBeInTheDocument() - expect( - screen.getByText('Deleting this workspace permanently removes these changes from disk.') - ).toBeVisible() fireEvent.click(trigger) expect(trigger).toHaveAttribute('aria-expanded', 'true') @@ -222,7 +219,7 @@ describe('DeleteWorktreeTargetPreview loaded paths', () => { expect(screen.queryByText(/No files|clean|0 changes/)).not.toBeInTheDocument() }) - it('expands each qualified batch target independently', () => { + it('opens the selected host preview and closes the previous preview', () => { const local = makeWorktree('same', 'collide', 'local') const runtime = makeWorktree('same', 'collide', 'runtime:runtime-7') const localKey = getWorktreeHostIdentity(local) @@ -251,11 +248,12 @@ describe('DeleteWorktreeTargetPreview loaded paths', () => { const localRow = screen.getByRole('listitem', { name: /Local/ }) const runtimeRow = screen.getByRole('listitem', { name: /Build Mac/ }) fireEvent.click(within(localRow).getByRole('button')) - expect(within(localRow).getByText('local.ts')).toBeVisible() + expect(screen.getByText('local.ts')).toBeVisible() expect(within(runtimeRow).queryByText('runtime.ts')).not.toBeInTheDocument() + expect(screen.getByLabelText('deleted')).toHaveTextContent('D') fireEvent.click(within(runtimeRow).getByRole('button')) - expect(within(runtimeRow).getByText('runtime.ts')).toBeVisible() - expect(within(localRow).getByLabelText('deleted')).toHaveTextContent('D') - expect(within(runtimeRow).getByLabelText('renamed')).toHaveTextContent('R') + expect(screen.getByText('runtime.ts')).toBeVisible() + expect(screen.queryByText('local.ts')).not.toBeInTheDocument() + expect(screen.getByLabelText('renamed')).toHaveTextContent('R') }) }) diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsx index 9253346f9d4..633fe7d17d4 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsx @@ -4,7 +4,10 @@ import { ScrollArea } from '@/components/ui/scroll-area' import type { Worktree } from '../../../../shared/worktree/types' import { getWorktreeHostIdentity } from '../../../../shared/worktree/host-qualified-identity' import { DeleteWorktreeDirtyChangeHint } from './DeleteWorktreeDirtyChangeHint' -import type { DeleteWorktreeDirtyChangePreview } from './delete-worktree-dirty-change-counts' +import type { + DeleteWorktreeChangeCheckState, + DeleteWorktreeDirtyChangePreview +} from './delete-worktree-dirty-change-counts' import type { AppState } from '@/store/types' import { getDeleteStateForWorktreeHost } from './worktree-delete-state-host-match' import { getWorktreeDeleteErrorToShow } from './worktree-delete-error-display' @@ -46,7 +49,8 @@ export function DeleteWorktreeTargetPreview({ hostLabelById, deleteStateByWorktreeId, dirtyChangeCountsByWorktreeId, - dirtyChangePreviewsByWorktreeId + dirtyChangePreviewsByWorktreeId, + changeCheckStatesByWorktreeId }: { isBatchDelete: boolean worktree: Worktree | null @@ -56,6 +60,7 @@ export function DeleteWorktreeTargetPreview({ deleteStateByWorktreeId: AppState['deleteStateByWorktreeId'] dirtyChangeCountsByWorktreeId: ReadonlyMap dirtyChangePreviewsByWorktreeId: ReadonlyMap + changeCheckStatesByWorktreeId?: ReadonlyMap }): JSX.Element | null { const targetIdPrefix = useId() const collisionIds = getCollisionIds(collisionWorktrees) @@ -94,6 +99,9 @@ export function DeleteWorktreeTargetPreview({ ) : null} { 0 ) expect(getDeleteWorktreeDirtyChangePreviews(input).size).toBe(0) + expect(getDeleteWorktreeChangeCheckStates(input).size).toBe(0) + }) +}) + +describe('deletion check states', () => { + it('distinguishes pending, failed and completed reads on the target host', () => { + const pending = worktree('same', 'ssh:pending') + const failed = worktree('same', 'ssh:failed') + const clean = worktree('same', 'local') + const input = { + deleteTargets: [pending, failed, clean], + gitStatusByWorktree: { same: [{ path: 'wrong-host.ts' }] }, + gitStatusByWorktreeIdentity: new Map([ + [getWorktreeHostIdentity(failed), null], + [getWorktreeHostIdentity(clean), []] + ]), + repoMap: new Map() + } + expect(getDeleteWorktreeChangeCheckStates(input)).toEqual( + new Map([ + [getWorktreeHostIdentity(pending), 'checking'], + [getWorktreeHostIdentity(failed), 'unavailable'], + [getWorktreeHostIdentity(clean), 'complete'] + ]) + ) + expect(getDeleteWorktreeDirtyChangeCounts({ ...input, deleteStateByWorktreeId: {} }).size).toBe( + 0 + ) + }) + + it('uses a hydrated read for a target without an explicit host', () => { + const target = worktree('legacy') + const entries: GitStatusEntry[] = [{ path: 'pending.ts', status: 'modified', area: 'unstaged' }] + const input = { + deleteTargets: [target], + gitStatusByWorktree: {}, + gitStatusByWorktreeIdentity: new Map([[getWorktreeHostIdentity(target), entries]]), + repoMap: new Map() + } + expect(getDeleteWorktreeChangeCheckStates(input).get('legacy')).toBe('complete') + expect( + getDeleteWorktreeDirtyChangeCounts({ ...input, deleteStateByWorktreeId: {} }).get('legacy') + ).toBe(1) + expect(getDeleteWorktreeDirtyChangePreviews(input).get('legacy')?.files[0]?.path).toBe( + 'pending.ts' + ) }) }) diff --git a/src/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.ts b/src/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.ts index ce3bab3e6be..0fdd7cdd7e2 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.ts @@ -6,6 +6,43 @@ import type { WorktreeDeleteState } from '../../store/slices/worktree-helpers' import { normalizeRelativePath } from '@/lib/path' import { buildStatusMap } from '../right-sidebar/status-display' import { isFolderWorkspaceDelete } from './delete-worktree-dialog-copy' +import { getDeleteStateForWorktreeHost } from './worktree-delete-state-host-match' + +export type DeleteWorktreeChangeCheckState = 'checking' | 'complete' | 'unavailable' + +type DeleteWorktreeStatusInput = { + deleteTargets: readonly Worktree[] + gitStatusByWorktree: Record + gitStatusByWorktreeIdentity?: ReadonlyMap + repoMap: ReadonlyMap +} + +function getStatusEntries( + item: Worktree, + input: DeleteWorktreeStatusInput +): readonly Entry[] | null | undefined { + return item.hostId + ? input.gitStatusByWorktreeIdentity?.get(getWorktreeHostIdentity(item)) + : (input.gitStatusByWorktree[item.id] ?? + input.gitStatusByWorktreeIdentity?.get(getWorktreeHostIdentity(item))) +} + +export function getDeleteWorktreeChangeCheckStates( + input: DeleteWorktreeStatusInput +): Map { + const result = new Map() + for (const item of input.deleteTargets) { + if (item.isMainWorktree || isFolderWorkspaceDelete(input.repoMap, item)) { + continue + } + const entries = getStatusEntries(item, input) + result.set( + item.hostId ? getWorktreeHostIdentity(item) : item.id, + entries === null ? 'unavailable' : entries === undefined ? 'checking' : 'complete' + ) + } + return result +} export function orderDeleteWorktreeStatusHydrationTargets({ targets, @@ -30,7 +67,6 @@ export function orderDeleteWorktreeStatusHydrationTargets({ .sort((left, right) => left.rank - right.rank || left.index - right.index) .map(({ target }) => target) } -import { getDeleteStateForWorktreeHost } from './worktree-delete-state-host-match' export function getDeleteWorktreeDirtyChangeCounts({ deleteTargets, @@ -42,7 +78,7 @@ export function getDeleteWorktreeDirtyChangeCounts({ deleteTargets: readonly Worktree[] deleteStateByWorktreeId: Record gitStatusByWorktree: Record - gitStatusByWorktreeIdentity?: ReadonlyMap + gitStatusByWorktreeIdentity?: ReadonlyMap repoMap: ReadonlyMap }): Map { const result = new Map() @@ -55,11 +91,12 @@ export function getDeleteWorktreeDirtyChangeCounts({ item, deleteStateByWorktreeId )?.forceDeleteReason - const changeCount = ( - item.hostId - ? gitStatusByWorktreeIdentity?.get(getWorktreeHostIdentity(item)) - : gitStatusByWorktree[item.id] - )?.length + const changeCount = getStatusEntries(item, { + deleteTargets, + gitStatusByWorktree, + gitStatusByWorktreeIdentity, + repoMap + })?.length if ((changeCount ?? 0) > 0) { result.set(resultKey, changeCount ?? 0) } else if (forceDeleteReason === 'dirty') { @@ -109,7 +146,7 @@ export function getDeleteWorktreeDirtyChangePreviews({ }: { deleteTargets: readonly Worktree[] gitStatusByWorktree: Record - gitStatusByWorktreeIdentity?: ReadonlyMap + gitStatusByWorktreeIdentity?: ReadonlyMap repoMap: ReadonlyMap }): Map { const result = new Map() @@ -118,9 +155,12 @@ export function getDeleteWorktreeDirtyChangePreviews({ continue } const resultKey = item.hostId ? getWorktreeHostIdentity(item) : item.id - const entries = item.hostId - ? gitStatusByWorktreeIdentity?.get(getWorktreeHostIdentity(item)) - : gitStatusByWorktree[item.id] + const entries = getStatusEntries(item, { + deleteTargets, + gitStatusByWorktree, + gitStatusByWorktreeIdentity, + repoMap + }) if (entries?.length) { result.set(resultKey, getDeleteWorktreeDirtyChangePreview(entries)) } diff --git a/src/renderer/src/components/sidebar/delete-worktree-dirty-change-hydration.test.tsx b/src/renderer/src/components/sidebar/delete-worktree-dirty-change-hydration.test.tsx index 297bfa66a0e..14755d8ead7 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-dirty-change-hydration.test.tsx +++ b/src/renderer/src/components/sidebar/delete-worktree-dirty-change-hydration.test.tsx @@ -11,6 +11,7 @@ import { DeleteWorktreeTargetPreview } from './DeleteWorktreeTargetPreview' import { DeleteWorktreeLineageNotice } from './DeleteWorktreeLineageNotice' import { useDeleteWorktreeStatusHydration } from './use-delete-worktree-status-hydration' import { + getDeleteWorktreeChangeCheckStates, getDeleteWorktreeDirtyChangeCounts, getDeleteWorktreeDirtyChangePreview, getDeleteWorktreeDirtyChangePreviews @@ -92,6 +93,7 @@ function Preview({ worktree }: { worktree: Worktree }): JSX.Element { collisionWorktrees={targets} hostLabelById={new Map()} deleteStateByWorktreeId={input.deleteStateByWorktreeId} + changeCheckStatesByWorktreeId={getDeleteWorktreeChangeCheckStates(input)} dirtyChangeCountsByWorktreeId={getDeleteWorktreeDirtyChangeCounts(input)} dirtyChangePreviewsByWorktreeId={getDeleteWorktreeDirtyChangePreviews(input)} /> @@ -102,6 +104,28 @@ afterEach(cleanup) beforeEach(() => vi.clearAllMocks()) describe('loaded deletion disclosure and existing hydration', () => { + it('shows pending and failed detail checks alongside a known dirty warning', async () => { + let failRead: (error: Error) => void = () => { + throw new Error('Status read has not started') + } + vi.mocked(getRuntimeGitStatus).mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + failRead = reject + }) + ) + render() + expect(screen.getByText('Uncommitted or untracked changes')).toBeVisible() + expect(screen.getByText('· Checking…')).toBeVisible() + await act(async () => { + failRead(new Error('Details unavailable')) + }) + expect(screen.getByText('Uncommitted or untracked changes')).toBeVisible() + expect(screen.getByText('· Details unavailable')).toBeVisible() + expect(screen.queryByRole('button')).not.toBeInTheDocument() + expect(screen.queryByText(/No uncommitted|0 changes/)).not.toBeInTheDocument() + }) + it('expands and collapses a hydrated snapshot without requesting status again', async () => { vi.mocked(getRuntimeGitStatus).mockResolvedValue({ entries: [ @@ -155,6 +179,7 @@ describe('loaded deletion disclosure and existing hydration', () => { }) await waitFor(() => expect(getRuntimeGitStatus).toHaveBeenCalledTimes(2)) expect(screen.getByText('Uncommitted or untracked changes')).toBeVisible() + expect(screen.getByText('· Details unavailable')).toBeVisible() expect(screen.queryByRole('button')).not.toBeInTheDocument() expect(screen.queryByText('old-host.ts')).not.toBeInTheDocument() expect(screen.queryByText(/No files|clean|0 changes/)).not.toBeInTheDocument() diff --git a/src/renderer/src/components/sidebar/use-delete-worktree-status-hydration.ts b/src/renderer/src/components/sidebar/use-delete-worktree-status-hydration.ts index 475f0f0d1de..5ed3fd6efd0 100644 --- a/src/renderer/src/components/sidebar/use-delete-worktree-status-hydration.ts +++ b/src/renderer/src/components/sidebar/use-delete-worktree-status-hydration.ts @@ -12,7 +12,7 @@ import { getWorktreeHostIdentity } from '../../../../shared/worktree/host-qualif import { isFolderWorkspaceDelete } from './delete-worktree-dialog-copy' import { orderDeleteWorktreeStatusHydrationTargets } from './delete-worktree-dirty-change-counts' -const EMPTY_STATUS_BY_IDENTITY = new Map() +const EMPTY_STATUS_BY_IDENTITY = new Map() export function useDeleteWorktreeStatusHydration({ isOpen, @@ -24,14 +24,14 @@ export function useDeleteWorktreeStatusHydration({ deleteTargets: readonly Worktree[] visibleTargets: readonly Worktree[] repoMap: ReadonlyMap -}): ReadonlyMap { +}): ReadonlyMap { const repos = useAppStore((state) => state.repos) const settings = useAppStore((state) => state.settings) const generation = isOpen ? deleteTargets.map(getWorktreeHostIdentity).join('\n') : '' const generationRef = useRef(generation) - const [statusByIdentity, setStatusByIdentity] = useState>( - () => new Map() - ) + const [statusByIdentity, setStatusByIdentity] = useState< + Map + >(() => new Map()) const currentStatusByIdentity = generationRef.current === generation ? statusByIdentity : EMPTY_STATUS_BY_IDENTITY @@ -89,7 +89,9 @@ export function useDeleteWorktreeStatusHydration({ } }) .catch(() => { - // Best effort only; deletion performs the authoritative backend check. + if (!controller.signal.aborted && generationRef.current === generation) { + setStatusByIdentity((current) => new Map(current).set(identity, null)) + } }) } return () => { diff --git a/src/renderer/src/i18n/en-runtime-required.json b/src/renderer/src/i18n/en-runtime-required.json index 9d6e29239ff..cc4ab94202a 100644 --- a/src/renderer/src/i18n/en-runtime-required.json +++ b/src/renderer/src/i18n/en-runtime-required.json @@ -1885,6 +1885,9 @@ "CacheTimer": { "07729cc155": "expired" }, + "DeleteWorktreeDirtyChangeHint": { + "8e2994ce28": "Deleting this workspace permanently removes these changes from disk." + }, "FolderWorkspaceComposerDialog": { "chooseSourceProject": "Choose task source", "connectFailed": "Failed to connect to project.", @@ -2883,7 +2886,6 @@ "openCurrentConversation": "Open the current conversation to continue.", "optionRejected": "The agent didn't accept this setting.", "outcomeUnknown": "Orca couldn't confirm what happened. Check the chat.", - "sendOutcomeLost": "Orca couldn't confirm your message reached the agent. Check the chat, then send it again if needed.", "ownerUnproven": "The previous agent in this chat may still be running.", "promptPending": "The agent is waiting for an answer to a question or approval.", "questionChanged": "This question was already answered or has changed.", @@ -2893,6 +2895,7 @@ "reopenChat": "Reopen the chat to check again.", "restartFailed": "The agent couldn't restart.", "savedByNewerOrca": "Chats were saved by a newer Orca.", + "sendOutcomeLost": "Orca couldn't confirm your message reached the agent. Check the chat, then send it again if needed.", "settleEarlierMessage": "Wait for your earlier message to go through, or retry it.", "startNewChat": "Start a new chat to continue.", "terminalAgentHoldsChat": "This chat is still open in a terminal agent.", diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 96802dc42f0..8991d2192cb 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -18360,6 +18360,16 @@ "workspaceReadyToast": "Workspace {{name}} is ready", "goToWorktree": "Go to worktree", "goToWorkspace": "Go to workspace" + }, + "delete": { + "changes": { + "permanentLoss": "Any uncommitted or untracked changes in Git workspaces will be permanently deleted.", + "clean": "No uncommitted or untracked changes", + "unavailable": "Changes could not be checked", + "checking": "Checking for changes…", + "checkingDetails": "Checking…", + "unavailableDetails": "Details unavailable" + } } }, "agentSessionContinuation": { diff --git a/tests/e2e/worktree-delete-dialog-layout.spec.ts b/tests/e2e/worktree-delete-dialog-layout.spec.ts new file mode 100644 index 00000000000..f2bf5804f30 --- /dev/null +++ b/tests/e2e/worktree-delete-dialog-layout.spec.ts @@ -0,0 +1,194 @@ +import { expect, test } from './helpers/orca-app' +import { waitForSessionReady } from './helpers/store' + +test.use({ minimumSeededWorktreeCount: 1 }) + +for (const scenario of ['single', 'children', 'batch', 'known-dirty'] as const) { + test(`keeps deletion geometry stable while checking ${scenario} workspaces`, async ({ + orcaPage: page, + electronApp + }, testInfo) => { + await waitForSessionReady(page) + if (scenario === 'batch') { + await page.emulateMedia({ reducedMotion: 'reduce' }) + } + await electronApp.evaluate(({ ipcMain }, knownDirty) => { + ipcMain.removeHandler('git:status') + ipcMain.handle('git:status', async (_event, args: { worktreePath: string }) => { + const childIndex = Number(args.worktreePath.match(/child-(\d+)$/)?.[1] ?? 0) + await new Promise((resolve) => setTimeout(resolve, 1800 + (childIndex % 3) * 180)) + if ( + args.worktreePath.endsWith('child-2') || + (knownDirty && args.worktreePath.endsWith('delete-layout-parent')) + ) { + throw new Error('Simulated disconnected execution host') + } + return { + entries: args.worktreePath.endsWith('child-1') + ? [] + : [{ path: 'src/pending-work.ts', status: 'modified', area: 'unstaged' }], + conflictOperation: 'unknown' + } + }) + }, scenario === 'known-dirty') + await page.evaluate((mode) => { + const store = window.__store + const state = store?.getState() + const repo = state?.repos[0] + const source = repo && state?.worktreesByRepo[repo.id]?.[0] + if (!store || !state || !repo || !source) { + throw new Error('Missing seeded workspace') + } + const parent = { + ...source, + id: 'delete-layout-parent', + instanceId: 'delete-layout-parent-instance', + displayName: 'workspace-with-pending-work', + path: `${repo.path}/delete-layout-parent`, + hostId: 'local' as const, + isMainWorktree: false, + lineage: null + } + const children = Array.from( + { length: mode === 'children' ? 31 : mode === 'batch' ? 3 : 0 }, + (_, index) => ({ + ...parent, + id: `delete-layout-child-${index}`, + instanceId: `delete-layout-child-instance-${index}`, + displayName: `child-workspace-${index}`, + path: `${repo.path}/delete-layout-child-${index}`, + lineage: + mode === 'children' + ? { + worktreeId: `delete-layout-child-${index}`, + worktreeInstanceId: `delete-layout-child-instance-${index}`, + parentWorktreeId: parent.id, + parentWorktreeInstanceId: parent.instanceId, + origin: 'manual' as const, + capture: { source: 'manual-action' as const, confidence: 'explicit' as const }, + createdAt: 1 + } + : null + }) + ) + store.setState({ + ...(state.settings + ? { + settings: { ...state.settings, theme: mode === 'children' ? 'dark' : 'light' } + } + : {}), + worktreesByRepo: { + ...state.worktreesByRepo, + [repo.id]: [...state.worktreesByRepo[repo.id], parent, ...children] + }, + gitStatusByWorktree: {}, + ...(mode === 'known-dirty' + ? { + deleteStateByWorktreeId: { + [parent.id]: { + isDeleting: false, + error: null, + canForceDelete: true, + forceDeleteReason: 'dirty', + executionHostId: parent.hostId + } + } + } + : {}) + }) + state.openModal( + 'delete-worktree', + mode === 'batch' + ? { worktreeIds: children.map((child) => child.id), allowSkipConfirm: false } + : { worktreeId: parent.id, allowSkipConfirm: false } + ) + }, scenario) + const dialog = page.getByRole('dialog', { name: /^Delete Workspace/ }) + await expect(dialog).toBeVisible() + await page.waitForTimeout(250) + await expect( + dialog.getByText(scenario === 'known-dirty' ? '· Checking…' : 'Checking for changes…').first() + ).toBeVisible() + await expect(dialog.getByText(/will be permanently deleted/)).toHaveCount(1) + await page.screenshot({ path: testInfo.outputPath('checking.png') }) + await dialog.screenshot({ path: testInfo.outputPath('checking-dialog.png') }) + const frames = await page.evaluate(async () => { + const samples: { + dialog: number[] + button: number[] + targets: number[] + confirmFocused: boolean + }[] = [] + const started = performance.now() + while (performance.now() - started < 2400) { + const dialog = document.querySelector('[data-slot="dialog-content"]') + const button = dialog?.querySelector('[data-slot="dialog-footer"] button:last-child') + if (!(dialog instanceof HTMLElement) || !(button instanceof HTMLElement)) { + throw new Error('Missing dialog') + } + const bounds = (element: Element): number[] => { + const rect = element.getBoundingClientRect() + return [rect.x, rect.y, rect.width, rect.height] + } + samples.push({ + dialog: bounds(dialog), + button: bounds(button), + confirmFocused: document.activeElement === button, + targets: Array.from(dialog.querySelectorAll('[role="listitem"], [role="region"], div')) + .filter( + (element) => + element.matches('[role="listitem"], [role="region"]') || + /^child-workspace-\d+$/.test(element.textContent ?? '') + ) + .flatMap(bounds) + }) + await new Promise((resolve) => requestAnimationFrame(() => resolve())) + } + return samples + }) + await expect( + dialog + .getByText( + scenario === 'known-dirty' + ? 'Uncommitted or untracked changes' + : '1 uncommitted or untracked change', + { exact: true } + ) + .first() + ).toBeVisible() + if (scenario === 'known-dirty') { + await expect(dialog.getByText('· Details unavailable')).toBeVisible() + } else if (scenario !== 'single') { + await expect(dialog.getByText('No uncommitted or untracked changes')).toBeVisible() + await expect(dialog.getByText('Changes could not be checked')).toBeVisible() + } + await page.screenshot({ path: testInfo.outputPath('loaded.png') }) + await dialog.screenshot({ path: testInfo.outputPath('loaded-dialog.png') }) + expect(frames.length).toBeGreaterThan(20) + expect(frames[0]?.confirmFocused).toBe(true) + for (const frame of frames) { + expect(frame).toEqual(frames[0]) + } + if (scenario === 'known-dirty') { + await expect(dialog.getByRole('button', { name: /Show loaded paths/ })).toHaveCount(0) + await dialog.getByRole('button', { name: 'Cancel', exact: true }).click() + await expect(dialog).toBeHidden() + return + } + const beforeDetails = await dialog.boundingBox() + await dialog + .getByRole('button', { name: /Show loaded paths/ }) + .first() + .click() + await expect(page.getByText('src/pending-work.ts', { exact: true })).toBeVisible() + await page.waitForTimeout(250) + await page.screenshot({ path: testInfo.outputPath('details.png') }) + await dialog.screenshot({ path: testInfo.outputPath('details-dialog.png') }) + expect(await dialog.boundingBox()).toEqual(beforeDetails) + await page.keyboard.press('Escape') + await expect(dialog).toBeVisible() + await expect(page.getByText('src/pending-work.ts', { exact: true })).toBeHidden() + await dialog.getByRole('button', { name: 'Cancel', exact: true }).click() + await expect(dialog).toBeHidden() + }) +}