mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
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 <path>`. 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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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 <runner>`; 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 <runner>`
|
||||
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.
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
+68
-4
@@ -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 <path>`, 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 })
|
||||
}
|
||||
|
||||
+38
-9
@@ -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 <path>`, 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' }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 <path>`, 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)
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user