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 66019980a3c8..875edffd9cdd 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,19 @@ 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); + let cwd = payload + .cwd + .as_str() + .get(..7) + .filter(|scheme| scheme.eq_ignore_ascii_case("file://")) + .and_then(|_| payload.cwd.to_inferred_path_uri()) + .map(LegacyAppPathString::from) + .unwrap_or_else(|| payload.cwd.clone()); ThreadItem::CommandExecution { id: payload.call_id.clone(), command: shlex_join(&payload.command), - cwd: payload.cwd.clone().into(), + cwd, process_id: payload.process_id.clone(), source: payload.source.into(), status: CommandExecutionStatus::InProgress, @@ -112,12 +120,20 @@ 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); + let cwd = payload + .cwd + .as_str() + .get(..7) + .filter(|scheme| scheme.eq_ignore_ascii_case("file://")) + .and_then(|_| payload.cwd.to_inferred_path_uri()) + .map(LegacyAppPathString::from) + .unwrap_or_else(|| payload.cwd.clone()); ThreadItem::CommandExecution { id: payload.call_id.clone(), command: shlex_join(&payload.command), - cwd: payload.cwd.clone().into(), + cwd, process_id: payload.process_id.clone(), source: payload.source.into(), status: (&payload.status).into(), @@ -128,14 +144,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 b892fcf505db..d6f8c7f6a7ae 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,6 @@ use super::*; +use codex_protocol::protocol::ExecCommandSource; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; #[test] @@ -25,7 +27,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 +41,54 @@ 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 cwd_uri = PathUri::parse(raw_uri).expect("raw file URI should parse"); + let expected_path = cwd_uri + .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"), + }]; + let payload = ExecCommandBeginEvent { + call_id: "call-1".to_string(), + process_id: None, + turn_id: "turn-1".to_string(), + started_at_ms: 0, + command: vec!["cat".to_string(), "file.txt".to_string()], + cwd, + parsed_cmd, + source: ExecCommandSource::Agent, + interaction_input: None, + }; + + assert_eq!( + build_command_execution_begin_item(&payload), + ThreadItem::CommandExecution { + id: "call-1".to_string(), + command: "cat file.txt".to_string(), + cwd: LegacyAppPathString::from(cwd_uri), + process_id: None, + source: CommandExecutionSource::Agent, + status: CommandExecutionStatus::InProgress, + command_actions: vec![CommandAction::Read { + command: "cat file.txt".to_string(), + name: "file.txt".to_string(), + path: expected_path, + }], + aggregated_output: None, + exit_code: None, + duration_ms: None, + } + ); +} diff --git a/codex-rs/core/src/tools/events.rs b/codex-rs/core/src/tools/events.rs index 757125a03d6d..42182da3690f 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 04f8914982b1..7de2e30152fc 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 fe6ce7fbb7a1..196d8333ad77 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 5a73ac4c90a0..132e263ec7cd 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 42e0db04d2bb..963b17c596d6 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, @@ -182,7 +182,14 @@ 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() + .get(..7) + .filter(|scheme| scheme.eq_ignore_ascii_case("file://")) + .and_then(|_| cwd.to_inferred_path_uri()) + .map(Into::into) + .unwrap_or_else(|| cwd.clone()) + .into_string(), parsed_cmd, source: *source, interaction_input: interaction_input.as_deref(), @@ -192,8 +199,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, @@ -242,7 +248,14 @@ 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() + .get(..7) + .filter(|scheme| scheme.eq_ignore_ascii_case("file://")) + .and_then(|_| cwd.to_inferred_path_uri()) + .map(Into::into) + .unwrap_or_else(|| cwd.clone()) + .into_string(), 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 200c35d94d7a..cfae00f00c8d 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!("file:///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!("file:///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 e4fdbe7c275e..0d3ad43d7d2e 100644 --- a/codex-rs/utils/path-uri/src/api_path_string.rs +++ b/codex-rs/utils/path-uri/src/api_path_string.rs @@ -81,7 +81,13 @@ impl LegacyAppPathString { }) } - /// Parses this API string as an absolute path using the convention inferred from its spelling. + fn has_serialized_file_uri_prefix(&self) -> bool { + self.0 + .get(..7) + .is_some_and(|scheme| scheme.eq_ignore_ascii_case("file://")) + } + + /// 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() } @@ -137,6 +143,18 @@ impl TryFrom for PathUri { type Error = LegacyAppPathStringError; fn try_from(path: LegacyAppPathString) -> Result { + // Check the original spelling before parsing because URL parsing + // canonicalizes relative aliases such as `file:repo`, losing the + // distinction from serialized absolute file URI spellings. + if path.has_serialized_file_uri_prefix() { + return PathUri::parse(path.as_str()).map_err(|_| { + LegacyAppPathStringError::InvalidNativePath { + path: path.0, + convention: None, + } + }); + } + let Some(convention) = path.infer_absolute_path_convention() else { return Err(LegacyAppPathStringError::InvalidNativePath { path: path.0, 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 c71a65abf3df..d80a4db1fea4 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,40 @@ fn converts_absolute_api_paths_using_the_inferred_convention() { } } +#[test] +fn converts_raw_file_uris_to_inferred_path_uris() { + for (raw_uri, expected_path) in [ + ("file:///workspace/file.rs", "/workspace/file.rs"), + ("file:///C:/workspace/file.rs", r"C:\workspace\file.rs"), + ("file://server/share/file.rs", r"\\server\share\file.rs"), + ] { + let path = serde_json::from_value::(serde_json::json!(raw_uri)) + .expect("raw file URI should deserialize as API text"); + let expected_path = + serde_json::from_value::(serde_json::json!(expected_path)) + .expect("expected native path should deserialize as API text"); + let expected_uri = PathUri::parse(raw_uri).expect("raw file URI should parse"); + + assert_eq!( + path.to_inferred_path_uri(), + Some(expected_uri.clone()), + "round-tripping {raw_uri:?}", + ); + assert_eq!(PathUri::try_from(path), Ok(expected_uri.clone())); + assert_eq!(LegacyAppPathString::from(expected_uri), expected_path); + } +} + +#[test] +fn inferred_path_uri_rejects_relative_file_uri_aliases() { + for raw_uri in ["file:repo", "file:../repo"] { + 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(), None, "parsing {raw_uri:?}"); + } +} + #[test] fn converts_native_api_path_to_inferred_absolute_path() { #[cfg(windows)]