From dff941d5fe0582336519eb39a19dd1bc3e4eb71d Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 1 Jul 2026 02:27:05 +0000 Subject: [PATCH] fix(coding-agent): preserved bash status while syncing cwd Propagated the native shell working directory in ShellRunResult so AgentSession can refresh cwd without running a hidden pwd command in the persistent shell. Added regression coverage for cd plus a failing command followed by echo $?, proving cwd sync no longer overwrites the user's last shell status. Fixes #3958 --- crates/pi-natives/src/shell.rs | 19 ++++--- crates/pi-shell/src/shell.rs | 49 +++++++++++-------- .../coding-agent/src/exec/bash-cwd-sync.ts | 31 ++---------- .../coding-agent/src/exec/bash-executor.ts | 2 + .../coding-agent/src/session/agent-session.ts | 3 +- .../coding-agent/test/bash-executor.test.ts | 14 ++++-- packages/natives/CHANGELOG.md | 4 ++ packages/natives/native/index.d.ts | 2 + 8 files changed, 64 insertions(+), 60 deletions(-) diff --git a/crates/pi-natives/src/shell.rs b/crates/pi-natives/src/shell.rs index 2b55d56d8..873aae7eb 100644 --- a/crates/pi-natives/src/shell.rs +++ b/crates/pi-natives/src/shell.rs @@ -162,25 +162,28 @@ 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 /// minimized text shown to the agent. `None` when nothing was rewritten. - pub minimized: Option, + pub minimized: Option, + /// Shell working directory after command completion. + pub working_dir: Option, } impl From for ShellRunResult { fn from(value: CoreShellRunResult) -> Self { Self { - exit_code: value.exit_code, - cancelled: value.cancelled, - timed_out: value.timed_out, - minimized: value.minimized.map(Into::into), + exit_code: value.exit_code, + cancelled: value.cancelled, + timed_out: value.timed_out, + minimized: value.minimized.map(Into::into), + working_dir: value.working_dir, } } } diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs index 608d9da04..e8796ffe2 100644 --- a/crates/pi-shell/src/shell.rs +++ b/crates/pi-shell/src/shell.rs @@ -99,10 +99,11 @@ pub struct MinimizerResult { #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct ShellRunResult { - pub exit_code: Option, - pub cancelled: bool, - pub timed_out: bool, - pub minimized: Option, + pub exit_code: Option, + pub cancelled: bool, + pub timed_out: bool, + pub minimized: Option, + pub working_dir: Option, } #[derive(Debug, Clone, Default)] @@ -322,10 +323,11 @@ async fn run_shell_session( } let _ = process_cancel_bridge.await; return Ok(ShellRunResult { - exit_code: None, - cancelled: matches!(reason, AbortReason::Signal), - timed_out: matches!(reason, AbortReason::Timeout), - minimized: None, + exit_code: None, + cancelled: matches!(reason, AbortReason::Signal), + timed_out: matches!(reason, AbortReason::Timeout), + minimized: None, + working_dir: None, }); } }; @@ -339,12 +341,13 @@ async fn run_shell_session( if !keepalive { *session.lock().await = None; } - let (exec, minimized) = res?; + let (exec, minimized, working_dir) = res?; Ok(ShellRunResult { exit_code: Some(exit_code(&exec)), cancelled: false, timed_out: false, minimized, + working_dir: Some(working_dir), }) } @@ -390,10 +393,11 @@ async fn run_shell_oneshot( } let _ = process_cancel_bridge.await; return Ok(ShellExecuteResult { - exit_code: None, - cancelled: matches!(reason, AbortReason::Signal), - timed_out: matches!(reason, AbortReason::Timeout), - minimized: None, + exit_code: None, + cancelled: matches!(reason, AbortReason::Signal), + timed_out: matches!(reason, AbortReason::Timeout), + minimized: None, + working_dir: None, }); }, }; @@ -402,12 +406,13 @@ async fn run_shell_oneshot( let _ = process_cancel_bridge.await; let res = run_result .unwrap_or_else(|err| Err(Error::msg(format!("Shell execution task failed: {err}")))); - let (exec, minimized) = res?; + let (exec, minimized, working_dir) = res?; Ok(ShellExecuteResult { exit_code: Some(exit_code(&exec)), cancelled: false, timed_out: false, minimized, + working_dir: Some(working_dir), }) } @@ -458,6 +463,7 @@ async fn run_shell_oneshot_streams( cancelled: matches!(reason, AbortReason::Signal), timed_out: matches!(reason, AbortReason::Timeout), minimized: None, + working_dir: None, }); }, }; @@ -468,10 +474,11 @@ async fn run_shell_oneshot_streams( .unwrap_or_else(|err| Err(Error::msg(format!("Shell execution task failed: {err}")))); let exec = res?; Ok(ShellExecuteResult { - exit_code: Some(exit_code(&exec)), - cancelled: false, - timed_out: false, - minimized: None, + exit_code: Some(exit_code(&exec)), + cancelled: false, + timed_out: false, + minimized: None, + working_dir: None, }) } @@ -760,7 +767,7 @@ async fn run_shell_command( on_chunk: Option>, cancel_token: CancellationToken, spawn_registry: Arc, -) -> Result<(ExecutionResult, Option)> { +) -> Result<(ExecutionResult, Option, String)> { if let Some(cwd) = options.cwd.as_deref() { session .shell @@ -802,7 +809,9 @@ async fn run_shell_command( .map_err(|err| Error::msg(format!("Failed to pop env scope: {err}")))?; } - result + result.map(|(exec, minimized)| { + (exec, minimized, session.shell.working_dir().to_string_lossy().into_owned()) + }) } async fn run_shell_command_single( diff --git a/packages/coding-agent/src/exec/bash-cwd-sync.ts b/packages/coding-agent/src/exec/bash-cwd-sync.ts index 1b6d854f5..ec97f0558 100644 --- a/packages/coding-agent/src/exec/bash-cwd-sync.ts +++ b/packages/coding-agent/src/exec/bash-cwd-sync.ts @@ -3,41 +3,20 @@ import * as path from "node:path"; import { logger } from "@oh-my-pi/pi-utils"; -import { type BashResult, executeBash } from "./bash-executor"; - -const CWD_QUERY_TIMEOUT_MS = 5_000; +import type { BashResult } from "./bash-executor"; export interface BashCwdSyncOptions { - /** Persistent bash session key whose current directory should be queried. */ - sessionKey: string; + /** Completed bash result whose native shell state carries the post-command cwd. */ + result: BashResult; /** Session cwd before the user bash command ran. */ currentCwd: string; - /** Run through the configured user shell when the original command did. */ - useUserShell?: boolean; /** Apply the discovered cwd to the owning session. */ applyCwd: (cwd: string) => Promise; } -/** Synchronize a persistent bash session's PWD back into the owning session. */ +/** Synchronize a completed bash command's native working directory back into the owning session. */ export async function syncBashSessionCwd(options: BashCwdSyncOptions): Promise { - let result: BashResult; - try { - result = await executeBash("pwd", { - sessionKey: options.sessionKey, - timeout: CWD_QUERY_TIMEOUT_MS, - useUserShell: options.useUserShell, - }); - } catch (error) { - logger.debug("Failed to query bash session cwd", { error: String(error) }); - return null; - } - - if (result.cancelled || result.exitCode !== 0) return null; - const nextCwd = result.output - .split(/\r?\n/) - .map(line => line.trim()) - .filter(Boolean) - .at(-1); + const nextCwd = options.result.workingDir; if (!nextCwd || !path.isAbsolute(nextCwd)) return null; if (path.resolve(nextCwd) === path.resolve(options.currentCwd)) return null; diff --git a/packages/coding-agent/src/exec/bash-executor.ts b/packages/coding-agent/src/exec/bash-executor.ts index 365d553b3..19397ad4f 100644 --- a/packages/coding-agent/src/exec/bash-executor.ts +++ b/packages/coding-agent/src/exec/bash-executor.ts @@ -51,6 +51,7 @@ export interface BashResult { outputLines: number; outputBytes: number; artifactId?: string; + workingDir?: string; } const shellSessions = new Map(); @@ -429,6 +430,7 @@ export async function executeBash(command: string, options?: BashExecutorOptions return { exitCode: winner.result.exitCode, cancelled: false, + workingDir: winner.result.workingDir, ...(await sink.dump()), }; } catch (err) { diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 6f71e6a82..d91bca141 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -12778,9 +12778,8 @@ export class AgentSession { }); await syncBashSessionCwd({ - sessionKey: this.sessionId, + result, currentCwd: cwd, - useUserShell: options?.useUserShell, applyCwd: async nextCwd => { await this.sessionManager.moveTo(nextCwd); setProjectDir(nextCwd); diff --git a/packages/coding-agent/test/bash-executor.test.ts b/packages/coding-agent/test/bash-executor.test.ts index fb8ba8b37..916d68dcf 100644 --- a/packages/coding-agent/test/bash-executor.test.ts +++ b/packages/coding-agent/test/bash-executor.test.ts @@ -122,17 +122,21 @@ describe("executeBash", () => { expect(result.output.trim()).toBe(fs.realpathSync(tempDir)); }); - it("syncs persistent shell directory changes back to the session", async () => { + it("syncs persistent shell directory changes back to the session without clobbering status", async () => { const childDir = path.join(tempDir, "child"); fs.mkdirSync(childDir); const sessionKey = `cwd-sync-${Date.now()}`; - await executeBash(`cd ${shellQuote(childDir)}`, { sessionKey, timeout: 5000, useUserShell: true }); + const result = await executeBash(`cd ${shellQuote(childDir)}; false`, { + sessionKey, + timeout: 5000, + useUserShell: true, + }); + expect(result.exitCode).toBe(1); const applied: string[] = []; const synced = await syncBashSessionCwd({ - sessionKey, + result, currentCwd: tempDir, - useUserShell: true, applyCwd: async cwd => { applied.push(cwd); }, @@ -140,6 +144,8 @@ describe("executeBash", () => { expect(synced).toBe(childDir); expect(applied).toEqual([childDir]); + const status = await executeBash("echo $?", { sessionKey, timeout: 5000, useUserShell: true }); + expect(status.output.trim()).toBe("1"); }); it("canonicalizes symlinked cwd before execution", async () => { diff --git a/packages/natives/CHANGELOG.md b/packages/natives/CHANGELOG.md index 658aa220b..0dabff7fd 100644 --- a/packages/natives/CHANGELOG.md +++ b/packages/natives/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added post-command `workingDir` metadata to native shell execution results so callers can sync persistent shell directory changes without running probe commands. + ## [16.2.11] - 2026-07-01 ### Fixed diff --git a/packages/natives/native/index.d.ts b/packages/natives/native/index.d.ts index 2e0756efb..abeebd636 100644 --- a/packages/natives/native/index.d.ts +++ b/packages/natives/native/index.d.ts @@ -1437,6 +1437,8 @@ export interface ShellRunResult { * minimized text shown to the agent. `None` when nothing was rewritten. */ minimized?: MinimizerResult + /** Shell working directory after command completion. */ + workingDir?: string } /**