From 3e240871113255b2c24efa5f5ea0a6e36bfc0738 Mon Sep 17 00:00:00 2001 From: Jeff Scott Ward Date: Tue, 7 Jul 2026 10:44:36 -0400 Subject: [PATCH] fix(coding-agent): persist bash shortcut cwd changes --- packages/coding-agent/CHANGELOG.md | 4 + .../coding-agent/src/exec/bash-executor.ts | 50 +++- .../modes/controllers/command-controller.ts | 42 +++- .../coding-agent/test/bash-executor.test.ts | 116 ++++++++- .../modes/controllers/bash-command.test.ts | 237 ++++++++++++++++++ 5 files changed, 446 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ad20be261..935582505 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed interactive bash shortcut `cd` commands leaving the OMP session and status-line working directory unchanged. + ## [17.0.3] - 2026-07-17 ### Changed diff --git a/packages/coding-agent/src/exec/bash-executor.ts b/packages/coding-agent/src/exec/bash-executor.ts index 3927532c0..32b9e8847 100644 --- a/packages/coding-agent/src/exec/bash-executor.ts +++ b/packages/coding-agent/src/exec/bash-executor.ts @@ -150,6 +150,53 @@ function isBashShell(shell: string): boolean { return basename.includes("bash"); } +const UNSUPPORTED_UNQUOTED_CD_CHARS = "\\$`;&|<>(){}*?[]!#\"'"; + +function hasUnsupportedUnquotedCdSyntax(value: string): boolean { + for (const char of value) { + if (/\s/.test(char) || UNSUPPORTED_UNQUOTED_CD_CHARS.includes(char)) return true; + } + return false; +} + +export function isPersistentShellCdCommand(command: string): boolean { + if (/[\r\n]/.test(command)) return false; + + const trimmed = command.trim(); + if (trimmed === "cd") return true; + if (!trimmed.startsWith("cd") || !/[ \t]/.test(trimmed[2] ?? "")) return false; + + let rest = trimmed.slice(2).trim(); + if (rest === "" || rest === "--") return true; + + let hasOptionTerminator = false; + if (/^--[ \t]/.test(rest)) { + hasOptionTerminator = true; + rest = rest.slice(2).trimStart(); + } + if (rest === "") return true; + + const quote = rest[0]; + let target: string; + let quoted = false; + if (quote === `"` || quote === "'") { + if (rest.length < 2 || rest[rest.length - 1] !== quote) return false; + target = rest.slice(1, -1); + if (target.includes(quote)) return false; + if (quote === `"` && /[\\$`\r\n]/.test(target)) return false; + quoted = true; + } else { + if (hasUnsupportedUnquotedCdSyntax(rest)) return false; + target = rest; + } + + if (target === "") return false; + if (/^[+-]\d+$/.test(target)) return false; + if (!hasOptionTerminator && target.startsWith("-") && target !== "-") return false; + if (!quoted && target.startsWith("~") && target !== "~" && !target.startsWith("~/")) return false; + return true; +} + function needsInteractiveShellArg(shell: string): boolean { const basename = shellBasename(shell); return basename.includes("zsh"); @@ -224,8 +271,9 @@ export async function executeBash(command: string, options?: BashExecutorOptions // Apply command prefix if configured const prefixedCommand = prefix ? `${prefix} ${command}` : command; + const runCdInPersistentShell = options?.useUserShell === true && !prefix && isPersistentShellCdCommand(command); const finalCommand = - options?.useUserShell === true && !bashShell + options?.useUserShell === true && !bashShell && !runCdInPersistentShell ? buildUserShellCommand(shell, args, prefixedCommand) : prefixedCommand; diff --git a/packages/coding-agent/src/modes/controllers/command-controller.ts b/packages/coding-agent/src/modes/controllers/command-controller.ts index 6d21be4de..9b7f17b06 100644 --- a/packages/coding-agent/src/modes/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/command-controller.ts @@ -13,6 +13,7 @@ import { import { Loader, Markdown, padding, Spacer, Text, visibleWidth } from "@oh-my-pi/pi-tui"; import { formatDuration, Snowflake, sanitizeText } from "@oh-my-pi/pi-utils"; import { shouldEnableAppendOnlyContext } from "../../config/append-only-context-mode"; +import { type BashResult, isPersistentShellCdCommand } from "../../exec/bash-executor"; import { type LoadedCustomShare, loadCustomShare } from "../../export/custom-share"; import { shareSession } from "../../export/share"; import type { CompactOptions } from "../../extensibility/extensions/types"; @@ -1100,6 +1101,12 @@ export class CommandController { async handleBashCommand(command: string, excludeFromContext = false): Promise { const isDeferred = this.ctx.session.isStreaming; + const shouldPersistCwd = isPersistentShellCdCommand(command); + if (isDeferred && shouldPersistCwd) { + this.ctx.showWarning("Wait for the current response to finish or abort it before changing directories."); + return; + } + this.ctx.bashComponent = new BashExecutionComponent(command, this.ctx.ui, excludeFromContext); if (isDeferred) { @@ -1120,7 +1127,6 @@ export class CommandController { }, { excludeFromContext, useUserShell: true }, ); - if (this.ctx.bashComponent) { const meta = outputMeta().truncationFromSummary(result, { direction: "tail" }).get(); this.ctx.bashComponent.setComplete(result.exitCode, result.cancelled, { @@ -1128,6 +1134,15 @@ export class CommandController { truncation: meta?.truncation, }); } + try { + if (shouldPersistCwd) await this.#applyBashResultCwd(result); + } catch (error) { + this.ctx.showError( + `Bash command completed, but OMP failed to update its working directory: ${ + error instanceof Error ? error.message : "Unknown error" + }`, + ); + } } catch (error) { if (this.ctx.bashComponent) { this.ctx.bashComponent.setComplete(undefined, false); @@ -1139,6 +1154,31 @@ export class CommandController { this.ctx.ui.requestRender(); } + async #moveInteractiveCwd(resolvedPath: string): Promise { + await this.ctx.sessionManager.moveTo(resolvedPath); + await this.ctx.applyCwdChange(resolvedPath); + this.ctx.updateEditorBorderColor(); + await this.ctx.reloadTodos(); + } + + async #applyBashResultCwd(result: BashResult): Promise { + if (result.cancelled || result.exitCode !== 0 || !result.workingDir) return; + if (!path.isAbsolute(result.workingDir)) return; + + const resolvedPath = path.resolve(result.workingDir); + if (resolvedPath === path.resolve(this.ctx.sessionManager.getCwd())) return; + + let isDirectory = false; + try { + isDirectory = (await fs.stat(resolvedPath)).isDirectory(); + } catch { + isDirectory = false; + } + if (!isDirectory) return; + + await this.#moveInteractiveCwd(resolvedPath); + } + async handlePythonCommand(code: string, excludeFromContext = false): Promise { const isDeferred = this.ctx.session.isStreaming; this.ctx.pythonComponent = new EvalExecutionComponent(code, this.ctx.ui, excludeFromContext); diff --git a/packages/coding-agent/test/bash-executor.test.ts b/packages/coding-agent/test/bash-executor.test.ts index d2b7c96bf..7badaa940 100644 --- a/packages/coding-agent/test/bash-executor.test.ts +++ b/packages/coding-agent/test/bash-executor.test.ts @@ -3,7 +3,11 @@ import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; import { resetSettingsForTest, Settings, type ShellMinimizerSettings } from "@oh-my-pi/pi-coding-agent/config/settings"; -import { buildMinimizerOptions, executeBash } from "@oh-my-pi/pi-coding-agent/exec/bash-executor"; +import { + buildMinimizerOptions, + executeBash, + isPersistentShellCdCommand, +} from "@oh-my-pi/pi-coding-agent/exec/bash-executor"; import { DEFAULT_MAX_BYTES } from "@oh-my-pi/pi-coding-agent/session/streaming-output"; import * as shellSnapshot from "@oh-my-pi/pi-coding-agent/utils/shell-snapshot"; import type { Shell, ShellRunResult } from "@oh-my-pi/pi-natives"; @@ -126,6 +130,34 @@ describe("executeBash", () => { legacyFilters: true, }); }); + + it.each([ + ["cd", true], + [" cd child ", true], + ["cd\tchild", true], + ["cd -", true], + ["cd --", true], + ["cd -- -P", true], + ['cd "#note"', true], + ['cd "two words"', true], + ["cd '~/literal'", true], + ["cd +1", false], + ['cd "+1"', false], + ["cd -- -1", false], + ["cd -- '+2'", false], + ["cd\npwd", false], + ["cd\rpwd", false], + ["cd -P", false], + ["cd -L /tmp", false], + ["cd #note", false], + ["cd child && pwd", false], + ["cd two words", false], + ['cd ""', false], + ["cd ~other", false], + ["echo cd child", false], + ] as const)("classifies persistent-shell cd routing for %j", (command, expected) => { + expect(isPersistentShellCdCommand(command)).toBe(expected); + }); it("returns non-zero exit codes without cancellation", async () => { const result = await executeBash("exit 7", { cwd: tempDir, timeout: 5000 }); expect(result.exitCode).toBe(7); @@ -227,6 +259,88 @@ exit 64 } }); + it("persists cd, bare cd, and cd - when shortcut commands use a non-bash user shell", async () => { + if (process.platform === "win32") return; + + const shellDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-cd-shellpath-")); + const marker = path.join(shellDir, "fake-shell-ran"); + const fakeShell = path.join(shellDir, "fake-shell"); + const childDir = path.join(tempDir, "child"); + fs.mkdirSync(childDir); + fs.writeFileSync( + fakeShell, + `#!/bin/sh +printf '%s\\n' "$*" > ${shellQuote(marker)} +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 sessionKey = `persistent-cd-${Date.now()}`; + const moved = await executeBash("cd child", { + cwd: tempDir, + timeout: 5000, + sessionKey, + useUserShell: true, + }); + + expect(moved.exitCode).toBe(0); + expect(moved.workingDir).toBe(childDir); + expect(fs.existsSync(marker)).toBe(false); + + const home = await executeBash("cd", { + cwd: childDir, + timeout: 5000, + sessionKey, + useUserShell: true, + }); + + expect(home.exitCode).toBe(0); + expect(home.workingDir).toBe(tempDir); + expect(fs.existsSync(marker)).toBe(false); + + const returned = await executeBash("cd -", { + cwd: tempDir, + timeout: 5000, + sessionKey, + useUserShell: true, + }); + + expect(returned.exitCode).toBe(0); + expect(returned.workingDir).toBe(childDir); + expect(fs.existsSync(marker)).toBe(false); + + const pwd = await executeBash("pwd", { + cwd: childDir, + timeout: 5000, + sessionKey, + useUserShell: true, + }); + expect(pwd.output.trim()).toBe(childDir); + expect(fs.existsSync(marker)).toBe(true); + } finally { + removeSyncWithRetries(shellDir); + } + }); + it("uses executable SHELL for user-shell shortcut commands", async () => { if (process.platform === "win32") { return; diff --git a/packages/coding-agent/test/modes/controllers/bash-command.test.ts b/packages/coding-agent/test/modes/controllers/bash-command.test.ts index fd2bd0e1f..12d17ac4e 100644 --- a/packages/coding-agent/test/modes/controllers/bash-command.test.ts +++ b/packages/coding-agent/test/modes/controllers/bash-command.test.ts @@ -1,4 +1,8 @@ import { beforeAll, describe, expect, it, vi } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { BashExecutionComponent } from "@oh-my-pi/pi-coding-agent/modes/components/bash-execution"; 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"; @@ -12,6 +16,51 @@ function createContainer() { }; } +function createCwdContext(sourceDir: string, isStreaming = false) { + const state = { cwd: sourceDir, executedCwds: [] as string[] }; + const executeBash = vi.fn(async (command: string) => { + state.executedCwds.push(state.cwd); + return { + output: command === "pwd" ? `${state.cwd}\n` : "ok", + exitCode: 0, + cancelled: false, + truncated: false, + totalLines: 1, + totalBytes: state.cwd.length, + outputLines: 1, + outputBytes: state.cwd.length, + workingDir: state.cwd, + }; + }); + const pendingMessagesContainer = createContainer(); + const present = vi.fn(); + const ctx = { + session: { + isStreaming, + executeBash, + }, + sessionManager: { + getCwd: () => state.cwd, + moveTo: vi.fn(async (cwd: string) => { + state.cwd = cwd; + }), + }, + chatContainer: createContainer(), + pendingMessagesContainer, + pendingBashComponents: [], + ui: { requestRender: vi.fn(), requestComponentRender: vi.fn() }, + present, + showError: vi.fn(), + showWarning: vi.fn(), + applyCwdChange: vi.fn(async (cwd: string) => { + expect(state.cwd).toBe(cwd); + }), + updateEditorBorderColor: vi.fn(), + reloadTodos: vi.fn(async () => {}), + } as unknown as InteractiveModeContext; + return { ctx, executeBash, pendingMessagesContainer, present, state }; +} + describe("bash shortcut command", () => { beforeAll(async () => { const theme = await getThemeByName("dark"); @@ -35,12 +84,18 @@ describe("bash shortcut command", () => { isStreaming: false, executeBash, }, + sessionManager: { + getCwd: () => "/tmp", + }, chatContainer: createContainer(), pendingMessagesContainer: createContainer(), pendingBashComponents: [], ui: { requestRender: vi.fn(), requestComponentRender: vi.fn() }, present: vi.fn(), showError: vi.fn(), + applyCwdChange: vi.fn(async () => {}), + updateEditorBorderColor: vi.fn(), + reloadTodos: vi.fn(async () => {}), } as unknown as InteractiveModeContext; const controller = new CommandController(ctx); @@ -51,4 +106,186 @@ describe("bash shortcut command", () => { useUserShell: true, }); }); + + it("persists standalone and bare cd before the next user-shell command", async () => { + const sourceDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-bash-cd-source-")); + const childDir = path.join(sourceDir, "child"); + await fs.mkdir(childDir); + try { + const { ctx, executeBash, state } = createCwdContext(sourceDir); + executeBash.mockImplementationOnce(async () => { + state.executedCwds.push(state.cwd); + return { + output: "", + exitCode: 0, + cancelled: false, + truncated: false, + totalLines: 0, + totalBytes: 0, + outputLines: 0, + outputBytes: 0, + workingDir: childDir, + }; + }); + executeBash.mockImplementationOnce(async () => { + state.executedCwds.push(state.cwd); + return { + output: "", + exitCode: 0, + cancelled: false, + truncated: false, + totalLines: 0, + totalBytes: 0, + outputLines: 0, + outputBytes: 0, + workingDir: sourceDir, + }; + }); + const controller = new CommandController(ctx); + + await controller.handleBashCommand("cd child"); + await controller.handleBashCommand("cd"); + await controller.handleBashCommand("pwd"); + + expect(state.cwd).toBe(sourceDir); + expect(state.executedCwds).toEqual([sourceDir, childDir, sourceDir]); + expect(executeBash).toHaveBeenCalledTimes(3); + expect(executeBash).toHaveBeenNthCalledWith(1, "cd child", expect.any(Function), { + excludeFromContext: false, + useUserShell: true, + }); + expect(executeBash).toHaveBeenNthCalledWith(2, "cd", expect.any(Function), { + excludeFromContext: false, + useUserShell: true, + }); + expect(ctx.applyCwdChange).toHaveBeenNthCalledWith(1, childDir); + expect(ctx.applyCwdChange).toHaveBeenNthCalledWith(2, sourceDir); + expect(ctx.updateEditorBorderColor).toHaveBeenCalledTimes(2); + expect(ctx.reloadTodos).toHaveBeenCalledTimes(2); + expect(ctx.showError).not.toHaveBeenCalled(); + } finally { + await fs.rm(sourceDir, { recursive: true, force: true }); + } + }); + + it("does not adopt cwd from a non-cd bash command", async () => { + const sourceDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-bash-cwd-sync-")); + const childDir = path.join(sourceDir, "child"); + await fs.mkdir(childDir); + try { + const { ctx, executeBash, state } = createCwdContext(sourceDir); + executeBash.mockImplementationOnce(async () => { + state.executedCwds.push(state.cwd); + return { + output: "", + exitCode: 0, + cancelled: false, + truncated: false, + totalLines: 0, + totalBytes: 0, + outputLines: 0, + outputBytes: 0, + workingDir: childDir, + }; + }); + const controller = new CommandController(ctx); + + await controller.handleBashCommand("pushd child >/dev/null"); + + expect(state.cwd).toBe(sourceDir); + expect(state.executedCwds).toEqual([sourceDir]); + expect(executeBash).toHaveBeenCalledTimes(1); + expect(ctx.applyCwdChange).not.toHaveBeenCalled(); + expect(ctx.updateEditorBorderColor).not.toHaveBeenCalled(); + expect(ctx.reloadTodos).not.toHaveBeenCalled(); + expect(ctx.showWarning).not.toHaveBeenCalled(); + expect(ctx.showError).not.toHaveBeenCalled(); + } finally { + await fs.rm(sourceDir, { recursive: true, force: true }); + } + }); + + it("rejects simple cd while streaming before queuing a bash block", async () => { + const sourceDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-bash-cd-streaming-")); + try { + const { ctx, executeBash, pendingMessagesContainer, present, state } = createCwdContext(sourceDir, true); + const controller = new CommandController(ctx); + + await controller.handleBashCommand("cd child"); + + expect(state.cwd).toBe(sourceDir); + expect(executeBash).not.toHaveBeenCalled(); + expect(present).not.toHaveBeenCalled(); + expect(pendingMessagesContainer.children).toHaveLength(0); + expect(ctx.pendingBashComponents).toHaveLength(0); + expect(ctx.showWarning).toHaveBeenCalledWith(expect.stringContaining("response")); + } finally { + await fs.rm(sourceDir, { recursive: true, force: true }); + } + }); + + it("does not adopt cwd or warn for a non-cd command while streaming", async () => { + const sourceDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-bash-cwd-deferred-")); + const childDir = path.join(sourceDir, "child"); + await fs.mkdir(childDir); + try { + const { ctx, executeBash, pendingMessagesContainer, state } = createCwdContext(sourceDir, true); + executeBash.mockImplementationOnce(async () => ({ + output: "", + exitCode: 0, + cancelled: false, + truncated: false, + totalLines: 0, + totalBytes: 0, + outputLines: 0, + outputBytes: 0, + workingDir: childDir, + })); + const controller = new CommandController(ctx); + + await controller.handleBashCommand("pushd child >/dev/null"); + + expect(state.cwd).toBe(sourceDir); + expect(ctx.applyCwdChange).not.toHaveBeenCalled(); + expect(pendingMessagesContainer.children).toHaveLength(1); + expect(ctx.pendingBashComponents).toHaveLength(1); + expect(ctx.showWarning).not.toHaveBeenCalled(); + } finally { + await fs.rm(sourceDir, { recursive: true, force: true }); + } + }); + + it("finalizes successful output before reporting a standalone cd refresh failure", async () => { + const sourceDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-bash-cwd-refresh-error-")); + const childDir = path.join(sourceDir, "child"); + await fs.mkdir(childDir); + try { + const { ctx, executeBash, present, state } = createCwdContext(sourceDir); + executeBash.mockImplementationOnce(async () => ({ + output: "final output", + exitCode: 0, + cancelled: false, + truncated: false, + totalLines: 1, + totalBytes: 12, + outputLines: 1, + outputBytes: 12, + workingDir: childDir, + })); + ctx.applyCwdChange = vi.fn(async () => { + throw new Error("refresh failed"); + }); + const controller = new CommandController(ctx); + + await controller.handleBashCommand("cd child"); + + const component = present.mock.calls[0]?.[0]; + expect(component).toBeInstanceOf(BashExecutionComponent); + expect((component as BashExecutionComponent).getOutput()).toContain("final output"); + expect(state.cwd).toBe(childDir); + expect(ctx.showError).toHaveBeenCalledWith(expect.stringContaining("completed, but")); + } finally { + await fs.rm(sourceDir, { recursive: true, force: true }); + } + }); });