From ea40c873158d6c181c07a3eacbae36bc1a18098b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 31 Jul 2026 14:03:26 -0700 Subject: [PATCH] [P2] fix(skills): keep the npx preflight in the forced-PowerShell setup terminal on Windows (#11814) Co-authored-by: Orca --- .../feature-tips/CliSkillSetupTerminal.tsx | 12 +++- ...inalOrchestrationDialog.freshness.test.tsx | 1 + .../settings/AgentSkillSetupPanel.tsx | 4 +- .../settings/CliSkillRuntimeSetup.test.tsx | 55 +++++++++++++++++++ .../settings/CliSkillRuntimeSetup.tsx | 44 ++++++++++++++- ...nt-skill-installed-command-callers.test.ts | 6 +- 6 files changed, 115 insertions(+), 7 deletions(-) diff --git a/src/renderer/src/components/feature-tips/CliSkillSetupTerminal.tsx b/src/renderer/src/components/feature-tips/CliSkillSetupTerminal.tsx index 2fa6f16ec..d1e025cbb 100644 --- a/src/renderer/src/components/feature-tips/CliSkillSetupTerminal.tsx +++ b/src/renderer/src/components/feature-tips/CliSkillSetupTerminal.tsx @@ -3,7 +3,10 @@ import { toast } from 'sonner' import { Button } from '@/components/ui/button' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { OnboardingInlineCommandTerminal } from '@/components/onboarding/OnboardingInlineCommandTerminal' -import { buildSkillCommandForRuntime } from '@/components/settings/CliSkillRuntimeSetup' +import { + buildSkillCommandForRuntime, + buildSkillSetupTerminalCommand +} from '@/components/settings/CliSkillRuntimeSetup' import { ORCA_CLI_ORCHESTRATION_SKILL_INSTALL_COMMAND } from '@/lib/agent-feature-install-commands' import { useActiveProjectSkillRuntime } from '@/hooks/useActiveProjectSkillRuntime' import { translate } from '@/i18n/i18n' @@ -18,6 +21,11 @@ export function CliSkillSetupTerminal(): React.JSX.Element { ORCA_CLI_ORCHESTRATION_SKILL_INSTALL_COMMAND, activeSkillRuntime.installDisabledReason ? undefined : activeSkillRuntime.agentRuntime ) + // The copied string stays as built; only what we execute is adapted. + const setupTerminalCommand = buildSkillSetupTerminalCommand( + skillCommand, + activeSkillRuntime.terminalShellOverride + ) const handleCopySkillCommand = async (): Promise => { try { @@ -70,7 +78,7 @@ export function CliSkillSetupTerminal(): React.JSX.Element { ({ vi.mock('@/components/settings/CliSkillRuntimeSetup', () => ({ buildSkillCommandForRuntime: (command: string) => command, + buildSkillSetupTerminalCommand: (command: string) => command, ensureWslCliAvailableForAgentSkillTerminal: vi.fn(), getWslCliDistroRequest: () => undefined })) diff --git a/src/renderer/src/components/settings/AgentSkillSetupPanel.tsx b/src/renderer/src/components/settings/AgentSkillSetupPanel.tsx index ee021d969..8db579cfa 100644 --- a/src/renderer/src/components/settings/AgentSkillSetupPanel.tsx +++ b/src/renderer/src/components/settings/AgentSkillSetupPanel.tsx @@ -5,6 +5,7 @@ import { IntegrationStatusPill } from '../integration-status-pill' import { SkillFreshnessStatusPill } from '../skills/SkillFreshnessStatusPill' import { OnboardingInlineCommandTerminal } from '../onboarding/OnboardingInlineCommandTerminal' import { AgentSkillSetupFailureNotice } from './AgentSkillSetupFailureNotice' +import { buildSkillSetupTerminalCommand } from './CliSkillRuntimeSetup' import type { AgentSkillSetupPanelProps } from './agent-skill-setup-panel-props' import { Button } from '../ui/button' import { Tooltip, TooltipContent, TooltipTrigger } from '../ui/tooltip' @@ -386,10 +387,11 @@ export function AgentSkillSetupPanel({ + {/* The copied string above stays as built; only what we run is adapted. */} { } }) + it('keeps the npx preflight in the PowerShell-forced setup terminal', () => { + const installCommand = buildAgentFeatureSkillInstallCommand(['orchestration']) + const windowsHost = { runtime: 'host', label: 'Windows' } as const + const previous = useAppStore.getState() + useAppStore.setState({ + settings: { ...getDefaultSettings('/tmp'), terminalWindowsShell: 'git-bash' } + }) + + try { + const copied = buildSkillCommandForRuntime(installCommand, windowsHost, 'win32') + expect(copied).toBe(installCommand) + // Orca forces its own setup terminal to powershell.exe, where cmd.exe works. + expect(buildSkillSetupTerminalCommand(copied, 'powershell.exe', 'win32')).toBe( + `${windowsNpxPreflightPrefix}${windowsNpxGuidance}) else (${installCommand})"` + ) + } finally { + useAppStore.setState({ settings: previous.settings }) + } + }) + + it('does not re-wrap the setup terminal command when no shell override applies', () => { + const installCommand = buildAgentFeatureSkillInstallCommand(['orchestration']) + const previous = useAppStore.getState() + useAppStore.setState({ + settings: { ...getDefaultSettings('/tmp'), terminalWindowsShell: 'cmd.exe' } + }) + + try { + const copied = buildSkillCommandForRuntime( + installCommand, + { runtime: 'host', label: 'Windows' }, + 'win32' + ) + expect(buildSkillSetupTerminalCommand(copied, undefined, 'win32')).toBe(copied) + // An already-wrapped command must not gain a second preflight. + expect(buildSkillSetupTerminalCommand(copied, 'powershell.exe', 'win32')).toBe(copied) + } finally { + useAppStore.setState({ settings: previous.settings }) + } + }) + + it('leaves WSL and non-Windows setup terminal commands untouched', () => { + const wslCommand = buildSkillCommandForRuntime( + 'npx skills add orchestration --global', + { runtime: 'wsl', wslDistro: 'Ubuntu', label: 'WSL Ubuntu' }, + 'win32' + ) + + expect(buildSkillSetupTerminalCommand(wslCommand, 'powershell.exe', 'win32')).toBe(wslCommand) + expect( + buildSkillSetupTerminalCommand('npx skills add orchestration --global', undefined, 'linux') + ).toBe('npx skills add orchestration --global') + }) + it('keeps the bare reinstall rewrite for POSIX-family Windows skill updates', () => { const installCommand = buildAgentFeatureSkillInstallCommand(['orchestration']) const previous = useAppStore.getState() diff --git a/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx b/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx index 005b5dfc5..039c2035d 100644 --- a/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx +++ b/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx @@ -89,7 +89,11 @@ export function buildSkillCommandForRuntime( currentPlatform ) if (resolvedRuntime.runtime !== 'wsl') { - return wrapWindowsSkillCommandWithNpxPrerequisite(normalizedCommand, currentPlatform) + return wrapWindowsSkillCommandWithNpxPrerequisite( + normalizedCommand, + currentPlatform, + 'copied-command' + ) } const distroArg = resolvedRuntime.wslDistro?.trim() @@ -127,9 +131,43 @@ function normalizeWindowsSkillUpdateCommand( return buildAgentFeatureSkillInstallCommand([updateMatch[1]]) } +/** + * Where a built skill command is going: the user's clipboard (their own shell) + * or the setup terminal Orca spawns itself. + */ +type SkillCommandTarget = 'copied-command' | 'orca-setup-terminal' + +/** + * Re-adds the npx preflight for Orca's own setup terminal, which + * `getAgentSkillTerminalShellOverride` forces onto powershell.exe. The copied + * string stays bare for POSIX-family shells; only the executed one is wrapped. + */ +export function buildSkillSetupTerminalCommand( + copiedCommand: string, + terminalShellOverride: string | undefined, + currentPlatform = getSkillCommandPlatform() +): string { + if (!isSetupTerminalForcedToPowerShell(terminalShellOverride)) { + return copiedCommand + } + return wrapWindowsSkillCommandWithNpxPrerequisite( + copiedCommand, + currentPlatform, + 'orca-setup-terminal' + ) +} + +function isSetupTerminalForcedToPowerShell(terminalShellOverride: string | undefined): boolean { + const trimmedOverride = terminalShellOverride?.trim() + return ( + Boolean(trimmedOverride) && resolveWindowsShellStartupFamily(trimmedOverride) === 'powershell' + ) +} + function wrapWindowsSkillCommandWithNpxPrerequisite( command: string, - currentPlatform: NodeJS.Platform + currentPlatform: NodeJS.Platform, + target: SkillCommandTarget ): string { const trimmedCommand = command.trim() if ( @@ -140,7 +178,7 @@ function wrapWindowsSkillCommandWithNpxPrerequisite( // Why: the copied command lands in the user's configured shell, and MSYS // shells rewrite cmd.exe's leading /d /s /c switches into drive paths, // starting an interactive cmd session instead of running the payload. - isPosixFamilyWindowsShellConfigured() || + (target === 'copied-command' && isPosixFamilyWindowsShellConfigured()) || !/^npx\s+skills\s+(?:add|update)\b/i.test(trimmedCommand) ) { return command diff --git a/src/renderer/src/components/settings/agent-skill-installed-command-callers.test.ts b/src/renderer/src/components/settings/agent-skill-installed-command-callers.test.ts index f0bab20bf..262876184 100644 --- a/src/renderer/src/components/settings/agent-skill-installed-command-callers.test.ts +++ b/src/renderer/src/components/settings/agent-skill-installed-command-callers.test.ts @@ -170,7 +170,11 @@ describe('AgentSkillSetupPanel installed-command call sites', () => { ) expect(source).toContain('buildSkillCommandForRuntime(') - expect(source).toContain('command={skillCommand}') + // The copied string stays bare for POSIX-family shells; the forced-PowerShell + // setup terminal keeps the npx preflight. + expect(source).toContain('writeClipboardText(skillCommand)') + expect(source).toContain('buildSkillSetupTerminalCommand(') + expect(source).toContain('command={setupTerminalCommand}') expect(source).toContain('shellOverride={activeSkillRuntime.terminalShellOverride}') expect(source).not.toContain('command={ORCA_CLI_ORCHESTRATION_SKILL_INSTALL_COMMAND}') // This terminal auto-pastes with no install gate, so a repair-required runtime