mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Preserve foreign paths in background terminal listings (#35850)
## Why Background terminal working directories can use a path convention from a different platform than the app-server host. Converting them to host-native absolute paths can reject otherwise valid terminal entries and fail the list request. ## What changed - Represent `ThreadBackgroundTerminal.cwd` with `LegacyAppPathString` at the protocol boundary. - Convert terminal `PathUri` values directly to their inferred native path strings instead of validating them against the host platform. ## Testing Add protocol round-trip coverage for POSIX, Windows drive, and Windows UNC working directories. GitOrigin-RevId: d766d54bdc191e75955fb419bec6e60c65f52298
This commit is contained in:
1 parent
03edf16f0b
commit
101d6b8cb2
4 files changed
+57
-12
No files matched your search
@@ -72,6 +72,57 @@ fn test_absolute_path() -> AbsolutePathBuf {
|
|||||||
absolute_path("readable")
|
absolute_path("readable")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn thread_background_terminals_list_response_round_trips_foreign_paths() {
|
||||||
|
for (uri, expected_cwd) in [
|
||||||
|
("file:///home/alice/repo", "/home/alice/repo"),
|
||||||
|
(
|
||||||
|
"file:///C:/Users/Alice%20Smith/repo",
|
||||||
|
r"C:\Users\Alice Smith\repo",
|
||||||
|
),
|
||||||
|
("file://server/share/repo", r"\\server\share\repo"),
|
||||||
|
] {
|
||||||
|
let response = ThreadBackgroundTerminalsListResponse {
|
||||||
|
data: vec![ThreadBackgroundTerminal {
|
||||||
|
item_id: "item_123".to_string(),
|
||||||
|
process_id: "42".to_string(),
|
||||||
|
command: "run server".to_string(),
|
||||||
|
cwd: PathUri::parse(uri)
|
||||||
|
.expect("cross-platform path URI should parse")
|
||||||
|
.into(),
|
||||||
|
os_pid: None,
|
||||||
|
cpu_percent: None,
|
||||||
|
rss_kb: None,
|
||||||
|
}],
|
||||||
|
next_cursor: None,
|
||||||
|
};
|
||||||
|
let expected = json!({
|
||||||
|
"data": [{
|
||||||
|
"itemId": "item_123",
|
||||||
|
"processId": "42",
|
||||||
|
"command": "run server",
|
||||||
|
"cwd": expected_cwd,
|
||||||
|
"osPid": null,
|
||||||
|
"cpuPercent": null,
|
||||||
|
"rssKb": null,
|
||||||
|
}],
|
||||||
|
"nextCursor": null,
|
||||||
|
});
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
serde_json::to_value(&response).expect("response should serialize"),
|
||||||
|
expected,
|
||||||
|
"serializing {uri}",
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
serde_json::from_value::<ThreadBackgroundTerminalsListResponse>(expected)
|
||||||
|
.expect("response should deserialize"),
|
||||||
|
response,
|
||||||
|
"deserializing {uri}",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn thread_sources_round_trip_as_scalar_labels() {
|
fn thread_sources_round_trip_as_scalar_labels() {
|
||||||
for (source, label) in [
|
for (source, label) in [
|
||||||
|
|||||||
@@ -1044,7 +1044,7 @@ pub struct ThreadBackgroundTerminal {
|
|||||||
pub item_id: String,
|
pub item_id: String,
|
||||||
pub process_id: String,
|
pub process_id: String,
|
||||||
pub command: String,
|
pub command: String,
|
||||||
pub cwd: AbsolutePathBuf,
|
pub cwd: LegacyAppPathString,
|
||||||
pub os_pid: Option<u32>,
|
pub os_pid: Option<u32>,
|
||||||
pub cpu_percent: Option<f64>,
|
pub cpu_percent: Option<f64>,
|
||||||
pub rss_kb: Option<u64>,
|
pub rss_kb: Option<u64>,
|
||||||
|
|||||||
@@ -1909,22 +1909,16 @@ impl ThreadRequestProcessor {
|
|||||||
.list_background_terminals()
|
.list_background_terminals()
|
||||||
.await
|
.await
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|terminal| {
|
.map(|terminal| ThreadBackgroundTerminal {
|
||||||
// TODO(anp): Migrate ThreadBackgroundTerminal to PathUri.
|
|
||||||
let cwd = terminal.cwd.to_abs_path().map_err(|err| {
|
|
||||||
internal_error(format!("background terminal has invalid cwd: {err}"))
|
|
||||||
})?;
|
|
||||||
Ok(ThreadBackgroundTerminal {
|
|
||||||
item_id: terminal.item_id,
|
item_id: terminal.item_id,
|
||||||
process_id: terminal.process_id,
|
process_id: terminal.process_id,
|
||||||
command: terminal.command,
|
command: terminal.command,
|
||||||
cwd,
|
cwd: terminal.cwd.into(),
|
||||||
os_pid: None,
|
os_pid: None,
|
||||||
cpu_percent: None,
|
cpu_percent: None,
|
||||||
rss_kb: None,
|
rss_kb: None,
|
||||||
})
|
})
|
||||||
})
|
.collect::<Vec<_>>();
|
||||||
.collect::<Result<Vec<_>, JSONRPCErrorError>>()?;
|
|
||||||
|
|
||||||
let (data, next_cursor) = paginate_background_terminals(&terminals, cursor, limit)?;
|
let (data, next_cursor) = paginate_background_terminals(&terminals, cursor, limit)?;
|
||||||
|
|
||||||
|
|||||||
@@ -39,7 +39,7 @@ mod thread_list_cwd_filter_tests {
|
|||||||
mod background_terminal_pagination_tests {
|
mod background_terminal_pagination_tests {
|
||||||
use super::super::paginate_background_terminals;
|
use super::super::paginate_background_terminals;
|
||||||
use codex_app_server_protocol::ThreadBackgroundTerminal;
|
use codex_app_server_protocol::ThreadBackgroundTerminal;
|
||||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
use codex_utils_path_uri::LegacyAppPathString;
|
||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
|
|
||||||
fn terminal(process_id: &str) -> ThreadBackgroundTerminal {
|
fn terminal(process_id: &str) -> ThreadBackgroundTerminal {
|
||||||
@@ -49,7 +49,7 @@ mod background_terminal_pagination_tests {
|
|||||||
item_id: format!("item-{process_id}"),
|
item_id: format!("item-{process_id}"),
|
||||||
process_id: process_id.to_string(),
|
process_id: process_id.to_string(),
|
||||||
command: format!("command-{process_id}"),
|
command: format!("command-{process_id}"),
|
||||||
cwd: AbsolutePathBuf::from_absolute_path(cwd).expect("absolute cwd"),
|
cwd: LegacyAppPathString::from_string(cwd),
|
||||||
os_pid: None,
|
os_pid: None,
|
||||||
cpu_percent: None,
|
cpu_percent: None,
|
||||||
rss_kb: None,
|
rss_kb: None,
|
||||||
|
|||||||
Reference in new issue
Block a user