fix: preserve bash cwd without pwd probe

This commit is contained in:
Jeff Scott Ward
2026-06-30 22:16:27 -04:00
parent dff941d5fe
commit 3da25618ea
7 changed files with 137 additions and 54 deletions
+4 -3
View File
@@ -162,11 +162,12 @@ impl From<CoreMinimizerResult> for MinimizerResult {
#[napi(object)]
pub struct ShellRunResult {
/// Exit code when the command completes normally.
pub exit_code: Option<i32>,
pub exit_code: Option<i32>,
/// Whether the command was cancelled via abort.
pub cancelled: bool,
pub cancelled: bool,
/// Whether the command timed out before completion.
pub timed_out: bool,
pub timed_out: bool,
/// When the minimizer rewrote the captured output, this carries the
/// original buffer + telemetry so the session layer can persist it as
/// an artifact and splice an `artifact://<id>` reference into the
+35 -21
View File
@@ -59,6 +59,24 @@ impl ShellAbortState {
}
}
fn shell_working_dir_matches(shell: &BrushShell, cwd: &str) -> bool {
let requested = std::path::Path::new(cwd);
if !requested.is_absolute() {
return false;
}
let current = shell.working_dir();
current == requested
}
fn set_shell_working_dir_if_changed(shell: &mut BrushShell, cwd: &str) -> Result<()> {
if shell_working_dir_matches(shell, cwd) {
return Ok(());
}
shell
.set_working_dir(cwd)
.map_err(|err| Error::msg(format!("Failed to set cwd: {err}")))
}
#[derive(Clone)]
struct ShellConfig {
session_env: Option<HashMap<String, String>>,
@@ -337,7 +355,7 @@ async fn run_shell_session(
let _ = process_cancel_bridge.await;
abort_state.clear().await;
let keepalive = res.as_ref().is_ok_and(|pair| session_keepalive(&pair.0));
let keepalive = res.as_ref().is_ok_and(|(exec, ..)| session_keepalive(exec));
if !keepalive {
*session.lock().await = None;
}
@@ -346,8 +364,8 @@ async fn run_shell_session(
exit_code: Some(exit_code(&exec)),
cancelled: false,
timed_out: false,
working_dir,
minimized,
working_dir: Some(working_dir),
})
}
@@ -411,8 +429,8 @@ async fn run_shell_oneshot(
exit_code: Some(exit_code(&exec)),
cancelled: false,
timed_out: false,
working_dir,
minimized,
working_dir: Some(working_dir),
})
}
@@ -472,13 +490,13 @@ async fn run_shell_oneshot_streams(
let _ = process_cancel_bridge.await;
let res = run_result
.unwrap_or_else(|err| Err(Error::msg(format!("Shell execution task failed: {err}"))));
let exec = res?;
let (exec, working_dir) = res?;
Ok(ShellExecuteResult {
exit_code: Some(exit_code(&exec)),
cancelled: false,
timed_out: false,
minimized: None,
working_dir: None,
exit_code: Some(exit_code(&exec)),
cancelled: false,
timed_out: false,
working_dir,
minimized: None,
})
}
@@ -767,12 +785,9 @@ async fn run_shell_command(
on_chunk: Option<Sender<String>>,
cancel_token: CancellationToken,
spawn_registry: Arc<process::SpawnRegistry>,
) -> Result<(ExecutionResult, Option<MinimizerResult>, String)> {
) -> Result<(ExecutionResult, Option<MinimizerResult>, Option<String>)> {
if let Some(cwd) = options.cwd.as_deref() {
session
.shell
.set_working_dir(cwd)
.map_err(|err| Error::msg(format!("Failed to set cwd: {err}")))?;
set_shell_working_dir_if_changed(&mut session.shell, cwd)?;
}
let env_scope_pushed = apply_command_env(&mut session.shell, options.env.as_ref())?;
@@ -810,7 +825,8 @@ async fn run_shell_command(
}
result.map(|(exec, minimized)| {
(exec, minimized, session.shell.working_dir().to_string_lossy().into_owned())
let working_dir = Some(session.shell.working_dir().to_string_lossy().into_owned());
(exec, minimized, working_dir)
})
}
@@ -1171,12 +1187,9 @@ async fn run_shell_command_streams(
streams: StreamSinks,
cancel_token: CancellationToken,
spawn_registry: Arc<process::SpawnRegistry>,
) -> Result<ExecutionResult> {
) -> Result<(ExecutionResult, Option<String>)> {
if let Some(cwd) = options.cwd.as_deref() {
session
.shell
.set_working_dir(cwd)
.map_err(|err| Error::msg(format!("Failed to set cwd: {err}")))?;
set_shell_working_dir_if_changed(&mut session.shell, cwd)?;
}
let env_scope_pushed = apply_command_env(&mut session.shell, options.env.as_ref())?;
@@ -1297,7 +1310,8 @@ async fn run_shell_command_streams(
let _ = cancel_bridge.await;
let result = result.map_err(|err| Error::msg(format!("Shell execution failed: {err}")))?;
Ok(result)
let working_dir = Some(session.shell.working_dir().to_string_lossy().into_owned());
Ok((result, working_dir))
}
async fn read_output_bytes(
@@ -14,13 +14,18 @@ export interface BashCwdSyncOptions {
applyCwd: (cwd: string) => Promise<void>;
}
/** Synchronize a completed bash command's native working directory back into the owning session. */
/**
* Synchronize a completed bash command's native working directory back into the owning session.
*
* Use real shell path strings for the no-op check and update so symlinked/logical cwd changes
* remain visible to the host. Existence validation still follows symlinks via `stat`.
*/
export async function syncBashSessionCwd(options: BashCwdSyncOptions): Promise<string | null> {
const nextCwd = options.result.workingDir;
if (!nextCwd || !path.isAbsolute(nextCwd)) return null;
if (path.resolve(nextCwd) === path.resolve(options.currentCwd)) return null;
try {
if (path.resolve(nextCwd) === path.resolve(options.currentCwd)) return null;
const stat = await fs.stat(nextCwd);
if (!stat.isDirectory()) return null;
await options.applyCwd(nextCwd);
@@ -3,7 +3,6 @@
*
* Uses brush-core via native bindings for shell execution.
*/
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 { isExecutable, type ShellConfig } from "@oh-my-pi/pi-utils/procmgr";
@@ -116,16 +115,10 @@ function quarantineShellSession(
.catch(() => undefined);
}
async function resolveShellCwd(cwd: string | undefined): Promise<string | undefined> {
if (!cwd) return undefined;
try {
// Brush preserves the working directory string verbatim, so resolve symlinks
// up front to keep `pwd` aligned with tools like `git worktree list`.
return await fs.realpath(cwd);
} catch {
return cwd;
}
function resolveShellCwd(cwd: string | undefined): string | undefined {
// Preserve the caller's logical cwd string. Brush uses this value to update `PWD` and its
// internal working directory, so realpathing here collapses symlinks before the shell sees them.
return cwd;
}
/** Translate `ShellMinimizerSettings` into native `MinimizerOptions`, or `undefined` when disabled. */
@@ -220,7 +213,7 @@ export async function executeBash(command: string, options?: BashExecutorOptions
const minimizer = buildMinimizerOptions(settings.getGroup("shellMinimizer"));
const commandCwd = await resolveShellCwd(options?.cwd);
const commandCwd = resolveShellCwd(options?.cwd);
const commandEnv = buildNonInteractiveEnv(options?.env);
// Apply command prefix if configured
@@ -12772,11 +12772,11 @@ export class AgentSession {
onChunk,
signal: abortController.signal,
sessionKey: this.sessionId,
cwd,
timeout: clampTimeout("bash") * 1000,
onMinimizedSave: originalText => this.#saveBashOriginalArtifact(originalText),
useUserShell: options?.useUserShell,
});
await syncBashSessionCwd({
result,
currentCwd: cwd,
@@ -32,6 +32,22 @@ function shellQuote(value: string): string {
return `'${value.replace(/'/g, "'\\''")}'`;
}
function configureBashUserShell(homeDir: string): boolean {
if (process.platform === "win32" || !fs.existsSync("/bin/bash")) return false;
Settings.instance.set("shellPath", "/bin/bash");
vi.spyOn(Settings.prototype, "getShellConfig").mockReturnValue({
shell: "/bin/bash",
args: ["-c"],
env: {
PATH: Bun.env.PATH ?? "",
HOME: homeDir,
SHELL: "/bin/bash",
},
prefix: undefined,
});
return true;
}
/** Resolve once `predicate()` holds or `deadlineMs` passes, polling every 2ms. */
async function pollUntil(predicate: () => boolean, deadlineMs: number): Promise<void> {
while (!predicate() && Date.now() < deadlineMs) {
@@ -119,19 +135,18 @@ describe("executeBash", () => {
it("honors cwd", async () => {
const result = await executeBash("pwd", { cwd: tempDir, timeout: 5000 });
expect(result.output.trim()).toBe(fs.realpathSync(tempDir));
expect(result.output.trim()).toBe(tempDir);
});
it("syncs persistent shell directory changes back to the session without clobbering status", async () => {
it("returns and syncs persistent shell directory changes back to the session", async () => {
if (!configureBashUserShell(tempDir)) return;
const childDir = path.join(tempDir, "child");
fs.mkdirSync(childDir);
const realChildDir = fs.realpathSync(childDir);
const sessionKey = `cwd-sync-${Date.now()}`;
const result = await executeBash(`cd ${shellQuote(childDir)}; false`, {
sessionKey,
timeout: 5000,
useUserShell: true,
});
expect(result.exitCode).toBe(1);
const result = await executeBash(`cd ${shellQuote(childDir)}`, { sessionKey, timeout: 5000, useUserShell: true });
expect(result.workingDir ? fs.realpathSync(result.workingDir) : undefined).toBe(realChildDir);
const applied: string[] = [];
const synced = await syncBashSessionCwd({
@@ -142,24 +157,79 @@ describe("executeBash", () => {
},
});
expect(synced).toBe(childDir);
expect(applied).toEqual([childDir]);
const status = await executeBash("echo $?", { sessionKey, timeout: 5000, useUserShell: true });
expect(synced ? fs.realpathSync(synced) : undefined).toBe(realChildDir);
expect(applied.map(cwd => fs.realpathSync(cwd))).toEqual([realChildDir]);
});
it("does not clobber the persistent shell status while syncing cwd", async () => {
if (!configureBashUserShell(tempDir)) return;
const childDir = path.join(tempDir, "child-status");
fs.mkdirSync(childDir);
const sessionKey = `cwd-status-${Date.now()}`;
const result = await executeBash(`cd ${shellQuote(childDir)}; false`, {
sessionKey,
cwd: tempDir,
timeout: 5000,
useUserShell: true,
});
expect(result.exitCode).toBe(1);
await syncBashSessionCwd({
result,
currentCwd: tempDir,
applyCwd: async () => {},
});
const status = await executeBash(`printf '%s\n' "$?"`, { sessionKey, timeout: 5000, useUserShell: true });
expect(status.output.trim()).toBe("1");
});
it("canonicalizes symlinked cwd before execution", async () => {
it("does not reset OLDPWD when synchronized cwd is passed to the next command", async () => {
if (!configureBashUserShell(tempDir)) return;
const childDir = path.join(tempDir, "child-oldpwd");
fs.mkdirSync(childDir);
const realTempDir = fs.realpathSync(tempDir);
const realChildDir = fs.realpathSync(childDir);
const sessionKey = `cwd-oldpwd-${Date.now()}`;
const result = await executeBash(`cd ${shellQuote(childDir)}`, {
sessionKey,
cwd: tempDir,
timeout: 5000,
useUserShell: true,
});
const synced = await syncBashSessionCwd({
result,
currentCwd: tempDir,
applyCwd: async () => {},
});
expect(synced ? fs.realpathSync(synced) : undefined).toBe(realChildDir);
const back = await executeBash("cd - >/dev/null; pwd", {
sessionKey,
cwd: synced ?? childDir,
timeout: 5000,
useUserShell: true,
});
expect(back.output.trim() ? fs.realpathSync(back.output.trim()) : undefined).toBe(realTempDir);
});
it("honors symlinked cwd requests in persistent shells", async () => {
if (process.platform === "win32") {
return;
}
if (!configureBashUserShell(tempDir)) return;
const realDir = path.join(tempDir, "real");
const linkDir = path.join(tempDir, "link");
fs.mkdirSync(realDir);
fs.symlinkSync(realDir, linkDir, "dir");
const sessionKey = `cwd-symlink-${Date.now()}`;
const result = await executeBash("pwd", { cwd: linkDir, timeout: 5000 });
expect(result.output.trim()).toBe(fs.realpathSync(linkDir));
await executeBash("pwd", { sessionKey, cwd: realDir, timeout: 5000, useUserShell: true });
const result = await executeBash("pwd", { sessionKey, cwd: linkDir, timeout: 5000, useUserShell: true });
expect(result.output.trim()).toBe(linkDir);
expect(result.workingDir).toBe(linkDir);
});
it("passes env vars", async () => {
+1 -1
View File
@@ -4,7 +4,7 @@
### Added
- Added post-command `workingDir` metadata to native shell execution results so callers can sync persistent shell directory changes without running probe commands.
- Added `workingDir` to `ShellRunResult` so hosts can sync session cwd without running a hidden probe command.
## [16.2.11] - 2026-07-01