diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3d140791e..d06d48306 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -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 diff --git a/packages/coding-agent/src/exec/bash-executor.ts b/packages/coding-agent/src/exec/bash-executor.ts index 0cb5bfe40..fb4de4a67 100644 --- a/packages/coding-agent/src/exec/bash-executor.ts +++ b/packages/coding-agent/src/exec/bash-executor.ts @@ -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; + /** 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 { + try { + await fs.access(shell, constants.X_OK); + return true; + } catch { + return false; + } +} + +async function resolveUserShellConfig(settings: Settings, baseConfig: ShellConfig): Promise { + 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 { 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({ diff --git a/packages/coding-agent/src/modes/controllers/command-controller.ts b/packages/coding-agent/src/modes/controllers/command-controller.ts index 7bec04c68..06d4832ce 100644 --- a/packages/coding-agent/src/modes/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/command-controller.ts @@ -934,7 +934,7 @@ export class CommandController { this.ctx.bashComponent.appendOutput(chunk); } }, - { excludeFromContext }, + { excludeFromContext, useUserShell: true }, ); if (this.ctx.bashComponent) { diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 5b1e50eb7..8e36fa6ae 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -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 { 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); diff --git a/packages/coding-agent/test/bash-executor.test.ts b/packages/coding-agent/test/bash-executor.test.ts index 52ab9c087..120b9e4ad 100644 --- a/packages/coding-agent/test/bash-executor.test.ts +++ b/packages/coding-agent/test/bash-executor.test.ts @@ -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", { diff --git a/packages/coding-agent/test/modes/controllers/bash-command.test.ts b/packages/coding-agent/test/modes/controllers/bash-command.test.ts new file mode 100644 index 000000000..70de12555 --- /dev/null +++ b/packages/coding-agent/test/modes/controllers/bash-command.test.ts @@ -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, + }); + }); +});