fix(settings): surface why CLI registration failed

The Settings CLI panel treated every resolved `cli:install` as a success,
so a refusal that arrives as data (conflict, missing launcher, unreadable
Windows PATH) produced a green "Registered `orca` in PATH." toast while the
switch stayed off. A thrown refusal fared little better: the raw Electron
`Error invoking remote method 'cli:install': ...` string went into a toast
that then disappeared, leaving the panel indistinguishable from "not yet
installed".

Inspect the returned status with the predicate the onboarding and
agent-skill flows already use (`state !== 'installed'`), unwrap the IPC
transport prefix off thrown installer messages, and persist the existing
main-process reason inline per STYLEGUIDE (toasts disappear; errors the
user must act on stay inline). No new error taxonomy — the reasons already
carry path and remedy; a conflict status, which names the path but not the
remedy, gets the installer's own remedy sentence.

Closes #3952
This commit is contained in:
Neil
2026-09-02 02:20:19 -07:00
parent 4bc20cb842
commit 280a6a2217
6 changed files with 472 additions and 74 deletions
@@ -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<void>; 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: () => <div data-testid="agent-skill-setup-panel" />
}))
vi.mock('./WslCliRegistration', () => ({ WslCliRegistration: () => null }))
vi.mock('./CliRegistrationDialog', () => ({
CliRegistrationDialog: function CliRegistrationDialog(props: {
onInstall: () => Promise<void>
open: boolean
}) {
dialog.props = props
return null
}
}))
function notInstalledStatus(overrides: Partial<CliInstallStatus> = {}): 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<CliInstallStatus>): Promise<void> {
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(<CliSection currentPlatform="darwin" settings={getDefaultSettings('/tmp')} />)
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()
})
})
@@ -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<CliInstallStatus | null>(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<void> => {
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<void> => {
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<void> => {
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 (
<section className="space-y-4" data-settings-section="cli">
<div className="space-y-1">
@@ -336,6 +278,31 @@ export function CliSection({
<p className="text-xs text-muted-foreground">{status.detail}</p>
) : null}
{installFailure ? (
<div
role="alert"
className="space-y-1 rounded-md border border-destructive/40 bg-destructive/10 px-3 py-2 text-xs text-destructive"
>
<p className="font-medium">
{translate(
'auto.components.settings.CliSection.a2b13efa94',
'Failed to register `{{value0}}` in PATH.',
{ value0: commandName }
)}
</p>
<p className="leading-snug">{installFailure.reason}</p>
{installFailure.conflictCommandPath ? (
<p className="leading-snug">
{translate(
'auto.components.settings.CliSection.installFailureConflictRemedy',
'Remove {{value0}} and register again if it is no longer needed.',
{ value0: installFailure.conflictCommandPath }
)}
</p>
) : null}
</div>
) : null}
<div className="flex items-center gap-2">
{status?.commandPath ? (
<Button
@@ -408,9 +375,9 @@ export function CliSection({
commandPath={status?.commandPath}
isEnabled={isEnabled}
isSupported={isSupported}
onInstall={handleInstall}
onInstall={install}
onOpenChange={setDialogOpen}
onRemove={handleRemove}
onRemove={remove}
open={dialogOpen}
/>
</section>
@@ -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> = {}): 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
})
})
})
@@ -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 '<channel>': Error: <message>`, 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
}
}
@@ -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<boolean>
onStatusChange: (status: CliInstallStatus) => void
onSettled: () => void
}
export type CliRegistrationActions = {
busyAction: 'install' | 'remove' | null
installFailure: CliInstallFailure | null
clearInstallFailure: () => void
install: () => Promise<void>
remove: () => Promise<void>
}
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<CliInstallFailure | null>(null)
const clearInstallFailure = useCallback((): void => setInstallFailure(null), [])
const install = useCallback(async (): Promise<void> => {
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<void> => {
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 }
}
+3 -1
View File
@@ -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",