fix(coding-agent): persist bash shortcut cwd changes

This commit is contained in:
Jeff Scott Ward
2026-07-07 10:44:36 -04:00
parent 48241afcc4
commit 3e24087111
5 changed files with 446 additions and 3 deletions
+4
View File
@@ -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
@@ -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;
@@ -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<void> {
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<void> {
await this.ctx.sessionManager.moveTo(resolvedPath);
await this.ctx.applyCwdChange(resolvedPath);
this.ctx.updateEditorBorderColor();
await this.ctx.reloadTodos();
}
async #applyBashResultCwd(result: BashResult): Promise<void> {
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<void> {
const isDeferred = this.ctx.session.isStreaming;
this.ctx.pythonComponent = new EvalExecutionComponent(code, this.ctx.ui, excludeFromContext);
@@ -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;
@@ -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 });
}
});
});