diff --git a/src/main/ipc/skills.test.ts b/src/main/ipc/skills.test.ts index e6f88345ff2..004de021e05 100644 --- a/src/main/ipc/skills.test.ts +++ b/src/main/ipc/skills.test.ts @@ -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 } + 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 + } + 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) + }) }) diff --git a/src/main/ipc/skills.ts b/src/main/ipc/skills.ts index e2d12ca57ba..f8fc42af0d9 100644 --- a/src/main/ipc/skills.ts +++ b/src/main/ipc/skills.ts @@ -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 => { return runner.getState() }) + + ipcMain.handle( + 'skills:isNpxOnPath', + async (_event, context?: PreflightRuntimeContext): Promise => { + return isNpxOnPathForSkillInstall(context) + } + ) } diff --git a/src/main/skills/skill-install-npx-preflight.test.ts b/src/main/skills/skill-install-npx-preflight.test.ts new file mode 100644 index 00000000000..25463918dac --- /dev/null +++ b/src/main/skills/skill-install-npx-preflight.test.ts @@ -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' }) + }) +}) diff --git a/src/main/skills/skill-install-npx-preflight.ts b/src/main/skills/skill-install-npx-preflight.ts new file mode 100644 index 00000000000..46468348752 --- /dev/null +++ b/src/main/skills/skill-install-npx-preflight.ts @@ -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 { + // Why: cold GUI launches need login-shell hydration to find nvm/brew-managed Node. + await hydrateShellPathForAgentDetection(context) + return isCommandOnPath('npx', getPreflightWslTarget(context) ?? undefined) +} diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 81796d17eb3..453074fa099 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -2526,6 +2526,7 @@ export type PreloadApi = { cancelUpdateRun: () => Promise acknowledgeUpdateRun: () => Promise getUpdateRun: () => Promise + isNpxOnPath: (context?: PreflightRuntimeContext) => Promise onUpdateRun: (callback: (run: SkillUpdateRun) => void) => () => void } pet: { diff --git a/src/preload/index.ts b/src/preload/index.ts index 7c63fd9756d..a8f7e21339a 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -2406,6 +2406,8 @@ const api = { cancelUpdateRun: (): Promise => ipcRenderer.invoke('skills:cancelUpdateRun'), acknowledgeUpdateRun: (): Promise => ipcRenderer.invoke('skills:acknowledgeUpdateRun'), getUpdateRun: (): Promise => ipcRenderer.invoke('skills:getUpdateRun'), + isNpxOnPath: (context?: PreflightRuntimeContext): Promise => + ipcRenderer.invoke('skills:isNpxOnPath', context), onUpdateRun: (callback: (run: SkillUpdateRun) => void): (() => void) => { const listener = (_event: Electron.IpcRendererEvent, run: SkillUpdateRun): void => callback(run) diff --git a/src/renderer/src/components/onboarding/FeatureSetupInlineTerminal.test.tsx b/src/renderer/src/components/onboarding/FeatureSetupInlineTerminal.test.tsx new file mode 100644 index 00000000000..8698314ef09 --- /dev/null +++ b/src/renderer/src/components/onboarding/FeatureSetupInlineTerminal.test.tsx @@ -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
{props.description}
+ } +})) + +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 { + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + await act(async () => { + root?.render( + + ) + }) + 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() + }) +}) diff --git a/src/renderer/src/components/onboarding/FeatureSetupInlineTerminal.tsx b/src/renderer/src/components/onboarding/FeatureSetupInlineTerminal.tsx index e15c796d4bf..0488eef7948 100644 --- a/src/renderer/src/components/onboarding/FeatureSetupInlineTerminal.tsx +++ b/src/renderer/src/components/onboarding/FeatureSetupInlineTerminal.tsx @@ -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 ( +
+ + {translate( + 'auto.components.onboarding.FeatureSetupInlineTerminal.checkingNpx', + 'Checking for npx…' + )} +
+ ) + } + + if (npxProbe.status === 'missing') { + return ( +
+
+ +
+

+ {translate( + 'auto.components.onboarding.FeatureSetupInlineTerminal.npxMissingTitle', + 'Node.js is required' + )} +

+

+ {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.' + )} +

+
+ + +
+
+
+
+ ) + } + return ( Integrations.", diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 2ae7a4d50c3..d2fd27f95e5 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -2966,6 +2966,8 @@ function createSkillsApi(): NonNullable['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: () => () => {} } }