codex: address PR review feedback (#27819)

codex-rs/exec-server/src/protocol.rs

@@ -79,8 +79,8 @@ pub struct EnvironmentInfo {
 impl EnvironmentInfo {
     /// Renders a path using this environment's native path syntax.
     ///
-    /// TODO(anp): Once `PathUri` carries an environment identifier, resolve the
-    /// environment from that identifier and verify it matches this metadata.
+    /// TODO(anp): Once `PathUri` carries an environment identifier, invert this
+    /// relationship and use the URI to resolve the environment information.
     pub fn render_path(&self, path: &PathUri) -> Result<NativePathString, NativePathStringError> {
         NativePathString::from_path_uri(path, self.path_convention)
     }

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

@@ -50,24 +50,21 @@ impl fmt::Display for PathConvention {
 /// "Native" refers to the supplied [`PathConvention`], which may be foreign to
 /// the operating system running this process. The inner string is private so
 /// path-producing code must render through [`Self::from_path_uri`] rather than
-/// accidentally applying the current host's path rules.
+/// accidentally applying the current host's path rules. Opaque fallback paths
+/// recoverable on the current host are converted to UTF-8 lossily at this API
+/// boundary because the value is serialized as a JSON string.
 #[derive(Clone, Debug, PartialEq, Eq, Hash, TS)]
 #[ts(type = "string")]
 pub struct NativePathString(String);
 
 impl NativePathString {
     /// Renders a path URI using the requested native path convention.
-    ///
-    /// TODO(anp): Once `PathUri` carries an environment identifier, resolve the path
-    /// convention from that identifier instead of requiring it explicitly.
     pub fn from_path_uri(
         path: &PathUri,
         convention: PathConvention,
     ) -> Result<Self, NativePathStringError> {
         if path.is_opaque_fallback() {
-            return Err(NativePathStringError::OpaqueFallback {
-                path: path.to_string(),
-            });
+            return render_opaque_fallback(path, convention).map(Self);
         }
         let value = match convention {
             PathConvention::Posix => render_posix_path(path)?,
@@ -85,6 +82,20 @@ impl NativePathString {
     }
 }
 
+fn render_opaque_fallback(
+    path: &PathUri,
+    convention: PathConvention,
+) -> Result<String, NativePathStringError> {
+    if convention != PathConvention::native() {
+        return Err(incompatible_convention(path, convention));
+    }
+    path.to_abs_path()
+        .map(|path| path.as_path().to_string_lossy().into_owned())
+        .map_err(|_| NativePathStringError::OpaqueFallback {
+            path: path.to_string(),
+        })
+}
+
 impl fmt::Display for NativePathString {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.write_str(&self.0)
@@ -222,7 +233,7 @@ fn incompatible_convention(path: &PathUri, convention: PathConvention) -> Native
 
 #[derive(Debug, Error, PartialEq, Eq)]
 pub enum NativePathStringError {
-    #[error("opaque fallback path URI `{path}` cannot be rendered as a UTF-8 native path")]
+    #[error("opaque fallback path URI `{path}` cannot be recovered as a native path")]
     OpaqueFallback { path: String },
     #[error("path URI `{path}` cannot be rendered using {convention} path syntax")]
     IncompatibleConvention {

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

@@ -100,17 +100,66 @@ fn rejects_paths_incompatible_with_the_convention() {
 }
 
 #[test]
-fn rejects_opaque_fallback_paths() {
+fn rejects_opaque_fallback_paths_that_cannot_be_recovered() {
     let path = PathUri::parse("file:///%00/bad/path/YQ").expect("canonical opaque fallback URI");
 
-    for convention in [PathConvention::Posix, PathConvention::Windows] {
-        assert_eq!(
-            NativePathString::from_path_uri(&path, convention),
-            Err(NativePathStringError::OpaqueFallback {
-                path: path.to_string(),
-            })
-        );
-    }
+    assert_eq!(
+        NativePathString::from_path_uri(&path, PathConvention::native()),
+        Err(NativePathStringError::OpaqueFallback {
+            path: path.to_string(),
+        })
+    );
+}
+
+#[cfg(unix)]
+#[test]
+fn renders_native_opaque_fallback_paths_lossily() {
+    use std::os::unix::ffi::OsStringExt;
+
+    let native_path = std::path::PathBuf::from(std::ffi::OsString::from_vec(
+        b"/tmp/null-\0-non-utf8-\xff".to_vec(),
+    ));
+    let path = PathUri::from_path(native_path).expect("absolute native path");
+
+    assert_eq!(
+        NativePathString::from_path_uri(&path, PathConvention::Posix)
+            .map(NativePathString::into_string),
+        Ok("/tmp/null-\0-non-utf8-<2D>".to_string())
+    );
+    assert_eq!(
+        NativePathString::from_path_uri(&path, PathConvention::Windows),
+        Err(NativePathStringError::IncompatibleConvention {
+            path: path.to_string(),
+            convention: PathConvention::Windows,
+        })
+    );
+}
+
+#[cfg(windows)]
+#[test]
+fn renders_native_opaque_fallback_paths_lossily() {
+    use std::os::windows::ffi::OsStringExt;
+
+    let native_path = std::path::PathBuf::from(std::ffi::OsString::from_wide(
+        &r"C:\bad\"
+            .encode_utf16()
+            .chain([0xd800])
+            .collect::<Vec<_>>(),
+    ));
+    let path = PathUri::from_path(native_path).expect("absolute native path");
+
+    assert_eq!(
+        NativePathString::from_path_uri(&path, PathConvention::Windows)
+            .map(NativePathString::into_string),
+        Ok(r"C:\bad\<5C>".to_string())
+    );
+    assert_eq!(
+        NativePathString::from_path_uri(&path, PathConvention::Posix),
+        Err(NativePathStringError::IncompatibleConvention {
+            path: path.to_string(),
+            convention: PathConvention::Posix,
+        })
+    );
 }
 
 #[test]