sim: merge PR #17230 (hand-resolved)

This commit is contained in:
Merge Sim
2026-08-30 16:10:23 -07:00
14 changed files with 447 additions and 8 deletions
@@ -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<string> {
const seen = new Set<string>()
@@ -94,7 +95,7 @@ export function DeleteWorktreeTargetPreview({
/>
{itemDeleteState?.error ? (
<div className="mt-1 whitespace-pre-wrap break-all text-destructive">
{itemDeleteState.error}
{getWorktreeRemovalErrorCopy(itemDeleteState.error)}
</div>
) : null}
</div>
@@ -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({
<div className="rounded-md border border-destructive/40 bg-destructive/8 px-3 py-2 text-xs text-destructive">
<div className="flex items-start gap-2">
<AlertTriangle className="mt-0.5 size-3.5 shrink-0" />
<div className="min-w-0 flex-1 whitespace-pre-wrap break-all">{deleteError}</div>
<div className="min-w-0 flex-1 whitespace-pre-wrap break-all">
{getWorktreeRemovalErrorCopy(deleteError)}
</div>
</div>
</div>
)}
@@ -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<void> {
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)
})
})
@@ -1,5 +1,5 @@
import { describe, expect, it } from 'vitest'
import { getDeleteWorktreeToastCopy } from './delete-worktree-toast'
import { getDeleteWorktreeToastCopy, type DeleteWorktreeToastCopy } from './delete-worktree-toast'
import {
classifyWorktreeForceDeleteReason,
isUnprovenOrphanedWorktreeDirectoryError,
@@ -10,7 +10,7 @@ import { translate } from '@/i18n/i18n'
// 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)
}
@@ -116,6 +116,30 @@ describe('getDeleteWorktreeToastCopy', () => {
})
})
// Why: Electron wraps rejected IPC calls; users should see the underlying reason, not plumbing.
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'")
})
it('falls back to human copy when the IPC 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',
@@ -1,4 +1,5 @@
import { translate } from '@/i18n/i18n'
import { getWorktreeRemovalErrorCopy } from './worktree-removal-error-copy'
import {
isHeldWorkspaceDirectoryRemovalError,
isLockedWorktreeRemovalError,
@@ -150,7 +151,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
}
}
@@ -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.'
)
)
}
@@ -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(
<DeleteWorktreeWarningPanels
isMainWorktree={false}
mainWorktreeBlocker=""
deleteError={ENVELOPED_REASON}
/>
)
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(
<DeleteWorktreeWarningPanels
isMainWorktree={false}
mainWorktreeBlocker=""
deleteError={ENVELOPE_ONLY}
/>
)
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(
<DeleteWorktreeTargetPreview
isBatchDelete
worktree={null}
worktrees={[worktree]}
collisionWorktrees={[worktree]}
hostLabelById={new Map()}
deleteStateByWorktreeId={{ [worktree.id]: deleteState(ENVELOPE_ONLY) }}
dirtyChangeCountsByWorktreeId={new Map()}
/>
)
expect(screen.queryByText(/invoking remote method/)).toBeNull()
expect(screen.getByText(UNREADABLE_COPY)).toBeInTheDocument()
})
})
@@ -70,6 +70,7 @@ import { toWorktreeDeleteIdentities } from '../sidebar/worktree-delete-request'
import { showWorkspaceListChangedToast } from '../sidebar/stale-workspace-list-toast'
import { runWorkspaceSpaceForceDelete } from './workspace-space-force-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 {
@@ -1141,7 +1142,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,
@@ -1169,7 +1172,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<HTMLButtonElement>): void => {
@@ -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 '<channel>': ${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(
<WorkspaceRow
worktree={worktree}
maxSize={4096}
selected={false}
inspected={false}
decisionDetails={decisionDetails}
deleteState={{
isDeleting: false,
error,
canForceDelete: false,
forceDeleteReason: null
}}
onToggleSelected={() => {}}
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()
})
})
+8
View File
@@ -5819,6 +5819,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": {
+72
View File
@@ -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')
})
})
+20
View File
@@ -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
@@ -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<AppState>)
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' })
@@ -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)
}
)
}