From 2f05d46d97dda8654cac7d3255c312d7de7ad79f Mon Sep 17 00:00:00 2001 From: OrcaWin <293788423+OrcaWin@users.noreply.github.com> Date: Tue, 4 Aug 2026 00:25:02 -0400 Subject: [PATCH] fix(worktrees): keep the cmd setup runner launchable from a Git Bash pane Adversarial review of this PR found that pinning the runner format per script reopened issue #6896 one layer down. - `WorktreeSetupLaunch.shell` had been redefined to mean "the format the runner file was written in". `resolveSetupRunnerCommand` consumes it as "the shell that types the launch command", so a Git Bash terminal with a batch setup script produced `cmd.exe /c "C:\...\setup-runner.cmd"` typed into a bash pane, where MSYS rewrites the `/c` switch into a drive path: cmd opens interactively and setup never runs. `shell` is the terminal's family again; the runner file's .cmd/.sh extension carries the format, and a batch runner launched from a POSIX pane reuses the existing PowerShell ProcessStartInfo launcher. - The cmd runner dropped a leading `#!` line and ran the rest as batch, so a bash script reaching cmd (PowerShell/cmd terminal, or any SSH-to-Windows host) got its interpreter-agnostic prefix executed before failing mid-way. It now prints why and exits 1 without running anything. - A `#!` line's option flags were discarded: `#!/usr/bin/env -S bash -euo pipefail` lost pipefail because the runner is launched as `bash `. The generated posix runner now replays declared flags via `set` and drops the duplicate interpreter line. - Docs cover the per-user setup command in repository hook settings, which goes through the same `#!` rule, and describe what the `#!` line does and does not select. Tests: composed launch command for a POSIX pane + cmd runner (hooks, shared runner command, setup sequencing gate, observed-setup signal), the cmd runner's shebang refusal, and shebang flag replay. Each fails with the source reverted. --- AGENTS.md | 2 +- docs/reference/windows-setup-shell.md | 36 +++++++++- src/main/hooks-runner.test.ts | 27 +++++-- src/main/hooks.test.ts | 72 +++++++++++++++++-- src/main/hooks.ts | 47 +++++++++--- .../setup-completion-signal.test.ts | 13 ++++ src/shared/setup-agent-sequencing.test.ts | 20 ++++++ src/shared/setup-runner-command.test.ts | 45 ++++++++++++ src/shared/setup-runner-command.ts | 24 +++++-- src/shared/setup-script-shebang.test.ts | 47 +++++++++++- src/shared/setup-script-shebang.ts | 68 +++++++++++++++--- 11 files changed, 362 insertions(+), 39 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9598089df08..6bc54a6f5c0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,7 +29,7 @@ Orca targets macOS, Linux, and Windows. Keep all platform-dependent behavior beh - **Keyboard shortcuts**: Never hardcode `e.metaKey`. Use a platform check (`navigator.userAgent.includes('Mac')`) to pick `metaKey` on Mac and `ctrlKey` on Linux/Windows. Electron menu accelerators should use `CmdOrCtrl`. - **Shortcut labels in UI**: Display `⌘` / `⇧` on Mac and `Ctrl+` / `Shift+` on other platforms. - **File paths**: Use `path.join` or Electron/Node path utilities — never assume `/` or `\`. -- **Windows setup scripts**: the setup/issue-command runner is a `.cmd` batch file unless the script starts with a `#!` line. Never derive it from the user's terminal-shell preference. See [`docs/reference/windows-setup-shell.md`](./docs/reference/windows-setup-shell.md). +- **Windows setup scripts**: the setup/issue-command runner is a `.cmd` batch file unless the script starts with a `#!` line — never derive that from the user's terminal-shell preference, and never launch a `.cmd` runner with a bare `cmd.exe /c` from a Git Bash pane (MSYS rewrites the `/c`). See [`docs/reference/windows-setup-shell.md`](./docs/reference/windows-setup-shell.md). - **Linux native modules**: keep the glibc floor at Ubuntu 20.04 / glibc 2.31. A module compiled from source on a newer runner can reference symbol versions absent on the floor and crash the app on startup. See [`docs/reference/linux-glibc-compatibility.md`](./docs/reference/linux-glibc-compatibility.md); packaging fails if a bundled native binary needs newer glibc. ## SSH Use Case diff --git a/docs/reference/windows-setup-shell.md b/docs/reference/windows-setup-shell.md index 2d465180a8e..c8231d078d5 100644 --- a/docs/reference/windows-setup-shell.md +++ b/docs/reference/windows-setup-shell.md @@ -35,13 +35,45 @@ language a project's setup script is written in. Deriving the runner from it had A `#!` line is per-project, explicit, and identical for everyone who checks the repo out. +The same rule applies to the per-user setup command in **Settings → repository hooks** +(`repo.hookSettings.scripts.setup`): it is merged into the same script that reaches the runner, so a +POSIX one-liner stored there needs its own `#!` line to run under bash on Windows. + +## What the `#!` line does and does not select + +The generated runner is always executed by bash (`bash `; Git Bash on native Windows), on +every platform. The `#!` line therefore does two things: + +- It declares the script is written for a POSIX shell, which is what selects the bash runner. +- Its option flags are replayed with `set`, so `#!/usr/bin/env -S bash -euo pipefail` really does + get `pipefail`. Without that replay the flags would be silently dropped, because `bash ` + never parses the interpreter line. + +The interpreter name itself is not honored beyond "is this a POSIX shell": `#!/bin/sh` and +`#!/bin/zsh` scripts run under bash, exactly as they already did on macOS and Linux. + ## Requirements for the bash runner A `#!` line only takes effect when Orca can actually launch bash from the configured terminal — the terminal shell must resolve to Git Bash (`resolveWindowsGitBashShellPath`). The generated runner uses MSYS `/c/...` paths, which Cygwin and the WSL shim do not accept, and the launch command is -typed into whatever shell the terminal opened with. When Git Bash is not available the script falls -back to the `.cmd` runner; the `#!` line is dropped rather than executed. +typed into whatever shell the terminal opened with. + +When bash is not available (a PowerShell/cmd terminal, or an SSH-to-Windows host, which always uses +the remote's `.cmd` runner) the `#!` script is **not** executed under cmd. The generated `.cmd` +runner prints why and exits 1, because running the interpreter-agnostic prefix of a bash script +(`pnpm install`, `git submodule update`) and only failing at the first bash-only line leaves a +half-set-up worktree that looks finished. + +## Launching a `.cmd` runner from a Git Bash terminal + +The runner format and the shell that types the launch command are independent: a Git Bash terminal +with a batch-syntax setup script gets a `.cmd` runner launched from a bash pane. `cmd.exe /c +"C:\..."` cannot be used there — MSYS rewrites the bare `/c` switch into a drive path, so cmd opens +interactively and the runner never executes (issue #6896). Those launches reuse the PowerShell +`ProcessStartInfo` launcher (`buildWindowsCmdRunnerDelayedLaunchCommand`), which carries the switch +and the runner path outside the command line. `WorktreeSetupLaunch.shell` therefore describes the +launching pane; the runner file's `.cmd`/`.sh` extension describes the format. WSL worktrees and non-Windows platforms are unaffected: they always use the bash runner. SSH hosts choose their runner from the remote path format, never from local Windows preferences. diff --git a/src/main/hooks-runner.test.ts b/src/main/hooks-runner.test.ts index fbf0bfe8567..8815ef652d3 100644 --- a/src/main/hooks-runner.test.ts +++ b/src/main/hooks-runner.test.ts @@ -113,7 +113,7 @@ describe('createSetupRunnerScript', () => { }) expect(vi.mocked(fs.writeFileSync)).toHaveBeenCalledWith( 'C:\\repo\\.git\\worktrees\\feature\\orca\\setup-runner.sh', - '#!/usr/bin/env bash\nset -e\n#!/usr/bin/env bash\npnpm install\n', + '#!/usr/bin/env bash\nset -e\npnpm install\n', 'utf-8' ) // Why: chmod over a native Windows path is meaningless; only the WSL branch sets the bit. @@ -186,15 +186,28 @@ describe('createSetupRunnerScript', () => { expect(runner).toContain('call echo hello!world!') }) - it('drops a leading shebang instead of calling it as a batch command', async () => { + it('refuses to run a shebang script as a batch command', async () => { const { buildWindowsRunnerScript } = await import('./hooks') - // Why: a shebang script falls back here when Git Bash is unavailable; `call #!/...` - // would abort on errorlevel before any real setup line ran. - const runner = buildWindowsRunnerScript('#!/usr/bin/env bash\npnpm install') + // Regression: a POSIX script reaching the cmd runner (no Git Bash terminal, or an SSH + // Windows host) must not have its interpreter-agnostic prefix executed under cmd — + // `pnpm install` would run and only a later bash-only line would fail, leaving the + // worktree half set up. + const runner = buildWindowsRunnerScript('#!/usr/bin/env bash\npnpm install\nsource .env') - expect(runner).not.toContain('#!') - expect(runner).toContain('call pnpm install') + expect(runner).not.toContain('call pnpm install') + expect(runner).not.toContain('call source .env') + expect(runner).toContain('needs a POSIX shell') + expect(runner).toContain('exit /b 1') + }) + + it('keeps a `#` comment that is not an interpreter line', async () => { + const { buildWindowsRunnerScript } = await import('./hooks') + + const runner = buildWindowsRunnerScript('# not a shebang\nrem still batch') + + expect(runner).toContain('call # not a shebang') + expect(runner).toContain('call rem still batch') }) it('derives ORCA_WORKSPACE_NAME from a POSIX worktree path', async () => { diff --git a/src/main/hooks.test.ts b/src/main/hooks.test.ts index 794f82dfeec..c39ea86cf2f 100644 --- a/src/main/hooks.test.ts +++ b/src/main/hooks.test.ts @@ -1282,7 +1282,7 @@ describe('createSetupRunnerScript', () => { ) expect(writeFileSyncMock).toHaveBeenCalledWith( 'C:\\repo\\.git\\orca\\setup-runner.sh', - '#!/usr/bin/env bash\nset -e\n#!/usr/bin/env bash\npnpm install\nnpm run build\n', + '#!/usr/bin/env bash\nset -e\npnpm install\nnpm run build\n', 'utf-8' ) expect(chmodSyncMock).not.toHaveBeenCalled() @@ -1327,13 +1327,76 @@ describe('createSetupRunnerScript', () => { ) expect(result).toMatchObject({ runnerScriptPath: 'C:\\repo\\.git\\orca\\setup-runner.cmd', - shell: { family: 'cmd' } + // Why: `shell` is the pane that types the launch command — still Git Bash here. The + // runner's own .cmd extension is what says the file is batch. + shell: { family: 'posix' } }) } finally { Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) } }) + it('hands a Git Bash pane a launch command MSYS cannot rewrite for a cmd runner', async () => { + // Regression (#6896): `cmd.exe /c "C:\...\setup-runner.cmd"` typed into Git Bash has its + // `/c` switch rewritten into a drive path, so cmd opens interactively and setup never runs. + gitExecFileSyncMock.mockReset() + gitExecFileSyncMock.mockReturnValue('C:\\repo\\.git\\orca\\setup-runner.cmd\n') + const originalPlatform = process.platform + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + + try { + const { createSetupRunnerScript } = await import('./hooks') + const { buildSetupRunnerCommand } = await import('../shared/setup-runner-command') + const result = createSetupRunnerScript( + makeRepo(), + 'C:\\repo-worktree', + 'copy .env.example .env', + undefined, + { family: 'posix' } + ) + + const command = buildSetupRunnerCommand(result.runnerScriptPath, 'windows', result.shell) + + expect(command).not.toContain('cmd.exe /c') + // Why: the batch runner must never be handed to bash either. + expect(command).not.toContain('bash ') + expect(command).toContain('powershell.exe') + } finally { + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + } + }) + + it('replays interpreter flags declared on the shebang line', async () => { + // Regression: the runner is launched as `bash `, so `-euo pipefail` on the script's + // own `#!` line never reaches the interpreter unless the runner re-applies it. + gitExecFileSyncMock.mockReset() + gitExecFileSyncMock.mockReturnValue('C:\\repo\\.git\\orca\\setup-runner.sh\n') + const fs = await import('node:fs') + const writeFileSyncMock = vi.mocked(fs.writeFileSync) + writeFileSyncMock.mockClear() + const originalPlatform = process.platform + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + + try { + const { createSetupRunnerScript } = await import('./hooks') + createSetupRunnerScript( + makeRepo(), + 'C:\\repo-worktree', + '#!/usr/bin/env -S bash -euo pipefail\nmake build | tee build.log', + undefined, + { family: 'posix' } + ) + + expect(writeFileSyncMock).toHaveBeenCalledWith( + 'C:\\repo\\.git\\orca\\setup-runner.sh', + '#!/usr/bin/env bash\nset -e\nset -euo pipefail\nmake build | tee build.log\n', + 'utf-8' + ) + } finally { + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + } + }) + it('preserves cmd.exe setup runner semantics for configured cmd users', async () => { gitExecFileSyncMock.mockReset() gitExecFileSyncMock.mockReturnValue('C:\\repo\\.git\\orca\\setup-runner.cmd\n') @@ -1473,7 +1536,7 @@ describe('createIssueCommandRunnerScript', () => { ) expect(writeFileSyncMock).toHaveBeenCalledWith( 'C:\\repo\\.git\\orca\\issue-command-runner.sh', - '#!/usr/bin/env bash\nset -e\n#!/usr/bin/env bash\ngh issue view 42\n', + '#!/usr/bin/env bash\nset -e\ngh issue view 42\n', 'utf-8' ) expect(result.shell).toEqual({ family: 'posix' }) @@ -1502,7 +1565,8 @@ describe('createIssueCommandRunnerScript', () => { ['rev-parse', '--git-path', 'orca/issue-command-runner.cmd'], { cwd: 'C:\\repo-worktree' } ) - expect(result.shell).toEqual({ family: 'cmd' }) + // Why: the runner file is batch (.cmd) while the pane that launches it is still Git Bash. + expect(result.shell).toEqual({ family: 'posix' }) } finally { Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) } diff --git a/src/main/hooks.ts b/src/main/hooks.ts index ab1c61ad106..21b56b8a597 100644 --- a/src/main/hooks.ts +++ b/src/main/hooks.ts @@ -9,7 +9,12 @@ import { shouldWaitForSetupBeforeAgentStartup } from '../shared/setup-agent-star import { TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV } from '../shared/terminal-git-credential-guard' import { parseOrcaYaml } from '../shared/orca-yaml' import { nativeWindowsPathToPosixShellPath } from '../shared/setup-runner-command' -import { isShebangLine, scriptDeclaresPosixShell } from '../shared/setup-script-shebang' +import { + isShebangLine, + parseSetupScriptShebang, + scriptDeclaresPosixShell, + stripLeadingShebangLine +} from '../shared/setup-script-shebang' import { resolveWindowsShellStartupFamily } from '../shared/windows-terminal-shell' import { resolveWindowsGitBashShellPath } from './git-bash' import { gitExecFileSync, promptGuardShellEnv } from './git/runner' @@ -398,19 +403,30 @@ function getHookWslContext( } } +// Why: cmd cannot run a POSIX script, and executing its interpreter-agnostic prefix (`pnpm +// install`, `git submodule update`) before dying on the first bash-only line is worse than not +// starting: half-applied setup looks like a working worktree. +const WINDOWS_RUNNER_SHEBANG_REFUSAL = [ + 'echo Orca setup: this script starts with a "#!" interpreter line, so it needs a POSIX shell. 1>&2', + 'echo Orca setup: this worktree runs setup through cmd.exe, which cannot execute it. 1>&2', + 'echo Orca setup: set the Windows terminal shell to Git Bash, or rewrite the script in cmd syntax. 1>&2', + 'exit /b 1', + '' +].join('\r\n') + export function buildWindowsRunnerScript(script: string): string { // Why: launchers invoke this runner under `cmd /v:on`, and EnableExtensions does not reset an // inherited delayed-expansion state — without this, every `!` in a user setup line is eaten. - let runnerScript = '@echo off\r\nsetlocal EnableExtensions DisableDelayedExpansion\r\n' + const header = '@echo off\r\nsetlocal EnableExtensions DisableDelayedExpansion\r\n' + let runnerScript = header let isFirstLine = true for (const rawLine of iterateLfScriptLines(script)) { const command = rawLine.trim() - // Why: a leading `#!` declares an interpreter; `call`ing it would abort the run on errorlevel. if (isFirstLine) { isFirstLine = false if (isShebangLine(command)) { - continue + return `${header}${WINDOWS_RUNNER_SHEBANG_REFUSAL}` } } if (!command) { @@ -471,7 +487,15 @@ export function getSetupRunnerEnvVars(repo: Repo, worktreePath: string): Record< } export function buildPosixRunnerScript(script: string): string { - return `#!/usr/bin/env bash\nset -e\n${normalizeCrlfScriptLineEndings(script)}\n` + // Why: the runner is always launched as `bash `, so flags on the script's own `#!` line + // are never seen by the interpreter — replay them through `set` or a script that asked for + // `-euo pipefail` silently loses pipefail, and drop the now-duplicate interpreter line. + const shebang = parseSetupScriptShebang(script) + const declaredOptions = shebang?.shellOptions.length + ? `set ${shebang.shellOptions.join(' ')}\n` + : '' + const body = shebang ? stripLeadingShebangLine(script) : script + return `#!/usr/bin/env bash\nset -e\n${declaredOptions}${normalizeCrlfScriptLineEndings(body)}\n` } function normalizeCrlfScriptLineEndings(script: string): string { @@ -546,8 +570,12 @@ function createWorktreeRunnerScript(args: { ? setupShell : { family: 'cmd' } : { family: 'posix' } + // Why: `shell` tells the launcher which shell types the command, not which format the runner + // file is in — the .cmd/.sh extension already carries that. Reporting the runner family here + // would make a Git Bash pane receive `cmd.exe /c ...`, whose `/c` MSYS rewrites into a drive + // path (issue #6896), so setup would open an interactive cmd and never run. const launchShell: SetupRunnerShell | undefined = nativeWindowsWorktree - ? runnerShell + ? (setupShell ?? { family: 'cmd' }) : process.platform === 'win32' && runtimeTarget?.wslDistro ? { family: 'posix', executable: 'wsl.exe' } : undefined @@ -598,7 +626,7 @@ function createWorktreeRunnerScript(args: { envVars, // Why: WSL git returns /mnt paths that Node converts back to C:\ for file // writes; retain the runtime signal so launch converts them to /mnt again. - // On native Windows it mirrors the family the runner file was written in. + // On native Windows it is the terminal's family, i.e. the shell that types the command. ...(launchShell ? { shell: launchShell } : {}), ...(waitForAgentStartup === true ? { waitForAgentStartup: true } : {}) } @@ -629,8 +657,9 @@ export function resolveSetupRunnerShell( options.resolveGitBashShellPath ?? ((shell: string) => resolveWindowsGitBashShellPath(shell, { platform })) if (resolveGitBashShellPath(configuredShell)) { - // Note: this only reports that a bash runner *could* launch here. Whether one is - // used is decided per script by its `#!` line — see docs/reference/windows-setup-shell.md. + // Note: this reports the terminal's family — the shell that types the launch command, and + // therefore also that a bash runner *could* launch here. Whether one is written is decided + // per script by its `#!` line — see docs/reference/windows-setup-shell.md. return { family: 'posix' } } } diff --git a/src/main/runtime/orchestration/setup-completion-signal.test.ts b/src/main/runtime/orchestration/setup-completion-signal.test.ts index baa5af51cf3..73f29371b50 100644 --- a/src/main/runtime/orchestration/setup-completion-signal.test.ts +++ b/src/main/runtime/orchestration/setup-completion-signal.test.ts @@ -64,6 +64,19 @@ describe('orchestration setup completion signal', () => { expect(command).toContain('bash /c/repo/.git/orca/setup-runner.sh') }) + it('keeps a batch runner on the Windows completion path from a Git Bash pane', () => { + // Regression (#6896): a Git Bash terminal with a batch setup script still gets a .cmd + // runner; observing it must not shell out to bash or type a bare `cmd.exe /c` switch. + const runnerPath = 'C:\\repo\\.git\\orca\\setup-runner.cmd' + const observed = buildObservedSetupCommand(runnerPath, 'windows', 'token-git-bash-cmd', { + family: 'posix' + }) + + expect(observed.command).toContain('powershell.exe -NoLogo -NoProfile -NonInteractive') + expect(observed.command).not.toContain('bash ') + expect(observed.env).toEqual({ ORCA_SETUP_RUNNER_PATH: runnerPath }) + }) + it('recognizes one completion signal across output chunk boundaries', () => { const onComplete = vi.fn() const scanner = createSetupCompletionScanner('token-chunks', onComplete) diff --git a/src/shared/setup-agent-sequencing.test.ts b/src/shared/setup-agent-sequencing.test.ts index bffeb0de351..828cd1eec65 100644 --- a/src/shared/setup-agent-sequencing.test.ts +++ b/src/shared/setup-agent-sequencing.test.ts @@ -218,6 +218,26 @@ describe('createSequencedSetupAgentCommands', () => { }) }) + it('keeps the cmd gate for a batch runner launched from a Git Bash pane', () => { + // Regression (#6896): a Git Bash terminal with a batch setup script still gets a .cmd + // runner, and the wait-for-setup gate must not try to run it under bash. + const result = createSequencedSetupAgentCommands({ + runnerScriptPath: 'C:\\repo\\.git\\orca\\setup-runner.cmd', + startupCommand: 'claude', + platform: 'windows', + shell: { family: 'posix' }, + nonce: 'nonce-gitbash-cmd' + }) + + expect(result.setupCommand).toContain( + 'powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand' + ) + expect(result.setupCommand).not.toContain('bash -lc') + expect(decodePowerShellScript(result.setupCommand)).toContain( + "$runner = 'C:\\repo\\.git\\orca\\setup-runner.cmd'" + ) + }) + it.skipIf(process.platform !== 'win32')( 'executes the native Windows setup-to-agent sequence through cmd.exe', async () => { diff --git a/src/shared/setup-runner-command.test.ts b/src/shared/setup-runner-command.test.ts index 99ba2e029c4..069130b1313 100644 --- a/src/shared/setup-runner-command.test.ts +++ b/src/shared/setup-runner-command.test.ts @@ -61,6 +61,51 @@ describe('buildSetupRunnerCommand', () => { 'bash /c/repo/.git/orca/setup-runner.sh' ) }) + + it('never hands a batch runner to bash, even from a Git Bash pane', () => { + // Regression: a Git Bash terminal with a batch-syntax setup script gets a .cmd runner, + // so the launch shell being POSIX must not be read as "the runner is a shell script". + const command = buildSetupRunnerCommand('C:\\repo\\.git\\orca\\setup-runner.cmd', 'windows', { + family: 'posix' + }) + + expect(command).not.toContain('bash ') + expect(command).not.toContain('/c/repo') + }) + + it('avoids the bare /c switch when a POSIX pane launches a batch runner', () => { + // Regression (#6896): MSYS rewrites `cmd.exe /c` into a drive path inside Git Bash, so cmd + // opens interactively and the runner payload never executes. + const command = buildSetupRunnerCommand('C:\\repo\\.git\\orca\\setup-runner.cmd', 'windows', { + family: 'posix' + }) + + expect(command).not.toContain('cmd.exe /c') + expect(command).toMatch( + /^powershell\.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand [A-Za-z0-9+/=]+$/ + ) + }) + + it('keeps the batch runner path in native form for a POSIX pane launch', () => { + // Why: the PowerShell launcher hands the path to cmd, which cannot read /c/... MSYS paths; + // marker and completion paths derive from this value too. + expect( + resolveSetupRunnerCommand('C:\\repo\\.git\\orca\\setup-runner.cmd', 'windows', { + family: 'posix' + }) + ).toMatchObject({ + runnerScriptPathForShell: 'C:\\repo\\.git\\orca\\setup-runner.cmd', + shell: 'windows' + }) + }) + + it('still uses bash for a POSIX runner launched from a POSIX pane', () => { + expect( + buildSetupRunnerCommand('C:\\repo\\.git\\orca\\setup-runner.sh', 'windows', { + family: 'posix' + }) + ).toBe('bash /c/repo/.git/orca/setup-runner.sh') + }) }) describe('buildSetupRunnerCommand cmd metacharacter guard', () => { diff --git a/src/shared/setup-runner-command.ts b/src/shared/setup-runner-command.ts index fe9cebed50b..7b1dee560fd 100644 --- a/src/shared/setup-runner-command.ts +++ b/src/shared/setup-runner-command.ts @@ -60,7 +60,10 @@ export function resolveSetupRunnerCommand( shell: 'posix' } } - if (shell?.family === 'posix' || /\.sh$/i.test(runnerScriptPath)) { + // Why: `shell` is the shell that types the command; the runner file's own extension decides + // what can execute it. A batch runner never goes to bash even from a Git Bash pane. + const cmdRunnerFile = isWindowsCmdRunnerPath(runnerScriptPath) + if (!cmdRunnerFile && (shell?.family === 'posix' || /\.sh$/i.test(runnerScriptPath))) { // Why: WSL shells need /mnt/... paths, while Git Bash expects /c/... when replaying deferred setup scripts. if (isWslExecutable(shell?.executable)) { const wslPath = nativeWindowsPathToWslShellPath(runnerScriptPath) @@ -79,11 +82,15 @@ export function resolveSetupRunnerCommand( } } return { - // Why: some path characters survive no amount of quoting on a cmd command line, so those - // paths take a delayed-expansion launcher instead. Every other path keeps the plain form. - command: windowsRunnerPathNeedsCmdGuard(runnerScriptPath) - ? buildWindowsCmdRunnerDelayedLaunchCommand(runnerScriptPath) - : `cmd.exe /c ${quoteWindowsArg(runnerScriptPath)}`, + // Why: some path characters survive no amount of quoting on a cmd command line, and a Git + // Bash pane rewrites the bare `/c` switch itself into a drive path (issue #6896) so cmd + // opens interactively and the runner never starts. Both take the delayed-expansion + // launcher, which passes the switch through a PowerShell ProcessStartInfo instead. Every + // other case keeps the plain form. + command: + shell?.family === 'posix' || windowsRunnerPathNeedsCmdGuard(runnerScriptPath) + ? buildWindowsCmdRunnerDelayedLaunchCommand(runnerScriptPath) + : `cmd.exe /c ${quoteWindowsArg(runnerScriptPath)}`, runnerScriptPathForShell: runnerScriptPath, shell: 'windows' } @@ -96,6 +103,11 @@ export function resolveSetupRunnerCommand( } } +/** True when the runner file is a batch script, which only cmd can execute. */ +export function isWindowsCmdRunnerPath(runnerScriptPath: string): boolean { + return /\.(cmd|bat)$/i.test(runnerScriptPath) +} + export function isWslUncPath(path: string): boolean { const normalized = path.replace(/\\/g, '/') return /^\/\/(wsl\.localhost|wsl\$)\//i.test(normalized) diff --git a/src/shared/setup-script-shebang.test.ts b/src/shared/setup-script-shebang.test.ts index 5d75deb39c9..5d85f5296a2 100644 --- a/src/shared/setup-script-shebang.test.ts +++ b/src/shared/setup-script-shebang.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect } from 'vitest' -import { isShebangLine, scriptDeclaresPosixShell } from './setup-script-shebang' +import { + isShebangLine, + parseSetupScriptShebang, + scriptDeclaresPosixShell, + stripLeadingShebangLine +} from './setup-script-shebang' describe('scriptDeclaresPosixShell', () => { it('accepts the common env and absolute-path forms', () => { @@ -30,6 +35,46 @@ describe('scriptDeclaresPosixShell', () => { }) }) +describe('parseSetupScriptShebang', () => { + it('keeps the interpreter flags a script declares', () => { + // Regression: the runner is launched as `bash `, so flags survive only if they are + // parsed out here and replayed with `set` — otherwise `pipefail` is silently lost. + expect(parseSetupScriptShebang('#!/usr/bin/env -S bash -euo pipefail\nmake')).toEqual({ + interpreter: 'bash', + shellOptions: ['-euo', 'pipefail'] + }) + expect(parseSetupScriptShebang('#!/bin/bash -e -x\nmake')).toEqual({ + interpreter: 'bash', + shellOptions: ['-e', '-x'] + }) + expect(parseSetupScriptShebang('#!/bin/sh\nmake')).toEqual({ + interpreter: 'sh', + shellOptions: [] + }) + }) + + it('ignores interpreter arguments that `set` cannot apply', () => { + expect(parseSetupScriptShebang('#!/bin/bash --norc\nmake')?.shellOptions).toEqual([]) + expect(parseSetupScriptShebang('#!/bin/bash -o\nmake')?.shellOptions).toEqual(['-o']) + }) + + it('returns null without an interpreter line', () => { + expect(parseSetupScriptShebang('pnpm install')).toBeNull() + expect(parseSetupScriptShebang('#!/usr/bin/env')).toBeNull() + }) +}) + +describe('stripLeadingShebangLine', () => { + it('removes only a leading interpreter line', () => { + expect(stripLeadingShebangLine('#!/usr/bin/env bash\npnpm install\n')).toBe('pnpm install\n') + expect(stripLeadingShebangLine('#!/usr/bin/env bash\r\npnpm install')).toBe('pnpm install') + expect(stripLeadingShebangLine('pnpm install\n#!/usr/bin/env bash')).toBe( + 'pnpm install\n#!/usr/bin/env bash' + ) + expect(stripLeadingShebangLine('#!/usr/bin/env bash')).toBe('') + }) +}) + describe('isShebangLine', () => { it('detects interpreter lines regardless of leading whitespace', () => { expect(isShebangLine('#!/usr/bin/env bash')).toBe(true) diff --git a/src/shared/setup-script-shebang.ts b/src/shared/setup-script-shebang.ts index 7b02594227b..ceb5741d44b 100644 --- a/src/shared/setup-script-shebang.ts +++ b/src/shared/setup-script-shebang.ts @@ -2,34 +2,84 @@ // not of the user's terminal preference, so a `#!` line is how a project declares it. const POSIX_SHELL_BASENAMES = new Set(['sh', 'bash', 'zsh', 'dash', 'ksh', 'ash']) +// Why: `-e`, `+e`, `-euo`; `--norc` and interpreter arguments are not `set` options. +const SHELL_OPTION_FLAG_PATTERN = /^[-+][a-zA-Z]+$/ +const SHELL_OPTION_NAME_PATTERN = /^[a-z_]+$/ + +export type SetupScriptShebang = { + /** Lowercased interpreter basename, e.g. `bash` for `#!/usr/bin/env -S bash -e`. */ + interpreter: string + /** Interpreter flags the generated runner replays through `set`, e.g. `['-euo', 'pipefail']`. */ + shellOptions: string[] +} /** True when `line` is a `#!` interpreter line (leading whitespace tolerated). */ export function isShebangLine(line: string): boolean { return line.trimStart().startsWith('#!') } -/** True when the script's first line is a `#!` line naming a POSIX shell. */ -export function scriptDeclaresPosixShell(script: string): boolean { +/** Parses the script's leading `#!` line, or null when it has none. */ +export function parseSetupScriptShebang(script: string): SetupScriptShebang | null { const firstLine = script.split('\n', 1)[0] ?? '' if (!isShebangLine(firstLine)) { - return false + return null } const tokens = firstLine.trim().slice(2).trim().split(/\s+/).filter(Boolean) - const interpreter = shebangInterpreterBasename(tokens) - return interpreter !== null && POSIX_SHELL_BASENAMES.has(interpreter) + const interpreterIndex = findInterpreterIndex(tokens) + if (interpreterIndex === -1) { + return null + } + + return { + interpreter: executableBasename(tokens[interpreterIndex]), + shellOptions: parseShellOptions(tokens.slice(interpreterIndex + 1)) + } } -function shebangInterpreterBasename(tokens: string[]): string | null { +/** True when the script's first line is a `#!` line naming a POSIX shell. */ +export function scriptDeclaresPosixShell(script: string): boolean { + const shebang = parseSetupScriptShebang(script) + return shebang !== null && POSIX_SHELL_BASENAMES.has(shebang.interpreter) +} + +/** Drops a leading `#!` line; the generated runner carries its own interpreter line. */ +export function stripLeadingShebangLine(script: string): string { + if (!isShebangLine(script.split('\n', 1)[0] ?? '')) { + return script + } + const lineEnd = script.indexOf('\n') + return lineEnd === -1 ? '' : script.slice(lineEnd + 1) +} + +function findInterpreterIndex(tokens: string[]): number { for (let index = 0; index < tokens.length; index++) { const basename = executableBasename(tokens[index]) // Why: `env` (and its `-S` split-string form) only forwards to the real interpreter. - if (basename === 'env' || basename.startsWith('-')) { + if (basename === '' || basename === 'env' || basename.startsWith('-')) { continue } - return basename || null + return index } - return null + return -1 +} + +function parseShellOptions(tokens: string[]): string[] { + const options: string[] = [] + for (let index = 0; index < tokens.length; index++) { + const token = tokens[index] + if (!SHELL_OPTION_FLAG_PATTERN.test(token)) { + continue + } + options.push(token) + // Why: `-o`/`-euo` take the option name as a separate argument (`pipefail`). + const optionName = tokens[index + 1] + if (token.endsWith('o') && optionName && SHELL_OPTION_NAME_PATTERN.test(optionName)) { + options.push(optionName) + index++ + } + } + return options } function executableBasename(token: string): string {