fix(skills): preflight npx before onboarding setup

This commit is contained in:
Brennan Benson
2026-07-30 16:42:24 -07:00
parent 35c76ba07a
commit 6052710dd2
11 changed files with 411 additions and 2 deletions
+25
View File
@@ -5,6 +5,7 @@ const {
discoverSkillsMock,
discoverSkillsInWslMock,
inventorySkillFreshnessMock,
isNpxOnPathForSkillInstallMock,
getDefaultWslDistroMock,
getWslHomeMock,
parseWslPathMock
@@ -13,6 +14,7 @@ const {
discoverSkillsMock: vi.fn(),
discoverSkillsInWslMock: vi.fn(),
inventorySkillFreshnessMock: vi.fn(),
isNpxOnPathForSkillInstallMock: vi.fn(),
getDefaultWslDistroMock: vi.fn(),
getWslHomeMock: vi.fn(),
parseWslPathMock: vi.fn()
@@ -39,6 +41,10 @@ vi.mock('../skills/skill-freshness-inventory', () => ({
inventorySkillFreshness: inventorySkillFreshnessMock
}))
vi.mock('../skills/skill-install-npx-preflight', () => ({
isNpxOnPathForSkillInstall: isNpxOnPathForSkillInstallMock
}))
vi.mock('../wsl', () => ({
getDefaultWslDistro: getDefaultWslDistroMock,
getWslHome: getWslHomeMock,
@@ -80,6 +86,7 @@ describe('registerSkillsHandlers', () => {
scanIssues: [],
scannedAt: 1
})
isNpxOnPathForSkillInstallMock.mockResolvedValue(true)
getWslHomeMock.mockReturnValue('\\\\wsl.localhost\\Ubuntu\\home\\alice')
Object.defineProperty(process, 'platform', {
configurable: true,
@@ -113,6 +120,15 @@ describe('registerSkillsHandlers', () => {
return call[1] as (_event: unknown) => Promise<unknown>
}
function getNpxPreflightHandler() {
registerSkillsHandlers(store as never)
const call = handleMock.mock.calls.find((entry: unknown[]) => entry[0] === 'skills:isNpxOnPath')
if (!call) {
throw new Error('skills:isNpxOnPath handler was not registered')
}
return call[1] as (_event: unknown, context?: unknown) => Promise<boolean>
}
it('uses host skill discovery when resolved project runtime overrides stale WSL target state', async () => {
const handler = getDiscoverHandler()
@@ -225,4 +241,13 @@ describe('registerSkillsHandlers', () => {
})
expect(getWslHomeMock).not.toHaveBeenCalled()
})
it('registers the npx preflight and forwards its runtime context', async () => {
const handler = getNpxPreflightHandler()
const context = { wslDistro: 'Ubuntu' }
await expect(handler(null, context)).resolves.toBe(true)
expect(isNpxOnPathForSkillInstallMock).toHaveBeenCalledWith(context)
})
})
+9
View File
@@ -11,9 +11,11 @@ import type {
SkillUpdateStartResult
} from '../../shared/skill-freshness'
import { inventorySkillFreshness } from '../skills/skill-freshness-inventory'
import { isNpxOnPathForSkillInstall } from '../skills/skill-install-npx-preflight'
import { SkillUpdateRunner } from '../skills/skill-update-run'
import { skillUpdateFailedNames } from '../skills/skill-update-outcome'
import { readGloballyUpdatableSkillLocks } from '../skills/skill-update-registration'
import type { PreflightRuntimeContext } from './preflight-runtime-target'
import {
discoverSkillsOnTarget,
resolveSkillDiscoveryTarget
@@ -79,4 +81,11 @@ export function registerSkillsHandlers(store: Store): void {
ipcMain.handle('skills:getUpdateRun', async (): Promise<SkillUpdateRun> => {
return runner.getState()
})
ipcMain.handle(
'skills:isNpxOnPath',
async (_event, context?: PreflightRuntimeContext): Promise<boolean> => {
return isNpxOnPathForSkillInstall(context)
}
)
}
@@ -0,0 +1,42 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const mocks = vi.hoisted(() => ({
hydrateShellPath: vi.fn(),
isCommandOnPath: vi.fn()
}))
vi.mock('../ipc/agent-detection-shell-path', () => ({
hydrateShellPathForAgentDetection: mocks.hydrateShellPath
}))
vi.mock('../ipc/preflight-command-exec', () => ({
isCommandOnPath: mocks.isCommandOnPath
}))
import { isNpxOnPathForSkillInstall } from './skill-install-npx-preflight'
describe('isNpxOnPathForSkillInstall', () => {
beforeEach(() => {
mocks.hydrateShellPath.mockReset()
mocks.hydrateShellPath.mockResolvedValue(undefined)
mocks.isCommandOnPath.mockReset()
mocks.isCommandOnPath.mockResolvedValue(true)
})
it('hydrates and checks the local host PATH', async () => {
await expect(isNpxOnPathForSkillInstall()).resolves.toBe(true)
expect(mocks.hydrateShellPath).toHaveBeenCalledWith(undefined)
expect(mocks.isCommandOnPath).toHaveBeenCalledWith('npx', undefined)
})
it('checks the selected WSL distro on Windows', async () => {
vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
const context = { wslDistro: 'Ubuntu' }
await expect(isNpxOnPathForSkillInstall(context)).resolves.toBe(true)
expect(mocks.hydrateShellPath).toHaveBeenCalledWith(context)
expect(mocks.isCommandOnPath).toHaveBeenCalledWith('npx', { distro: 'Ubuntu' })
})
})
@@ -0,0 +1,14 @@
import { hydrateShellPathForAgentDetection } from '../ipc/agent-detection-shell-path'
import { isCommandOnPath } from '../ipc/preflight-command-exec'
import {
getPreflightWslTarget,
type PreflightRuntimeContext
} from '../ipc/preflight-runtime-target'
export async function isNpxOnPathForSkillInstall(
context?: PreflightRuntimeContext
): Promise<boolean> {
// Why: cold GUI launches need login-shell hydration to find nvm/brew-managed Node.
await hydrateShellPathForAgentDetection(context)
return isCommandOnPath('npx', getPreflightWslTarget(context) ?? undefined)
}
+1
View File
@@ -2526,6 +2526,7 @@ export type PreloadApi = {
cancelUpdateRun: () => Promise<void>
acknowledgeUpdateRun: () => Promise<void>
getUpdateRun: () => Promise<SkillUpdateRun>
isNpxOnPath: (context?: PreflightRuntimeContext) => Promise<boolean>
onUpdateRun: (callback: (run: SkillUpdateRun) => void) => () => void
}
pet: {
+2
View File
@@ -2406,6 +2406,8 @@ const api = {
cancelUpdateRun: (): Promise<void> => ipcRenderer.invoke('skills:cancelUpdateRun'),
acknowledgeUpdateRun: (): Promise<void> => ipcRenderer.invoke('skills:acknowledgeUpdateRun'),
getUpdateRun: (): Promise<SkillUpdateRun> => ipcRenderer.invoke('skills:getUpdateRun'),
isNpxOnPath: (context?: PreflightRuntimeContext): Promise<boolean> =>
ipcRenderer.invoke('skills:isNpxOnPath', context),
onUpdateRun: (callback: (run: SkillUpdateRun) => void): (() => void) => {
const listener = (_event: Electron.IpcRendererEvent, run: SkillUpdateRun): void =>
callback(run)
@@ -0,0 +1,176 @@
// @vitest-environment happy-dom
import { act } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { FeatureSetupInlineTerminal } from './FeatureSetupInlineTerminal'
import { getFeatureSetupNpxPreflightContext } from './feature-setup-npx-preflight'
import { DEFAULT_ONBOARDING_FEATURE_SETUP_SELECTION } from './onboarding-feature-setup'
const INSTALL_COMMAND = 'npx skills add https://github.com/stablyai/orca --skill orca-cli --global'
const RUN_DESCRIPTION =
'Press Enter to run the command and confirm npx if asked. You can also set this up later in Settings.'
const mocks = vi.hoisted(() => ({
terminalProps: [] as { command: string; description: string }[],
settings: {} as {
activeRuntimeEnvironmentId?: string | null
terminalWindowsShell?: string
terminalWindowsWslDistro?: string | null
},
isNpxOnPath: vi.fn(),
openUrl: vi.fn()
}))
vi.mock('./OnboardingInlineCommandTerminal', () => ({
OnboardingInlineCommandTerminal: (props: { command: string; description: string }) => {
mocks.terminalProps.push(props)
return <div data-testid="inline-command-terminal">{props.description}</div>
}
}))
vi.mock('@/store', () => ({
useAppStore: (selector: (state: unknown) => unknown) => selector({ settings: mocks.settings })
}))
vi.mock('@/lib/telemetry', () => ({
track: vi.fn()
}))
let container: HTMLElement | null = null
let root: Root | null = null
async function renderTerminal(): Promise<void> {
container = document.createElement('div')
document.body.appendChild(container)
root = createRoot(container)
await act(async () => {
root?.render(
<FeatureSetupInlineTerminal
command={INSTALL_COMMAND}
selection={DEFAULT_ONBOARDING_FEATURE_SETUP_SELECTION}
/>
)
})
await act(async () => {})
}
describe('FeatureSetupInlineTerminal', () => {
beforeEach(() => {
mocks.terminalProps.length = 0
mocks.settings = {}
mocks.isNpxOnPath.mockReset()
mocks.isNpxOnPath.mockResolvedValue(true)
mocks.openUrl.mockReset()
Object.defineProperty(window, 'api', {
configurable: true,
value: {
platform: { get: () => ({ platform: 'darwin' }) },
shell: { openUrl: mocks.openUrl },
skills: { isNpxOnPath: mocks.isNpxOnPath }
}
})
})
afterEach(async () => {
if (root) {
await act(async () => {
root?.unmount()
})
}
root = null
container?.remove()
container = null
Reflect.deleteProperty(window, 'api')
})
it('keeps the run instruction when npx is on PATH', async () => {
await renderTerminal()
expect(mocks.isNpxOnPath).toHaveBeenCalledTimes(1)
expect(mocks.terminalProps.at(-1)?.description).toBe(RUN_DESCRIPTION)
})
it('blocks the raw command and shows actionable guidance when npx is missing', async () => {
mocks.isNpxOnPath.mockResolvedValue(false)
await renderTerminal()
expect(container?.textContent).toContain('Node.js is required')
expect(container?.textContent).toContain('npx was not found in this terminal environment')
expect(container?.textContent).toContain('Download Node.js')
expect(container?.textContent).toContain('Re-check')
expect(mocks.terminalProps).toHaveLength(0)
})
it('skips the local probe when a remote runtime environment is focused', async () => {
mocks.settings = { activeRuntimeEnvironmentId: 'ssh-host-1' }
mocks.isNpxOnPath.mockResolvedValue(false)
await renderTerminal()
expect(mocks.isNpxOnPath).not.toHaveBeenCalled()
expect(mocks.terminalProps.at(-1)?.description).toBe(RUN_DESCRIPTION)
})
it('fails open when the probe rejects', async () => {
mocks.isNpxOnPath.mockRejectedValue(new Error('ipc unavailable'))
await renderTerminal()
expect(mocks.terminalProps.at(-1)?.description).toBe(RUN_DESCRIPTION)
})
it('re-checks after Node.js is installed', async () => {
mocks.isNpxOnPath.mockResolvedValueOnce(false).mockResolvedValueOnce(true)
await renderTerminal()
const recheck = Array.from(container?.querySelectorAll('button') ?? []).find(
(button) => button.textContent?.trim() === 'Re-check'
)
expect(recheck).toBeDefined()
await act(async () => {
recheck?.dispatchEvent(new MouseEvent('click', { bubbles: true }))
})
await act(async () => {})
expect(mocks.isNpxOnPath).toHaveBeenCalledTimes(2)
expect(mocks.terminalProps.at(-1)?.command).toBe(INSTALL_COMMAND)
})
it('opens the Node.js download in the external browser', async () => {
mocks.isNpxOnPath.mockResolvedValue(false)
await renderTerminal()
const download = Array.from(container?.querySelectorAll('button') ?? []).find(
(button) => button.textContent?.trim() === 'Download Node.js'
)
download?.dispatchEvent(new MouseEvent('click', { bubbles: true }))
expect(mocks.openUrl).toHaveBeenCalledWith('https://nodejs.org/')
})
})
describe('getFeatureSetupNpxPreflightContext', () => {
it('targets the configured WSL distro', () => {
expect(getFeatureSetupNpxPreflightContext('win32', 'wsl.exe', ' Ubuntu ')).toEqual({
wslDistro: 'Ubuntu'
})
})
it('targets the default distro when none is configured', () => {
expect(
getFeatureSetupNpxPreflightContext('win32', 'C:\\Windows\\System32\\wsl.exe', null)
).toEqual({ wslDefault: true })
})
it.each([
['darwin', 'zsh'],
['linux', 'bash'],
['win32', 'powershell.exe'],
['win32', 'cmd.exe'],
['win32', 'C:\\Program Files\\Git\\bin\\bash.exe']
] as const)('uses the host PATH for %s with %s', (platform, shell) => {
expect(getFeatureSetupNpxPreflightContext(platform, shell, null)).toBeUndefined()
})
})
@@ -1,11 +1,16 @@
import { useCallback, useMemo, useRef, type KeyboardEvent } from 'react'
import { useCallback, useEffect, useMemo, useRef, useState, type KeyboardEvent } from 'react'
import { ExternalLink, Loader2, RefreshCw, TriangleAlert } from 'lucide-react'
import { Button } from '@/components/ui/button'
import { getRendererAppPlatform } from '@/lib/renderer-app-platform'
import { track } from '@/lib/telemetry'
import { notifyInstalledAgentSkillsChanged } from '@/hooks/useInstalledAgentSkills'
import { useAppStore } from '@/store'
import { OnboardingInlineCommandTerminal } from './OnboardingInlineCommandTerminal'
import {
onboardingFeatureSetupTelemetrySelection,
type OnboardingFeatureSetupSelection
} from './onboarding-feature-setup'
import { getFeatureSetupNpxPreflightContext } from './feature-setup-npx-preflight'
import { translate } from '@/i18n/i18n'
type FeatureSetupInlineTerminalProps = {
@@ -13,12 +18,63 @@ type FeatureSetupInlineTerminalProps = {
selection: OnboardingFeatureSetupSelection
}
type NpxProbeStatus = 'checking' | 'available' | 'missing'
export function FeatureSetupInlineTerminal({
command,
selection
}: FeatureSetupInlineTerminalProps): React.JSX.Element {
const terminalOpenedTrackedRef = useRef(false)
const terminalInteractedTrackedRef = useRef(false)
const activeRuntimeEnvironmentId = useAppStore((s) => s.settings?.activeRuntimeEnvironmentId)
const terminalWindowsShell = useAppStore((s) => s.settings?.terminalWindowsShell)
const terminalWindowsWslDistro = useAppStore((s) => s.settings?.terminalWindowsWslDistro)
const isRemoteRuntimeFocused = Boolean(activeRuntimeEnvironmentId?.trim())
const platform = getRendererAppPlatform()
const [probeRevision, setProbeRevision] = useState(0)
const probeKey = [
isRemoteRuntimeFocused,
platform,
terminalWindowsShell,
terminalWindowsWslDistro,
probeRevision
].join(':')
const initialProbeStatus = isRemoteRuntimeFocused ? 'available' : 'checking'
const [npxProbe, setNpxProbe] = useState<{ key: string; status: NpxProbeStatus }>({
key: probeKey,
status: initialProbeStatus
})
if (npxProbe.key !== probeKey) {
setNpxProbe({ key: probeKey, status: initialProbeStatus })
}
useEffect(() => {
if (isRemoteRuntimeFocused) {
return
}
let cancelled = false
const context = getFeatureSetupNpxPreflightContext(
platform,
terminalWindowsShell,
terminalWindowsWslDistro
)
window.api.skills.isNpxOnPath(context).then(
(onPath) => {
if (!cancelled) {
setNpxProbe({ key: probeKey, status: onPath ? 'available' : 'missing' })
}
},
() => {
if (!cancelled) {
setNpxProbe({ key: probeKey, status: 'available' })
}
}
)
return () => {
cancelled = true
}
}, [isRemoteRuntimeFocused, platform, probeKey, terminalWindowsShell, terminalWindowsWslDistro])
const selectionTelemetry = useMemo(
() => onboardingFeatureSetupTelemetrySelection(selection),
@@ -54,6 +110,70 @@ export function FeatureSetupInlineTerminal({
[selectionTelemetry]
)
if (npxProbe.status === 'checking') {
return (
<div
role="status"
className="mt-4 flex min-h-24 items-center justify-center gap-2 rounded-xl border border-border bg-card text-sm text-muted-foreground"
>
<Loader2 className="size-4 animate-spin" />
{translate(
'auto.components.onboarding.FeatureSetupInlineTerminal.checkingNpx',
'Checking for npx…'
)}
</div>
)
}
if (npxProbe.status === 'missing') {
return (
<section className="mt-4 rounded-xl border border-border bg-card p-4" aria-live="polite">
<div className="flex items-start gap-3">
<TriangleAlert className="mt-0.5 size-4 shrink-0 text-destructive" />
<div className="min-w-0">
<h3 className="text-sm font-medium text-foreground">
{translate(
'auto.components.onboarding.FeatureSetupInlineTerminal.npxMissingTitle',
'Node.js is required'
)}
</h3>
<p className="mt-1 text-xs leading-relaxed text-muted-foreground">
{translate(
'auto.components.onboarding.FeatureSetupInlineTerminal.npxMissingNotice',
'npx was not found in this terminal environment. Install Node.js LTS, then re-check to continue skill setup.'
)}
</p>
<div className="mt-3 flex flex-wrap gap-2">
<Button
type="button"
size="sm"
onClick={() => void window.api.shell.openUrl('https://nodejs.org/')}
>
<ExternalLink className="size-3.5" />
{translate(
'auto.components.onboarding.FeatureSetupInlineTerminal.downloadNode',
'Download Node.js'
)}
</Button>
<Button
type="button"
variant="outline"
size="sm"
onClick={() => setProbeRevision((revision) => revision + 1)}
>
<RefreshCw className="size-3.5" />
{translate(
'auto.components.onboarding.FeatureSetupInlineTerminal.recheckNpx',
'Re-check'
)}
</Button>
</div>
</div>
</div>
</section>
)
}
return (
<OnboardingInlineCommandTerminal
command={command}
@@ -0,0 +1,13 @@
import { isWslShellName } from '../../../../shared/local-windows-terminal-runtime'
export function getFeatureSetupNpxPreflightContext(
platform: NodeJS.Platform,
terminalWindowsShell: string | undefined,
terminalWindowsWslDistro: string | null | undefined
): { wslDistro?: string; wslDefault?: boolean } | undefined {
if (platform !== 'win32' || !isWslShellName(terminalWindowsShell)) {
return undefined
}
const distro = terminalWindowsWslDistro?.trim()
return distro ? { wslDistro: distro } : { wslDefault: true }
}
+6 -1
View File
@@ -11440,7 +11440,12 @@
"FeatureSetupInlineTerminal": {
"789b59936e": "Press Enter to run the command and confirm npx if asked. You can also set this up later in Settings.",
"47fc6cc6dc": "Skill setup command",
"c767ab7061": "Skill setup"
"c767ab7061": "Skill setup",
"checkingNpx": "Checking for npx…",
"downloadNode": "Download Node.js",
"npxMissingNotice": "npx was not found in this terminal environment. Install Node.js LTS, then re-check to continue skill setup.",
"npxMissingTitle": "Node.js is required",
"recheckNpx": "Re-check"
},
"IntegrationsStep": {
"277f30eb34": "Linear, GitLab, Bitbucket, Azure DevOps, Gitea, and Jira live in Settings > Integrations.",
+2
View File
@@ -2966,6 +2966,8 @@ function createSkillsApi(): NonNullable<Partial<PreloadApi>['skills']> {
cancelUpdateRun: () => Promise.resolve(),
acknowledgeUpdateRun: () => Promise.resolve(),
getUpdateRun: () => Promise.resolve({ state: 'idle' as const }),
// Why: the browser client cannot inspect the host PATH; avoid a false block.
isNpxOnPath: () => Promise.resolve(true),
onUpdateRun: () => () => {}
}
}