diff --git a/crates/pi-natives/src/shell.rs b/crates/pi-natives/src/shell.rs index 873aae7eb..47402b5fc 100644 --- a/crates/pi-natives/src/shell.rs +++ b/crates/pi-natives/src/shell.rs @@ -162,11 +162,12 @@ impl From for MinimizerResult { #[napi(object)] pub struct ShellRunResult { /// Exit code when the command completes normally. - pub exit_code: Option, + pub exit_code: Option, /// 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://` reference into the diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs index e8796ffe2..3a79ed800 100644 --- a/crates/pi-shell/src/shell.rs +++ b/crates/pi-shell/src/shell.rs @@ -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>, @@ -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>, cancel_token: CancellationToken, spawn_registry: Arc, -) -> Result<(ExecutionResult, Option, String)> { +) -> Result<(ExecutionResult, Option, Option)> { 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, -) -> Result { +) -> Result<(ExecutionResult, Option)> { 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( diff --git a/packages/coding-agent/src/exec/bash-cwd-sync.ts b/packages/coding-agent/src/exec/bash-cwd-sync.ts index ec97f0558..2e9546113 100644 --- a/packages/coding-agent/src/exec/bash-cwd-sync.ts +++ b/packages/coding-agent/src/exec/bash-cwd-sync.ts @@ -14,13 +14,18 @@ export interface BashCwdSyncOptions { applyCwd: (cwd: string) => Promise; } -/** 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 { 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); diff --git a/packages/coding-agent/src/exec/bash-executor.ts b/packages/coding-agent/src/exec/bash-executor.ts index 19397ad4f..dfdcee8a3 100644 --- a/packages/coding-agent/src/exec/bash-executor.ts +++ b/packages/coding-agent/src/exec/bash-executor.ts @@ -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 { - 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 diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index d91bca141..cae0b2794 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -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, diff --git a/packages/coding-agent/test/bash-executor.test.ts b/packages/coding-agent/test/bash-executor.test.ts index 916d68dcf..0271c48af 100644 --- a/packages/coding-agent/test/bash-executor.test.ts +++ b/packages/coding-agent/test/bash-executor.test.ts @@ -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 { 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 () => { diff --git a/packages/natives/CHANGELOG.md b/packages/natives/CHANGELOG.md index 0dabff7fd..33de27029 100644 --- a/packages/natives/CHANGELOG.md +++ b/packages/natives/CHANGELOG.md @@ -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