From f8dbb3669fe31512be748f73de5b9a163151d278 Mon Sep 17 00:00:00 2001 From: can1357 Date: Sun, 26 Jul 2026 20:27:16 +0200 Subject: [PATCH 1/2] feat(utils): implemented windows shell resolution for bash execution - Added resolveWindowsShell to locate Git Bash, scoop installs, and path binaries with a fallback to cmd.exe. - Updated bash-executor to prevent wrapping user commands in cmd.exe when using fallback shell paths. - Updated installation script to report optional shell status rather than failing when bash is absent. --- .../test/google-vertex-discovery.test.ts | 4 +- .../coding-agent/src/exec/bash-executor.ts | 6 +- packages/coding-agent/src/tools/bash.ts | 9 +- packages/utils/src/procmgr.ts | 100 +++++++++++------- packages/utils/test/procmgr.test.ts | 56 +++++++++- scripts/install.ps1 | 14 ++- 6 files changed, 134 insertions(+), 55 deletions(-) diff --git a/packages/catalog/test/google-vertex-discovery.test.ts b/packages/catalog/test/google-vertex-discovery.test.ts index fac2331f2..7b7cf814e 100644 --- a/packages/catalog/test/google-vertex-discovery.test.ts +++ b/packages/catalog/test/google-vertex-discovery.test.ts @@ -85,9 +85,9 @@ describe("google-vertex model catalog", () => { expect(options.fetchDynamicModels).toBeUndefined(); expect(options.staticModels).toBeUndefined(); - const result = await resolveProviderModels(options, "offline"); + const result = await resolveProviderModels({ ...options, cacheDbPath: ":memory:" }, "offline"); expect(result.stale).toBe(false); - expect(result.models.some(model => model.id === "deepseek-ai/deepseek-v3.2-maas")).toBe(true); + expect(result.models.some(model => model.id.endsWith("-maas") && model.api === "openai-completions")).toBe(true); expect(result.models.some(model => model.id === "gemini-3.5-flash")).toBe(true); expect(result.models.some(model => model.id === "gemini-1.5-pro")).toBe(false); }); diff --git a/packages/coding-agent/src/exec/bash-executor.ts b/packages/coding-agent/src/exec/bash-executor.ts index e9a0f104e..2306d17c5 100644 --- a/packages/coding-agent/src/exec/bash-executor.ts +++ b/packages/coding-agent/src/exec/bash-executor.ts @@ -5,7 +5,7 @@ */ import { ExponentialYield } from "@oh-my-pi/pi-agent-core/utils/yield"; import { type MinimizerOptions, Shell, type ShellRunResult } from "@oh-my-pi/pi-natives"; -import { isExecutable, type ShellConfig } from "@oh-my-pi/pi-utils/procmgr"; +import { isCmdShell, isExecutable, type ShellConfig } from "@oh-my-pi/pi-utils/procmgr"; import { Settings, type ShellMinimizerSettings } from "../config/settings"; import { OutputSink } from "../session/streaming-output"; import { resolveOutputMaxColumns, resolveOutputSinkHeadBytes } from "../tools/output-meta"; @@ -367,8 +367,10 @@ export async function executeBash(command: string, options?: BashExecutorOptions }); const commandEnv = buildNonInteractiveEnv(preflight.env); const runCdInPersistentShell = options?.useUserShell === true && !prefix && isPersistentShellCdCommand(command); + // Never wrap in cmd.exe: it is only the Windows no-bash fallback for spawn + // paths, and the embedded brush shell runs the POSIX line better directly. const finalCommand = - options?.useUserShell === true && !bashShell && !runCdInPersistentShell + options?.useUserShell === true && !bashShell && !isCmdShell(shell) && !runCdInPersistentShell ? buildUserShellCommand(shell, args, preflight.command) : preflight.command; diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index 86bf6a730..c3870723b 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -72,12 +72,13 @@ const BASH_PATTERN_APPROVAL_VALUES = new Set(["allow", "deny", "prompt"]); * containing a space, pipe, `&&`, redirect, or `$(...)`. * * The wrap reuses the same shell binary + args the local `bash-executor` would - * pick via `settings.getShellConfig()` — Git Bash / `bash.exe` on Windows, + * pick via `settings.getShellConfig()` — Git Bash / `bash.exe` on Windows + * (`cmd.exe /c` as the last-resort fallback when no bash exists on the host), * `$SHELL` (bash/zsh) with the `sh` fallback on POSIX — so the ACP path * preserves `bash` tool semantics (`$VAR`, `$(...)`, `source`, POSIX quoting, - * `-l`) instead of dropping to `cmd.exe` on Windows. The agent host's shell - * path is used as a proxy for the client's, matching the near-universal - * ACP deployment shape of an editor spawning omp as a co-hosted subprocess. + * `-l`) wherever a POSIX shell is available. The agent host's shell path is + * used as a proxy for the client's, matching the near-universal ACP + * deployment shape of an editor spawning omp as a co-hosted subprocess. */ export function wrapShellLineForClientTerminal( line: string, diff --git a/packages/utils/src/procmgr.ts b/packages/utils/src/procmgr.ts index 75b4ea3c7..6d2a615d2 100644 --- a/packages/utils/src/procmgr.ts +++ b/packages/utils/src/procmgr.ts @@ -49,14 +49,22 @@ function buildSpawnEnv(shell: string): Record { } /** - * Get shell args, optionally including login shell flag. - * Supports PI_BASH_NO_LOGIN and CLAUDE_BASH_NO_LOGIN to skip -l. + * Get shell args for the resolved shell. + * cmd.exe takes `/c`; POSIX shells take `-c`, with `-l` unless + * PI_BASH_NO_LOGIN / CLAUDE_BASH_NO_LOGIN is set. */ -function getShellArgs(): string[] { +function getShellArgs(shell: string): string[] { + if (isCmdShell(shell)) return ["/c"]; const noLogin = $env.PI_BASH_NO_LOGIN || $env.CLAUDE_BASH_NO_LOGIN; return noLogin ? ["-c"] : ["-l", "-c"]; } +/** Whether the shell is Windows cmd.exe (spawn paths must use `/c`, not `-c`). */ +export function isCmdShell(shell: string): boolean { + const basename = shell.replace(/\\/g, "/").split("/").pop()?.toLowerCase(); + return basename === "cmd.exe" || basename === "cmd"; +} + /** * Get shell prefix for wrapping commands (profilers, strace, etc.). */ @@ -70,7 +78,7 @@ function getShellPrefix(): string | undefined { function buildConfig(shell: string): ShellConfig { return { shell, - args: getShellArgs(), + args: getShellArgs(shell), env: buildSpawnEnv(shell), prefix: getShellPrefix(), }; @@ -100,11 +108,59 @@ export function resolveBasicShell(): string | undefined { return undefined; } +/** + * Resolve the external shell to advertise on Windows. + * + * A host bash is OPTIONAL: bash tool commands always execute in the embedded + * brush-core shell. The resolved binary only serves the spawn-a-shell paths + * (interactive PTY sessions, ACP client terminals, SHELL env), so this + * prefers a real Git Bash when one exists and otherwise falls back to + * cmd.exe — it never fails. + * + * Search order: + * 1. Git for Windows install roots (machine + per-user installers) + * 2. scoop installs — scoop's git manifest sets GIT_INSTALL_ROOT and shims + * sh.exe/git.exe but never bash.exe, so PATH lookup alone misses it + * 3. bash.exe on PATH (Cygwin, MSYS2, ...) + * 4. sh.exe on PATH (Git for Windows' sh.exe is bash; prefer a sibling + * bash.exe when present) + * 5. cmd.exe from ComSpec + * + * Exported for tests; `env` overrides Bun.env-based discovery. + */ +export function resolveWindowsShell(env: Record = Bun.env): string { + const gitRoots = [ + env.ProgramFiles && path.join(env.ProgramFiles, "Git"), + env["ProgramFiles(x86)"] && path.join(env["ProgramFiles(x86)"], "Git"), + env.LOCALAPPDATA && path.join(env.LOCALAPPDATA, "Programs", "Git"), + env.GIT_INSTALL_ROOT, + env.SCOOP && path.join(env.SCOOP, "apps", "git", "current"), + env.USERPROFILE && path.join(env.USERPROFILE, "scoop", "apps", "git", "current"), + ]; + for (const root of gitRoots) { + if (!root) continue; + const candidate = path.join(root, "bin", "bash.exe"); + if (fs.existsSync(candidate)) return candidate; + } + + const bashOnPath = $which("bash.exe"); + if (bashOnPath) return bashOnPath; + + const shOnPath = $which("sh.exe"); + if (shOnPath) { + const siblingBash = path.join(path.dirname(shOnPath), "bash.exe"); + return fs.existsSync(siblingBash) ? siblingBash : shOnPath; + } + + return env.ComSpec || env.COMSPEC || "C:\\Windows\\System32\\cmd.exe"; +} + /** * Get shell configuration based on platform. * Resolution order: * 1. User-specified shellPath from the active settings source - * 2. On Windows: Git Bash in known locations, then bash on PATH + * 2. On Windows: Git Bash / bash / sh discovery, then cmd.exe (see + * {@link resolveWindowsShell}) — never fails * 3. On Unix: $SHELL if bash/zsh, then fallback paths * 4. Fallback: sh */ @@ -127,38 +183,8 @@ export function getShellConfig(customShellPath?: string, options: ShellConfigOpt } if (process.platform === "win32") { - // 2. Try Git Bash in known locations - const paths: string[] = []; - const programFiles = Bun.env.ProgramFiles; - if (programFiles) { - paths.push(`${programFiles}\\Git\\bin\\bash.exe`); - } - const programFilesX86 = Bun.env["ProgramFiles(x86)"]; - if (programFilesX86) { - paths.push(`${programFilesX86}\\Git\\bin\\bash.exe`); - } - - for (const path of paths) { - if (fs.existsSync(path)) { - cachedShellConfig = buildConfig(path); - return cachedShellConfig; - } - } - - // 3. Fallback: search bash.exe on PATH (Cygwin, MSYS2, WSL, etc.) - const bashOnPath = $which("bash.exe"); - if (bashOnPath) { - cachedShellConfig = buildConfig(bashOnPath); - return cachedShellConfig; - } - - throw new Error( - `No bash shell found. Options:\n` + - ` 1. Install Git for Windows: https://git-scm.com/download/win\n` + - ` 2. Add your bash to PATH (Cygwin, MSYS2, etc.)\n` + - ` 3. Set shellPath in ${configSource}\n\n` + - `Searched Git Bash in:\n${paths.map(p => ` ${p}`).join("\n")}`, - ); + cachedShellConfig = buildConfig(resolveWindowsShell()); + return cachedShellConfig; } // Unix: prefer user's shell from $SHELL if it's bash/zsh and executable diff --git a/packages/utils/test/procmgr.test.ts b/packages/utils/test/procmgr.test.ts index 70878de73..1e9e8ff61 100644 --- a/packages/utils/test/procmgr.test.ts +++ b/packages/utils/test/procmgr.test.ts @@ -1,8 +1,9 @@ -import { describe, expect, it } from "bun:test"; +import { afterEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; import { getAgentDir, MAIN_CONFIG_FILENAMES } from "../src/dirs"; -import { getShellConfig } from "../src/procmgr"; +import { getShellConfig, resolveWindowsShell } from "../src/procmgr"; describe("getShellConfig", () => { it("directs invalid custom shell paths to the canonical config file", () => { @@ -13,3 +14,54 @@ describe("getShellConfig", () => { ); }); }); + +describe("resolveWindowsShell", () => { + const tempDirs: string[] = []; + + afterEach(() => { + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + function makeGitRoot(): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "omp-git-root-")); + tempDirs.push(root); + fs.mkdirSync(path.join(root, "bin"), { recursive: true }); + fs.writeFileSync(path.join(root, "bin", "bash.exe"), ""); + return root; + } + + it("finds scoop's Git Bash via GIT_INSTALL_ROOT despite bash.exe missing from PATH", () => { + // scoop's git manifest sets GIT_INSTALL_ROOT and shims sh.exe/git.exe but + // never bash.exe, so PATH lookup alone misses the install. + const root = makeGitRoot(); + expect(resolveWindowsShell({ GIT_INSTALL_ROOT: root })).toBe(path.join(root, "bin", "bash.exe")); + }); + + it("finds Git Bash in the default scoop app dir via USERPROFILE", () => { + const profile = fs.mkdtempSync(path.join(os.tmpdir(), "omp-profile-")); + tempDirs.push(profile); + const root = path.join(profile, "scoop", "apps", "git", "current"); + fs.mkdirSync(path.join(root, "bin"), { recursive: true }); + fs.writeFileSync(path.join(root, "bin", "bash.exe"), ""); + expect(resolveWindowsShell({ USERPROFILE: profile })).toBe(path.join(root, "bin", "bash.exe")); + }); + + it("prefers a Git for Windows install root over the cmd.exe fallback", () => { + const programFiles = fs.mkdtempSync(path.join(os.tmpdir(), "omp-programfiles-")); + tempDirs.push(programFiles); + const bash = path.join(programFiles, "Git", "bin", "bash.exe"); + fs.mkdirSync(path.dirname(bash), { recursive: true }); + fs.writeFileSync(bash, ""); + expect(resolveWindowsShell({ ProgramFiles: programFiles, ComSpec: "C:\\Windows\\System32\\cmd.exe" })).toBe(bash); + }); + + // On a real Windows host bash.exe/sh.exe may resolve from PATH before the + // cmd.exe fallback is reached, so the fallback contract is only + // deterministic off-Windows. + it.skipIf(process.platform === "win32")("falls back to cmd.exe instead of failing when no bash exists", () => { + expect(resolveWindowsShell({})).toBe("C:\\Windows\\System32\\cmd.exe"); + expect(resolveWindowsShell({ ComSpec: "D:\\win\\cmd.exe" })).toBe("D:\\win\\cmd.exe"); + }); +}); diff --git a/scripts/install.ps1 b/scripts/install.ps1 index cb26ada9b..d47f6cac7 100755 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -146,14 +146,12 @@ function Configure-BashShell { Write-Host "✓ Configured shell path in $settingsFile" -ForegroundColor Green } else { Write-Host "" - Write-Host "⚠ No bash shell found!" -ForegroundColor Yellow - Write-Host " OMP requires a bash shell on Windows. Options:" -ForegroundColor Yellow - Write-Host " 1. Install Git for Windows: https://git-scm.com/download/win" -ForegroundColor Yellow - Write-Host " 2. Use WSL, Cygwin, or MSYS2" -ForegroundColor Yellow - Write-Host "" - Write-Host " After installing, you can set a custom path in:" -ForegroundColor Yellow - Write-Host " $settingsFile" -ForegroundColor Yellow - Write-Host ' { "shellPath": "C:\\path\\to\\bash.exe" }' -ForegroundColor Yellow + Write-Host "No bash shell found - OMP will use its built-in shell." -ForegroundColor Cyan + Write-Host " For shell snapshots and interactive terminals, install Git for Windows:" -ForegroundColor Cyan + Write-Host " https://git-scm.com/download/win" -ForegroundColor Cyan + Write-Host " Or set a custom path in:" -ForegroundColor Cyan + Write-Host " $settingsFile" -ForegroundColor Cyan + Write-Host ' { "shellPath": "C:\\path\\to\\bash.exe" }' -ForegroundColor Cyan } } catch { Write-Host "⚠ Could not configure bash shell: $_" -ForegroundColor Yellow From 403931b9687012ec83754e0826b7d97f551e5a17 Mon Sep 17 00:00:00 2001 From: can1357 Date: Sun, 26 Jul 2026 21:00:39 +0200 Subject: [PATCH 2/2] docs(changelog): documented windows built-in shell fallback in 17.1.4 - Recorded the scoop/no-bash startup fix (No bash shell found) in pi-utils and pi-coding-agent 17.1.4 sections, which were finalized before the fix commit landed on the release. --- packages/coding-agent/CHANGELOG.md | 1 + packages/utils/CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e4352d056..8286d275b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -15,6 +15,7 @@ ### Fixed +- Fixed `omp` refusing to start on Windows when no `bash.exe` is discoverable — most visibly with scoop-installed Git, whose manifest shims `sh.exe`/`git.exe` but never `bash.exe`, so PATH lookup missed it. Startup threw `No bash shell found` while merely building the bash tool description, even though bash tool commands always execute in the embedded brush-core shell and need no host bash. Shell discovery now also checks `GIT_INSTALL_ROOT`, scoop and per-user Git for Windows install roots, and `sh.exe` on PATH, then falls back to `cmd.exe` for the spawn-only paths (interactive PTY, ACP client terminals) instead of failing; the cmd fallback is never used to wrap user-shell commands — brush runs the POSIX line directly. - Fixed dragging an image whose path contains unescaped spaces (e.g. macOS screenshot names like `Screenshot 2026-07-24 at 1.55.12 PM.png`) into the terminal — the bracketed-paste image extraction route now has the same whole-text-as-path fallback as the clipboard keybind route, so both routes share identical detection and attach the image instead of inserting the raw path as literal text ([#6578](https://github.com/can1357/oh-my-pi/issues/6578)). The shared fallback only claims payloads that hold a single path: one carrying a second absolute-path anchor after unescaped whitespace (`/tmp/a.png /tmp/b shot.png` — dragging two files at once when either name has spaces) now pastes as text on both routes instead of being fused into one unresolvable path, which on the clipboard route previously attached nothing and swallowed the text behind an "Image not found" status. - Fixed transient reasonless request aborts that arrived after a tool call finished streaming ending the turn instead of entering recovery, which left edit calls and task subagents dead until the user manually resumed. The session now continues from the synthetic unexecuted tool result under the normal retry policy without replaying completed side effects ([#6668](https://github.com/can1357/oh-my-pi/issues/6668)). - Fixed prewalk silently dropping a same-model hand-off that only lowers the thinking level: the arm/switch guard compared model identity alone and discarded the resolved `thinkingLevel`, so a legal effort-downgrade target (e.g. `prewalk: "@task"` resolving to the same model at a cheaper effort) never applied and the session paid the plan/continue nudges for nothing. Prewalk now compares `(provider, id, effective thinking level)`, applies effort-only hand-offs, and emits a notice on a genuine no-op instead of returning silently ([#6659](https://github.com/can1357/oh-my-pi/issues/6659)). diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index e97c793ff..6b11f37e9 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -6,6 +6,7 @@ ### Fixed +- `getShellConfig` no longer throws `No bash shell found` on Windows hosts without a discoverable bash. `resolveWindowsShell` searches Git for Windows install roots (machine, per-user, `GIT_INSTALL_ROOT`, scoop app dirs — scoop shims `sh.exe`/`git.exe` but never `bash.exe`), then `bash.exe`/`sh.exe` on PATH, and finally falls back to `cmd.exe` from ComSpec with `/c` args, so shell resolution always succeeds. - Fixed postmortem signal and fatal shutdown exits being intercepted by temporary `process.exit` guards during extension startup ([#6488](https://github.com/can1357/oh-my-pi/issues/6488)). - Corrected Windows shell resolution errors to identify the active global, project, overlay, or runtime source for `shellPath` instead of directing every user to the retired `settings.json` file ([#6579](https://github.com/can1357/oh-my-pi/issues/6579)). - Contained timed-out child lifecycle rejections so `ptree` callers cannot leak an unhandled `TimeoutError` after settling ([#6635](https://github.com/can1357/oh-my-pi/issues/6635)).