diff --git a/src/renderer/src/components/settings/CliSection.install-failure.test.tsx b/src/renderer/src/components/settings/CliSection.install-failure.test.tsx new file mode 100644 index 00000000000..414a5ace240 --- /dev/null +++ b/src/renderer/src/components/settings/CliSection.install-failure.test.tsx @@ -0,0 +1,154 @@ +// @vitest-environment happy-dom + +import { act, cleanup, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { getDefaultSettings } from '../../../../shared/constants' +import type { CliInstallStatus } from '../../../../shared/cli-install-types' +import { CliSection } from './CliSection' + +const toasts = vi.hoisted(() => ({ error: vi.fn(), success: vi.fn() })) +const dialog = vi.hoisted(() => ({ + props: null as null | { onInstall: () => Promise; open: boolean } +})) + +vi.mock('sonner', () => ({ toast: toasts })) + +vi.mock('@/hooks/useInstalledAgentSkills', () => ({ + GLOBAL_AGENT_SKILL_SOURCE_KINDS: ['global'], + useInstalledAgentSkill: () => ({ + installed: false, + loading: false, + error: null, + refresh: vi.fn() + }) +})) + +vi.mock('@/hooks/useActiveProjectSkillRuntime', () => ({ + useActiveProjectSkillRuntime: () => ({ canUseLocalSkillFreshness: true }) +})) + +vi.mock('./AgentSkillSetupPanel', () => ({ + AgentSkillSetupPanel: () =>
+})) + +vi.mock('./WslCliRegistration', () => ({ WslCliRegistration: () => null })) + +vi.mock('./CliRegistrationDialog', () => ({ + CliRegistrationDialog: function CliRegistrationDialog(props: { + onInstall: () => Promise + open: boolean + }) { + dialog.props = props + return null + } +})) + +function notInstalledStatus(overrides: Partial = {}): CliInstallStatus { + return { + platform: 'darwin', + commandName: 'orca', + commandPath: '/usr/local/bin/orca', + pathDirectory: '/usr/local/bin', + pathConfigured: true, + launcherPath: '/Applications/Orca.app/Contents/Resources/bin/orca', + installMethod: 'symlink', + supported: true, + state: 'not_installed', + currentTarget: null, + unsupportedReason: null, + detail: 'Register /usr/local/bin/orca to use Orca from the terminal.', + ...overrides + } +} + +async function renderCliSectionAndInstall(install: () => Promise): Promise { + Object.assign(window, { + api: { + cli: { + getInstallStatus: vi.fn().mockResolvedValue(notInstalledStatus()), + getWslInstallStatus: vi.fn(), + install: vi.fn(install), + remove: vi.fn() + }, + shell: { openPath: vi.fn() } + } + }) + + render() + await screen.findByRole('switch') + await act(async () => { + await dialog.props?.onInstall() + }) +} + +afterEach(() => { + cleanup() + dialog.props = null + toasts.error.mockReset() + toasts.success.mockReset() +}) + +describe('CliSection install failure surfacing', () => { + it('shows the thrown conflict reason and its remedy instead of a success toast', async () => { + await renderCliSectionAndInstall(async () => { + throw new Error( + "Error invoking remote method 'cli:install': Error: Refusing to replace non-Orca " + + 'command at /usr/local/bin/orca. Remove it and register again if it is no longer needed.' + ) + }) + + const alert = screen.getByRole('alert') + expect(alert.textContent).toContain('Failed to register `orca` in PATH.') + expect(alert.textContent).toContain( + 'Refusing to replace non-Orca command at /usr/local/bin/orca.' + ) + expect(alert.textContent).toContain('Remove it and register again if it is no longer needed.') + // The Electron transport wrapper must not leak into the panel. + expect(alert.textContent).not.toContain('invoking remote method') + expect(toasts.success).not.toHaveBeenCalled() + expect(toasts.error).toHaveBeenCalledTimes(1) + }) + + it('names the path and the remedy when install resolves with a conflict', async () => { + await renderCliSectionAndInstall(async () => + notInstalledStatus({ + state: 'conflict', + detail: '/usr/local/bin/orca exists but is not an Orca symlink.' + }) + ) + + const alert = screen.getByRole('alert') + expect(alert.textContent).toContain('/usr/local/bin/orca exists but is not an Orca symlink.') + expect(alert.textContent).toContain( + 'Remove /usr/local/bin/orca and register again if it is no longer needed.' + ) + expect(toasts.success).not.toHaveBeenCalled() + }) + + it('does not claim success when install resolves without registering', async () => { + await renderCliSectionAndInstall(async () => + notInstalledStatus({ + state: 'unsupported', + supported: false, + unsupportedReason: 'launcher_missing', + detail: 'The bundled CLI launcher is missing from this Orca build.' + }) + ) + + expect(screen.getByRole('alert').textContent).toContain( + 'The bundled CLI launcher is missing from this Orca build.' + ) + expect(toasts.success).not.toHaveBeenCalled() + expect(toasts.error).toHaveBeenCalledTimes(1) + }) + + it('keeps the success toast and shows no failure notice when registration lands', async () => { + await renderCliSectionAndInstall(async () => + notInstalledStatus({ state: 'installed', detail: null }) + ) + + expect(screen.queryByRole('alert')).toBeNull() + expect(toasts.success).toHaveBeenCalledTimes(1) + expect(toasts.error).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/settings/CliSection.tsx b/src/renderer/src/components/settings/CliSection.tsx index 32cbb3e5766..364a866df5b 100644 --- a/src/renderer/src/components/settings/CliSection.tsx +++ b/src/renderer/src/components/settings/CliSection.tsx @@ -33,6 +33,7 @@ import { getWslCliDistroRequest } from './CliSkillRuntimeSetup' import { WslCliRegistration } from './WslCliRegistration' +import { useCliRegistrationActions } from './use-cli-registration-actions' import { useLocalCliSkillFreshnessName } from './use-local-cli-skill-freshness-name' import { translate } from '@/i18n/i18n' @@ -81,7 +82,6 @@ export function CliSection({ const [status, setStatus] = useState(null) const [loading, setLoading] = useState(true) const [dialogOpen, setDialogOpen] = useState(false) - const [busyAction, setBusyAction] = useState<'install' | 'remove' | null>(null) const mountedRef = useMountedRef() const agentRuntime = useMemo( () => @@ -132,8 +132,19 @@ export function CliSection({ [mountedRef] ) + const closeDialog = useCallback((): void => setDialogOpen(false), []) + const commandName = status?.commandName ?? getFallbackCommandName(currentPlatform) + const { busyAction, installFailure, clearInstallFailure, install, remove } = + useCliRegistrationActions({ + commandName, + mountedRef, + onStatusChange: handleStatusChange, + onSettled: closeDialog + }) + const refreshStatus = useCallback(async (): Promise => { setLoading(true) + clearInstallFailure() try { handleStatusChange(await window.api.cli.getInstallStatus()) } catch (error) { @@ -152,7 +163,7 @@ export function CliSection({ setLoading(false) } } - }, [handleStatusChange, mountedRef]) + }, [clearInstallFailure, handleStatusChange, mountedRef]) useEffect(() => { void refreshStatus() @@ -163,78 +174,9 @@ export function CliSection({ const isSupported = status?.supported ?? false const isBrowserManaged = status?.unsupportedReason === 'launch_mode_unavailable' const revealLabel = getRevealLabel(currentPlatform) - const commandName = status?.commandName ?? getFallbackCommandName(currentPlatform) const canRevealCommandPath = status?.commandPath != null && ['installed', 'stale', 'conflict'].includes(status.state) - const handleInstall = async (): Promise => { - setBusyAction('install') - try { - const next = await window.api.cli.install() - if (mountedRef.current) { - setStatus(next) - setDialogOpen(false) - toast.success( - translate( - 'auto.components.settings.CliSection.9cbcd31338', - 'Registered `{{value0}}` in PATH.', - { value0: next.commandName } - ) - ) - } - } catch (error) { - if (mountedRef.current) { - toast.error( - error instanceof Error - ? error.message - : translate( - 'auto.components.settings.CliSection.a2b13efa94', - 'Failed to register `{{value0}}` in PATH.', - { value0: commandName } - ) - ) - } - } finally { - if (mountedRef.current) { - setBusyAction(null) - } - } - } - - const handleRemove = async (): Promise => { - setBusyAction('remove') - try { - const next = await window.api.cli.remove() - if (mountedRef.current) { - setStatus(next) - setDialogOpen(false) - toast.success( - translate( - 'auto.components.settings.CliSection.af5540930c', - 'Removed `{{value0}}` from PATH.', - { value0: next.commandName } - ) - ) - } - } catch (error) { - if (mountedRef.current) { - toast.error( - error instanceof Error - ? error.message - : translate( - 'auto.components.settings.CliSection.d77352f2df', - 'Failed to remove `{{value0}}` from PATH.', - { value0: commandName } - ) - ) - } - } finally { - if (mountedRef.current) { - setBusyAction(null) - } - } - } - return (
@@ -336,6 +278,31 @@ export function CliSection({

{status.detail}

) : null} + {installFailure ? ( +
+

+ {translate( + 'auto.components.settings.CliSection.a2b13efa94', + 'Failed to register `{{value0}}` in PATH.', + { value0: commandName } + )} +

+

{installFailure.reason}

+ {installFailure.conflictCommandPath ? ( +

+ {translate( + 'auto.components.settings.CliSection.installFailureConflictRemedy', + 'Remove {{value0}} and register again if it is no longer needed.', + { value0: installFailure.conflictCommandPath } + )} +

+ ) : null} +
+ ) : null} +
{status?.commandPath ? (
diff --git a/src/renderer/src/components/settings/cli-install-failure.test.ts b/src/renderer/src/components/settings/cli-install-failure.test.ts new file mode 100644 index 00000000000..dfd39f15649 --- /dev/null +++ b/src/renderer/src/components/settings/cli-install-failure.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, it } from 'vitest' +import type { CliInstallStatus } from '../../../../shared/cli-install-types' +import { readCliInstallFailure, readCliInstallRejection } from './cli-install-failure' + +const FALLBACK = 'Orca could not finish CLI registration and reported no reason.' + +function cliStatus(overrides: Partial = {}): CliInstallStatus { + return { + platform: 'darwin', + commandName: 'orca', + commandPath: '/usr/local/bin/orca', + pathDirectory: '/usr/local/bin', + pathConfigured: true, + launcherPath: '/Applications/Orca.app/Contents/Resources/bin/orca', + installMethod: 'symlink', + supported: true, + state: 'installed', + currentTarget: null, + unsupportedReason: null, + detail: null, + ...overrides + } +} + +describe('readCliInstallFailure', () => { + it('reports no failure for a landed registration', () => { + expect(readCliInstallFailure(cliStatus(), FALLBACK)).toBeNull() + }) + + it('surfaces the main-process reason verbatim without re-classifying it', () => { + expect( + readCliInstallFailure( + cliStatus({ + state: 'unsupported', + supported: false, + unsupportedReason: 'launcher_missing', + detail: 'The bundled CLI launcher is missing from this Orca build.' + }), + FALLBACK + ) + ).toEqual({ + reason: 'The bundled CLI launcher is missing from this Orca build.', + conflictCommandPath: null + }) + }) + + it('names the conflicting path so the panel can offer the remedy', () => { + expect( + readCliInstallFailure( + cliStatus({ + state: 'conflict', + detail: '/usr/local/bin/orca exists but is not an Orca symlink.' + }), + FALLBACK + ) + ).toEqual({ + reason: '/usr/local/bin/orca exists but is not an Orca symlink.', + conflictCommandPath: '/usr/local/bin/orca' + }) + }) + + it('falls back when the main process reported no detail', () => { + expect(readCliInstallFailure(cliStatus({ state: 'not_installed' }), FALLBACK)).toEqual({ + reason: FALLBACK, + conflictCommandPath: null + }) + }) +}) + +describe('readCliInstallRejection', () => { + it('strips the Electron transport prefix off the installer message', () => { + expect( + readCliInstallRejection( + new Error( + "Error invoking remote method 'cli:install': Error: Refusing to replace non-Orca " + + 'command at /usr/local/bin/orca. Remove it and register again if it is no longer needed.' + ), + FALLBACK + ) + ).toEqual({ + reason: + 'Refusing to replace non-Orca command at /usr/local/bin/orca. ' + + 'Remove it and register again if it is no longer needed.', + conflictCommandPath: null + }) + }) + + it('keeps the registration-lock remedy that names the lock file', () => { + const failure = readCliInstallRejection( + new Error( + "Error invoking remote method 'cli:install': Error: Timed out waiting for another Orca " + + 'process to finish CLI registration (waited 330s). If no other Orca is running, remove ' + + '/home/u/.cache/orca/appimage/.cli-registration.lock and retry.' + ), + FALLBACK + ) + + expect(failure.reason).toContain('.cli-registration.lock and retry.') + expect(failure.reason.startsWith('Timed out waiting')).toBe(true) + }) + + it('falls back for a non-Error rejection with no message', () => { + expect(readCliInstallRejection(new Error(' '), FALLBACK)).toEqual({ + reason: FALLBACK, + conflictCommandPath: null + }) + }) +}) diff --git a/src/renderer/src/components/settings/cli-install-failure.ts b/src/renderer/src/components/settings/cli-install-failure.ts new file mode 100644 index 00000000000..8ca55a43299 --- /dev/null +++ b/src/renderer/src/components/settings/cli-install-failure.ts @@ -0,0 +1,40 @@ +import type { CliInstallStatus } from '../../../../shared/cli-install-types' + +// Why: Electron re-wraps a rejected `ipcMain.handle` as +// `Error invoking remote method '': Error: `, so the installer's +// own sentence is buried behind transport noise by the time it reaches the panel. +const IPC_INVOKE_PREFIX = /^Error invoking remote method '[^']*':\s*(?:Error:\s*)?/ + +export type CliInstallFailure = { + /** The main-process reason verbatim; installer throws already embed their own remedy. */ + reason: string + /** Set only for a conflict, whose status detail names the path but stops short of the remedy. */ + conflictCommandPath: string | null +} + +/** + * A registration call that resolved without landing. The main process already + * reported why in `detail`, so this only decides that it failed — it does not + * re-classify the reason. + */ +export function readCliInstallFailure( + status: CliInstallStatus, + fallbackReason: string +): CliInstallFailure | null { + if (status.state === 'installed') { + return null + } + return { + reason: status.detail?.trim() || fallbackReason, + conflictCommandPath: status.state === 'conflict' ? status.commandPath : null + } +} + +/** A registration call that threw: unwrap the transport prefix off the installer's message. */ +export function readCliInstallRejection(error: unknown, fallbackReason: string): CliInstallFailure { + const message = error instanceof Error ? error.message : String(error) + return { + reason: message.replace(IPC_INVOKE_PREFIX, '').trim() || fallbackReason, + conflictCommandPath: null + } +} diff --git a/src/renderer/src/components/settings/use-cli-registration-actions.ts b/src/renderer/src/components/settings/use-cli-registration-actions.ts new file mode 100644 index 00000000000..29737764e9c --- /dev/null +++ b/src/renderer/src/components/settings/use-cli-registration-actions.ts @@ -0,0 +1,127 @@ +import { useCallback, useState, type MutableRefObject } from 'react' +import { toast } from 'sonner' +import type { CliInstallStatus } from '../../../../shared/cli-install-types' +import { translate } from '@/i18n/i18n' +import { + readCliInstallFailure, + readCliInstallRejection, + type CliInstallFailure +} from './cli-install-failure' + +type CliRegistrationActionsOptions = { + commandName: string + mountedRef: MutableRefObject + onStatusChange: (status: CliInstallStatus) => void + onSettled: () => void +} + +export type CliRegistrationActions = { + busyAction: 'install' | 'remove' | null + installFailure: CliInstallFailure | null + clearInstallFailure: () => void + install: () => Promise + remove: () => Promise +} + +function unknownReason(): string { + return translate( + 'auto.components.settings.CliSection.installFailureUnknownReason', + 'Orca could not finish CLI registration and reported no reason.' + ) +} + +function failedTitle(commandName: string): string { + return translate( + 'auto.components.settings.CliSection.a2b13efa94', + 'Failed to register `{{value0}}` in PATH.', + { value0: commandName } + ) +} + +export function useCliRegistrationActions({ + commandName, + mountedRef, + onStatusChange, + onSettled +}: CliRegistrationActionsOptions): CliRegistrationActions { + const [busyAction, setBusyAction] = useState<'install' | 'remove' | null>(null) + const [installFailure, setInstallFailure] = useState(null) + const clearInstallFailure = useCallback((): void => setInstallFailure(null), []) + + const install = useCallback(async (): Promise => { + setBusyAction('install') + try { + const next = await window.api.cli.install() + if (!mountedRef.current) { + return + } + onStatusChange(next) + onSettled() + // Why: `install()` resolves with the post-registration status, so a refusal + // (conflict, unsupported build, unreadable PATH) arrives as data, not a throw. + const failure = readCliInstallFailure(next, unknownReason()) + setInstallFailure(failure) + if (failure) { + toast.error(failedTitle(next.commandName), { description: failure.reason }) + return + } + toast.success( + translate( + 'auto.components.settings.CliSection.9cbcd31338', + 'Registered `{{value0}}` in PATH.', + { value0: next.commandName } + ) + ) + } catch (error) { + if (!mountedRef.current) { + return + } + const failure = readCliInstallRejection(error, unknownReason()) + setInstallFailure(failure) + // Why: closing reveals the persistent notice the toast is only a preview of. + onSettled() + toast.error(failedTitle(commandName), { description: failure.reason }) + } finally { + if (mountedRef.current) { + setBusyAction(null) + } + } + }, [commandName, mountedRef, onSettled, onStatusChange]) + + const remove = useCallback(async (): Promise => { + setBusyAction('remove') + try { + const next = await window.api.cli.remove() + if (mountedRef.current) { + onStatusChange(next) + onSettled() + setInstallFailure(null) + toast.success( + translate( + 'auto.components.settings.CliSection.af5540930c', + 'Removed `{{value0}}` from PATH.', + { value0: next.commandName } + ) + ) + } + } catch (error) { + if (mountedRef.current) { + toast.error( + error instanceof Error + ? error.message + : translate( + 'auto.components.settings.CliSection.d77352f2df', + 'Failed to remove `{{value0}}` from PATH.', + { value0: commandName } + ) + ) + } + } finally { + if (mountedRef.current) { + setBusyAction(null) + } + } + }, [commandName, mountedRef, onSettled, onStatusChange]) + + return { busyAction, installFailure, clearInstallFailure, install, remove } +} diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 99805af4c89..a403978ce44 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -6753,7 +6753,9 @@ "8a9b784c60": "stale", "d363e5929b": "Checking CLI registration…", "cliSkillTerminalTitle": "CLI skill setup", - "cliSkillTerminalAria": "CLI skill install terminal" + "cliSkillTerminalAria": "CLI skill install terminal", + "installFailureUnknownReason": "Orca could not finish CLI registration and reported no reason.", + "installFailureConflictRemedy": "Remove {{value0}} and register again if it is no longer needed." }, "CliSkillRuntimeSetup": { "04325573f8": "WSL",