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
This commit is contained in:
@@ -162,25 +162,28 @@ 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
|
||||
/// minimized text shown to the agent. `None` when nothing was rewritten.
|
||||
pub minimized: Option<MinimizerResult>,
|
||||
pub minimized: Option<MinimizerResult>,
|
||||
/// Shell working directory after command completion.
|
||||
pub working_dir: Option<String>,
|
||||
}
|
||||
|
||||
impl From<CoreShellRunResult> 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,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -99,10 +99,11 @@ pub struct MinimizerResult {
|
||||
|
||||
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
|
||||
pub struct ShellRunResult {
|
||||
pub exit_code: Option<i32>,
|
||||
pub cancelled: bool,
|
||||
pub timed_out: bool,
|
||||
pub minimized: Option<MinimizerResult>,
|
||||
pub exit_code: Option<i32>,
|
||||
pub cancelled: bool,
|
||||
pub timed_out: bool,
|
||||
pub minimized: Option<MinimizerResult>,
|
||||
pub working_dir: Option<String>,
|
||||
}
|
||||
|
||||
#[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<Sender<String>>,
|
||||
cancel_token: CancellationToken,
|
||||
spawn_registry: Arc<process::SpawnRegistry>,
|
||||
) -> Result<(ExecutionResult, Option<MinimizerResult>)> {
|
||||
) -> Result<(ExecutionResult, Option<MinimizerResult>, 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(
|
||||
|
||||
@@ -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<void>;
|
||||
}
|
||||
|
||||
/** 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<string | null> {
|
||||
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;
|
||||
|
||||
|
||||
@@ -51,6 +51,7 @@ export interface BashResult {
|
||||
outputLines: number;
|
||||
outputBytes: number;
|
||||
artifactId?: string;
|
||||
workingDir?: string;
|
||||
}
|
||||
|
||||
const shellSessions = new Map<string, Shell>();
|
||||
@@ -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) {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
Vendored
+2
@@ -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
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user