From e1092291ae4e90d18b1d896e9f8e8c3ef056480f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:41:51 -0700 Subject: [PATCH] fix(settings): keep copied skill commands bare for POSIX-family Windows shells (#11599) The cmd.exe npx preflight added in #10453 silently no-ops when pasted into Git Bash: MSYS rewrites the leading /d /s /c switches into drive paths, so cmd.exe starts an interactive session and never runs the payload. Skip the wrapper when the configured Windows shell resolves to the posix family, matching the shell the copied command actually lands in. --- .../settings/CliSkillRuntimeSetup.test.tsx | 49 +++++++++++++++++++ .../settings/CliSkillRuntimeSetup.tsx | 12 +++++ 2 files changed, 61 insertions(+) diff --git a/src/renderer/src/components/settings/CliSkillRuntimeSetup.test.tsx b/src/renderer/src/components/settings/CliSkillRuntimeSetup.test.tsx index 6150bd808eb..269074ff895 100644 --- a/src/renderer/src/components/settings/CliSkillRuntimeSetup.test.tsx +++ b/src/renderer/src/components/settings/CliSkillRuntimeSetup.test.tsx @@ -243,6 +243,55 @@ describe('CliSkillRuntimeSetup runtime helpers', () => { } }) + it('skips the Windows preflight when the configured Windows shell is POSIX-family', () => { + const installCommand = buildAgentFeatureSkillInstallCommand(['orchestration']) + const windowsHost = { runtime: 'host', label: 'Windows' } as const + const previous = useAppStore.getState() + + try { + // MSYS rewrites cmd.exe's leading /d /s /c switches into drive paths, so + // the copied command must stay bare for a Git Bash / wsl.exe paste target. + for (const terminalWindowsShell of ['git-bash', 'C:\\Program Files\\Git\\bin\\bash.exe']) { + useAppStore.setState({ + settings: { ...getDefaultSettings('/tmp'), terminalWindowsShell } + }) + expect(buildSkillCommandForRuntime(installCommand, windowsHost, 'win32')).toBe( + installCommand + ) + } + + // cmd-family shells still need the preflight wrapper. + useAppStore.setState({ + settings: { ...getDefaultSettings('/tmp'), terminalWindowsShell: 'cmd.exe' } + }) + expect(buildSkillCommandForRuntime(installCommand, windowsHost, 'win32')).toBe( + `${windowsNpxPreflightPrefix}${windowsNpxGuidance}) else (${installCommand})"` + ) + } finally { + useAppStore.setState({ settings: previous.settings }) + } + }) + + it('keeps the bare reinstall rewrite for POSIX-family Windows skill updates', () => { + const installCommand = buildAgentFeatureSkillInstallCommand(['orchestration']) + const previous = useAppStore.getState() + useAppStore.setState({ + settings: { ...getDefaultSettings('/tmp'), terminalWindowsShell: 'git-bash' } + }) + + try { + expect( + buildSkillCommandForRuntime( + 'npx skills update orchestration --global', + { runtime: 'host', label: 'Windows' }, + 'win32' + ) + ).toBe(installCommand) + } finally { + useAppStore.setState({ settings: previous.settings }) + } + }) + it('does not wrap unrelated Windows host commands', () => { expect( buildSkillCommandForRuntime( diff --git a/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx b/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx index bee39049fc5..005b5dfc5c6 100644 --- a/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx +++ b/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx @@ -8,6 +8,7 @@ import { quotePowerShellNativeArgument } from '../../../../shared/powershell-native-argument' import { buildWslLoginShellCommand } from '../../../../shared/wsl-login-shell-command' +import { resolveWindowsShellStartupFamily } from '../../../../shared/windows-terminal-shell' import { getProjectAgentSkillTerminalShellOverride } from '@/lib/project-skill-runtime' import { useAppStore } from '@/store' import { buildAgentFeatureSkillInstallCommand } from '../../../../shared/agent-feature-install-commands' @@ -136,6 +137,10 @@ function wrapWindowsSkillCommandWithNpxPrerequisite( // Why: skill setup terminals spawn on the focused runtime environment, so a // Windows client must not hand a cmd.exe command to a remote host. isRemoteRuntimeEnvironmentFocused() || + // 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() || !/^npx\s+skills\s+(?:add|update)\b/i.test(trimmedCommand) ) { return command @@ -149,6 +154,13 @@ function wrapWindowsSkillCommandWithNpxPrerequisite( return `cmd.exe /d /s /c "where.exe npx >nul 2>nul & if errorlevel 1 (${missingNpxGuidance}) else (${trimmedCommand})"` } +function isPosixFamilyWindowsShellConfigured(): boolean { + return ( + resolveWindowsShellStartupFamily(useAppStore.getState().settings?.terminalWindowsShell) === + 'posix' + ) +} + function isRemoteRuntimeEnvironmentFocused(): boolean { // Why: the terminal router also weighs how many environments are saved, but // that slice has no subscriber here. Read only the focused id, which every