mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
unified-exec: retain PathUri in command events (#28780)
## Why App-server must report command events containing foreign-platform paths without changing existing client or rollout path-string formats. ## What changed - retain `PathUri` through exec command begin/end events - convert cwd values to `LegacyAppPathString` at the app-server compatibility boundary - drop command actions with foreign paths and log them - serialize rollout-trace cwd values using their inferred native path representation - restore Wine coverage for retained Windows cwd values and successful completion
This commit is contained in:
committed by
GitHub
Unverified
parent
285eff6c3e
commit
3931bc2bde
@@ -70,11 +70,21 @@ impl LegacyAppPathString {
|
||||
PathUri::from_absolute_native_path(&self.0, convention).ok_or_else(|| {
|
||||
LegacyAppPathStringError::InvalidNativePath {
|
||||
path: self.0.clone(),
|
||||
convention,
|
||||
convention: Some(convention),
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/// Parses this API string as an absolute path using the convention inferred from its spelling.
|
||||
pub fn to_inferred_path_uri(&self) -> Option<PathUri> {
|
||||
PathUri::try_from(self.clone()).ok()
|
||||
}
|
||||
|
||||
/// Parses this API string as a host-native absolute path.
|
||||
pub fn to_inferred_abs_path(&self) -> Option<AbsolutePathBuf> {
|
||||
AbsolutePathBuf::try_from(self.clone()).ok()
|
||||
}
|
||||
|
||||
/// Infers the path convention of an absolute API path from its spelling.
|
||||
///
|
||||
/// Relative paths and ambiguous spellings return `None`. In particular,
|
||||
@@ -111,6 +121,44 @@ impl From<AbsolutePathBuf> for LegacyAppPathString {
|
||||
}
|
||||
}
|
||||
|
||||
impl From<PathUri> for LegacyAppPathString {
|
||||
fn from(path: PathUri) -> Self {
|
||||
Self(path.inferred_native_path_string())
|
||||
}
|
||||
}
|
||||
|
||||
impl TryFrom<LegacyAppPathString> for PathUri {
|
||||
type Error = LegacyAppPathStringError;
|
||||
|
||||
fn try_from(path: LegacyAppPathString) -> Result<Self, Self::Error> {
|
||||
let Some(convention) = path.infer_absolute_path_convention() else {
|
||||
return Err(LegacyAppPathStringError::InvalidNativePath {
|
||||
path: path.0,
|
||||
convention: None,
|
||||
});
|
||||
};
|
||||
PathUri::from_absolute_native_path(path.as_str(), convention).ok_or({
|
||||
LegacyAppPathStringError::InvalidNativePath {
|
||||
path: path.0,
|
||||
convention: Some(convention),
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
impl TryFrom<LegacyAppPathString> for AbsolutePathBuf {
|
||||
type Error = LegacyAppPathStringError;
|
||||
|
||||
fn try_from(path: LegacyAppPathString) -> Result<Self, Self::Error> {
|
||||
AbsolutePathBuf::from_absolute_path_checked(path.as_str()).map_err(|_| {
|
||||
LegacyAppPathStringError::InvalidNativePath {
|
||||
path: path.0,
|
||||
convention: None,
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
fn render_opaque_fallback(
|
||||
path: &PathUri,
|
||||
path_bytes: &[u8],
|
||||
@@ -279,10 +327,13 @@ pub enum LegacyAppPathStringError {
|
||||
path: String,
|
||||
convention: PathConvention,
|
||||
},
|
||||
#[error("path `{path}` is not absolute using {convention} path syntax")]
|
||||
#[error(
|
||||
"path `{path}` is not absolute{convention}",
|
||||
convention = .convention.map(|convention| format!(" using {convention} path syntax")).unwrap_or_default()
|
||||
)]
|
||||
InvalidNativePath {
|
||||
path: String,
|
||||
convention: PathConvention,
|
||||
convention: Option<PathConvention>,
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
@@ -369,7 +369,21 @@ fn relative_api_path_is_invalid_when_converted_to_a_path_uri() {
|
||||
path.to_path_uri(PathConvention::Posix),
|
||||
Err(LegacyAppPathStringError::InvalidNativePath {
|
||||
path: raw_path.to_string(),
|
||||
convention: PathConvention::Posix,
|
||||
convention: Some(PathConvention::Posix),
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
PathUri::try_from(path.clone()),
|
||||
Err(LegacyAppPathStringError::InvalidNativePath {
|
||||
path: raw_path.to_string(),
|
||||
convention: None,
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
AbsolutePathBuf::try_from(path),
|
||||
Err(LegacyAppPathStringError::InvalidNativePath {
|
||||
path: raw_path.to_string(),
|
||||
convention: None,
|
||||
})
|
||||
);
|
||||
}
|
||||
@@ -388,7 +402,7 @@ fn other_non_absolute_api_paths_cannot_be_converted_to_path_uris() {
|
||||
path.to_path_uri(convention),
|
||||
Err(LegacyAppPathStringError::InvalidNativePath {
|
||||
path: raw_path.to_string(),
|
||||
convention,
|
||||
convention: Some(convention),
|
||||
})
|
||||
);
|
||||
}
|
||||
@@ -423,6 +437,51 @@ fn infers_absolute_path_conventions_from_api_text() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn converts_absolute_api_paths_using_the_inferred_convention() {
|
||||
for (raw_path, convention, expected_uri) in [
|
||||
(
|
||||
r"C:\workspace\file.rs",
|
||||
PathConvention::Windows,
|
||||
"file:///C:/workspace/file.rs",
|
||||
),
|
||||
(
|
||||
"/workspace/file.rs",
|
||||
PathConvention::Posix,
|
||||
"file:///workspace/file.rs",
|
||||
),
|
||||
] {
|
||||
let path = serde_json::from_value::<LegacyAppPathString>(serde_json::json!(raw_path))
|
||||
.expect("absolute API path should deserialize");
|
||||
|
||||
assert_eq!(
|
||||
path.to_inferred_path_uri(),
|
||||
Some(PathUri::parse(expected_uri).expect("expected URI should parse")),
|
||||
);
|
||||
assert_eq!(
|
||||
PathUri::try_from(path.clone()),
|
||||
path.to_path_uri(convention)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn converts_native_api_path_to_inferred_absolute_path() {
|
||||
#[cfg(windows)]
|
||||
let raw_path = r"C:\workspace\file.rs";
|
||||
#[cfg(not(windows))]
|
||||
let raw_path = "/workspace/file.rs";
|
||||
let path = serde_json::from_value::<LegacyAppPathString>(serde_json::json!(raw_path))
|
||||
.expect("absolute API path should deserialize");
|
||||
let expected = AbsolutePathBuf::try_from(raw_path).expect("native absolute path should parse");
|
||||
|
||||
assert_eq!(
|
||||
AbsolutePathBuf::try_from(path.clone()),
|
||||
Ok(expected.clone())
|
||||
);
|
||||
assert_eq!(path.to_inferred_abs_path(), Some(expected));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn foreign_absolute_syntax_deserializes_without_host_interpretation() {
|
||||
for (raw_path, convention) in [
|
||||
|
||||
@@ -119,6 +119,11 @@ fn inferred_native_path_string_uses_the_inferred_convention() {
|
||||
expected,
|
||||
"rendering {uri}"
|
||||
);
|
||||
assert_eq!(
|
||||
LegacyAppPathString::from(path).as_str(),
|
||||
expected,
|
||||
"rendering typed API path {uri}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user