From a9cbc76065368e12f4486b474ccbe9b820483ef6 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 29 Aug 2026 08:46:22 -0700 Subject: [PATCH] fix(workspaces): stop a failed delete showing Electron's IPC envelope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workspace delete that the classifier does not recognise fell through to a branch that renders the failure string verbatim. For an IPC rejection that string is Electron's own wrapper — "Error invoking remote method 'worktrees:remove': ..." — so a safe refusal read as a broken app. Adds stripIpcInvokeEnvelope to the canonical ipc-error module: it returns the reason behind the wrapper, or null when the wrapper carried no readable reason (Electron builds the tail from the main side's error.toString(), so a message-less handler failure arrives as a bare class name). The eight user-facing sites in the delete flow now route through one copy function that falls back to a human sentence on null. Nothing is swallowed: deleteStateByWorktreeId keeps the raw string, the renderer still console.warns the rejection, and Electron itself logs the handler's original error with its stack in the main process. extractIpcErrorMessage keeps its fail-open contract byte-for-byte so its 18 existing call sites are unaffected. --- .../sidebar/DeleteWorktreeTargetPreview.tsx | 3 +- .../sidebar/DeleteWorktreeWarningPanels.tsx | 5 +- ...-worktree-dialog-force-delete-copy.test.ts | 63 ++++++++++++ .../delete-worktree-dialog-force-delete.ts | 5 +- .../sidebar/delete-worktree-toast.test.ts | 34 ++++++- .../sidebar/delete-worktree-toast.ts | 5 +- .../sidebar/worktree-removal-error-copy.ts | 34 +++++++ .../worktree-removal-error-surfaces.test.tsx | 85 ++++++++++++++++ .../status-bar/WorkspaceSpaceManagerPanel.tsx | 12 ++- ...rkspace-space-row-removal-failure.test.tsx | 99 +++++++++++++++++++ src/renderer/src/i18n/locales/en.json | 8 ++ src/renderer/src/lib/ipc-error.test.ts | 72 ++++++++++++++ src/renderer/src/lib/ipc-error.ts | 20 ++++ .../worktrees-metadata-persistence.test.ts | 20 ++++ .../teardown/force-delete-preserved-branch.ts | 3 +- 15 files changed, 457 insertions(+), 11 deletions(-) create mode 100644 src/renderer/src/components/sidebar/delete-worktree-dialog-force-delete-copy.test.ts create mode 100644 src/renderer/src/components/sidebar/worktree-removal-error-copy.ts create mode 100644 src/renderer/src/components/sidebar/worktree-removal-error-surfaces.test.tsx create mode 100644 src/renderer/src/components/status-bar/workspace-space-row-removal-failure.test.tsx create mode 100644 src/renderer/src/lib/ipc-error.test.ts diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsx index fa5a47715ca..a5e86ac7c32 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsx @@ -12,6 +12,7 @@ import { type ExecutionHostId } from '../../../../shared/execution-host' import { translate } from '@/i18n/i18n' +import { getWorktreeRemovalErrorCopy } from './worktree-removal-error-copy' function getCollisionIds(worktrees: readonly Worktree[]): ReadonlySet { const seen = new Set() @@ -94,7 +95,7 @@ export function DeleteWorktreeTargetPreview({ /> {itemDeleteState?.error ? (
- {itemDeleteState.error} + {getWorktreeRemovalErrorCopy(itemDeleteState.error)}
) : null} diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeWarningPanels.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeWarningPanels.tsx index b143148249c..7a8dd66bab2 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeWarningPanels.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeWarningPanels.tsx @@ -1,6 +1,7 @@ import type { JSX } from 'react' import { AlertTriangle } from 'lucide-react' import { translate } from '@/i18n/i18n' +import { getWorktreeRemovalErrorCopy } from './worktree-removal-error-copy' export function DeleteWorktreeWarningPanels({ isMainWorktree, @@ -42,7 +43,9 @@ export function DeleteWorktreeWarningPanels({
-
{deleteError}
+
+ {getWorktreeRemovalErrorCopy(deleteError)} +
)} diff --git a/src/renderer/src/components/sidebar/delete-worktree-dialog-force-delete-copy.test.ts b/src/renderer/src/components/sidebar/delete-worktree-dialog-force-delete-copy.test.ts new file mode 100644 index 00000000000..cdbbd5a4c73 --- /dev/null +++ b/src/renderer/src/components/sidebar/delete-worktree-dialog-force-delete-copy.test.ts @@ -0,0 +1,63 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const toastError = vi.fn() +vi.mock('sonner', () => ({ toast: { error: toastError, info: vi.fn() } })) +// Why: this pulls in the real app store; the focus commit is irrelevant to the toast copy. +vi.mock('./active-worktree-focus-after-delete', () => ({ + prepareActiveWorktreeFocusAfterDelete: () => () => {} +})) + +const { runDialogForceDelete } = await import('./delete-worktree-dialog-force-delete') +import type { Worktree } from '../../../../shared/worktree/types' + +const ENVELOPE_ONLY = "Error invoking remote method 'worktrees:remove': Error" +const UNREADABLE_COPY = + 'Orca could not delete this workspace, and the failure did not include a readable reason. Retry, and send app diagnostics to support if it keeps failing.' + +const worktree = { + id: 'repo1::/w/feature', + repoId: 'repo1', + path: '/w/feature', + head: 'abc123', + branch: 'feature', + isBare: false, + isMainWorktree: false, + displayName: 'feature' +} as Worktree + +function runForceDelete( + removeWorktree: () => Promise<{ ok: false; error: string }> +): Promise { + runDialogForceDelete({ + worktreeId: worktree.id, + currentWorktrees: [worktree], + removeWorktree: removeWorktree as never, + closeModal: () => {}, + onDeleted: null + }) + return Promise.resolve().then(() => { + // Two microtask turns: the removal promise, then the .then/.catch that toasts. + }) +} + +beforeEach(() => { + toastError.mockClear() +}) + +// Why: pressing Force Delete from the failure toast runs a second removal, and its own +// failure toast read `result.error` verbatim — the same Electron envelope one click later. +describe('dialog Force Delete failure copy', () => { + it('never shows the IPC envelope when the retry resolves as failed', async () => { + await runForceDelete(() => Promise.resolve({ ok: false as const, error: ENVELOPE_ONLY })) + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(toastError).toHaveBeenCalledTimes(1) + expect(toastError.mock.calls[0][1].description).toBe(UNREADABLE_COPY) + }) + + it('never shows the IPC envelope when the retry rejects', async () => { + await runForceDelete(() => Promise.reject(new Error(ENVELOPE_ONLY)) as never) + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(toastError).toHaveBeenCalledTimes(1) + expect(toastError.mock.calls[0][1].description).toBe(UNREADABLE_COPY) + }) +}) diff --git a/src/renderer/src/components/sidebar/delete-worktree-dialog-force-delete.ts b/src/renderer/src/components/sidebar/delete-worktree-dialog-force-delete.ts index 756952c0fac..563e866b6b0 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-dialog-force-delete.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-dialog-force-delete.ts @@ -9,6 +9,7 @@ import type { RemoveWorktreeOptions } from '@/store/slices/worktree-removal-opti import type { RendererRemoveWorktreeResult } from '@/store/slices/renderer-remove-worktree-result' import { prepareActiveWorktreeFocusAfterDelete } from './active-worktree-focus-after-delete' import { showWorkspaceListChangedToast } from './stale-workspace-list-toast' +import { getWorktreeRemovalErrorCopy } from './worktree-removal-error-copy' /** * The dialog's explicit "Force Delete" retry. @@ -60,7 +61,7 @@ export function runDialogForceDelete(args: { 'Force delete failed' ), { - description: result.error + description: getWorktreeRemovalErrorCopy(result.error) } ) return @@ -75,7 +76,7 @@ export function runDialogForceDelete(args: { 'Failed to delete workspace' ), { - description: err instanceof Error ? err.message : String(err) + description: getWorktreeRemovalErrorCopy(err instanceof Error ? err.message : String(err)) } ) }) diff --git a/src/renderer/src/components/sidebar/delete-worktree-toast.test.ts b/src/renderer/src/components/sidebar/delete-worktree-toast.test.ts index dffedf06337..ee59ec457ec 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-toast.test.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-toast.test.ts @@ -1,11 +1,11 @@ import { describe, expect, it } from 'vitest' -import { getDeleteWorktreeToastCopy } from './delete-worktree-toast' +import { getDeleteWorktreeToastCopy, type DeleteWorktreeToastCopy } from './delete-worktree-toast' import { classifyWorktreeForceDeleteReason } from '../../../../shared/worktree/removal' // Why: production never hands this function a literal reason — the store derives it from // classifyWorktreeForceDeleteReason (store/slices/worktrees.ts). Passing one in would let a // message the classifier rejects still render the force copy, testing a UI that never runs. -function toastCopyForRemovalError(worktreeName: string, error: string): unknown { +function toastCopyForRemovalError(worktreeName: string, error: string): DeleteWorktreeToastCopy { return getDeleteWorktreeToastCopy(worktreeName, classifyWorktreeForceDeleteReason(error), error) } @@ -111,6 +111,36 @@ describe('getDeleteWorktreeToastCopy', () => { }) }) + // Why: Electron builds every invoke rejection as `Error invoking remote method '': ${err}` + // (renderer/api/ipc-renderer.ts) from the main side's `error.toString()`. A failure the classifier + // does not recognise reaches this branch, so without stripping, the delete toast shows the user + // Orca's IPC plumbing instead of a reason. + it('never shows the Electron IPC envelope when the failure carries a reason', () => { + const description = toastCopyForRemovalError( + 'feature/foo', + "Error invoking remote method 'worktrees:remove': Error: EPERM: operation not permitted, unlink '/w/.git'" + ).description + expect(description).not.toContain('invoking remote method') + expect(description).toBe("EPERM: operation not permitted, unlink '/w/.git'") + }) + + // Why: `error.toString()` on a message-less main-side error is the bare class name, so the + // envelope arrives with nothing behind it. Rendering what is left ("Error") is still plumbing, + // so this case has to reach human copy rather than a stripped remnant. + it('falls back to human copy when the envelope carries no readable reason', () => { + expect( + toastCopyForRemovalError( + 'feature/foo', + "Error invoking remote method 'worktrees:remove': Error" + ) + ).toEqual({ + title: 'Failed to delete workspace feature/foo', + description: + 'Orca could not delete this workspace, and the failure did not include a readable reason. Retry, and send app diagnostics to support if it keeps failing.', + isDestructive: true + }) + }) + it('preserves the raw error when force delete is unavailable', () => { expect(getDeleteWorktreeToastCopy('feature/foo', null, 'permission denied')).toEqual({ title: 'Failed to delete workspace feature/foo', diff --git a/src/renderer/src/components/sidebar/delete-worktree-toast.ts b/src/renderer/src/components/sidebar/delete-worktree-toast.ts index 1c013da0da6..a1632493346 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-toast.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-toast.ts @@ -1,4 +1,5 @@ import { translate } from '@/i18n/i18n' +import { getWorktreeRemovalErrorCopy } from './worktree-removal-error-copy' import { isLockedWorktreeRemovalError, isProvenLivePtyRemovalError, @@ -112,7 +113,9 @@ export function getDeleteWorktreeToastCopy( 'Failed to delete workspace {{value0}}', { value0: worktreeName } ), - description: error, + // Why: this branch renders whatever string the failure carried, so an unclassified + // failure used to show the user Electron's own IPC envelope instead of a reason. + description: getWorktreeRemovalErrorCopy(error), isDestructive: true } } diff --git a/src/renderer/src/components/sidebar/worktree-removal-error-copy.ts b/src/renderer/src/components/sidebar/worktree-removal-error-copy.ts new file mode 100644 index 00000000000..0c57006f83d --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-removal-error-copy.ts @@ -0,0 +1,34 @@ +import { translate } from '@/i18n/i18n' +import { stripIpcInvokeEnvelope } from '@/lib/ipc-error' + +/** + * User-facing text for a workspace-removal failure Orca could not classify. + * + * Every unclassified failure reaches a user through this one function, so Electron's IPC + * envelope cannot leak into the delete toast, the delete dialog, or the space manager. + * Nothing is discarded: `deleteStateByWorktreeId` keeps the raw string, the renderer logs + * the rejection, and Electron logs the handler's original error with its stack in main. + */ +export function getWorktreeRemovalErrorCopy(error: string): string { + return ( + stripIpcInvokeEnvelope(error) ?? + translate( + 'auto.components.sidebar.worktree.removal.error.copy.unreadable', + 'Orca could not delete this workspace, and the failure did not include a readable reason. Retry, and send app diagnostics to support if it keeps failing.' + ) + ) +} + +/** + * Same contract for the branch a deleted workspace left behind: the preserved-branch toast + * runs its own IPC call, so it can surface the same envelope from a different channel. + */ +export function getPreservedBranchDeletionErrorCopy(error: string): string { + return ( + stripIpcInvokeEnvelope(error) ?? + translate( + 'auto.components.sidebar.worktree.removal.error.copy.branchUnreadable', + 'Orca could not delete this branch, and the failure did not include a readable reason. Retry, and send app diagnostics to support if it keeps failing.' + ) + ) +} diff --git a/src/renderer/src/components/sidebar/worktree-removal-error-surfaces.test.tsx b/src/renderer/src/components/sidebar/worktree-removal-error-surfaces.test.tsx new file mode 100644 index 00000000000..7c5fe333176 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-removal-error-surfaces.test.tsx @@ -0,0 +1,85 @@ +// @vitest-environment happy-dom + +import '@testing-library/jest-dom/vitest' +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it } from 'vitest' +import { DeleteWorktreeWarningPanels } from './DeleteWorktreeWarningPanels' +import { DeleteWorktreeTargetPreview } from './DeleteWorktreeTargetPreview' +import type { Worktree } from '../../../../shared/worktree/types' +import type { WorktreeDeleteState } from '../../store/slices/worktree-delete-state-types' + +// The exact shape Electron produces when a `worktrees:remove` handler rejects: the renderer +// wraps the main side's `error.toString()` (renderer/api/ipc-renderer.ts). The second form is +// what a message-less handler failure looks like — envelope, nothing behind it. +const ENVELOPED_REASON = + "Error invoking remote method 'worktrees:remove': Error: Failed to delete worktree at /w/feature. ?? scratch.txt" +const ENVELOPE_ONLY = "Error invoking remote method 'worktrees:remove': Error" + +const UNREADABLE_COPY = + 'Orca could not delete this workspace, and the failure did not include a readable reason. Retry, and send app diagnostics to support if it keeps failing.' + +function deleteState(error: string): WorktreeDeleteState { + return { isDeleting: false, error, canForceDelete: false, forceDeleteReason: null } +} + +function makeWorktree(): Worktree { + return { + id: 'repo1::/w/feature', + repoId: 'repo1', + path: '/w/feature', + head: 'abc123', + branch: 'feature', + isBare: false, + isMainWorktree: false, + displayName: 'feature' + } as Worktree +} + +afterEach(cleanup) + +// Why: all three surfaces read the same `deleteStateByWorktreeId[...].error` the toast reads, +// so a fix confined to the toast would still show plumbing in the dialog and the space manager. +describe('workspace-removal failure surfaces', () => { + it('shows only the reason in the delete dialog warning panel', () => { + render( + + ) + expect(screen.queryByText(/invoking remote method/)).toBeNull() + expect( + screen.getByText('Failed to delete worktree at /w/feature. ?? scratch.txt') + ).toBeInTheDocument() + }) + + it('shows human copy in the delete dialog warning panel when nothing readable arrived', () => { + render( + + ) + expect(screen.queryByText(/invoking remote method/)).toBeNull() + expect(screen.getByText(UNREADABLE_COPY)).toBeInTheDocument() + }) + + it('shows only the reason on a delete-target row', () => { + const worktree = makeWorktree() + render( + + ) + expect(screen.queryByText(/invoking remote method/)).toBeNull() + expect(screen.getByText(UNREADABLE_COPY)).toBeInTheDocument() + }) +}) diff --git a/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx b/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx index af74109cae9..46a15cb7881 100644 --- a/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx +++ b/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx @@ -67,6 +67,7 @@ import { toWorktreeDeleteIdentities } from '../sidebar/worktree-delete-request' import { showWorkspaceListChangedToast } from '../sidebar/stale-workspace-list-toast' import { prepareActiveWorktreeFocusAfterDelete } from '../sidebar/active-worktree-focus-after-delete' import { branchDisplayName } from '../sidebar/WorktreeCardHelpers' +import { getWorktreeRemovalErrorCopy } from '../sidebar/worktree-removal-error-copy' import { Badge } from '../ui/badge' import { Button } from '../ui/button' import { @@ -1124,7 +1125,9 @@ function BreakdownRow({ ) } -function WorkspaceRow({ +// Exported for test: the panel above it is store-connected, and this row is the only +// place a workspace-removal failure is rendered in Space. +export function WorkspaceRow({ worktree, maxSize, selected, @@ -1152,7 +1155,10 @@ function WorkspaceRow({ onForceDelete: () => void }): React.JSX.Element { const isDeleting = deleteState?.isDeleting ?? false - const deleteError = deleteState?.error ?? null + // Why: this row shows the same removal failure the delete toast does, so it needs the + // same guard against rendering Electron's IPC envelope — in the title attribute too. + const rawDeleteError = deleteState?.error ?? null + const deleteError = rawDeleteError === null ? null : getWorktreeRemovalErrorCopy(rawDeleteError) const canForceDelete = deleteState?.canForceDelete ?? false const canDelete = isWorkspaceSpaceRowReadyToDelete(worktree, decisionDetails) && !isDeleting const handleForceDelete = (event: React.MouseEvent): void => { @@ -1786,7 +1792,7 @@ export function WorkspaceSpaceManagerPanel(): React.JSX.Element { 'Force delete failed' ), { - description: result.error + description: getWorktreeRemovalErrorCopy(result.error) } ) return diff --git a/src/renderer/src/components/status-bar/workspace-space-row-removal-failure.test.tsx b/src/renderer/src/components/status-bar/workspace-space-row-removal-failure.test.tsx new file mode 100644 index 00000000000..f47a175e715 --- /dev/null +++ b/src/renderer/src/components/status-bar/workspace-space-row-removal-failure.test.tsx @@ -0,0 +1,99 @@ +// @vitest-environment happy-dom + +import '@testing-library/jest-dom/vitest' +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it } from 'vitest' +import { WorkspaceRow } from './WorkspaceSpaceManagerPanel' +import type { WorkspaceSpaceWorktree } from '../../../../shared/workspace-space-types' + +// Electron wraps every invoke rejection as `Error invoking remote method '': ${err}`, +// and this row reads the same deleteState the sidebar toast does — the plumbing reaches Space too. +const ENVELOPED_REASON = + "Error invoking remote method 'worktrees:remove': Error: Failed to delete worktree at /w/feature. ?? scratch.txt" +const ENVELOPE_ONLY = "Error invoking remote method 'worktrees:remove': Error" +const UNREADABLE_COPY = + 'Orca could not delete this workspace, and the failure did not include a readable reason. Retry, and send app diagnostics to support if it keeps failing.' + +const worktree: WorkspaceSpaceWorktree = { + worktreeId: 'repo1::/w/feature', + repoId: 'repo1', + repoDisplayName: 'repo1', + repoPath: '/w', + displayName: 'feature', + path: '/w/feature', + branch: 'feature', + isMainWorktree: false, + isRemote: false, + isSparse: false, + canDelete: true, + lastActivityAt: 0, + status: 'ok', + error: null, + scannedAt: 0, + sizeBytes: 1024, + reclaimableBytes: 0, + skippedEntryCount: 0, + topLevelItems: [], + omittedTopLevelItemCount: 0, + omittedTopLevelSizeBytes: 0 +} + +const decisionDetails = { + isActive: false, + canOpenWorkspace: true, + terminalTabCount: 0, + liveTerminalCount: 0, + activeAgentCount: 0, + completedAgentCount: 0, + openEditorFileCount: 0, + dirtyEditorBufferCount: 0, + browserTabCount: 0, + changedFileCount: 0, + branchStatus: null, + reviewLabel: null, + issueLabel: null, + linearIssueLabel: null +} + +function renderRow(error: string): void { + render( + {}} + onInspect={() => {}} + onOpenWorkspace={() => {}} + onDelete={() => {}} + onForceDelete={() => {}} + /> + ) +} + +afterEach(cleanup) + +describe('Space workspace row removal failure', () => { + it('shows only the reason', () => { + renderRow(ENVELOPED_REASON) + expect(screen.queryByText(/invoking remote method/)).toBeNull() + expect( + screen.getByText('Failed to delete worktree at /w/feature. ?? scratch.txt') + ).toBeInTheDocument() + }) + + // Why: the row also puts this string in a `title` attribute, which the envelope leaked into + // even when the visible text was clipped. + it('shows human copy, in the tooltip too, when nothing readable arrived', () => { + renderRow(ENVELOPE_ONLY) + expect(screen.queryByText(/invoking remote method/)).toBeNull() + expect(screen.getByTitle(UNREADABLE_COPY)).toBeInTheDocument() + }) +}) diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 6660bb6c200..64d54b3e3fb 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -5806,6 +5806,14 @@ "6798dc7c94": "In review", "5076efc3d2": "Done" } + }, + "removal": { + "error": { + "copy": { + "unreadable": "Orca could not delete this workspace, and the failure did not include a readable reason. Retry, and send app diagnostics to support if it keeps failing.", + "branchUnreadable": "Orca could not delete this branch, and the failure did not include a readable reason. Retry, and send app diagnostics to support if it keeps failing." + } + } } }, "CacheTimer": { diff --git a/src/renderer/src/lib/ipc-error.test.ts b/src/renderer/src/lib/ipc-error.test.ts new file mode 100644 index 00000000000..7119cf84c60 --- /dev/null +++ b/src/renderer/src/lib/ipc-error.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest' +import { extractIpcErrorMessage, stripIpcInvokeEnvelope } from './ipc-error' + +// Electron builds every invoke rejection in renderer/api/ipc-renderer.ts as +// new Error(`Error invoking remote method '${channel}': ${error}`) +// where `error` is the main side's `error.toString()` (browser/api/web-contents.ts). +// Both halves of that template are reproduced below. +describe('stripIpcInvokeEnvelope', () => { + it('returns the underlying reason', () => { + expect( + stripIpcInvokeEnvelope( + "Error invoking remote method 'worktrees:remove': Error: Worktree has uncommitted changes" + ) + ).toBe('Worktree has uncommitted changes') + }) + + it('removes an envelope a caller has already prefixed', () => { + expect( + stripIpcInvokeEnvelope( + "SSH connection failed: Error invoking remote method 'ssh:connect': Error: Relay package not found." + ) + ).toBe('SSH connection failed: Relay package not found.') + }) + + // Why: `String(new Error(''))` is 'Error', so a message-less handler failure reaches the + // renderer as an envelope with only the class name behind it. Returning that remnant would + // hand a user 'Error' as the explanation, so the whole shape counts as unreadable. + it('reports no readable reason when only the error class survives', () => { + expect(stripIpcInvokeEnvelope("Error invoking remote method 'worktrees:remove': Error")).toBe( + null + ) + expect(stripIpcInvokeEnvelope("Error invoking remote method 'worktrees:remove': ")).toBe(null) + expect(stripIpcInvokeEnvelope("Error invoking remote method 'worktrees:remove'")).toBe(null) + }) + + it('keeps a message that never crossed IPC untouched', () => { + expect(stripIpcInvokeEnvelope('permission denied')).toBe('permission denied') + }) + + // Why: `(.+)` in the older matcher stops at the first newline, which drops the rest of a + // multi-line git stderr — the part naming the files that blocked the delete. + it('keeps every line of a multi-line reason', () => { + expect( + stripIpcInvokeEnvelope( + "Error invoking remote method 'worktrees:remove': Error: fatal: cannot remove\n?? scratch.txt" + ) + ).toBe('fatal: cannot remove\n?? scratch.txt') + }) +}) + +// Why: 18 call sites read this one, and its documented contract is to fall back to the raw +// message rather than to null. Adding the stricter reader must not quietly retune them. +describe('extractIpcErrorMessage', () => { + it('still unwraps the envelope', () => { + expect( + extractIpcErrorMessage( + new Error("Error invoking remote method 'fs:readFile': Error: Access denied"), + 'fallback' + ) + ).toBe('Access denied') + }) + + it('still returns the raw message when the envelope has no tail', () => { + expect( + extractIpcErrorMessage(new Error("Error invoking remote method 'fs:readFile'"), 'fallback') + ).toBe("Error invoking remote method 'fs:readFile'") + }) + + it('still returns the fallback for a non-Error', () => { + expect(extractIpcErrorMessage('boom', 'fallback')).toBe('fallback') + }) +}) diff --git a/src/renderer/src/lib/ipc-error.ts b/src/renderer/src/lib/ipc-error.ts index 282cfd3f9ad..270fb6c7e35 100644 --- a/src/renderer/src/lib/ipc-error.ts +++ b/src/renderer/src/lib/ipc-error.ts @@ -3,6 +3,26 @@ * "Error invoking remote method 'channel': Error: actual message" * Strip the wrapper so users see only the meaningful part. */ + +// Why: the renderer builds this from the main side's `error.toString()`, so a handler that +// threw a message-less error arrives as a bare class name ("…': Error") — the tail can be +// empty even though the envelope is present. Global because a caller may have prefixed it. +const IPC_INVOKE_ENVELOPE = /Error invoking remote method '[^']*'(?::[ \t]*(?:\w*Error:[ \t]*)?)?/g +const BARE_ERROR_CLASS_RESIDUE = /^\w*Error:?$/ + +/** + * The failure behind the IPC envelope, or null when the envelope carried no readable reason. + * Callers that must never render plumbing branch on null instead of falling back to the + * wrapper text — which is what `extractIpcErrorMessage` does. + */ +export function stripIpcInvokeEnvelope(message: string): string | null { + const stripped = message.replace(IPC_INVOKE_ENVELOPE, '').trim() + if (stripped === '' || BARE_ERROR_CLASS_RESIDUE.test(stripped)) { + return null + } + return stripped +} + export function extractIpcErrorMessage(err: unknown, fallback: string): string { if (!(err instanceof Error)) { return fallback diff --git a/src/renderer/src/store/slices/worktrees-metadata-persistence.test.ts b/src/renderer/src/store/slices/worktrees-metadata-persistence.test.ts index f7770e8b8b8..b51d1e6984d 100644 --- a/src/renderer/src/store/slices/worktrees-metadata-persistence.test.ts +++ b/src/renderer/src/store/slices/worktrees-metadata-persistence.test.ts @@ -100,6 +100,26 @@ describe('worktree remote runtime mutations', () => { expect(mockApi.worktrees.forceDeletePreservedBranch).not.toHaveBeenCalled() }) + // Why: this toast runs its own IPC call, so it can surface Electron's invoke envelope from a + // different channel than the workspace delete that produced the preserved branch. + it('never shows the IPC envelope when preserved-branch deletion fails', async () => { + const store = createTestStore() + const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', path: '/path/wt1' }) + store.setState({ worktreesByRepo: { repo1: [wt] } } as Partial) + mockApi.worktrees.forceDeletePreservedBranch.mockRejectedValueOnce( + new Error("Error invoking remote method 'worktrees:forceDeletePreservedBranch': Error") + ) + + const result = await store + .getState() + .forceDeletePreservedBranch(wt.id, 'feature/test', 'abc123') + + expect(result.ok).toBe(false) + expect(vi.mocked(toast.error).mock.calls[0]?.[1]?.description).toBe( + 'Orca could not delete this branch, and the failure did not include a readable reason. Retry, and send app diagnostics to support if it keeps failing.' + ) + }) + it('suppresses per-branch feedback for an aggregate delete', async () => { const store = createTestStore() const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', path: '/path/wt1' }) diff --git a/src/renderer/src/store/slices/worktrees/teardown/force-delete-preserved-branch.ts b/src/renderer/src/store/slices/worktrees/teardown/force-delete-preserved-branch.ts index 5875371035b..e359a6fa394 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/force-delete-preserved-branch.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/force-delete-preserved-branch.ts @@ -10,6 +10,7 @@ import type { ForceDeleteWorktreeBranchResult } from '../../../../../../shared/w import type { PreservedBranchCleanup } from '../../../../../../shared/preserved-branch-cleanup' import { preservedBranchRuntimeTargetByCleanupKey } from './preserved-branch-cleanup-target' import { settingsForWorktreeOwner } from '../listing/worktree-owner-settings' +import { getPreservedBranchDeletionErrorCopy } from '@/components/sidebar/worktree-removal-error-copy' export function createForceDeletePreservedBranch( _set: WorktreeSliceSet, @@ -105,7 +106,7 @@ export function createForceDeletePreservedBranch( toast.error( translate('auto.store.slices.worktrees.0216895fb5', 'Failed to delete branch'), { - description: error + description: getPreservedBranchDeletionErrorCopy(error) } ) }