fix(coding-agent): resolve shell config from session settings

Stop shell resolution from consulting ambient process.cwd() during bash execution and use the active session's shellPath setting instead.

closes #3452
This commit is contained in:
Mario Zechner
2026-04-20 22:32:11 +02:00
parent 5a4e22ea44
commit c40efa9bab
5 changed files with 48 additions and 34 deletions

View File

@@ -13,6 +13,7 @@
### Fixed ### 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 `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 <pkg>@latest` ([#3000](https://github.com/badlogic/pi-mono/issues/3000)) - Fixed `pi update` reinstalling npm packages that are already at the latest published version by checking the installed package version before running `npm install <pkg>@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)) - 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))

View File

@@ -2320,6 +2320,7 @@ export class AgentSession {
}): void { }): void {
const autoResizeImages = this.settingsManager.getImageAutoResize(); const autoResizeImages = this.settingsManager.getImageAutoResize();
const shellCommandPrefix = this.settingsManager.getShellCommandPrefix(); const shellCommandPrefix = this.settingsManager.getShellCommandPrefix();
const shellPath = this.settingsManager.getShellPath();
const baseToolDefinitions = this._baseToolsOverride const baseToolDefinitions = this._baseToolsOverride
? Object.fromEntries( ? Object.fromEntries(
Object.entries(this._baseToolsOverride).map(([name, tool]) => [ Object.entries(this._baseToolsOverride).map(([name, tool]) => [
@@ -2329,7 +2330,7 @@ export class AgentSession {
) )
: createAllToolDefinitions(this._cwd, { : createAllToolDefinitions(this._cwd, {
read: { autoResizeImages }, read: { autoResizeImages },
bash: { commandPrefix: shellCommandPrefix }, bash: { commandPrefix: shellCommandPrefix, shellPath },
}); });
this._baseToolDefinitions = new Map( 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) // Apply command prefix if configured (e.g., "shopt -s expand_aliases" for alias support)
const prefix = this.settingsManager.getShellCommandPrefix(); const prefix = this.settingsManager.getShellCommandPrefix();
const shellPath = this.settingsManager.getShellPath();
const resolvedCommand = prefix ? `${prefix}\n${command}` : command; const resolvedCommand = prefix ? `${prefix}\n${command}` : command;
try { try {
const result = await executeBashWithOperations( const result = await executeBashWithOperations(
resolvedCommand, resolvedCommand,
this.sessionManager.getCwd(), this.sessionManager.getCwd(),
options?.operations ?? createLocalBashOperations(), options?.operations ?? createLocalBashOperations({ shellPath }),
{ {
onChunk, onChunk,
signal: this._bashAbortController.signal, signal: this._bashAbortController.signal,

View File

@@ -72,11 +72,11 @@ export interface BashOperations {
* This is useful for extensions that intercept user_bash and still want pi's * This is useful for extensions that intercept user_bash and still want pi's
* standard local shell behavior while wrapping or rewriting commands. * standard local shell behavior while wrapping or rewriting commands.
*/ */
export function createLocalBashOperations(): BashOperations { export function createLocalBashOperations(options?: { shellPath?: string }): BashOperations {
return { return {
exec: (command, cwd, { onData, signal, timeout, env }) => { exec: (command, cwd, { onData, signal, timeout, env }) => {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
const { shell, args } = getShellConfig(); const { shell, args } = getShellConfig(options?.shellPath);
if (!existsSync(cwd)) { if (!existsSync(cwd)) {
reject(new Error(`Working directory does not exist: ${cwd}\nCannot execute bash commands.`)); reject(new Error(`Working directory does not exist: ${cwd}\nCannot execute bash commands.`));
return; return;
@@ -154,6 +154,8 @@ export interface BashToolOptions {
operations?: BashOperations; operations?: BashOperations;
/** Command prefix prepended to every command (for example shell setup commands) */ /** Command prefix prepended to every command (for example shell setup commands) */
commandPrefix?: string; commandPrefix?: string;
/** Optional explicit shell path from settings */
shellPath?: string;
/** Hook to adjust command, cwd, or env before execution */ /** Hook to adjust command, cwd, or env before execution */
spawnHook?: BashSpawnHook; spawnHook?: BashSpawnHook;
} }
@@ -272,7 +274,7 @@ export function createBashToolDefinition(
cwd: string, cwd: string,
options?: BashToolOptions, options?: BashToolOptions,
): ToolDefinition<typeof bashSchema, BashToolDetails | undefined, BashRenderState> { ): ToolDefinition<typeof bashSchema, BashToolDetails | undefined, BashRenderState> {
const ops = options?.operations ?? createLocalBashOperations(); const ops = options?.operations ?? createLocalBashOperations({ shellPath: options?.shellPath });
const commandPrefix = options?.commandPrefix; const commandPrefix = options?.commandPrefix;
const spawnHook = options?.spawnHook; const spawnHook = options?.spawnHook;
return { return {

View File

@@ -1,10 +1,12 @@
import { existsSync } from "node:fs"; import { existsSync } from "node:fs";
import { delimiter } from "node:path"; import { delimiter } from "node:path";
import { spawn, spawnSync } from "child_process"; import { spawn, spawnSync } from "child_process";
import { getBinDir, getSettingsPath } from "../config.js"; import { getBinDir } from "../config.js";
import { SettingsManager } from "../core/settings-manager.js";
let cachedShellConfig: { shell: string; args: string[] } | null = null; export interface ShellConfig {
shell: string;
args: string[];
}
/** /**
* Find bash executable on PATH (cross-platform) * 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: * 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 * 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 * 3. On Unix: /bin/bash, then bash on PATH, then fallback to sh
*/ */
export function getShellConfig(): { shell: string; args: string[] } { export function getShellConfig(customShellPath?: string): ShellConfig {
if (cachedShellConfig) {
return cachedShellConfig;
}
const settings = SettingsManager.create(process.cwd());
const customShellPath = settings.getShellPath();
// 1. Check user-specified shell path // 1. Check user-specified shell path
if (customShellPath) { if (customShellPath) {
if (existsSync(customShellPath)) { if (existsSync(customShellPath)) {
cachedShellConfig = { shell: customShellPath, args: ["-c"] }; return { shell: customShellPath, args: ["-c"] };
return cachedShellConfig;
} }
throw new Error( throw new Error(`Custom shell path not found: ${customShellPath}`);
`Custom shell path not found: ${customShellPath}\nPlease update shellPath in ${getSettingsPath()}`,
);
} }
if (process.platform === "win32") { if (process.platform === "win32") {
@@ -81,41 +73,36 @@ export function getShellConfig(): { shell: string; args: string[] } {
for (const path of paths) { for (const path of paths) {
if (existsSync(path)) { if (existsSync(path)) {
cachedShellConfig = { shell: path, args: ["-c"] }; return { shell: path, args: ["-c"] };
return cachedShellConfig;
} }
} }
// 3. Fallback: search bash.exe on PATH (Cygwin, MSYS2, WSL, etc.) // 3. Fallback: search bash.exe on PATH (Cygwin, MSYS2, WSL, etc.)
const bashOnPath = findBashOnPath(); const bashOnPath = findBashOnPath();
if (bashOnPath) { if (bashOnPath) {
cachedShellConfig = { shell: bashOnPath, args: ["-c"] }; return { shell: bashOnPath, args: ["-c"] };
return cachedShellConfig;
} }
throw new Error( throw new Error(
`No bash shell found. Options:\n` + `No bash shell found. Options:\n` +
` 1. Install Git for Windows: https://git-scm.com/download/win\n` + ` 1. Install Git for Windows: https://git-scm.com/download/win\n` +
` 2. Add your bash to PATH (Cygwin, MSYS2, etc.)\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")}`, `Searched Git Bash in:\n${paths.map((p) => ` ${p}`).join("\n")}`,
); );
} }
// Unix: try /bin/bash, then bash on PATH, then fallback to sh // Unix: try /bin/bash, then bash on PATH, then fallback to sh
if (existsSync("/bin/bash")) { if (existsSync("/bin/bash")) {
cachedShellConfig = { shell: "/bin/bash", args: ["-c"] }; return { shell: "/bin/bash", args: ["-c"] };
return cachedShellConfig;
} }
const bashOnPath = findBashOnPath(); const bashOnPath = findBashOnPath();
if (bashOnPath) { if (bashOnPath) {
cachedShellConfig = { shell: bashOnPath, args: ["-c"] }; return { shell: bashOnPath, args: ["-c"] };
return cachedShellConfig;
} }
cachedShellConfig = { shell: "sh", args: ["-c"] }; return { shell: "sh", args: ["-c"] };
return cachedShellConfig;
} }
export function getShellEnv(): NodeJS.ProcessEnv { export function getShellEnv(): NodeJS.ProcessEnv {

View File

@@ -409,6 +409,28 @@ describe("Coding Agent Tools", () => {
await expect(bashWithBadShell.execute("test-call-12", { command: "echo test" })).rejects.toThrow(/ENOENT/); 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 () => { it("should prepend command prefix when configured", async () => {
const bashWithPrefix = createBashTool(testDir, { const bashWithPrefix = createBashTool(testDir, {
commandPrefix: "export TEST_VAR=hello", commandPrefix: "export TEST_VAR=hello",