fix(coding-agent): respect user shell for shortcuts

2026-06-05 11:16:25 +09:00
parent e3501e3f76
commit 8b9c4fa1b9
6 changed files with 299 additions and 5 deletions
@@ -631,6 +631,9 @@
 - Fixed inline images rendering as a wall of empty PUA box glyphs with laggy scrolling on Kitty-protocol terminals that do not honor Unicode placeholders (most notably WezTerm and tmux/screen passthrough to a non-Kitty outer terminal). The 15.9 placeholder rollout enabled the `U=1`/U+10EEEE grid for every Kitty-protocol path; it now defaults on only for `kitty` and `ghostty`, with `PI_NO_KITTY_PLACEHOLDERS=1` as a hard opt-out and `PI_KITTY_PLACEHOLDERS=1` as opt-in for terminals (e.g. wezterm nightlies) that have since added support ([#1877](https://github.com/can1357/oh-my-pi/issues/1877)).
 - Fixed auto session-title generation failures being swallowed without an actionable diagnostic. Title generation now logs structured start, missing-model/API-key, provider-error, empty-result, and exception outcomes with the session id and resolved title model; the interactive auto-title caller also logs uncaught persistence/generation errors instead of dropping them. ([#1892](https://github.com/can1357/oh-my-pi/issues/1892))
 - Fixed `TranscriptContainer` reporting the live block boundary to the TUI again, so ED3-risk foreground streaming can append newly sealed transcript blocks to native scrollback once while deferring only the active live block.
+### Fixed
+
+- Fixed interactive `!`/`!!` shell shortcuts to run non-bash commands through the configured user shell, including interactive startup for zsh/fish aliases and functions ([#1816](https://github.com/can1357/oh-my-pi/issues/1816)).

 ## [15.9.1] - 2026-06-04

@@ -3,9 +3,11 @@
 *
 * Uses brush-core via native bindings for shell execution.
 */
+import { constants } from "node:fs";
 import * as fs from "node:fs/promises";
 import { ExponentialYield } from "@oh-my-pi/pi-agent-core/utils/yield";
 import { executeShell, type MinimizerOptions, Shell, type ShellRunResult } from "@oh-my-pi/pi-natives";
+import 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";
@@ -22,6 +24,8 @@ export interface BashExecutorOptions {
 	sessionKey?: string;
 	/** Additional environment variables to inject */
 	env?: Record<string, string>;
+	/** Run through the configured user shell instead of brush parsing directly. */
+	useUserShell?: boolean;
 	/** Artifact path/id for full output storage */
 	artifactPath?: string;
 	artifactId?: string;
@@ -100,10 +104,85 @@ export function buildMinimizerOptions(group: ShellMinimizerSettings): MinimizerO
 	};
 }

+function shellBasename(shell: string): string {
+	return shell.replace(/\\/g, "/").split("/").pop()?.toLowerCase() ?? "";
+}
+
+function isBashShell(shell: string): boolean {
+	const basename = shellBasename(shell);
+	return basename.includes("bash");
+}
+
+function needsInteractiveShellArg(shell: string): boolean {
+	const basename = shellBasename(shell);
+	return basename.includes("zsh") || basename.includes("fish");
+}
+
+function hasInteractiveShellArg(args: string[]): boolean {
+	return args.some(arg => arg === "--interactive" || /^-[^-]*i/.test(arg));
+}
+
+function ensureInteractiveShellArgs(shell: string, args: string[]): string[] {
+	if (!needsInteractiveShellArg(shell) || hasInteractiveShellArg(args)) return args;
+
+	const commandIndex = args.findIndex(arg => arg === "-c" || arg === "--command");
+	if (commandIndex !== -1) {
+		return [...args.slice(0, commandIndex), "-i", ...args.slice(commandIndex)];
+	}
+
+	const compactCommandIndex = args.findIndex(arg => /^-[^-]*c[^-]*$/.test(arg));
+	if (compactCommandIndex !== -1) {
+		return args.map((arg, index) => (index === compactCommandIndex ? arg.replace("c", "ic") : arg));
+	}
+
+	return [...args, "-i"];
+}
+
+function quoteShellArg(value: string): string {
+	return `'${value.replace(/'/g, "'\\''")}'`;
+}
+
+function buildUserShellCommand(shell: string, args: string[], command: string): string {
+	return [shell, ...ensureInteractiveShellArgs(shell, args), command].map(quoteShellArg).join(" ");
+}
+
+async function isExecutableShell(shell: string): Promise<boolean> {
+	try {
+		await fs.access(shell, constants.X_OK);
+		return true;
+	} catch {
+		return false;
+	}
+}
+
+async function resolveUserShellConfig(settings: Settings, baseConfig: ShellConfig): Promise<ShellConfig> {
+	const customShellPath = settings.get("shellPath");
+	const envShell = Bun.env.SHELL;
+	if (customShellPath || process.platform === "win32" || !envShell || envShell === baseConfig.shell) {
+		return baseConfig;
+	}
+	if (!(await isExecutableShell(envShell))) {
+		return baseConfig;
+	}
+
+	return {
+		...baseConfig,
+		shell: envShell,
+		env: {
+			...baseConfig.env,
+			SHELL: envShell,
+		},
+	};
+}
+
 export async function executeBash(command: string, options?: BashExecutorOptions): Promise<BashResult> {
 	const settings = await Settings.init();
-	const { shell, env: shellEnv, prefix } = settings.getShellConfig();
-	const snapshotPath = shell.includes("bash") ? await getOrCreateSnapshot(shell, shellEnv) : null;
+	const baseShellConfig = settings.getShellConfig();
+	const shellConfig =
+		options?.useUserShell === true ? await resolveUserShellConfig(settings, baseShellConfig) : baseShellConfig;
+	const { shell, args, env: shellEnv, prefix } = shellConfig;
+	const bashShell = isBashShell(shell);
+	const snapshotPath = bashShell ? await getOrCreateSnapshot(shell, shellEnv) : null;

 	const minimizer = buildMinimizerOptions(settings.getGroup("shellMinimizer"));

@@ -112,7 +191,10 @@ export async function executeBash(command: string, options?: BashExecutorOptions

 	// Apply command prefix if configured
 	const prefixedCommand = prefix ? `${prefix} ${command}` : command;
-	const finalCommand = prefixedCommand;
+	const finalCommand =
+		options?.useUserShell === true && !bashShell
+			? buildUserShellCommand(shell, args, prefixedCommand)
+			: prefixedCommand;

 	// Create output sink for truncation and artifact handling
 	const sink = new OutputSink({
@@ -934,7 +934,7 @@ export class CommandController {
 						this.ctx.bashComponent.appendOutput(chunk);
 					}
 				},
-				{ excludeFromContext },
+				{ excludeFromContext, useUserShell: true },
 			);

 			if (this.ctx.bashComponent) {
@@ -8615,11 +8615,12 @@ export class AgentSession {
 	 * @param command The bash command to execute
 	 * @param onChunk Optional streaming callback for output
 	 * @param options.excludeFromContext If true, command output won't be sent to LLM (!! prefix)
+	 * @param options.useUserShell If true, run via the configured user shell for interactive ! commands
 	 */
 	async executeBash(
 		command: string,
 		onChunk?: (chunk: string) => void,
-		options?: { excludeFromContext?: boolean },
+		options?: { excludeFromContext?: boolean; useUserShell?: boolean },
 	): Promise<BashResult> {
 		const excludeFromContext = options?.excludeFromContext === true;
 		const cwd = this.sessionManager.getCwd();
@@ -8647,6 +8648,7 @@ export class AgentSession {
 				sessionKey: this.sessionId,
 				timeout: clampTimeout("bash") * 1000,
 				onMinimizedSave: originalText => this.#saveBashOriginalArtifact(originalText),
+				useUserShell: options?.useUserShell,
 			});

 			this.recordBashResult(command, result, options);
@@ -128,6 +128,160 @@ describe("executeBash", () => {
 		expect(result.output.trim()).toBe("0:hello");
 	});

+	it("runs non-bash shellPath commands through the configured shell", async () => {
+		if (process.platform === "win32") {
+			return;
+		}
+
+		const shellDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-shellpath-"));
+		const marker = path.join(shellDir, "fake-shell-ran");
+		const markerEscaped = marker.replace(/'/g, "'\\''");
+		const fakeShell = path.join(shellDir, "fake-shell");
+		fs.writeFileSync(
+			fakeShell,
+			`#!/bin/sh
+printf '%s\\n' "$*" > '${markerEscaped}'
+while [ "$#" -gt 0 ]; do
+	if [ "$1" = "-c" ]; then
+		shift
+		exec /bin/sh -c "$1"
+	fi
+	shift
+done
+exit 64
+`,
+		);
+		fs.chmodSync(fakeShell, 0o755);
+		Settings.instance.set("shellPath", fakeShell);
+
+		vi.spyOn(Settings.prototype, "getShellConfig").mockReturnValue({
+			shell: fakeShell,
+			args: ["-l", "-c"],
+			env: {
+				PATH: Bun.env.PATH ?? "",
+				HOME: tempDir,
+			},
+			prefix: undefined,
+		});
+
+		try {
+			const result = await executeBash("printf 'shell-ok\\n'", {
+				cwd: tempDir,
+				timeout: 5000,
+				sessionKey: "custom-shell-path",
+				useUserShell: true,
+			});
+
+			expect(result.cancelled).toBe(false);
+			expect(result.exitCode).toBe(0);
+			expect(result.output.trim()).toBe("shell-ok");
+			expect(fs.readFileSync(marker, "utf8")).toContain("-l -c");
+		} finally {
+			fs.rmSync(shellDir, { recursive: true, force: true });
+		}
+	});
+
+	it("uses executable SHELL for user-shell shortcut commands", async () => {
+		if (process.platform === "win32") {
+			return;
+		}
+
+		const originalShell = Bun.env.SHELL;
+		const shellDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-env-shell-"));
+		const marker = path.join(shellDir, "env-shell-ran");
+		const markerEscaped = marker.replace(/'/g, "'\\''");
+		const fakeShell = path.join(shellDir, "fish");
+		fs.writeFileSync(
+			fakeShell,
+			`#!/bin/sh
+printf '%s\\n' "$*" > '${markerEscaped}'
+while [ "$#" -gt 0 ]; do
+	if [ "$1" = "-c" ]; then
+		shift
+		exec /bin/sh -c "$1"
+	fi
+	shift
+done
+exit 64
+`,
+		);
+		fs.chmodSync(fakeShell, 0o755);
+		Bun.env.SHELL = fakeShell;
+
+		vi.spyOn(Settings.prototype, "getShellConfig").mockReturnValue({
+			shell: "/bin/bash",
+			args: ["-l", "-c"],
+			env: {
+				PATH: Bun.env.PATH ?? "",
+				HOME: tempDir,
+				SHELL: "/bin/bash",
+			},
+			prefix: undefined,
+		});
+
+		try {
+			const result = await executeBash("printf 'env-shell-ok\\n'", {
+				cwd: tempDir,
+				timeout: 5000,
+				sessionKey: "env-user-shell",
+				useUserShell: true,
+			});
+
+			expect(result.cancelled).toBe(false);
+			expect(result.exitCode).toBe(0);
+			expect(result.output.trim()).toBe("env-shell-ok");
+			expect(fs.readFileSync(marker, "utf8")).toContain("-l -i -c");
+		} finally {
+			if (originalShell === undefined) {
+				delete Bun.env.SHELL;
+			} else {
+				Bun.env.SHELL = originalShell;
+			}
+			fs.rmSync(shellDir, { recursive: true, force: true });
+		}
+	});
+
+	it("loads zshrc aliases for user-shell shortcut commands", async () => {
+		if (process.platform === "win32") {
+			return;
+		}
+
+		const zshPath = ["/bin/zsh", "/usr/bin/zsh", "/usr/local/bin/zsh", "/opt/homebrew/bin/zsh"].find(candidate =>
+			fs.existsSync(candidate),
+		);
+		if (!zshPath) {
+			return;
+		}
+
+		const shellDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-zsh-shellpath-"));
+		fs.writeFileSync(path.join(shellDir, ".zshrc"), "alias pi_shell_alias='printf zsh-alias-ok\\\\n'\n");
+
+		vi.spyOn(Settings.prototype, "getShellConfig").mockReturnValue({
+			shell: zshPath,
+			args: ["-l", "-c"],
+			env: {
+				PATH: Bun.env.PATH ?? "",
+				HOME: shellDir,
+			},
+			prefix: undefined,
+		});
+
+		try {
+			const result = await executeBash("pi_shell_alias", {
+				cwd: tempDir,
+				timeout: 5000,
+				sessionKey: "zsh-shell-path",
+				useUserShell: true,
+			});
+
+			expect(result.cancelled).toBe(false);
+			expect(result.exitCode).toBe(0);
+			expect(result.output.trim()).toBe("zsh-alias-ok");
+		} finally {
+			fs.rmSync(shellDir, { recursive: true, force: true });
+		}
+	});
+
 	it("invokes onChunk with command output", async () => {
 		let seenChunk: string | null = null;
 		const result = await executeBash("echo hello", {
@@ -0,0 +1,53 @@
+import { beforeAll, describe, expect, it, vi } from "bun:test";
+import { CommandController } from "@oh-my-pi/pi-coding-agent/modes/controllers/command-controller";
+import { getThemeByName, setThemeInstance } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
+import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types";
+
+function createContainer() {
+	return {
+		children: [] as unknown[],
+		addChild(child: unknown) {
+			this.children.push(child);
+		},
+	};
+}
+
+describe("bash shortcut command", () => {
+	beforeAll(async () => {
+		const theme = await getThemeByName("dark");
+		if (!theme) throw new Error("Expected dark theme");
+		setThemeInstance(theme);
+	});
+
+	it("runs interactive ! commands through the configured user shell", async () => {
+		const executeBash = vi.fn().mockResolvedValue({
+			output: "ok",
+			exitCode: 0,
+			cancelled: false,
+			truncated: false,
+			totalLines: 1,
+			totalBytes: 2,
+			outputLines: 1,
+			outputBytes: 2,
+		});
+		const ctx = {
+			session: {
+				isStreaming: false,
+				executeBash,
+			},
+			chatContainer: createContainer(),
+			pendingMessagesContainer: createContainer(),
+			pendingBashComponents: [],
+			ui: { requestRender: vi.fn() },
+			showError: vi.fn(),
+		} as unknown as InteractiveModeContext;
+		const controller = new CommandController(ctx);
+
+		await controller.handleBashCommand("echo hi");
+
+		expect(executeBash).toHaveBeenCalledWith("echo hi", expect.any(Function), {
+			excludeFromContext: false,
+			useUserShell: true,
+		});
+	});
+});