codex: address PR review feedback (#29472)

codex-rs/app-server-protocol/src/protocol/item_builders.rs

@@ -94,7 +94,7 @@ pub fn build_command_execution_begin_item(payload: &ExecCommandBeginEvent) -> Th
     ThreadItem::CommandExecution {
         id: payload.call_id.clone(),
         command: shlex_join(&payload.command),
-        cwd: normalized_app_server_cwd(&payload.cwd),
+        cwd: payload.cwd.normalize_file_uri(),
         process_id: payload.process_id.clone(),
         source: payload.source.into(),
         status: CommandExecutionStatus::InProgress,
@@ -117,7 +117,7 @@ pub fn build_command_execution_end_item(payload: &ExecCommandEndEvent) -> Thread
     ThreadItem::CommandExecution {
         id: payload.call_id.clone(),
         command: shlex_join(&payload.command),
-        cwd: normalized_app_server_cwd(&payload.cwd),
+        cwd: payload.cwd.normalize_file_uri(),
         process_id: payload.process_id.clone(),
         source: payload.source.into(),
         status: (&payload.status).into(),
@@ -128,12 +128,6 @@ pub fn build_command_execution_end_item(payload: &ExecCommandEndEvent) -> Thread
     }
 }
 
-fn normalized_app_server_cwd(cwd: &LegacyAppPathString) -> LegacyAppPathString {
-    cwd.to_file_uri()
-        .map(LegacyAppPathString::from)
-        .unwrap_or_else(|| cwd.clone())
-}
-
 fn command_actions_for_legacy_cwd(
     parsed_cmd: &[ParsedCommand],
     cwd: &LegacyAppPathString,

codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs

@@ -60,10 +60,7 @@ fn raw_file_uri_cwd_is_converted_for_command_actions() {
         path: PathBuf::from("file.txt"),
     }];
 
-    assert_eq!(
-        normalized_app_server_cwd(&cwd),
-        LegacyAppPathString::from(cwd_uri),
-    );
+    assert_eq!(cwd.normalize_file_uri(), LegacyAppPathString::from(cwd_uri));
     assert_eq!(
         command_actions_for_legacy_cwd(&parsed_cmd, &cwd),
         vec![CommandAction::Read {

codex-rs/rollout-trace/src/protocol_event.rs

@@ -182,10 +182,7 @@ impl<'a> From<&'a ExecCommandBeginEvent> for ExecCommandBeginTracePayload<'a> {
             turn_id,
             started_at_ms: *started_at_ms,
             command,
-            cwd: cwd
-                .to_file_uri()
-                .map(|cwd| cwd.inferred_native_path_string())
-                .unwrap_or_else(|| cwd.as_str().to_string()),
+            cwd: cwd.normalize_file_uri().into_string(),
             parsed_cmd,
             source: *source,
             interaction_input: interaction_input.as_deref(),
@@ -244,10 +241,7 @@ impl<'a> From<&'a ExecCommandEndEvent> for ExecCommandEndTracePayload<'a> {
             turn_id,
             completed_at_ms: *completed_at_ms,
             command,
-            cwd: cwd
-                .to_file_uri()
-                .map(|cwd| cwd.inferred_native_path_string())
-                .unwrap_or_else(|| cwd.as_str().to_string()),
+            cwd: cwd.normalize_file_uri().into_string(),
             parsed_cmd,
             source: *source,
             interaction_input: interaction_input.as_deref(),

codex-rs/utils/path-uri/src/api_path_string.rs

@@ -83,6 +83,9 @@ impl LegacyAppPathString {
 
     /// Parses this API string as a serialized file URI.
     pub fn to_file_uri(&self) -> Option<PathUri> {
+        // 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.
         let scheme = self.0.get(..7)?;
         if !scheme.eq_ignore_ascii_case("file://") {
             return None;
@@ -90,6 +93,15 @@ impl LegacyAppPathString {
         PathUri::parse(&self.0).ok()
     }
 
+    /// Converts a serialized file URI spelling to its inferred native path spelling.
+    ///
+    /// Non-URI path strings are returned unchanged.
+    pub fn normalize_file_uri(&self) -> Self {
+        self.to_file_uri()
+            .map(Self::from)
+            .unwrap_or_else(|| self.clone())
+    }
+
     /// 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> {
         self.to_file_uri()

codex-rs/utils/path-uri/src/api_path_string_tests.rs

@@ -467,13 +467,16 @@ 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",
+    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::<LegacyAppPathString>(serde_json::json!(raw_uri))
             .expect("raw file URI should deserialize as API text");
+        let expected_path =
+            serde_json::from_value::<LegacyAppPathString>(serde_json::json!(expected_path))
+                .expect("expected native path should deserialize as API text");
 
         assert_eq!(
             path.to_inferred_path_uri(),
@@ -481,6 +484,7 @@ fn converts_raw_file_uris_to_inferred_path_uris() {
             "round-tripping {raw_uri:?}",
         );
         assert_eq!(path.to_file_uri(), path.to_inferred_path_uri());
+        assert_eq!(path.normalize_file_uri(), expected_path);
     }
 }