codex: address PR review feedback (#28463)

codex-rs/core/tests/suite/remote_env.rs

@@ -28,8 +28,6 @@ use codex_protocol::request_permissions::RequestPermissionProfile;
 use codex_protocol::request_permissions::RequestPermissionsResponse;
 use codex_protocol::user_input::UserInput;
 use codex_utils_absolute_path::AbsolutePathBuf;
-use codex_utils_path_uri::ApiPathString;
-use codex_utils_path_uri::PathConvention;
 use codex_utils_path_uri::PathUri;
 use core_test_support::PathBufExt;
 use core_test_support::PathExt;
@@ -1082,15 +1080,6 @@ async fn remote_test_env_sandboxed_read_rejects_symlink_parent_dotdot_escape() -
     let outside_dir_uri = PathUri::from_abs_path(&outside_dir);
     let secret_path_uri = PathUri::from_abs_path(&secret_path);
     let symlink_path_uri = allowed_dir_uri.join("link")?;
-    let windows_root = ApiPathString::from_path_uri(&root_uri, PathConvention::Windows)?;
-    let windows_allowed_dir =
-        ApiPathString::from_path_uri(&allowed_dir_uri, PathConvention::Windows)?;
-    let windows_outside_dir =
-        ApiPathString::from_path_uri(&outside_dir_uri, PathConvention::Windows)?;
-    let windows_secret_path =
-        ApiPathString::from_path_uri(&secret_path_uri, PathConvention::Windows)?;
-    let windows_symlink_path =
-        ApiPathString::from_path_uri(&symlink_path_uri, PathConvention::Windows)?;
     let linux_setup_command = format!(
         "rm -rf {root}; mkdir -p {allowed} {outside}; printf nope > {secret}; ln -s {outside} {allowed}/link",
         root = root.display(),
@@ -1110,11 +1099,11 @@ New-Item -ItemType Directory -Path $allowed -Force | Out-Null
 New-Item -ItemType Directory -Path $outside -Force | Out-Null
 [System.IO.File]::WriteAllText($secret, 'nope')
 New-Item -ItemType SymbolicLink -Path $link -Target $outside | Out-Null"#,
-        root = windows_root.as_str(),
-        allowed = windows_allowed_dir.as_str(),
-        outside = windows_outside_dir.as_str(),
-        secret = windows_secret_path.as_str(),
-        link = windows_symlink_path.as_str(),
+        root = root_uri.native_path_display(),
+        allowed = allowed_dir_uri.native_path_display(),
+        outside = outside_dir_uri.native_path_display(),
+        secret = secret_path_uri.native_path_display(),
+        link = symlink_path_uri.native_path_display(),
     );
     remote_exec(&linux_setup_command, &windows_setup_command)?;
 
@@ -1131,7 +1120,7 @@ New-Item -ItemType SymbolicLink -Path $link -Target $outside | Out-Null"#,
         r#"$ErrorActionPreference = 'Stop'
 $root = '{root}'
 Remove-Item -LiteralPath $root -Recurse -Force -ErrorAction SilentlyContinue"#,
-        root = windows_root.as_str(),
+        root = root_uri.native_path_display(),
     );
     remote_exec(&linux_cleanup_command, &windows_cleanup_command)?;
     Ok(())

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

@@ -164,6 +164,17 @@ impl PathUri {
         }
     }
 
+    /// Renders native path text using the convention represented by this URI.
+    ///
+    /// This uses [`Self::infer_path_convention`] rather than the current host's
+    /// convention, so a Windows URI renders with Windows separators on POSIX
+    /// and vice versa. An opaque fallback that does not encode a recognizable
+    /// absolute path, or a URI that the inferred convention cannot represent,
+    /// displays as its canonical URI instead.
+    pub fn native_path_display(&self) -> impl fmt::Display + '_ {
+        NativePathDisplay(self)
+    }
+
     /// Returns the decoded final URI path segment, or `None` for the URI root
     /// or an opaque fallback URI created by [`Self::from_abs_path`].
     ///
@@ -370,6 +381,20 @@ impl fmt::Display for PathUri {
     }
 }
 
+struct NativePathDisplay<'a>(&'a PathUri);
+
+impl fmt::Display for NativePathDisplay<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let path = self.0;
+        if let Some(convention) = path.infer_path_convention()
+            && let Ok(native_path) = ApiPathString::from_path_uri(path, convention)
+        {
+            return native_path.fmt(f);
+        }
+        path.fmt(f)
+    }
+}
+
 impl Serialize for PathUri {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where

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

@@ -94,6 +94,31 @@ fn drive_shaped_posix_uri_is_intentionally_inferred_as_windows() {
     assert_eq!(path.infer_path_convention(), Some(PathConvention::Windows));
 }
 
+#[test]
+fn displays_native_paths_using_the_inferred_convention() {
+    for (uri, expected) in [
+        ("file:///home/Alice%20Smith/src", "/home/Alice Smith/src"),
+        (
+            "file:///C:/Users/Alice%20Smith/src",
+            r"C:\Users\Alice Smith\src",
+        ),
+        ("file://server/share/src", r"\\server\share\src"),
+    ] {
+        let path = PathUri::parse(uri).expect("valid path URI");
+
+        assert_eq!(path.native_path_display().to_string(), expected);
+    }
+}
+
+#[test]
+fn native_path_display_falls_back_to_the_canonical_uri() {
+    for uri in ["file:///%00/bad/path/YQ", "file://server/"] {
+        let path = PathUri::parse(uri).expect("valid path URI");
+
+        assert_eq!(path.native_path_display().to_string(), path.to_string());
+    }
+}
+
 #[cfg(windows)]
 #[test]
 fn file_uri_falls_back_for_windows_prefixes_without_a_uri_representation() {