diff --git a/src/main/git/remove-worktree-clean-preflight.test.ts b/src/main/git/remove-worktree-clean-preflight.test.ts index ebaec39cee4..b872d1fc721 100644 --- a/src/main/git/remove-worktree-clean-preflight.test.ts +++ b/src/main/git/remove-worktree-clean-preflight.test.ts @@ -43,7 +43,8 @@ beforeEach(() => { describe('assertWorktreeCleanForRemoval', () => { beforeEach(() => { - gitExecFileAsyncMock.mockReset() + // Calls after the status read are the dirty refusal's `.gitmodules` probe: no file, exit 1. + gitExecFileAsyncMock.mockReset().mockRejectedValue(new Error('git config exited with code 1')) }) it('returns without checking git status for force removals', async () => { @@ -57,7 +58,7 @@ describe('assertWorktreeCleanForRemoval', () => { await expect(assertWorktreeCleanForRemoval('/repo-feature')).resolves.toBeUndefined() expect(gitExecFileAsyncMock).toHaveBeenCalledWith( - ['status', '--porcelain', '--untracked-files=all'], + ['status', '--porcelain', '--untracked-files=all', '--ignore-submodules=none'], { cwd: '/repo-feature', timeout: WORKTREE_REMOVAL_PREFLIGHT_TIMEOUT_MS } ) }) @@ -81,7 +82,7 @@ describe('assertWorktreeCleanForRemoval', () => { ).resolves.toBeUndefined() expect(gitExecFileAsyncMock).toHaveBeenCalledWith( - ['status', '--porcelain', '-z', '--untracked-files=all'], + ['status', '--porcelain', '-z', '--untracked-files=all', '--ignore-submodules=none'], { cwd: '/repo-feature', timeout: WORKTREE_REMOVAL_PREFLIGHT_TIMEOUT_MS } ) }) @@ -94,7 +95,7 @@ describe('assertWorktreeCleanForRemoval', () => { ).rejects.toThrow('git timed out.') expect(gitExecFileAsyncMock).toHaveBeenCalledWith( - ['status', '--porcelain', '--untracked-files=all'], + ['status', '--porcelain', '--untracked-files=all', '--ignore-submodules=none'], { cwd: '/repo-feature', timeout: 25 } ) }) diff --git a/src/main/git/worktree-removal-preflight.ts b/src/main/git/worktree-removal-preflight.ts index 1e3ee3e8c3d..64ea39f8991 100644 --- a/src/main/git/worktree-removal-preflight.ts +++ b/src/main/git/worktree-removal-preflight.ts @@ -1,3 +1,6 @@ +import { stat } from 'node:fs/promises' +import { join } from 'node:path' +import { SUBMODULE_DIRTY_REMOVAL_DETAIL } from '../../shared/worktree/removal' import { gitExecFileAsync } from './runner' import type { WorktreeRemovalPreflightOptions } from './worktree-operation-options' import { WORKTREE_REMOVAL_PREFLIGHT_TIMEOUT_MS, gitExecOptions } from './worktree-operation-options' @@ -17,7 +20,14 @@ export async function assertWorktreeCleanForRemoval( const { ignoredUntrackedPaths = [], ...gitOptions } = options const useNullTerminatedStatus = ignoredUntrackedPaths.length > 0 const { stdout } = await gitExecFileAsync( - ['status', '--porcelain', ...(useNullTerminatedStatus ? ['-z'] : []), '--untracked-files=all'], + [ + 'status', + '--porcelain', + ...(useNullTerminatedStatus ? ['-z'] : []), + '--untracked-files=all', + // Why: a user's ignore-submodules setting would hide submodule edits from this dirty check. + '--ignore-submodules=none' + ], { ...gitExecOptions(worktreePath, gitOptions), timeout: gitOptions.timeout ?? WORKTREE_REMOVAL_PREFLIGHT_TIMEOUT_MS @@ -32,15 +42,53 @@ export async function assertWorktreeCleanForRemoval( return } - const error = new Error('Worktree has uncommitted or untracked changes.') + const blockingStatus = blockingEntries ? blockingEntries.join('\n') : stdout + if (await hasPopulatedSubmodule(worktreePath, gitOptions)) { + // Why: Force Delete also destroys the submodules' never-pushed commits, which the parent's + // status cannot show, so the refusal must say submodules. Status lines go after the detail + // because the formatter prefers stdout over the message. + throw new Error(`${SUBMODULE_DIRTY_REMOVAL_DETAIL}\n${blockingStatus.trim()}`) + } + // Why not the raw stdout: `-z` output is NUL-delimited and `.trim()` leaves // interior NULs, so attaching it verbatim put raw control bytes into the // user-facing removal error — and listed the tolerated shared link, the one // entry that is not the user's work and cannot be committed away. - ;(error as Error & { stdout?: string }).stdout = blockingEntries - ? blockingEntries.join('\n') - : stdout - throw error + throw Object.assign(new Error('Worktree has uncommitted or untracked changes.'), { + stdout: blockingStatus + }) +} + +/** Git's main submodule refusal test: a `.gitmodules` path whose directory holds a `.git`. */ +async function hasPopulatedSubmodule( + worktreePath: string, + gitOptions: Omit +): Promise { + try { + const { stdout } = await gitExecFileAsync( + ['config', '-z', '--file', '.gitmodules', '--get-regexp', '^submodule\\..*\\.path$'], + { + ...gitExecOptions(worktreePath, gitOptions), + timeout: gitOptions.timeout ?? WORKTREE_REMOVAL_PREFLIGHT_TIMEOUT_MS + } + ) + const paths = stdout + .split('\0') + .map((entry) => entry.slice(entry.indexOf('\n') + 1)) + .filter(Boolean) + const populated = await Promise.all( + paths.map((path) => + stat(join(worktreePath, path, '.git')).then( + () => true, + () => false + ) + ) + ) + return populated.includes(true) + } catch { + // No .gitmodules exits non-zero. Any failure keeps the plain dirty refusal, which still blocks. + return false + } } /** The `git status --porcelain -z` entries that genuinely block removal: diff --git a/src/main/git/worktree-removal-real-git.test.ts b/src/main/git/worktree-removal-real-git.test.ts index 0f0641d2bf9..fba4ebb9594 100644 --- a/src/main/git/worktree-removal-real-git.test.ts +++ b/src/main/git/worktree-removal-real-git.test.ts @@ -9,7 +9,9 @@ import { performance } from 'node:perf_hooks' import { promisify } from 'node:util' import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { removeTree } from '../../shared/windows-transient-lock-removal' -import { listWorktreesStrict, removeWorktree } from './worktree' +import { classifyWorktreeForceDeleteReason } from '../../shared/worktree/removal' +import { formatWorktreeRemovalError } from '../ipc/worktree-logic' +import { assertWorktreeCleanForRemoval, listWorktreesStrict, removeWorktree } from './worktree' import { areWorktreePathsEqual } from './worktree-path-comparison' import { isPrunableGitFileWorktree } from '../worktree-prunable-git-file' import { removeStaleLocalWorktreeRegistration } from '../local-worktree-removal-recovery' @@ -95,6 +97,67 @@ describe('worktree removal against the real Git binary', () => { expect(await isRegistered(worktreePath)).toBe(true) }) + // A checkout whose initialized submodule holds a commit only it has; the parent does not record it. + async function addSubmoduleCheckoutWithLocalCommit(): Promise<{ + subWorktreePath: string + vendorPath: string + localCommit: string + }> { + const emptyCommit = ['-c', 'user.name=o', '-c', 'user.email=o@example.invalid', 'commit'] + const originPath = join(scratchDir, 'origin') + await mkdir(originPath) + await git(['init', '-q'], originPath) + await git([...emptyCommit, '-q', '--allow-empty', '-m', 'origin'], originPath) + await git( + ['-c', 'protocol.file.allow=always', 'submodule', 'add', '-q', originPath, 'vendor'], + repoPath + ) + await git(['commit', '-qm', 'add vendor'], repoPath) + const subWorktreePath = join(workspaceRoot, 'repo', 'with-sub') + await git(['worktree', 'add', '-q', subWorktreePath, '-b', 'with-sub'], repoPath) + await git( + ['-c', 'protocol.file.allow=always', 'submodule', 'update', '-q', '--init'], + subWorktreePath + ) + const vendorPath = join(subWorktreePath, 'vendor') + await git([...emptyCommit, '-q', '--allow-empty', '-m', 'local only'], vendorPath) + const localCommit = (await git(['rev-parse', 'HEAD'], vendorPath)).trim() + return { subWorktreePath, vendorPath, localCommit } + } + + it('refuses a clean checkout with a submodule and offers Force Delete for it', async () => { + // Why: --force would delete submodule commits that exist only in this checkout, so Git's + // refusal must reach the user as a Force Delete choice rather than be retried away. + const { subWorktreePath, vendorPath, localCommit } = await addSubmoduleCheckoutWithLocalCommit() + // Recorded by the parent so its status stays clean. + await git(['commit', '-qam', 'bump vendor'], subWorktreePath) + + const refusal = await removeWorktree(repoPath, subWorktreePath, false).catch((error) => error) + expect(refusal).toBeInstanceOf(Error) + expect( + classifyWorktreeForceDeleteReason(formatWorktreeRemovalError(refusal, subWorktreePath, false)) + ).toBe('submodules') + expect(await isRegistered(subWorktreePath)).toBe(true) + expect((await git(['cat-file', '-t', localCommit], vendorPath)).trim()).toBe('commit') + + await removeWorktree(repoPath, subWorktreePath, true) + expect(await isRegistered(subWorktreePath)).toBe(false) + }) + + it('names submodules when the dirty check refuses a checkout that has them', async () => { + // Why: the parent's status shows only its own file, yet Force Delete destroys the submodule commit. + const { subWorktreePath } = await addSubmoduleCheckoutWithLocalCommit() + await git(['commit', '-qam', 'bump vendor'], subWorktreePath) + await writeFile(join(subWorktreePath, 'notes.txt'), 'draft\n') + + const refusal = await assertWorktreeCleanForRemoval(subWorktreePath).catch((error) => error) + const message = formatWorktreeRemovalError(refusal, subWorktreePath, false) + expect(classifyWorktreeForceDeleteReason(message)).toBe('submodules') + expect(message).toContain('?? notes.txt') + // Older apps offer Force Delete only for this sentence. + expect(message).toContain('Worktree has uncommitted or untracked changes') + }) + it('does not delete a checkout through a malformed registration that names its git file', async () => { const markerPath = join(worktreePath, '.git') const marker = await readFile(markerPath, 'utf8') diff --git a/src/renderer/src/components/sidebar/delete-worktree-always-force-preference.test.tsx b/src/renderer/src/components/sidebar/delete-worktree-always-force-preference.test.tsx index c372f5204c6..346f82a15d6 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-always-force-preference.test.tsx +++ b/src/renderer/src/components/sidebar/delete-worktree-always-force-preference.test.tsx @@ -128,7 +128,7 @@ it('waits for the preference to persist before force-deleting and dismissing', a expect(toast.dismiss).not.toHaveBeenCalled() await act(async () => finishSaving?.()) expect(onForceDelete).toHaveBeenCalledOnce() - expect(toast.dismiss).toHaveBeenCalledWith('delete-worktree-failure:wt-1') + expect(toast.dismiss).toHaveBeenCalledWith('delete-worktree-failure:|wt-1') }) it('keeps recovery available when saving the preference fails', async () => { diff --git a/src/renderer/src/components/sidebar/delete-worktree-failure-toast.test.tsx b/src/renderer/src/components/sidebar/delete-worktree-failure-toast.test.tsx index 861d1e9e0d5..14e13b4fd1d 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-failure-toast.test.tsx +++ b/src/renderer/src/components/sidebar/delete-worktree-failure-toast.test.tsx @@ -67,7 +67,7 @@ describe('showDeleteWorktreeFailureToast', () => { expect(toast.info).toHaveBeenCalledWith( 'Failed to delete workspace feature/foo', expect.objectContaining({ - id: 'delete-worktree-failure:wt-1', + id: 'delete-worktree-failure:|wt-1', dismissible: true, duration: Infinity }) @@ -82,11 +82,11 @@ describe('showDeleteWorktreeFailureToast', () => { expect(body.textContent).toContain('It has changed files.') clickButton(body, 'View') - expect(toast.dismiss).toHaveBeenCalledWith('delete-worktree-failure:wt-1') + expect(toast.dismiss).toHaveBeenCalledWith('delete-worktree-failure:|wt-1') expect(onViewChanges).toHaveBeenCalled() clickButton(body, 'Force Delete') - expect(toast.dismiss).toHaveBeenCalledWith('delete-worktree-failure:wt-1') + expect(toast.dismiss).toHaveBeenCalledWith('delete-worktree-failure:|wt-1') expect(onForceDelete).toHaveBeenCalled() }) @@ -107,7 +107,7 @@ describe('showDeleteWorktreeFailureToast', () => { expect(toast.error).toHaveBeenCalledWith( 'Failed to delete workspace feature/bar', expect.objectContaining({ - id: 'delete-worktree-failure:wt-2', + id: 'delete-worktree-failure:|wt-2', dismissible: true, duration: 10000 }) @@ -118,7 +118,7 @@ describe('showDeleteWorktreeFailureToast', () => { expect(body.textContent).not.toContain('Force Delete') clickButton(body, 'View') - expect(toast.dismiss).toHaveBeenCalledWith('delete-worktree-failure:wt-2') + expect(toast.dismiss).toHaveBeenCalledWith('delete-worktree-failure:|wt-2') expect(onViewChanges).toHaveBeenCalled() }) @@ -150,7 +150,7 @@ describe('showDeleteWorktreeFailureToast', () => { expect(body.textContent).not.toContain('Force Delete') clickButton(body, 'Delete Anyway') - expect(toast.dismiss).toHaveBeenCalledWith('delete-worktree-failure:wt-archive') + expect(toast.dismiss).toHaveBeenCalledWith('delete-worktree-failure:|wt-archive') expect(onDeleteAnyway).toHaveBeenCalled() }) diff --git a/src/renderer/src/components/sidebar/delete-worktree-failure-toast.tsx b/src/renderer/src/components/sidebar/delete-worktree-failure-toast.tsx index 8dc934ae426..e32050f755f 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-failure-toast.tsx +++ b/src/renderer/src/components/sidebar/delete-worktree-failure-toast.tsx @@ -5,8 +5,11 @@ import { translate } from '@/i18n/i18n' import { DeleteNestedWorktreesDialog } from './DeleteNestedWorktreesDialog' import { WorktreeForceDeleteButton } from './WorktreeForceDeleteButton' import { isNestedWorktreeRemovalError } from '../../../../shared/worktree/nested-removal' +import { composeWorktreeHostIdentity } from '../../../../shared/worktree/host-qualified-identity' +import type { ExecutionHostId } from '../../../../shared/execution-host' import { isLockedWorktreeRemovalError, + SUBMODULE_DIRTY_REMOVAL_DETAIL, type WorktreeForceDeleteReason, type WorktreeRemovalTarget } from '../../../../shared/worktree/removal' @@ -24,13 +27,20 @@ type DeleteWorktreeFailureToastOptions = { onAlwaysForceDelete?: () => Promise onDeleteAnyway: () => void worktreeId: string + executionHostId?: ExecutionHostId | null worktreeName: string nestedRemovalTarget?: WorktreeRemovalTarget onNestedDeleted?: () => void } -function deleteWorktreeFailureToastId(worktreeId: string): string { - return `delete-worktree-failure:${worktreeId}` +// Host-qualified: the same worktree id can exist on two hosts. +function deleteWorktreeFailureToastId(target: WorktreeRemovalTarget): string { + return `delete-worktree-failure:${composeWorktreeHostIdentity(target.executionHostId ?? undefined, target.id)}` +} + +/** A workspace deleted from another surface must not keep a toast offering to delete it. */ +export function dismissDeleteWorktreeFailureToast(target: WorktreeRemovalTarget): void { + toast.dismiss(deleteWorktreeFailureToastId(target)) } function DeleteWorktreeFailureToastBody({ @@ -120,6 +130,7 @@ export function showDeleteWorktreeFailureToast({ onAlwaysForceDelete, onDeleteAnyway, worktreeId, + executionHostId, worktreeName, nestedRemovalTarget, onNestedDeleted @@ -131,7 +142,10 @@ export function showDeleteWorktreeFailureToast({ lockReason ?? null ) const showToast = toastCopy.isDestructive ? toast.error : toast.info - const id = deleteWorktreeFailureToastId(worktreeId) + const id = deleteWorktreeFailureToastId({ + id: worktreeId, + executionHostId: executionHostId ?? null + }) const nestedTarget = isNestedWorktreeRemovalError(error) && window.api?.worktrees.previewNestedRemoval ? nestedRemovalTarget @@ -146,7 +160,12 @@ export function showDeleteWorktreeFailureToast({ description={toastCopy.description} canForceDelete={canForceDelete} canWaiveArchiveHook={canWaiveArchiveHook === true} - showViewChanges={!isLockedWorktreeRemovalError(error) || hasKnownChanges === true} + showViewChanges={ + !( + isLockedWorktreeRemovalError(error) || + (forceDeleteReason === 'submodules' && !error.includes(SUBMODULE_DIRTY_REMOVAL_DETAIL)) + ) || hasKnownChanges === true + } onViewChanges={onViewChanges} onForceDelete={onForceDelete} onAlwaysForceDelete={onAlwaysForceDelete} diff --git a/src/renderer/src/components/sidebar/delete-worktree-toast.ts b/src/renderer/src/components/sidebar/delete-worktree-toast.ts index dc32abfc40f..2a9bbc0471b 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-toast.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-toast.ts @@ -11,6 +11,13 @@ export type DeleteWorktreeToastCopy = { isDestructive: boolean } +export function getSubmoduleRemovalRefusalCopy(): string { + return translate( + 'auto.components.sidebar.delete.worktree.toast.submodules', + 'This workspace contains submodules, so Git deletes it only with Force Delete. Force Delete also permanently removes anything in those submodules that was never pushed: commits, local branches and stashes.' + ) +} + export function getDeleteWorktreeToastCopy( worktreeName: string, forceDeleteReason: WorktreeForceDeleteReason | null, @@ -98,6 +105,17 @@ export function getDeleteWorktreeToastCopy( isDestructive: false } } + if (forceDeleteReason === 'submodules') { + return { + title: translate( + 'auto.components.sidebar.delete.worktree.toast.1d0fa5c0a5', + 'Failed to delete workspace {{value0}}', + { value0: worktreeName } + ), + description: getSubmoduleRemovalRefusalCopy(), + isDestructive: false + } + } if (forceDeleteReason === 'missing-registration') { return { title: translate( diff --git a/src/renderer/src/components/sidebar/run-worktree-delete-with-toast.ts b/src/renderer/src/components/sidebar/run-worktree-delete-with-toast.ts index 640ac5fc15a..d8cc321b89e 100644 --- a/src/renderer/src/components/sidebar/run-worktree-delete-with-toast.ts +++ b/src/renderer/src/components/sidebar/run-worktree-delete-with-toast.ts @@ -78,6 +78,7 @@ export function runWorktreeDeleteWithToast( withViewAction: true }), worktreeId, + executionHostId: target.executionHostId, worktreeName, nestedRemovalTarget: route && !route.runtimeEnvironmentId ? target : undefined, onNestedDeleted: () => options.onForceDeleted?.(target) diff --git a/src/renderer/src/components/sidebar/worktree-delete-error-display.ts b/src/renderer/src/components/sidebar/worktree-delete-error-display.ts index f4697df6b72..e8c4c5b8cf5 100644 --- a/src/renderer/src/components/sidebar/worktree-delete-error-display.ts +++ b/src/renderer/src/components/sidebar/worktree-delete-error-display.ts @@ -1,4 +1,11 @@ +import { isSubmoduleWorktreeRemovalRefusal } from '../../../../shared/worktree/removal' import type { Worktree } from '../../../../shared/worktree/types' +import { getSubmoduleRemovalRefusalCopy } from './delete-worktree-toast' + +/** A delete error as a user reads it: Git's submodule refusal becomes plain copy. */ +export function describeWorktreeDeleteError(error: string): string { + return isSubmoduleWorktreeRemovalRefusal(error) ? getSubmoduleRemovalRefusalCopy() : error +} /** * The delete error to show for a row. A failed delete the host lists wins over renderer state, so @@ -8,5 +15,6 @@ export function getWorktreeDeleteErrorToShow( row: Pick | null | undefined, state: { isDeleting: boolean; error: string | null } | undefined ): string | null { - return row?.removalError && !state?.isDeleting ? row.removalError : (state?.error ?? null) + const error = row?.removalError && !state?.isDeleting ? row.removalError : (state?.error ?? null) + return error === null ? null : describeWorktreeDeleteError(error) } diff --git a/src/renderer/src/components/status-bar/use-workspace-space-manager-panel.ts b/src/renderer/src/components/status-bar/use-workspace-space-manager-panel.ts index 9380c30b8b7..6002bc8e6eb 100644 --- a/src/renderer/src/components/status-bar/use-workspace-space-manager-panel.ts +++ b/src/renderer/src/components/status-bar/use-workspace-space-manager-panel.ts @@ -1,11 +1,15 @@ import { useCallback } from 'react' import { toast } from 'sonner' import type { WorkspaceSpaceWorktree } from '../../../../shared/workspace-space-types' -import type { WorktreeRemovalTarget } from '../../../../shared/worktree/removal' +import { + toWorktreeRemovalTarget, + type WorktreeRemovalTarget +} from '../../../../shared/worktree/removal' import { composeWorktreeHostIdentity } from '../../../../shared/worktree/host-qualified-identity' import { useAppStore } from '../../store' import { getWorktreeOnHostFromState } from '../../store/selectors' import { runWorktreeBatchDelete } from '../sidebar/delete-worktree-flow' +import { dismissDeleteWorktreeFailureToast } from '../sidebar/delete-worktree-failure-toast' import { showWorkspaceListChangedToast } from '../sidebar/stale-workspace-list-toast' import { toWorktreeDeleteIdentities } from '../sidebar/worktree-delete-request' import { prepareActiveWorktreeFocusAfterDelete } from '../sidebar/active-worktree-focus-after-delete' @@ -103,6 +107,9 @@ export function useWorkspaceSpaceManagerPanel() { return } removeWorkspaceSpaceWorktrees(deletedTargets) + for (const target of deletedTargets) { + dismissDeleteWorktreeFailureToast(target) + } void window.api.workspaceSpace.requestWorktreeRefresh?.() const deletedIdentities = new Set( deletedTargets.map((target) => @@ -185,6 +192,12 @@ export function useWorkspaceSpaceManagerPanel() { // Why: Space keeps normal deletes non-force so uncommitted work is not // discarded silently; a failed row gets this explicit recovery path. const commitFocus = prepareActiveWorktreeFocusAfterDelete(worktree.worktreeId) + // The failure toast is keyed by the store row's host, which a host-less saved Space row lacks. + const storeRow = getWorktreeOnHostFromState( + useAppStore.getState(), + worktree.worktreeId, + worktree.executionHostId ?? undefined + ) // Why (#11960): explicit force recovery, so it may also waive PTY-stop proof. void removeWorktree( { id: worktree.worktreeId, executionHostId: worktree.executionHostId ?? null }, @@ -203,6 +216,9 @@ export function useWorkspaceSpaceManagerPanel() { return } commitFocus() + if (storeRow) { + dismissDeleteWorktreeFailureToast(toWorktreeRemovalTarget(storeRow)) + } handleDeletedWorktrees([ { id: worktree.worktreeId, executionHostId: worktree.executionHostId ?? null } ]) diff --git a/src/renderer/src/components/status-bar/workspace-space-worktree-row.tsx b/src/renderer/src/components/status-bar/workspace-space-worktree-row.tsx index a2da169b3d7..c9be0652fe6 100644 --- a/src/renderer/src/components/status-bar/workspace-space-worktree-row.tsx +++ b/src/renderer/src/components/status-bar/workspace-space-worktree-row.tsx @@ -24,6 +24,7 @@ import { CheckButton } from './workspace-space-selection-controls' import { WorkspaceDecisionHoverCard } from './workspace-space-decision-hover-card' import { WorkspaceSpaceStatusBadge } from './workspace-space-status-badge' import { WorkspaceSpaceSizeBar } from './workspace-space-breakdown-list' +import { describeWorktreeDeleteError } from '../sidebar/worktree-delete-error-display' export function WorkspaceSpaceWorktreeRow({ worktree, @@ -53,7 +54,7 @@ export function WorkspaceSpaceWorktreeRow({ onForceDelete: () => void }): React.JSX.Element { const isDeleting = deleteState?.isDeleting ?? false - const deleteError = deleteState?.error ?? null + const deleteError = deleteState?.error ? describeWorktreeDeleteError(deleteState.error) : null const canForceDelete = deleteState?.canForceDelete ?? false const canDelete = isWorkspaceSpaceRowReadyToDelete(worktree, decisionDetails) && !isDeleting const handleForceDelete = (event: React.MouseEvent): void => { diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 1700f0de34b..8d492659482 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -6258,7 +6258,8 @@ "unstoppedPty": "Orca could not confirm every terminal in this workspace has exited, so it stopped before deleting any files. Use Force Delete to remove it anyway.", "unstoppedPtyLive": "This workspace still has running terminals, so Orca stopped before deleting any files. Force Delete will kill them and discard any uncommitted work they hold.", "runningAgentSession": "Orca could not confirm every agent session in this workspace has closed, so it stopped before deleting any files. Use Force Delete to remove it anyway.", - "runningAgentSessionLive": "This workspace still has running agent sessions that Orca could not close, so it stopped before deleting any files. Force Delete will discard any work they hold." + "runningAgentSessionLive": "This workspace still has running agent sessions that Orca could not close, so it stopped before deleting any files. Force Delete will discard any work they hold.", + "submodules": "This workspace contains submodules, so Git deletes it only with Force Delete. Force Delete also permanently removes anything in those submodules that was never pushed: commits, local branches and stashes." }, "failure": { "archive": { diff --git a/src/shared/git-command-failure-text.ts b/src/shared/git-command-failure-text.ts index 92265cab98d..06a3ac72ebd 100644 --- a/src/shared/git-command-failure-text.ts +++ b/src/shared/git-command-failure-text.ts @@ -5,9 +5,8 @@ * Why all three streams and not just `message` + `stderr`: the errors Orca classifies * do not all come straight out of `execFile`. Node puts Git's stderr in both `message` * and `stderr`, but Orca also throws its own failures with the Git output on `stdout` - * (`worktree remove`'s submodule retry attaches `git status --porcelain` output that - * way on both the local runner and the relay). Reading all three is what keeps the - * local and relay classifiers from disagreeing about the same error object. + * (the removal dirty check attaches `git status --porcelain` output that way). Reading + * all three is what keeps the classifiers from disagreeing about the same error object. * * Against a real binary this reads no differently: Git emits every refusal this module * classifies through `error()`/`die()`, i.e. stderr only, on 2.25 through 2.55. diff --git a/src/shared/worktree/removal.ts b/src/shared/worktree/removal.ts index 237d73630df..48edfa68e1c 100644 --- a/src/shared/worktree/removal.ts +++ b/src/shared/worktree/removal.ts @@ -16,6 +16,7 @@ export type WorktreeForceDeleteReason = | 'missing-registration' | 'unstopped-pty' | 'running-agent-session' + | 'submodules' // Why: everything before this separator is the worktree id — a user-chosen filesystem path. // Only the detail after it is Orca's own wording, so verdict matchers anchor on the boundary @@ -104,6 +105,21 @@ export function getLockedWorktreeRemovalReason(error: string): string | null { return reason || null } +// Why: Orca's dirty check refuses before Git's submodule refusal can, so it says submodules itself. +// Keeps the dirty sentence so older classifiers still offer Force Delete. +export const SUBMODULE_DIRTY_REMOVAL_DETAIL = + 'Worktree has uncommitted or untracked changes, and it contains submodules.' + +// Why: non-force `git worktree remove` refuses any checkout with an initialized submodule, clean or +// not; only --force deletes it, and with it the submodule's never-pushed history. Git output is +// pinned to English on every host, and the renderer classifies, so older hosts get this too. +export function isSubmoduleWorktreeRemovalRefusal(error: string): boolean { + return ( + error.includes('working trees containing submodules cannot be moved or removed') || + error.includes(SUBMODULE_DIRTY_REMOVAL_DETAIL) + ) +} + const FORMATTED_DIRTY_WORKTREE_REMOVAL_PATTERN = /Failed to delete worktree at [^\n]*\.\s*(?:(?:[MADRCUT][ MADRCUT]| [MADRCUT]|\?\?)\s+\S)/ @@ -133,6 +149,9 @@ export function classifyWorktreeForceDeleteReason( if (force) { return null } + if (isSubmoduleWorktreeRemovalRefusal(error)) { + return 'submodules' + } if (error.includes('Worktree is no longer registered with Git but its directory remains')) { return 'orphan-directory' }