diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c3e220be..ca0bf693 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -13,6 +13,7 @@ ### Fixed +- Fixed shell-path resolution to stop consulting ambient `process.cwd()` state during bash execution, so session/project-specific `shellPath` settings now follow the active coding-agent session cwd instead of the launcher cwd ([#3452](https://github.com/badlogic/pi-mono/issues/3452)) - Fixed `ctx.ui.setWorkingIndicator()` custom frames to render verbatim instead of forcing the theme accent color, so extensions now own working-indicator coloring when they customize it ([#3467](https://github.com/badlogic/pi-mono/issues/3467)) - Fixed `pi update` reinstalling npm packages that are already at the latest published version by checking the installed package version before running `npm install @latest` ([#3000](https://github.com/badlogic/pi-mono/issues/3000)) - Fixed built-in tool wrapping to use the same extension-runner context path as extension tools, so built-in tools receive execution context and `read` can warn when the current model does not support images ([#3429](https://github.com/badlogic/pi-mono/issues/3429)) diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 86a9c3f0..f90ee4c7 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -2320,6 +2320,7 @@ export class AgentSession { }): void { const autoResizeImages = this.settingsManager.getImageAutoResize(); const shellCommandPrefix = this.settingsManager.getShellCommandPrefix(); + const shellPath = this.settingsManager.getShellPath(); const baseToolDefinitions = this._baseToolsOverride ? Object.fromEntries( Object.entries(this._baseToolsOverride).map(([name, tool]) => [ @@ -2329,7 +2330,7 @@ export class AgentSession { ) : createAllToolDefinitions(this._cwd, { read: { autoResizeImages }, - bash: { commandPrefix: shellCommandPrefix }, + bash: { commandPrefix: shellCommandPrefix, shellPath }, }); this._baseToolDefinitions = new Map( @@ -2551,13 +2552,14 @@ export class AgentSession { // Apply command prefix if configured (e.g., "shopt -s expand_aliases" for alias support) const prefix = this.settingsManager.getShellCommandPrefix(); + const shellPath = this.settingsManager.getShellPath(); const resolvedCommand = prefix ? `${prefix}\n${command}` : command; try { const result = await executeBashWithOperations( resolvedCommand, this.sessionManager.getCwd(), - options?.operations ?? createLocalBashOperations(), + options?.operations ?? createLocalBashOperations({ shellPath }), { onChunk, signal: this._bashAbortController.signal, diff --git a/packages/coding-agent/src/core/tools/bash.ts b/packages/coding-agent/src/core/tools/bash.ts index 6c425fd9..736ff66e 100644 --- a/packages/coding-agent/src/core/tools/bash.ts +++ b/packages/coding-agent/src/core/tools/bash.ts @@ -72,11 +72,11 @@ export interface BashOperations { * This is useful for extensions that intercept user_bash and still want pi's * standard local shell behavior while wrapping or rewriting commands. */ -export function createLocalBashOperations(): BashOperations { +export function createLocalBashOperations(options?: { shellPath?: string }): BashOperations { return { exec: (command, cwd, { onData, signal, timeout, env }) => { return new Promise((resolve, reject) => { - const { shell, args } = getShellConfig(); + const { shell, args } = getShellConfig(options?.shellPath); if (!existsSync(cwd)) { reject(new Error(`Working directory does not exist: ${cwd}\nCannot execute bash commands.`)); return; @@ -154,6 +154,8 @@ export interface BashToolOptions { operations?: BashOperations; /** Command prefix prepended to every command (for example shell setup commands) */ commandPrefix?: string; + /** Optional explicit shell path from settings */ + shellPath?: string; /** Hook to adjust command, cwd, or env before execution */ spawnHook?: BashSpawnHook; } @@ -272,7 +274,7 @@ export function createBashToolDefinition( cwd: string, options?: BashToolOptions, ): ToolDefinition { - const ops = options?.operations ?? createLocalBashOperations(); + const ops = options?.operations ?? createLocalBashOperations({ shellPath: options?.shellPath }); const commandPrefix = options?.commandPrefix; const spawnHook = options?.spawnHook; return { diff --git a/packages/coding-agent/src/utils/shell.ts b/packages/coding-agent/src/utils/shell.ts index 1e9b5ed5..1c235802 100644 --- a/packages/coding-agent/src/utils/shell.ts +++ b/packages/coding-agent/src/utils/shell.ts @@ -1,10 +1,12 @@ import { existsSync } from "node:fs"; import { delimiter } from "node:path"; import { spawn, spawnSync } from "child_process"; -import { getBinDir, getSettingsPath } from "../config.js"; -import { SettingsManager } from "../core/settings-manager.js"; +import { getBinDir } from "../config.js"; -let cachedShellConfig: { shell: string; args: string[] } | null = null; +export interface ShellConfig { + shell: string; + args: string[]; +} /** * Find bash executable on PATH (cross-platform) @@ -42,29 +44,19 @@ function findBashOnPath(): string | null { } /** - * Get shell configuration based on platform. + * Resolve shell configuration based on platform and an optional explicit shell path. * Resolution order: - * 1. User-specified shellPath in settings.json + * 1. User-specified shellPath * 2. On Windows: Git Bash in known locations, then bash on PATH * 3. On Unix: /bin/bash, then bash on PATH, then fallback to sh */ -export function getShellConfig(): { shell: string; args: string[] } { - if (cachedShellConfig) { - return cachedShellConfig; - } - - const settings = SettingsManager.create(process.cwd()); - const customShellPath = settings.getShellPath(); - +export function getShellConfig(customShellPath?: string): ShellConfig { // 1. Check user-specified shell path if (customShellPath) { if (existsSync(customShellPath)) { - cachedShellConfig = { shell: customShellPath, args: ["-c"] }; - return cachedShellConfig; + return { shell: customShellPath, args: ["-c"] }; } - throw new Error( - `Custom shell path not found: ${customShellPath}\nPlease update shellPath in ${getSettingsPath()}`, - ); + throw new Error(`Custom shell path not found: ${customShellPath}`); } if (process.platform === "win32") { @@ -81,41 +73,36 @@ export function getShellConfig(): { shell: string; args: string[] } { for (const path of paths) { if (existsSync(path)) { - cachedShellConfig = { shell: path, args: ["-c"] }; - return cachedShellConfig; + return { shell: path, args: ["-c"] }; } } // 3. Fallback: search bash.exe on PATH (Cygwin, MSYS2, WSL, etc.) const bashOnPath = findBashOnPath(); if (bashOnPath) { - cachedShellConfig = { shell: bashOnPath, args: ["-c"] }; - return cachedShellConfig; + return { shell: bashOnPath, args: ["-c"] }; } 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 ${getSettingsPath()}\n\n` + + " 3. Set shellPath in settings.json\n\n" + `Searched Git Bash in:\n${paths.map((p) => ` ${p}`).join("\n")}`, ); } // Unix: try /bin/bash, then bash on PATH, then fallback to sh if (existsSync("/bin/bash")) { - cachedShellConfig = { shell: "/bin/bash", args: ["-c"] }; - return cachedShellConfig; + return { shell: "/bin/bash", args: ["-c"] }; } const bashOnPath = findBashOnPath(); if (bashOnPath) { - cachedShellConfig = { shell: bashOnPath, args: ["-c"] }; - return cachedShellConfig; + return { shell: bashOnPath, args: ["-c"] }; } - cachedShellConfig = { shell: "sh", args: ["-c"] }; - return cachedShellConfig; + return { shell: "sh", args: ["-c"] }; } export function getShellEnv(): NodeJS.ProcessEnv { diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index 2d45dbae..dace48b2 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -409,6 +409,28 @@ describe("Coding Agent Tools", () => { await expect(bashWithBadShell.execute("test-call-12", { command: "echo test" })).rejects.toThrow(/ENOENT/); }); + it("should pass shellPath through to shell resolution", async () => { + const getShellConfigSpy = vi.spyOn(shellModule, "getShellConfig"); + const bashWithCustomShell = createBashTool(testDir, { + shellPath: "/custom/bash", + operations: { + exec: async () => ({ exitCode: 0 }), + }, + }); + + await bashWithCustomShell.execute("test-call-12b", { command: "echo test" }); + + expect(getShellConfigSpy).not.toHaveBeenCalled(); + + const ops = createLocalBashOperations({ shellPath: "/custom/bash" }); + await expect( + ops.exec("echo test", testDir, { + onData: () => {}, + }), + ).rejects.toThrow("Custom shell path not found: /custom/bash"); + expect(getShellConfigSpy).toHaveBeenCalledWith("/custom/bash"); + }); + it("should prepend command prefix when configured", async () => { const bashWithPrefix = createBashTool(testDir, { commandPrefix: "export TEST_VAR=hello",