From 1544a003db2c5c59cc8d73fc9e2ed2e8333a3e2d Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Mon, 22 Jun 2026 19:17:23 +0000 Subject: [PATCH] app-server: preserve legacy cwd strings in exec events --- .../src/protocol/item_builders.rs | 27 +++++++++------- .../src/protocol/item_builders_tests.rs | 32 ++++++++++++++++++- codex-rs/core/src/tools/events.rs | 4 +-- .../remote_env_windows_test.rs | 3 +- codex-rs/core/tests/suite/unified_exec.rs | 9 ++++-- codex-rs/protocol/src/protocol.rs | 5 +-- codex-rs/rollout-trace/src/protocol_event.rs | 13 ++++---- .../rollout-trace/src/protocol_event_tests.rs | 4 +-- .../utils/path-uri/src/api_path_string.rs | 6 ++-- .../path-uri/src/api_path_string_tests.rs | 18 +++++++++++ 10 files changed, 90 insertions(+), 31 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/item_builders.rs b/codex-rs/app-server-protocol/src/protocol/item_builders.rs index 66019980a3..53c0beb9bd 100644 --- a/codex-rs/app-server-protocol/src/protocol/item_builders.rs +++ b/codex-rs/app-server-protocol/src/protocol/item_builders.rs @@ -35,8 +35,8 @@ use codex_protocol::protocol::PatchApplyBeginEvent; use codex_protocol::protocol::PatchApplyEndEvent; use codex_shell_command::parse_command::parse_command; use codex_shell_command::parse_command::shlex_join; +use codex_utils_path_uri::LegacyAppPathString; use codex_utils_path_uri::PathConvention; -use codex_utils_path_uri::PathUri; use std::collections::HashMap; use std::path::PathBuf; use tracing::warn; @@ -90,11 +90,11 @@ pub fn build_command_execution_approval_request_item( } pub fn build_command_execution_begin_item(payload: &ExecCommandBeginEvent) -> ThreadItem { - let command_actions = command_actions_for_path_uri(&payload.parsed_cmd, &payload.cwd); + let command_actions = command_actions_for_legacy_cwd(&payload.parsed_cmd, &payload.cwd); ThreadItem::CommandExecution { id: payload.call_id.clone(), command: shlex_join(&payload.command), - cwd: payload.cwd.clone().into(), + cwd: payload.cwd.clone(), process_id: payload.process_id.clone(), source: payload.source.into(), status: CommandExecutionStatus::InProgress, @@ -112,12 +112,12 @@ pub fn build_command_execution_end_item(payload: &ExecCommandEndEvent) -> Thread Some(payload.aggregated_output.clone()) }; let duration_ms = i64::try_from(payload.duration.as_millis()).unwrap_or(i64::MAX); - let command_actions = command_actions_for_path_uri(&payload.parsed_cmd, &payload.cwd); + let command_actions = command_actions_for_legacy_cwd(&payload.parsed_cmd, &payload.cwd); ThreadItem::CommandExecution { id: payload.call_id.clone(), command: shlex_join(&payload.command), - cwd: payload.cwd.clone().into(), + cwd: payload.cwd.clone(), process_id: payload.process_id.clone(), source: payload.source.into(), status: (&payload.status).into(), @@ -128,14 +128,19 @@ pub fn build_command_execution_end_item(payload: &ExecCommandEndEvent) -> Thread } } -fn command_actions_for_path_uri(parsed_cmd: &[ParsedCommand], cwd: &PathUri) -> Vec { +fn command_actions_for_legacy_cwd( + parsed_cmd: &[ParsedCommand], + cwd: &LegacyAppPathString, +) -> Vec { // TODO(anp): Carry PathUri into CommandAction so foreign Read actions retain resolved paths. // Until then, omit those actions rather than project a foreign cwd onto the host. - let native_cwd = if cwd.infer_path_convention() == Some(PathConvention::native()) { - cwd.to_abs_path().ok() - } else { - None - }; + let native_cwd = cwd.to_inferred_path_uri().and_then(|cwd| { + if cwd.infer_path_convention() == Some(PathConvention::native()) { + cwd.to_abs_path().ok() + } else { + None + } + }); parsed_cmd .iter() diff --git a/codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs b/codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs index b892fcf505..a622affc0d 100644 --- a/codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs @@ -1,4 +1,5 @@ use super::*; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; #[test] @@ -25,7 +26,7 @@ fn foreign_read_is_omitted_without_dropping_other_command_actions() { ]; assert_eq!( - command_actions_for_path_uri(&parsed_cmd, &cwd), + command_actions_for_legacy_cwd(&parsed_cmd, &cwd.into()), vec![ CommandAction::ListFiles { command: "ls".to_string(), @@ -39,3 +40,32 @@ fn foreign_read_is_omitted_without_dropping_other_command_actions() { ] ); } + +#[test] +fn raw_file_uri_cwd_is_converted_for_command_actions() { + #[cfg(windows)] + let raw_uri = "file:///C:/src"; + #[cfg(not(windows))] + let raw_uri = "file:///usr/local/src"; + let expected_path = PathUri::parse(raw_uri) + .expect("raw file URI should parse") + .to_abs_path() + .expect("raw file URI should be native") + .join("file.txt"); + let cwd = serde_json::from_value::(serde_json::json!(raw_uri)) + .expect("raw file URI should deserialize as a legacy API path"); + let parsed_cmd = vec![ParsedCommand::Read { + cmd: "cat file.txt".to_string(), + name: "file.txt".to_string(), + path: PathBuf::from("file.txt"), + }]; + + assert_eq!( + command_actions_for_legacy_cwd(&parsed_cmd, &cwd), + vec![CommandAction::Read { + command: "cat file.txt".to_string(), + name: "file.txt".to_string(), + path: expected_path, + }], + ); +} diff --git a/codex-rs/core/src/tools/events.rs b/codex-rs/core/src/tools/events.rs index 757125a03d..42182da369 100644 --- a/codex-rs/core/src/tools/events.rs +++ b/codex-rs/core/src/tools/events.rs @@ -111,7 +111,7 @@ pub(crate) async fn emit_exec_command_begin( turn_id: ctx.turn.sub_id.clone(), started_at_ms: now_unix_timestamp_ms(), command: command.to_vec(), - cwd: cwd.clone(), + cwd: cwd.clone().into(), parsed_cmd: parsed_cmd.to_vec(), source, interaction_input, @@ -551,7 +551,7 @@ async fn emit_exec_end( turn_id: ctx.turn.sub_id.clone(), completed_at_ms: now_unix_timestamp_ms(), command: exec_input.command.to_vec(), - cwd: exec_input.cwd.clone(), + cwd: exec_input.cwd.clone().into(), parsed_cmd: exec_input.parsed_cmd.to_vec(), source: exec_input.source, interaction_input: exec_input.interaction_input.map(str::to_owned), diff --git a/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs b/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs index 04f8914982..7de2e30152 100644 --- a/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs +++ b/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs @@ -194,7 +194,8 @@ async fn windows_exec_server_runs_with_native_shell_and_cwd() -> Result<()> { assert_eq!(&begin.command[1..], ["-NoProfile", "-Command", COMMAND]); let end = end.context("exec_command should emit an end event")?; - let expected_cwd = PathUri::parse("file:///C:/windows")?; + let expected_cwd: LegacyAppPathString = + PathUri::parse("file:///C:/windows")?.into(); assert_eq!((&begin.cwd, &end.cwd), (&expected_cwd, &expected_cwd)); assert_eq!((end.exit_code, end.status), (0, ExecCommandStatus::Completed)); diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index fe6ce7fbb7..196d8333ad 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -437,7 +437,10 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> { assert_command(&begin_event.command, "-lc", "/bin/echo hello unified exec"); - assert_eq!(begin_event.cwd, PathUri::from_host_native_path(&cwd)?); + assert_eq!( + begin_event.cwd, + PathUri::from_host_native_path(&cwd)?.into() + ); wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_)) @@ -508,7 +511,7 @@ async fn unified_exec_resolves_relative_workdir() -> Result<()> { assert_eq!( begin_event.cwd, - PathUri::from_host_native_path(&workdir)?, + PathUri::from_host_native_path(&workdir)?.into(), "exec_command cwd should resolve relative workdir against turn cwd", ); @@ -570,7 +573,7 @@ async fn unified_exec_respects_workdir_override() -> Result<()> { assert_eq!( begin_event.cwd, - PathUri::from_host_native_path(&workdir)?, + PathUri::from_host_native_path(&workdir)?.into(), "exec_command cwd should reflect the requested workdir override" ); diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 5a73ac4c90..132e263ec7 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -54,6 +54,7 @@ use crate::request_permissions::RequestPermissionsResponse; use crate::request_user_input::RequestUserInputResponse; use crate::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::LegacyAppPathString; use codex_utils_path_uri::PathUri; use schemars::JsonSchema; use serde::Deserialize; @@ -3345,7 +3346,7 @@ pub struct ExecCommandBeginEvent { /// The command to be executed. pub command: Vec, /// The command's working directory if not the default cwd for the agent. - pub cwd: PathUri, + pub cwd: LegacyAppPathString, pub parsed_cmd: Vec, /// Where the command originated. Defaults to Agent for backward compatibility. #[serde(default)] @@ -3371,7 +3372,7 @@ pub struct ExecCommandEndEvent { /// The command that was executed. pub command: Vec, /// The command's working directory if not the default cwd for the agent. - pub cwd: PathUri, + pub cwd: LegacyAppPathString, pub parsed_cmd: Vec, /// Where the command originated. Defaults to Agent for backward compatibility. #[serde(default)] diff --git a/codex-rs/rollout-trace/src/protocol_event.rs b/codex-rs/rollout-trace/src/protocol_event.rs index 42e0db04d2..dac9db029d 100644 --- a/codex-rs/rollout-trace/src/protocol_event.rs +++ b/codex-rs/rollout-trace/src/protocol_event.rs @@ -147,7 +147,7 @@ impl Serialize for ToolRuntimePayload<'_> { /// Rollout-trace representation of an exec begin event. /// /// Rollout traces share the rollout compatibility requirement that paths remain path-flavored -/// strings on disk, even though live events carry `PathUri` internally. +/// strings on disk. #[derive(Serialize)] struct ExecCommandBeginTracePayload<'a> { call_id: &'a str, @@ -156,7 +156,7 @@ struct ExecCommandBeginTracePayload<'a> { turn_id: &'a str, started_at_ms: i64, command: &'a [String], - cwd: String, + cwd: &'a str, parsed_cmd: &'a [codex_protocol::parse_command::ParsedCommand], source: ExecCommandSource, #[serde(skip_serializing_if = "Option::is_none")] @@ -182,7 +182,7 @@ impl<'a> From<&'a ExecCommandBeginEvent> for ExecCommandBeginTracePayload<'a> { turn_id, started_at_ms: *started_at_ms, command, - cwd: cwd.inferred_native_path_string(), + cwd: cwd.as_str(), parsed_cmd, source: *source, interaction_input: interaction_input.as_deref(), @@ -192,8 +192,7 @@ impl<'a> From<&'a ExecCommandBeginEvent> for ExecCommandBeginTracePayload<'a> { /// Rollout-trace representation of an exec end event. /// -/// Like [`ExecCommandBeginTracePayload`], this renders `cwd` as an inferred native path to preserve -/// the on-disk format rather than serializing the internal `PathUri`. +/// Like [`ExecCommandBeginTracePayload`], this preserves the native-path `cwd` spelling. #[derive(Serialize)] struct ExecCommandEndTracePayload<'a> { call_id: &'a str, @@ -202,7 +201,7 @@ struct ExecCommandEndTracePayload<'a> { turn_id: &'a str, completed_at_ms: i64, command: &'a [String], - cwd: String, + cwd: &'a str, parsed_cmd: &'a [codex_protocol::parse_command::ParsedCommand], source: ExecCommandSource, #[serde(skip_serializing_if = "Option::is_none")] @@ -242,7 +241,7 @@ impl<'a> From<&'a ExecCommandEndEvent> for ExecCommandEndTracePayload<'a> { turn_id, completed_at_ms: *completed_at_ms, command, - cwd: cwd.inferred_native_path_string(), + cwd: cwd.as_str(), parsed_cmd, source: *source, interaction_input: interaction_input.as_deref(), diff --git a/codex-rs/rollout-trace/src/protocol_event_tests.rs b/codex-rs/rollout-trace/src/protocol_event_tests.rs index 200c35d94d..3c29a5a6a5 100644 --- a/codex-rs/rollout-trace/src/protocol_event_tests.rs +++ b/codex-rs/rollout-trace/src/protocol_event_tests.rs @@ -60,7 +60,7 @@ fn exec_command_trace_payloads_use_inferred_native_cwd() -> anyhow::Result<()> { turn_id: "turn-1".to_string(), started_at_ms: 1234, command: vec!["pwd".to_string()], - cwd: "file:///C:/windows".parse()?, + cwd: serde_json::from_value(json!(r"C:\windows"))?, parsed_cmd: Vec::new(), source: ExecCommandSource::Agent, interaction_input: None, @@ -71,7 +71,7 @@ fn exec_command_trace_payloads_use_inferred_native_cwd() -> anyhow::Result<()> { turn_id: "turn-1".to_string(), completed_at_ms: 2345, command: vec!["pwd".to_string()], - cwd: "file:///workspace/project".parse()?, + cwd: serde_json::from_value(json!("/workspace/project"))?, parsed_cmd: Vec::new(), source: ExecCommandSource::UnifiedExecInteraction, interaction_input: Some("input".to_string()), diff --git a/codex-rs/utils/path-uri/src/api_path_string.rs b/codex-rs/utils/path-uri/src/api_path_string.rs index e4fdbe7c27..3bd9e3948a 100644 --- a/codex-rs/utils/path-uri/src/api_path_string.rs +++ b/codex-rs/utils/path-uri/src/api_path_string.rs @@ -81,9 +81,11 @@ impl LegacyAppPathString { }) } - /// Parses this API string as an absolute path using the convention inferred from its spelling. + /// Parses this API string as a file URI or as an absolute path using its inferred convention. pub fn to_inferred_path_uri(&self) -> Option { - PathUri::try_from(self.clone()).ok() + PathUri::parse(&self.0) + .ok() + .or_else(|| PathUri::try_from(self.clone()).ok()) } /// Parses this API string as a host-native absolute path. diff --git a/codex-rs/utils/path-uri/src/api_path_string_tests.rs b/codex-rs/utils/path-uri/src/api_path_string_tests.rs index c71a65abf3..55484f0af3 100644 --- a/codex-rs/utils/path-uri/src/api_path_string_tests.rs +++ b/codex-rs/utils/path-uri/src/api_path_string_tests.rs @@ -465,6 +465,24 @@ fn converts_absolute_api_paths_using_the_inferred_convention() { } } +#[test] +fn converts_raw_file_uris_to_inferred_path_uris() { + for raw_uri in [ + "file:///workspace/file.rs", + "file:///C:/workspace/file.rs", + "file://server/share/file.rs", + ] { + let path = serde_json::from_value::(serde_json::json!(raw_uri)) + .expect("raw file URI should deserialize as API text"); + + assert_eq!( + path.to_inferred_path_uri(), + Some(PathUri::parse(raw_uri).expect("raw file URI should parse")), + "round-tripping {raw_uri:?}", + ); + } +} + #[test] fn converts_native_api_path_to_inferred_absolute_path() { #[cfg(windows)]