From 1065aa5205160b078ada58f8963d87a953901afc Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 14 Jun 2026 02:32:57 +0000 Subject: [PATCH 1/4] tests: require native remote environment context --- .../remote_env_windows_test.rs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs b/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs index 208dfa554d6..7a8693edbea 100644 --- a/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs +++ b/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs @@ -38,6 +38,7 @@ use core_test_support::wait_for_event; use codex_utils_path_uri::LegacyAppPathString; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; +use serde_json::Value; use serde_json::json; use std::collections::BTreeMap; use tempfile::TempDir; @@ -246,6 +247,42 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul ) .await??; + let requests = server + .received_requests() + .await + .context("failed to fetch received requests")?; + let first_request = requests + .iter() + .find(|request| request.url.path().ends_with("/responses")) + .context("turn should send a Responses request")?; + let body = first_request.body_json::()?; + let environment_context = body["input"] + .as_array() + .into_iter() + .flatten() + .filter(|item| item.get("role").and_then(Value::as_str) == Some("user")) + .filter_map(|item| item.get("content").and_then(Value::as_array)) + .flatten() + .filter_map(|content| content.get("text").and_then(Value::as_str)) + .find(|text| text.starts_with("")) + .context("environment context should be model visible")?; + // The model should see the remote environment's shell, not the Linux app-server's + // host shell. + assert_eq!( + environment_context + .lines() + .find(|line| line.trim_start().starts_with("")), + Some(" powershell"), + ); + // The model should see cwd using the remote environment's native path convention, not + // the Linux app-server's host path convention. + assert_eq!( + environment_context + .lines() + .find(|line| line.trim_start().starts_with("")), + Some(r" C:\windows"), + ); + Ok(()) }) .await From 0062995e82f228fc43de792f3e800c1c61af0823 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 14 Jun 2026 03:07:21 +0000 Subject: [PATCH 2/4] core: render remote environment cwd natively --- .../core/src/context/environment_context.rs | 64 ++++++------ .../src/context/environment_context_tests.rs | 98 ++++++++++++++++--- .../remote_env_windows_test.rs | 17 +++- 3 files changed, 133 insertions(+), 46 deletions(-) diff --git a/codex-rs/core/src/context/environment_context.rs b/codex-rs/core/src/context/environment_context.rs index f0cc9f80bc6..46b396f14f8 100644 --- a/codex-rs/core/src/context/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -10,6 +10,8 @@ use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::TurnContextItem; use codex_protocol::protocol::TurnContextNetworkItem; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::ApiPathString; +use codex_utils_path_uri::PathUri; use std::collections::HashSet; use std::path::PathBuf; @@ -28,12 +30,12 @@ pub(crate) struct EnvironmentContext { #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct EnvironmentContextEnvironment { pub(crate) id: String, - pub(crate) cwd: AbsolutePathBuf, + pub(crate) cwd: PathUri, pub(crate) shell: String, } impl EnvironmentContextEnvironment { - fn legacy(cwd: AbsolutePathBuf, shell: String) -> Self { + fn legacy(cwd: PathUri, shell: String) -> Self { Self { id: String::new(), cwd, @@ -44,18 +46,14 @@ impl EnvironmentContextEnvironment { fn from_turn_environments(environments: &[TurnEnvironment], shell: &Shell) -> Vec { environments .iter() - .filter_map(|environment| { - // TODO(anp): Migrate EnvironmentContextEnvironment to PathUri so foreign - // environments remain visible in model context. - Some(Self { - id: environment.environment_id.clone(), - cwd: environment.cwd().to_abs_path().ok()?, - shell: environment - .shell - .as_ref() - .map(|shell| shell.name().to_string()) - .unwrap_or_else(|| shell.name().to_string()), - }) + .map(|environment| Self { + id: environment.environment_id.clone(), + cwd: environment.cwd().clone(), + shell: environment + .shell + .as_ref() + .map(|shell| shell.name().to_string()) + .unwrap_or_else(|| shell.name().to_string()), }) .collect() } @@ -388,7 +386,7 @@ impl EnvironmentContext { let before_filesystem = Self::filesystem_from_turn_context_item(before); let environments = match &after.environments { EnvironmentContextEnvironments::Single(environment) => { - if before.cwd.as_path() != environment.cwd.as_path() { + if turn_context_item_cwd_uri(before) != environment.cwd { EnvironmentContextEnvironments::Single(EnvironmentContextEnvironment::legacy( environment.cwd.clone(), environment.shell.clone(), @@ -444,13 +442,10 @@ impl EnvironmentContext { turn_context_item: &TurnContextItem, shell: String, ) -> Self { - let cwd = match AbsolutePathBuf::try_from(turn_context_item.cwd.clone()) { - Ok(cwd) => cwd, - Err(_) => AbsolutePathBuf::resolve_path_against_base(&turn_context_item.cwd, "/"), - }; Self::new_with_environments( EnvironmentContextEnvironments::from_vec(vec![EnvironmentContextEnvironment::legacy( - cwd, shell, + turn_context_item_cwd_uri(turn_context_item), + shell, )]), turn_context_item.current_date.clone(), turn_context_item.timezone.clone(), @@ -547,20 +542,16 @@ impl ContextualUserFragment for EnvironmentContext { let mut lines = Vec::new(); match &self.environments { EnvironmentContextEnvironments::Single(environment) => { - lines.push(format!( - " {}", - environment.cwd.to_string_lossy() - )); + let cwd = render_environment_cwd(&environment.cwd); + lines.push(format!(" {cwd}")); lines.push(format!(" {}", environment.shell)); } EnvironmentContextEnvironments::Multiple(environments) => { lines.push(" ".to_string()); for environment in environments { lines.push(format!(" ", environment.id)); - lines.push(format!( - " {}", - environment.cwd.to_string_lossy() - )); + let cwd = render_environment_cwd(&environment.cwd); + lines.push(format!(" {cwd}")); lines.push(format!(" {}", environment.shell)); lines.push(" ".to_string()); } @@ -595,6 +586,23 @@ impl ContextualUserFragment for EnvironmentContext { } } +fn turn_context_item_cwd_uri(turn_context_item: &TurnContextItem) -> PathUri { + let cwd = match AbsolutePathBuf::try_from(turn_context_item.cwd.clone()) { + Ok(cwd) => cwd, + Err(_) => AbsolutePathBuf::resolve_path_against_base(&turn_context_item.cwd, "/"), + }; + PathUri::from_abs_path(&cwd) +} + +fn render_environment_cwd(cwd: &PathUri) -> String { + let Some(convention) = cwd.infer_path_convention() else { + return cwd.to_string(); + }; + ApiPathString::from_path_uri(cwd, convention) + .map(ApiPathString::into_string) + .unwrap_or_else(|_| cwd.to_string()) +} + #[cfg(test)] #[path = "environment_context_tests.rs"] mod tests; diff --git a/codex-rs/core/src/context/environment_context_tests.rs b/codex-rs/core/src/context/environment_context_tests.rs index 5f391ca387f..b4f86fb034f 100644 --- a/codex-rs/core/src/context/environment_context_tests.rs +++ b/codex-rs/core/src/context/environment_context_tests.rs @@ -36,7 +36,7 @@ fn serialize_workspace_write_environment_context() { let context = EnvironmentContext::new( vec![EnvironmentContextEnvironment { id: "local".to_string(), - cwd: cwd.abs(), + cwd: PathUri::from_abs_path(&cwd.abs()), shell: fake_shell_name(), }], Some("2026-02-26".to_string()), @@ -58,6 +58,29 @@ fn serialize_workspace_write_environment_context() { assert_eq!(context.render(), expected); } +#[test] +fn serialize_environment_context_with_foreign_windows_cwd() { + let context = EnvironmentContext::new( + vec![EnvironmentContextEnvironment { + id: "remote".to_string(), + cwd: PathUri::parse("file:///C:/windows").expect("Windows cwd URI"), + shell: "powershell".to_string(), + }], + /*current_date*/ None, + /*timezone*/ None, + /*network*/ None, + /*subagents*/ None, + ); + + assert_eq!( + context.render(), + r#" + C:\windows + powershell +"# + ); +} + #[test] fn serialize_environment_context_with_network() { let network = NetworkContext::new( @@ -67,7 +90,7 @@ fn serialize_environment_context_with_network() { let context = EnvironmentContext::new( vec![EnvironmentContextEnvironment { id: "local".to_string(), - cwd: test_path_buf("/repo").abs(), + cwd: PathUri::from_abs_path(&test_abs_path("/repo")), shell: fake_shell_name(), }], Some("2026-02-26".to_string()), @@ -129,7 +152,7 @@ fn serialize_environment_context_with_full_filesystem_profile() { let mut context = EnvironmentContext::new( vec![EnvironmentContextEnvironment { id: "local".to_string(), - cwd: test_path_buf("/repo").abs(), + cwd: PathUri::from_abs_path(&test_abs_path("/repo")), shell: fake_shell_name(), }], /*current_date*/ None, @@ -211,6 +234,53 @@ fn turn_context_item_filesystem_uses_workspace_roots_instead_of_cwd() { ); } +#[cfg(unix)] +#[test] +fn legacy_projected_windows_cwd_matches_environment_uri() { + let item = TurnContextItem { + turn_id: None, + cwd: PathBuf::from("/C:/windows"), + workspace_roots: None, + current_date: None, + timezone: None, + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + permission_profile: Some(PermissionProfile::Disabled), + network: None, + file_system_sandbox_policy: None, + model: "gpt-5".to_string(), + comp_hash: None, + personality: None, + collaboration_mode: None, + multi_agent_version: None, + realtime_active: None, + effort: None, + summary: codex_protocol::config_types::ReasoningSummary::Auto, + }; + let current = EnvironmentContext::new( + vec![EnvironmentContextEnvironment { + id: "remote".to_string(), + cwd: PathUri::parse("file:///C:/windows").expect("Windows cwd URI"), + shell: "powershell".to_string(), + }], + /*current_date*/ None, + /*timezone*/ None, + /*network*/ None, + /*subagents*/ None, + ); + + assert_eq!( + EnvironmentContext::diff_from_turn_context_item(&item, ¤t), + EnvironmentContext::new( + Vec::new(), + /*current_date*/ None, + /*timezone*/ None, + /*network*/ None, + /*subagents*/ None, + ) + ); +} + #[test] fn serialize_read_only_environment_context() { let context = EnvironmentContext::new( @@ -234,7 +304,7 @@ fn equals_except_shell_compares_cwd() { let context1 = EnvironmentContext::new( vec![EnvironmentContextEnvironment { id: "local".to_string(), - cwd: test_abs_path("/repo"), + cwd: PathUri::from_abs_path(&test_abs_path("/repo")), shell: fake_shell_name(), }], /*current_date*/ None, @@ -245,7 +315,7 @@ fn equals_except_shell_compares_cwd() { let context2 = EnvironmentContext::new( vec![EnvironmentContextEnvironment { id: "local".to_string(), - cwd: test_abs_path("/repo"), + cwd: PathUri::from_abs_path(&test_abs_path("/repo")), shell: fake_shell_name(), }], /*current_date*/ None, @@ -261,7 +331,7 @@ fn equals_except_shell_compares_cwd_differences() { let context1 = EnvironmentContext::new( vec![EnvironmentContextEnvironment { id: "local".to_string(), - cwd: test_abs_path("/repo1"), + cwd: PathUri::from_abs_path(&test_abs_path("/repo1")), shell: fake_shell_name(), }], /*current_date*/ None, @@ -272,7 +342,7 @@ fn equals_except_shell_compares_cwd_differences() { let context2 = EnvironmentContext::new( vec![EnvironmentContextEnvironment { id: "local".to_string(), - cwd: test_abs_path("/repo2"), + cwd: PathUri::from_abs_path(&test_abs_path("/repo2")), shell: fake_shell_name(), }], /*current_date*/ None, @@ -289,7 +359,7 @@ fn equals_except_shell_ignores_shell() { let context1 = EnvironmentContext::new( vec![EnvironmentContextEnvironment { id: "local".to_string(), - cwd: test_abs_path("/repo"), + cwd: PathUri::from_abs_path(&test_abs_path("/repo")), shell: "bash".to_string(), }], /*current_date*/ None, @@ -300,7 +370,7 @@ fn equals_except_shell_ignores_shell() { let context2 = EnvironmentContext::new( vec![EnvironmentContextEnvironment { id: "other".to_string(), - cwd: test_abs_path("/repo"), + cwd: PathUri::from_abs_path(&test_abs_path("/repo")), shell: "zsh".to_string(), }], /*current_date*/ None, @@ -317,7 +387,7 @@ fn serialize_environment_context_with_subagents() { let context = EnvironmentContext::new( vec![EnvironmentContextEnvironment { id: "local".to_string(), - cwd: test_path_buf("/repo").abs(), + cwd: PathUri::from_abs_path(&test_abs_path("/repo")), shell: fake_shell_name(), }], Some("2026-02-26".to_string()), @@ -351,12 +421,12 @@ fn serialize_environment_context_with_multiple_selected_environments() { vec![ EnvironmentContextEnvironment { id: "local".to_string(), - cwd: local_cwd.abs(), + cwd: PathUri::from_abs_path(&local_cwd.abs()), shell: "bash".to_string(), }, EnvironmentContextEnvironment { id: "remote".to_string(), - cwd: remote_cwd.abs(), + cwd: PathUri::from_abs_path(&remote_cwd.abs()), shell: "bash".to_string(), }, ], @@ -396,12 +466,12 @@ fn serialize_environment_context_prefers_environment_shell_when_present() { vec![ EnvironmentContextEnvironment { id: "local".to_string(), - cwd: local_cwd.abs(), + cwd: PathUri::from_abs_path(&local_cwd.abs()), shell: "powershell".to_string(), }, EnvironmentContextEnvironment { id: "remote".to_string(), - cwd: remote_cwd.abs(), + cwd: PathUri::from_abs_path(&remote_cwd.abs()), shell: "cmd".to_string(), }, ], diff --git a/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs b/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs index 7a8693edbea..c092c7dcc4c 100644 --- a/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs +++ b/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs @@ -271,17 +271,26 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul assert_eq!( environment_context .lines() - .find(|line| line.trim_start().starts_with("")), - Some(" powershell"), + .find(|line| line.trim_start().starts_with("")) + .map(str::trim), + Some("powershell"), ); // The model should see cwd using the remote environment's native path convention, not // the Linux app-server's host path convention. assert_eq!( environment_context .lines() - .find(|line| line.trim_start().starts_with("")), - Some(r" C:\windows"), + .find(|line| line.trim_start().starts_with("")) + .map(str::trim), + Some(r"C:\windows"), ); + let host_workspace_roots = format!( + "{}", + codex_home.path().display() + ); + // TODO(anp): Derive model-visible workspace roots from the selected remote environment + // and render them using its native path convention. + assert!(environment_context.contains(&host_workspace_roots)); Ok(()) }) From 6afd483fe047ac0d14a07bdd7d2aed0bd110defe Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Tue, 16 Jun 2026 17:36:17 +0000 Subject: [PATCH 3/4] codex: address PR review feedback (#28152) --- codex-rs/core/src/codex_thread.rs | 2 +- .../core/src/context/environment_context.rs | 52 ++++-------- .../src/context/environment_context_tests.rs | 53 +----------- .../core/src/context_manager/history_tests.rs | 9 +- codex-rs/core/src/context_manager/updates.rs | 42 +++++----- codex-rs/core/src/prompt_debug.rs | 2 +- codex-rs/core/src/session/mod.rs | 23 ++--- .../session/rollout_reconstruction_tests.rs | 17 ++-- codex-rs/core/src/session/tests.rs | 62 ++++++++++---- codex-rs/core/src/session/turn.rs | 12 ++- codex-rs/core/src/session/turn_context.rs | 11 ++- codex-rs/core/tests/suite/resume_warning.rs | 3 +- codex-rs/exec/src/lib.rs | 6 +- codex-rs/protocol/src/protocol.rs | 82 ++++++++++++++---- codex-rs/rollout/src/metadata.rs | 2 +- codex-rs/rollout/src/metadata_tests.rs | 3 +- codex-rs/rollout/src/recorder.rs | 5 +- codex-rs/rollout/src/recorder_tests.rs | 3 +- codex-rs/state/src/extract.rs | 84 ++++++++++++++----- codex-rs/state/src/runtime/threads.rs | 2 +- codex-rs/thread-store/src/live_thread.rs | 4 +- .../thread-store/src/thread_metadata_sync.rs | 69 ++++++++++----- codex-rs/utils/path-uri/src/lib.rs | 13 +++ codex-rs/utils/path-uri/src/tests.rs | 22 +++++ 24 files changed, 371 insertions(+), 212 deletions(-) diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 451b497827b..e19bad0eb8e 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -458,7 +458,7 @@ impl CodexThread { self.codex .session .record_context_updates_and_set_reference_context_item(turn_context.as_ref()) - .await; + .await?; } self.codex .session diff --git a/codex-rs/core/src/context/environment_context.rs b/codex-rs/core/src/context/environment_context.rs index 46b396f14f8..fe5f9b1d0dc 100644 --- a/codex-rs/core/src/context/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -10,7 +10,6 @@ use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::TurnContextItem; use codex_protocol::protocol::TurnContextNetworkItem; use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_path_uri::ApiPathString; use codex_utils_path_uri::PathUri; use std::collections::HashSet; use std::path::PathBuf; @@ -381,12 +380,12 @@ impl EnvironmentContext { pub(crate) fn diff_from_turn_context_item( before: &TurnContextItem, after: &EnvironmentContext, - ) -> Self { + ) -> std::io::Result { let before_network = Self::network_from_turn_context_item(before); - let before_filesystem = Self::filesystem_from_turn_context_item(before); + let before_filesystem = Self::filesystem_from_turn_context_item(before)?; let environments = match &after.environments { EnvironmentContextEnvironments::Single(environment) => { - if turn_context_item_cwd_uri(before) != environment.cwd { + if before.cwd != environment.cwd { EnvironmentContextEnvironments::Single(EnvironmentContextEnvironment::legacy( environment.cwd.clone(), environment.shell.clone(), @@ -410,14 +409,14 @@ impl EnvironmentContext { } else { before_filesystem }; - EnvironmentContext::new_with_environments( + Ok(EnvironmentContext::new_with_environments( environments, after.current_date.clone(), after.timezone.clone(), network, filesystem, /*subagents*/ None, - ) + )) } pub(crate) fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self { @@ -441,18 +440,18 @@ impl EnvironmentContext { pub(crate) fn from_turn_context_item( turn_context_item: &TurnContextItem, shell: String, - ) -> Self { - Self::new_with_environments( + ) -> std::io::Result { + Ok(Self::new_with_environments( EnvironmentContextEnvironments::from_vec(vec![EnvironmentContextEnvironment::legacy( - turn_context_item_cwd_uri(turn_context_item), + turn_context_item.cwd.clone(), shell, )]), turn_context_item.current_date.clone(), turn_context_item.timezone.clone(), Self::network_from_turn_context_item(turn_context_item), - Self::filesystem_from_turn_context_item(turn_context_item), + Self::filesystem_from_turn_context_item(turn_context_item)?, /*subagents*/ None, - ) + )) } pub(crate) fn with_subagents(mut self, subagents: String) -> Self { @@ -499,11 +498,11 @@ impl EnvironmentContext { fn filesystem_from_turn_context_item( turn_context_item: &TurnContextItem, - ) -> Option { - Some(FileSystemContext::from_permission_profile( - &turn_context_item.permission_profile(), + ) -> std::io::Result> { + Ok(Some(FileSystemContext::from_permission_profile( + &turn_context_item.permission_profile()?, &workspace_roots_from_turn_context_item(turn_context_item), - )) + ))) } } @@ -516,7 +515,7 @@ fn workspace_roots_from_turn_context_item( // Older rollout items did not persist workspace roots. Fall back to the // legacy cwd binding only when reconstructing that historical context. - match AbsolutePathBuf::try_from(turn_context_item.cwd.clone()) { + match turn_context_item.cwd.to_abs_path() { Ok(cwd) => vec![cwd], Err(_) => Vec::new(), } @@ -542,7 +541,7 @@ impl ContextualUserFragment for EnvironmentContext { let mut lines = Vec::new(); match &self.environments { EnvironmentContextEnvironments::Single(environment) => { - let cwd = render_environment_cwd(&environment.cwd); + let cwd = environment.cwd.inferred_native_path_string(); lines.push(format!(" {cwd}")); lines.push(format!(" {}", environment.shell)); } @@ -550,7 +549,7 @@ impl ContextualUserFragment for EnvironmentContext { lines.push(" ".to_string()); for environment in environments { lines.push(format!(" ", environment.id)); - let cwd = render_environment_cwd(&environment.cwd); + let cwd = environment.cwd.inferred_native_path_string(); lines.push(format!(" {cwd}")); lines.push(format!(" {}", environment.shell)); lines.push(" ".to_string()); @@ -586,23 +585,6 @@ impl ContextualUserFragment for EnvironmentContext { } } -fn turn_context_item_cwd_uri(turn_context_item: &TurnContextItem) -> PathUri { - let cwd = match AbsolutePathBuf::try_from(turn_context_item.cwd.clone()) { - Ok(cwd) => cwd, - Err(_) => AbsolutePathBuf::resolve_path_against_base(&turn_context_item.cwd, "/"), - }; - PathUri::from_abs_path(&cwd) -} - -fn render_environment_cwd(cwd: &PathUri) -> String { - let Some(convention) = cwd.infer_path_convention() else { - return cwd.to_string(); - }; - ApiPathString::from_path_uri(cwd, convention) - .map(ApiPathString::into_string) - .unwrap_or_else(|_| cwd.to_string()) -} - #[cfg(test)] #[path = "environment_context_tests.rs"] mod tests; diff --git a/codex-rs/core/src/context/environment_context_tests.rs b/codex-rs/core/src/context/environment_context_tests.rs index b4f86fb034f..383a38b55c7 100644 --- a/codex-rs/core/src/context/environment_context_tests.rs +++ b/codex-rs/core/src/context/environment_context_tests.rs @@ -190,7 +190,7 @@ fn turn_context_item_filesystem_uses_workspace_roots_instead_of_cwd() { let repo_private = repo.join("private"); let item = TurnContextItem { turn_id: None, - cwd: test_path_buf("/not-the-workspace"), + cwd: PathUri::from_abs_path(&test_abs_path("/not-the-workspace")), workspace_roots: Some(vec![repo.clone(), other_repo.clone()]), current_date: None, timezone: None, @@ -209,7 +209,9 @@ fn turn_context_item_filesystem_uses_workspace_roots_instead_of_cwd() { summary: codex_protocol::config_types::ReasoningSummary::Auto, }; - let context = EnvironmentContext::from_turn_context_item(&item, fake_shell_name()).render(); + let context = EnvironmentContext::from_turn_context_item(&item, fake_shell_name()) + .expect("turn context should hydrate") + .render(); assert!( context.contains(&format!( @@ -234,53 +236,6 @@ fn turn_context_item_filesystem_uses_workspace_roots_instead_of_cwd() { ); } -#[cfg(unix)] -#[test] -fn legacy_projected_windows_cwd_matches_environment_uri() { - let item = TurnContextItem { - turn_id: None, - cwd: PathBuf::from("/C:/windows"), - workspace_roots: None, - current_date: None, - timezone: None, - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::DangerFullAccess, - permission_profile: Some(PermissionProfile::Disabled), - network: None, - file_system_sandbox_policy: None, - model: "gpt-5".to_string(), - comp_hash: None, - personality: None, - collaboration_mode: None, - multi_agent_version: None, - realtime_active: None, - effort: None, - summary: codex_protocol::config_types::ReasoningSummary::Auto, - }; - let current = EnvironmentContext::new( - vec![EnvironmentContextEnvironment { - id: "remote".to_string(), - cwd: PathUri::parse("file:///C:/windows").expect("Windows cwd URI"), - shell: "powershell".to_string(), - }], - /*current_date*/ None, - /*timezone*/ None, - /*network*/ None, - /*subagents*/ None, - ); - - assert_eq!( - EnvironmentContext::diff_from_turn_context_item(&item, ¤t), - EnvironmentContext::new( - Vec::new(), - /*current_date*/ None, - /*timezone*/ None, - /*network*/ None, - /*subagents*/ None, - ) - ); -} - #[test] fn serialize_read_only_environment_context() { let context = EnvironmentContext::new( diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index a98061b5b09..0b538be1109 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -23,13 +23,13 @@ use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::TurnContextItem; use codex_utils_output_truncation::TruncationPolicy; use codex_utils_output_truncation::truncate_text; +use codex_utils_path_uri::PathUri; use image::ImageBuffer; use image::ImageFormat; use image::Luma; use image::Rgba; use pretty_assertions::assert_eq; use regex_lite::Regex; -use std::path::PathBuf; const EXEC_FORMAT_MAX_BYTES: usize = 10_000; const EXEC_FORMAT_MAX_TOKENS: usize = 2_500; @@ -127,7 +127,12 @@ fn developer_msg_with_fragments(texts: &[&str]) -> ResponseItem { fn reference_context_item() -> TurnContextItem { TurnContextItem { turn_id: Some("reference-turn".to_string()), - cwd: PathBuf::from("/tmp/reference-cwd"), + cwd: PathUri::from_path( + std::env::current_dir() + .expect("current directory") + .join("reference-cwd"), + ) + .expect("absolute reference cwd"), workspace_roots: None, current_date: Some("2026-03-23".to_string()), timezone: Some("America/Los_Angeles".to_string()), diff --git a/codex-rs/core/src/context_manager/updates.rs b/codex-rs/core/src/context_manager/updates.rs index 0ce02240b00..d9c0b2f03ee 100644 --- a/codex-rs/core/src/context_manager/updates.rs +++ b/codex-rs/core/src/context_manager/updates.rs @@ -22,40 +22,44 @@ fn build_environment_update_item( previous: Option<&TurnContextItem>, next: &TurnContext, shell: &Shell, -) -> Option { +) -> std::io::Result> { if !next.config.include_environment_context { - return None; + return Ok(None); } - let prev = previous?; - let prev_context = EnvironmentContext::from_turn_context_item(prev, shell.name().to_string()); + let Some(prev) = previous else { + return Ok(None); + }; + let prev_context = EnvironmentContext::from_turn_context_item(prev, shell.name().to_string())?; let next_context = EnvironmentContext::from_turn_context(next, shell); if prev_context.equals_except_shell(&next_context) { - return None; + return Ok(None); } - Some(ContextualUserFragment::into( - EnvironmentContext::diff_from_turn_context_item(prev, &next_context), - )) + Ok(Some(ContextualUserFragment::into( + EnvironmentContext::diff_from_turn_context_item(prev, &next_context)?, + ))) } fn build_permissions_update_item( previous: Option<&TurnContextItem>, next: &TurnContext, exec_policy: &Policy, -) -> Option { +) -> std::io::Result> { if !next.config.include_permissions_instructions { - return None; + return Ok(None); } - let prev = previous?; - if prev.permission_profile() == next.permission_profile() + let Some(prev) = previous else { + return Ok(None); + }; + if prev.permission_profile()? == next.permission_profile() && prev.approval_policy == next.approval_policy.value() { - return None; + return Ok(None); } - Some( + Ok(Some( PermissionsInstructions::from_permission_profile( &next.permission_profile, next.approval_policy.value(), @@ -67,7 +71,7 @@ fn build_permissions_update_item( next.features.enabled(Feature::RequestPermissionsTool), ) .render(), - ) + )) } fn build_collaboration_mode_update_item( @@ -214,17 +218,17 @@ pub(crate) fn build_settings_update_items( shell: &Shell, exec_policy: &Policy, personality_feature_enabled: bool, -) -> Vec { +) -> std::io::Result> { // TODO(ccunningham): build_settings_update_items still does not cover every // model-visible item emitted by build_initial_context. Persist the remaining // inputs or add explicit replay events so fork/resume can diff everything // deterministically. - let contextual_user_message = build_environment_update_item(previous, next, shell); + let contextual_user_message = build_environment_update_item(previous, next, shell)?; let developer_update_sections = [ // Keep model-switch instructions first so model-specific guidance is read before // any other context diffs on this turn. build_model_instructions_update_item(previous_turn_settings, next), - build_permissions_update_item(previous, next, exec_policy), + build_permissions_update_item(previous, next, exec_policy)?, build_collaboration_mode_update_item(previous, next), build_realtime_update_item(previous, previous_turn_settings, next), build_personality_update_item(previous, next, personality_feature_enabled), @@ -240,5 +244,5 @@ pub(crate) fn build_settings_update_items( if let Some(contextual_user_message) = contextual_user_message { items.push(contextual_user_message); } - items + Ok(items) } diff --git a/codex-rs/core/src/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs index 0da42d8a4e3..f3da9b93fe4 100644 --- a/codex-rs/core/src/prompt_debug.rs +++ b/codex-rs/core/src/prompt_debug.rs @@ -77,7 +77,7 @@ pub(crate) async fn build_prompt_input_from_session( ) -> CodexResult> { let turn_context = sess.new_default_turn().await; sess.record_context_updates_and_set_reference_context_item(turn_context.as_ref()) - .await; + .await?; if !input.is_empty() { let response_item = sess.response_item_from_user_input(turn_context.as_ref(), input); diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 242e35f3756..81e07dabfa5 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1619,7 +1619,7 @@ impl Session { &self, reference_context_item: Option<&TurnContextItem>, current_context: &TurnContext, - ) -> Vec { + ) -> CodexResult> { // TODO: Make context updates a pure diff of persisted previous/current TurnContextItem // state so replay/backtracking is deterministic. Runtime inputs that affect model-visible // context (shell, exec policy, feature gates, previous-turn bridge) should be persisted @@ -1630,13 +1630,15 @@ impl Session { }; let shell = self.user_shell(); let exec_policy = self.services.exec_policy.current(); - crate::context_manager::updates::build_settings_update_items( - reference_context_item, - previous_turn_settings.as_ref(), - current_context, - shell.as_ref(), - exec_policy.as_ref(), - self.features.enabled(Feature::Personality), + Ok( + crate::context_manager::updates::build_settings_update_items( + reference_context_item, + previous_turn_settings.as_ref(), + current_context, + shell.as_ref(), + exec_policy.as_ref(), + self.features.enabled(Feature::Personality), + )?, ) } @@ -3161,7 +3163,7 @@ impl Session { pub(crate) async fn record_context_updates_and_set_reference_context_item( &self, turn_context: &TurnContext, - ) { + ) -> CodexResult<()> { let reference_context_item = { let state = self.state.lock().await; state.reference_context_item() @@ -3172,7 +3174,7 @@ impl Session { } else { // Steady-state path: append only context diffs to minimize token overhead. self.build_settings_update_items(reference_context_item.as_ref(), turn_context) - .await + .await? }; let turn_context_item = turn_context.to_turn_context_item(); if !context_items.is_empty() { @@ -3188,6 +3190,7 @@ impl Session { // context items. This keeps later runtime diffing aligned with the current turn state. let mut state = self.state.lock().await; state.set_reference_context_item(Some(turn_context_item)); + Ok(()) } pub(crate) async fn update_token_usage_info( diff --git a/codex-rs/core/src/session/rollout_reconstruction_tests.rs b/codex-rs/core/src/session/rollout_reconstruction_tests.rs index d53e3866c99..67632aa5365 100644 --- a/codex-rs/core/src/session/rollout_reconstruction_tests.rs +++ b/codex-rs/core/src/session/rollout_reconstruction_tests.rs @@ -9,6 +9,7 @@ use codex_protocol::protocol::CompactedItem; use codex_protocol::protocol::InitialHistory; use codex_protocol::protocol::InterAgentCommunication; use codex_protocol::protocol::ResumedHistory; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use std::path::PathBuf; @@ -88,7 +89,7 @@ async fn record_initial_history_resumed_bare_turn_context_does_not_hydrate_previ let previous_context_item = TurnContextItem { turn_id: Some(turn_context.sub_id.clone()), #[allow(deprecated)] - cwd: turn_context.cwd.to_path_buf(), + cwd: PathUri::from_abs_path(&turn_context.cwd), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -128,7 +129,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif let mut previous_context_item = TurnContextItem { turn_id: Some(turn_context.sub_id.clone()), #[allow(deprecated)] - cwd: turn_context.cwd.to_path_buf(), + cwd: PathUri::from_abs_path(&turn_context.cwd), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -989,7 +990,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis let previous_context_item = TurnContextItem { turn_id: Some(turn_context.sub_id.clone()), #[allow(deprecated)] - cwd: turn_context.cwd.to_path_buf(), + cwd: PathUri::from_abs_path(&turn_context.cwd), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -1071,7 +1072,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis serde_json::to_value(Some(TurnContextItem { turn_id: Some(turn_context.sub_id.clone()), #[allow(deprecated)] - cwd: turn_context.cwd.to_path_buf(), + cwd: PathUri::from_abs_path(&turn_context.cwd), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -1101,7 +1102,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu let previous_context_item = TurnContextItem { turn_id: Some(turn_context.sub_id.clone()), #[allow(deprecated)] - cwd: turn_context.cwd.to_path_buf(), + cwd: PathUri::from_abs_path(&turn_context.cwd), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -1223,7 +1224,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo let current_context_item = TurnContextItem { turn_id: Some(current_turn_id.clone()), #[allow(deprecated)] - cwd: turn_context.cwd.to_path_buf(), + cwd: PathUri::from_abs_path(&turn_context.cwd), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -1343,7 +1344,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea let previous_context_item = TurnContextItem { turn_id: Some(turn_context.sub_id.clone()), #[allow(deprecated)] - cwd: turn_context.cwd.to_path_buf(), + cwd: PathUri::from_abs_path(&turn_context.cwd), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -1506,7 +1507,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear let previous_context_item = TurnContextItem { turn_id: Some(turn_context.sub_id.clone()), #[allow(deprecated)] - cwd: turn_context.cwd.to_path_buf(), + cwd: PathUri::from_abs_path(&turn_context.cwd), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index b4b786c902a..ce8b439bfa1 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -1874,7 +1874,8 @@ async fn resumed_history_injects_initial_context_on_first_context_update_only() session .record_context_updates_and_set_reference_context_item(&turn_context) - .await; + .await + .expect("context updates should hydrate"); let initial_context = session.build_initial_context(&turn_context).await; expected.extend(initial_context); let history_after_seed = session.clone_history().await; @@ -1882,7 +1883,8 @@ async fn resumed_history_injects_initial_context_on_first_context_update_only() session .record_context_updates_and_set_reference_context_item(&turn_context) - .await; + .await + .expect("context updates should hydrate"); let history_after_second_seed = session.clone_history().await; assert_eq!( history_after_seed.raw_items(), @@ -2673,7 +2675,7 @@ async fn record_initial_history_forked_hydrates_previous_turn_settings() { let previous_context_item = TurnContextItem { turn_id: Some(turn_context.sub_id.clone()), #[allow(deprecated)] - cwd: turn_context.cwd.to_path_buf(), + cwd: PathUri::from_abs_path(&turn_context.cwd), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -7290,7 +7292,8 @@ async fn build_settings_update_items_emits_environment_item_for_network_changes( let reference_context_item = previous_context.to_turn_context_item(); let update_items = session .build_settings_update_items(Some(&reference_context_item), ¤t_context) - .await; + .await + .expect("settings updates should hydrate"); let environment_update = user_input_texts(&update_items) .into_iter() @@ -7360,7 +7363,8 @@ async fn build_settings_update_items_emits_environment_item_for_time_changes() { let reference_context_item = previous_context.to_turn_context_item(); let update_items = session .build_settings_update_items(Some(&reference_context_item), ¤t_context) - .await; + .await + .expect("settings updates should hydrate"); let environment_update = user_input_texts(&update_items) .into_iter() @@ -7388,7 +7392,8 @@ async fn build_settings_update_items_omits_environment_item_when_disabled() { let reference_context_item = previous_context.to_turn_context_item(); let update_items = session .build_settings_update_items(Some(&reference_context_item), ¤t_context) - .await; + .await + .expect("settings updates should hydrate"); let user_texts = user_input_texts(&update_items); assert!( @@ -7416,7 +7421,8 @@ async fn build_settings_update_items_emits_realtime_start_when_session_becomes_l Some(&previous_context.to_turn_context_item()), ¤t_context, ) - .await; + .await + .expect("settings updates should hydrate"); let developer_texts = developer_input_texts(&update_items); assert!( @@ -7444,7 +7450,8 @@ async fn build_settings_update_items_emits_realtime_end_when_session_stops_being Some(&previous_context.to_turn_context_item()), ¤t_context, ) - .await; + .await + .expect("settings updates should hydrate"); let developer_texts = developer_input_texts(&update_items); assert!( @@ -7478,7 +7485,8 @@ async fn build_settings_update_items_uses_previous_turn_settings_for_realtime_en .await; let update_items = session .build_settings_update_items(Some(&previous_context_item), ¤t_context) - .await; + .await + .expect("settings updates should hydrate"); let developer_texts = developer_input_texts(&update_items); assert!( @@ -8130,6 +8138,21 @@ async fn turn_context_item_uses_turn_context_comp_hash_snapshot() { ); } +#[tokio::test] +async fn turn_context_item_stores_primary_environment_cwd_uri() { + let (_session, mut turn_context) = make_session_and_context().await; + let environment = turn_context.environments.turn_environments[0].clone(); + let cwd = PathUri::parse("file:///C:/windows").expect("Windows cwd URI"); + turn_context.environments.turn_environments[0] = TurnEnvironment::new( + "remote".to_string(), + environment.environment, + cwd.clone(), + environment.shell, + ); + + assert_eq!(turn_context.to_turn_context_item().cwd, cwd); +} + #[tokio::test] async fn turn_context_item_omits_legacy_equivalent_file_system_sandbox_policy() { let (_session, turn_context) = make_session_and_context().await; @@ -8171,7 +8194,8 @@ async fn record_context_updates_and_set_reference_context_item_injects_full_cont let (session, turn_context) = make_session_and_context().await; session .record_context_updates_and_set_reference_context_item(&turn_context) - .await; + .await + .expect("context updates should hydrate"); let history = session.clone_history().await; let initial_context = session.build_initial_context(&turn_context).await; assert_eq!(history.raw_items().to_vec(), initial_context); @@ -8202,7 +8226,8 @@ async fn record_context_updates_and_set_reference_context_item_reinjects_full_co .await; session .record_context_updates_and_set_reference_context_item(&turn_context) - .await; + .await + .expect("context updates should hydrate"); { let mut state = session.state.lock().await; state.set_reference_context_item(/*item*/ None); @@ -8216,7 +8241,8 @@ async fn record_context_updates_and_set_reference_context_item_reinjects_full_co session .record_context_updates_and_set_reference_context_item(&turn_context) - .await; + .await + .expect("context updates should hydrate"); let history = session.clone_history().await; let mut expected_history = vec![compacted_summary]; @@ -8246,12 +8272,14 @@ async fn record_context_updates_and_set_reference_context_item_persists_baseline let update_items = session .build_settings_update_items(Some(&previous_context_item), &turn_context) - .await; + .await + .expect("settings updates should hydrate"); assert_eq!(update_items, Vec::new()); session .record_context_updates_and_set_reference_context_item(&turn_context) - .await; + .await + .expect("context updates should hydrate"); assert_eq!( session.clone_history().await.raw_items().to_vec(), @@ -8298,7 +8326,8 @@ async fn record_context_updates_and_set_reference_context_item_persists_split_fi session .record_context_updates_and_set_reference_context_item(&turn_context) - .await; + .await + .expect("context updates should hydrate"); session.ensure_rollout_materialized().await; session.flush_rollout().await.expect("rollout should flush"); @@ -8382,7 +8411,8 @@ async fn record_context_updates_and_set_reference_context_item_persists_full_rei .await; session .record_context_updates_and_set_reference_context_item(&turn_context) - .await; + .await + .expect("context updates should hydrate"); session.ensure_rollout_materialized().await; session.flush_rollout().await.expect("rollout should flush"); diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index a105aeffeed..fbbd00f846f 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -156,8 +156,16 @@ pub(crate) async fn run_turn( return None; } - sess.record_context_updates_and_set_reference_context_item(turn_context.as_ref()) - .await; + if let Err(err) = sess + .record_context_updates_and_set_reference_context_item(turn_context.as_ref()) + .await + { + let error = err.to_codex_protocol_error(); + sess.emit_turn_error_lifecycle(turn_context.as_ref(), error.clone()) + .await; + error!(%err, "failed to hydrate persisted turn context"); + return None; + } let (injection_items, explicitly_enabled_connectors) = build_skills_and_plugins(&sess, turn_context.as_ref(), &input, &cancellation_token).await?; diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index aed5fae681f..25ad962fd49 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -396,10 +396,17 @@ impl TurnContext { pub(crate) fn to_turn_context_item(&self) -> TurnContextItem { let workspace_roots = self.config.effective_workspace_roots(); + let cwd = self + .environments + .primary() + .map(|environment| environment.cwd().clone()) + .unwrap_or_else(|| { + #[allow(deprecated)] + PathUri::from_abs_path(&self.cwd) + }); TurnContextItem { turn_id: Some(self.sub_id.clone()), - #[allow(deprecated)] - cwd: self.cwd.to_path_buf(), + cwd, workspace_roots: (!workspace_roots.is_empty()).then_some(workspace_roots), current_date: self.current_date.clone(), timezone: self.timezone.clone(), diff --git a/codex-rs/core/tests/suite/resume_warning.rs b/codex-rs/core/tests/suite/resume_warning.rs index 0ba39229846..001c1129952 100644 --- a/codex-rs/core/tests/suite/resume_warning.rs +++ b/codex-rs/core/tests/suite/resume_warning.rs @@ -14,6 +14,7 @@ use codex_protocol::protocol::TurnContextItem; use codex_protocol::protocol::TurnStartedEvent; use codex_protocol::protocol::UserMessageEvent; use codex_protocol::protocol::WarningEvent; +use codex_utils_path_uri::PathUri; use core::time::Duration; use core_test_support::load_default_config_for_test; use core_test_support::wait_for_event; @@ -27,7 +28,7 @@ fn resume_history( let turn_id = "resume-warning-seed-turn".to_string(); let turn_ctx = TurnContextItem { turn_id: Some(turn_id.clone()), - cwd: config.cwd.to_path_buf(), + cwd: PathUri::from_abs_path(&config.cwd), workspace_roots: None, current_date: None, timezone: None, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index f8ac851bf97..c7ab8cfa0ad 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -1423,7 +1423,11 @@ async fn parse_latest_turn_context_cwd(path: &Path) -> Option { continue; }; if let RolloutItem::TurnContext(item) = rollout_line.item { - return Some(item.cwd); + return item + .cwd + .to_abs_path() + .ok() + .map(AbsolutePathBuf::into_path_buf); } } None diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 88c08bdb68d..7275c0667fc 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -53,6 +53,7 @@ use crate::request_permissions::RequestPermissionsResponse; use crate::request_user_input::RequestUserInputResponse; use crate::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; use schemars::JsonSchema; use serde::Deserialize; @@ -2988,7 +2989,7 @@ pub struct TurnContextNetworkItem { pub struct TurnContextItem { #[serde(default, skip_serializing_if = "Option::is_none")] pub turn_id: Option, - pub cwd: PathBuf, + pub cwd: PathUri, /// Effective workspace roots used to materialize symbolic /// `:workspace_roots` filesystem permissions in `permission_profile`. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -3026,21 +3027,45 @@ pub struct TurnContextItem { } impl TurnContextItem { - pub fn permission_profile(&self) -> PermissionProfile { - self.permission_profile.clone().unwrap_or_else(|| { - let file_system_sandbox_policy = - self.file_system_sandbox_policy.clone().unwrap_or_else(|| { - FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( - &self.sandbox_policy, - &self.cwd, + pub fn permission_profile(&self) -> std::io::Result { + if let Some(permission_profile) = self.permission_profile.clone() { + return Ok(permission_profile); + } + let file_system_sandbox_policy = match self.file_system_sandbox_policy.clone() { + Some(file_system_sandbox_policy) => file_system_sandbox_policy, + None => { + let convention = self.cwd.infer_path_convention().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!( + "cannot infer a path convention for legacy permission profile cwd {}", + self.cwd + ), ) - }); + })?; + if convention != PathConvention::native() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!( + "cannot hydrate legacy permission profile for {convention} cwd {} on a {} host", + self.cwd, + PathConvention::native(), + ), + )); + } + FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( + &self.sandbox_policy, + self.cwd.to_abs_path()?.as_path(), + ) + } + }; + Ok( PermissionProfile::from_runtime_permissions_with_enforcement( SandboxEnforcement::from_legacy_sandbox_policy(&self.sandbox_policy), &file_system_sandbox_policy, NetworkSandboxPolicy::from(&self.sandbox_policy), - ) - }) + ), + ) } } @@ -5301,21 +5326,50 @@ mod tests { } #[test] - fn turn_context_item_deserializes_without_network() -> Result<()> { + fn turn_context_item_accepts_legacy_cwd_and_serializes_path_uri() -> Result<()> { + let legacy_cwd = test_path_buf("/tmp"); let item: TurnContextItem = serde_json::from_value(json!({ - "cwd": test_path_buf("/tmp"), + "cwd": legacy_cwd, "approval_policy": "never", "sandbox_policy": { "type": "danger-full-access" }, "model": "gpt-5", "summary": "auto", }))?; + let expected_cwd = PathUri::from_path(legacy_cwd)?; + assert_eq!(item.cwd, expected_cwd); + assert_eq!( + serde_json::to_value(&item)?["cwd"], + json!(expected_cwd.to_string()) + ); assert_eq!(item.network, None); assert_eq!(item.file_system_sandbox_policy, None); assert_eq!(item.comp_hash, None); Ok(()) } + #[test] + fn turn_context_item_rejects_legacy_policy_hydration_for_foreign_cwd() -> Result<()> { + let foreign_cwd = if cfg!(windows) { + "file:///tmp" + } else { + "file:///C:/windows" + }; + let item: TurnContextItem = serde_json::from_value(json!({ + "cwd": foreign_cwd, + "approval_policy": "never", + "sandbox_policy": { "type": "workspace-write" }, + "model": "gpt-5", + "summary": "auto", + }))?; + + let err = item + .permission_profile() + .expect_err("foreign cwd should not hydrate a legacy permission profile"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + Ok(()) + } + #[test] fn multi_agent_version_uses_newest_present_session_meta_value() -> Result<()> { let thread_id = ThreadId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8")?; @@ -5353,7 +5407,7 @@ mod tests { fn turn_context_item_serializes_network_when_present() -> Result<()> { let item = TurnContextItem { turn_id: None, - cwd: test_path_buf("/tmp"), + cwd: PathUri::from_abs_path(&test_path_buf("/tmp").abs()), workspace_roots: None, current_date: None, timezone: None, diff --git a/codex-rs/rollout/src/metadata.rs b/codex-rs/rollout/src/metadata.rs index 19e9a9320c9..6eabd21ae64 100644 --- a/codex-rs/rollout/src/metadata.rs +++ b/codex-rs/rollout/src/metadata.rs @@ -111,7 +111,7 @@ pub async fn extract_metadata_from_rollout( })?; let mut metadata = builder.build(default_provider); for item in &items { - apply_rollout_item(&mut metadata, item, default_provider); + apply_rollout_item(&mut metadata, item, default_provider)?; } if let Some(updated_at) = file_modified_time_utc(rollout_path).await { metadata.updated_at = updated_at; diff --git a/codex-rs/rollout/src/metadata_tests.rs b/codex-rs/rollout/src/metadata_tests.rs index 03d251e1b30..43aa62e646f 100644 --- a/codex-rs/rollout/src/metadata_tests.rs +++ b/codex-rs/rollout/src/metadata_tests.rs @@ -69,7 +69,8 @@ async fn extract_metadata_from_rollout_uses_session_meta() { let builder = builder_from_session_meta(&session_meta_line, path.as_path()).expect("builder"); let mut expected = builder.build("openai"); - apply_rollout_item(&mut expected, &rollout_line.item, "openai"); + apply_rollout_item(&mut expected, &rollout_line.item, "openai") + .expect("rollout item should apply"); expected.updated_at = file_modified_time_utc(&path).await.expect("mtime"); assert_eq!(outcome.metadata, expected); diff --git a/codex-rs/rollout/src/recorder.rs b/codex-rs/rollout/src/recorder.rs index 5dc207049fe..a3e5c0514b0 100644 --- a/codex-rs/rollout/src/recorder.rs +++ b/codex-rs/rollout/src/recorder.rs @@ -1765,15 +1765,16 @@ async fn resume_candidate_matches_cwd( if let Ok((items, _, _)) = RolloutRecorder::load_rollout_items(rollout_path).await && let Some(latest_turn_context_cwd) = items.iter().rev().find_map(|item| match item { - RolloutItem::TurnContext(turn_context) => Some(turn_context.cwd.as_path()), + RolloutItem::TurnContext(turn_context) => Some(&turn_context.cwd), RolloutItem::SessionMeta(_) | RolloutItem::ResponseItem(_) | RolloutItem::InterAgentCommunication(_) | RolloutItem::Compacted(_) | RolloutItem::EventMsg(_) => None, }) + && let Ok(latest_turn_context_cwd) = latest_turn_context_cwd.to_abs_path() { - return cwd_matches(latest_turn_context_cwd, cwd); + return cwd_matches(latest_turn_context_cwd.as_path(), cwd); } metadata::extract_metadata_from_rollout(rollout_path, default_provider) diff --git a/codex-rs/rollout/src/recorder_tests.rs b/codex-rs/rollout/src/recorder_tests.rs index 68896ad202f..c49a477944b 100644 --- a/codex-rs/rollout/src/recorder_tests.rs +++ b/codex-rs/rollout/src/recorder_tests.rs @@ -1134,7 +1134,8 @@ async fn resume_candidate_matches_cwd_reads_latest_turn_context() -> std::io::Re timestamp: "2025-01-03T13:00:01Z".to_string(), item: RolloutItem::TurnContext(TurnContextItem { turn_id: Some("turn-1".to_string()), - cwd: latest_cwd.clone(), + cwd: serde_json::from_value(serde_json::json!(&latest_cwd)) + .expect("absolute latest cwd"), workspace_roots: None, current_date: None, timezone: None, diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index ce2da4156de..2254b4da896 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -16,10 +16,10 @@ pub fn apply_rollout_item( metadata: &mut ThreadMetadata, item: &RolloutItem, default_provider: &str, -) { +) -> std::io::Result<()> { match item { RolloutItem::SessionMeta(meta_line) => apply_session_meta_from_item(metadata, meta_line), - RolloutItem::TurnContext(turn_ctx) => apply_turn_context(metadata, turn_ctx), + RolloutItem::TurnContext(turn_ctx) => apply_turn_context(metadata, turn_ctx)?, RolloutItem::EventMsg(event) => apply_event_msg(metadata, event), RolloutItem::ResponseItem(item) => apply_response_item(metadata, item), RolloutItem::InterAgentCommunication(_) => {} @@ -28,6 +28,7 @@ pub fn apply_rollout_item( if metadata.model_provider.is_empty() { metadata.model_provider = default_provider.to_string(); } + Ok(()) } /// Return whether this rollout item can mutate thread metadata stored in SQLite. @@ -72,15 +73,21 @@ fn apply_session_meta_from_item(metadata: &mut ThreadMetadata, meta_line: &Sessi } } -fn apply_turn_context(metadata: &mut ThreadMetadata, turn_ctx: &TurnContextItem) { - if metadata.cwd.as_os_str().is_empty() { - metadata.cwd = turn_ctx.cwd.clone(); +fn apply_turn_context( + metadata: &mut ThreadMetadata, + turn_ctx: &TurnContextItem, +) -> std::io::Result<()> { + if metadata.cwd.as_os_str().is_empty() + && let Ok(cwd) = turn_ctx.cwd.to_abs_path() + { + metadata.cwd = cwd.into_path_buf(); } metadata.model = Some(turn_ctx.model.clone()); metadata.reasoning_effort = turn_ctx.effort.clone(); metadata.sandbox_policy = - serde_json::to_string(&turn_ctx.permission_profile()).unwrap_or_default(); + serde_json::to_string(&turn_ctx.permission_profile()?).unwrap_or_default(); metadata.approval_mode = enum_to_string(&turn_ctx.approval_policy); + Ok(()) } fn apply_event_msg(metadata: &mut ThreadMetadata, event: &EventMsg) { @@ -194,7 +201,8 @@ mod tests { metadata: None, }); - apply_rollout_item(&mut metadata, &item, "test-provider"); + apply_rollout_item(&mut metadata, &item, "test-provider") + .expect("rollout item should apply"); assert_eq!(metadata.first_user_message, None); assert_eq!(metadata.preview, None); @@ -213,7 +221,8 @@ mod tests { ..Default::default() })); - apply_rollout_item(&mut metadata, &item, "test-provider"); + apply_rollout_item(&mut metadata, &item, "test-provider") + .expect("rollout item should apply"); assert_eq!( metadata.first_user_message.as_deref(), @@ -235,7 +244,8 @@ mod tests { ..Default::default() })); - apply_rollout_item(&mut metadata, &item, "test-provider"); + apply_rollout_item(&mut metadata, &item, "test-provider") + .expect("rollout item should apply"); assert_eq!( metadata.first_user_message.as_deref(), @@ -260,7 +270,8 @@ mod tests { ..Default::default() })); - apply_rollout_item(&mut metadata, &item, "test-provider"); + apply_rollout_item(&mut metadata, &item, "test-provider") + .expect("rollout item should apply"); assert_eq!(metadata.first_user_message, None); assert_eq!(metadata.preview, None); @@ -286,7 +297,8 @@ mod tests { }, })); - apply_rollout_item(&mut metadata, &goal_item, "test-provider"); + apply_rollout_item(&mut metadata, &goal_item, "test-provider") + .expect("rollout item should apply"); assert_eq!(metadata.preview.as_deref(), Some("optimize the benchmark")); assert_eq!(metadata.first_user_message, None); @@ -301,7 +313,8 @@ mod tests { ..Default::default() })); - apply_rollout_item(&mut metadata, &user_item, "test-provider"); + apply_rollout_item(&mut metadata, &user_item, "test-provider") + .expect("rollout item should apply"); assert_eq!(metadata.preview.as_deref(), Some("optimize the benchmark")); assert_eq!( @@ -344,12 +357,18 @@ mod tests { git: None, }), "test-provider", - ); + ) + .expect("rollout item should apply"); apply_rollout_item( &mut metadata, &RolloutItem::TurnContext(TurnContextItem { turn_id: Some("turn-1".to_string()), - cwd: PathBuf::from("/parent/workspace"), + cwd: serde_json::from_value(serde_json::json!( + std::env::current_dir() + .expect("current directory") + .join("parent/workspace") + )) + .expect("absolute parent cwd"), workspace_roots: None, current_date: None, timezone: None, @@ -368,7 +387,8 @@ mod tests { summary: codex_protocol::config_types::ReasoningSummary::Auto, }), "test-provider", - ); + ) + .expect("rollout item should apply"); assert_eq!(metadata.cwd, PathBuf::from("/child/worktree")); let permission_profile: PermissionProfile = PermissionProfile::Disabled; @@ -388,7 +408,12 @@ mod tests { &mut metadata, &RolloutItem::TurnContext(TurnContextItem { turn_id: Some("turn-1".to_string()), - cwd: PathBuf::from("/workspace"), + cwd: serde_json::from_value(serde_json::json!( + std::env::current_dir() + .expect("current directory") + .join("workspace") + )) + .expect("absolute workspace cwd"), workspace_roots: None, current_date: None, timezone: None, @@ -407,7 +432,8 @@ mod tests { summary: codex_protocol::config_types::ReasoningSummary::Auto, }), "test-provider", - ); + ) + .expect("rollout item should apply"); assert_eq!( metadata.sandbox_policy, @@ -419,12 +445,16 @@ mod tests { fn turn_context_sets_cwd_when_session_cwd_missing() { let mut metadata = metadata_for_test(); metadata.cwd = PathBuf::new(); + let fallback_cwd = std::env::current_dir() + .expect("current directory") + .join("fallback/workspace"); apply_rollout_item( &mut metadata, &RolloutItem::TurnContext(TurnContextItem { turn_id: Some("turn-1".to_string()), - cwd: PathBuf::from("/fallback/workspace"), + cwd: serde_json::from_value(serde_json::json!(&fallback_cwd)) + .expect("absolute fallback cwd"), workspace_roots: None, current_date: None, timezone: None, @@ -443,9 +473,10 @@ mod tests { summary: codex_protocol::config_types::ReasoningSummary::Auto, }), "test-provider", - ); + ) + .expect("rollout item should apply"); - assert_eq!(metadata.cwd, PathBuf::from("/fallback/workspace")); + assert_eq!(metadata.cwd, fallback_cwd); } #[test] @@ -456,7 +487,12 @@ mod tests { &mut metadata, &RolloutItem::TurnContext(TurnContextItem { turn_id: Some("turn-1".to_string()), - cwd: PathBuf::from("/fallback/workspace"), + cwd: serde_json::from_value(serde_json::json!( + std::env::current_dir() + .expect("current directory") + .join("fallback/workspace") + )) + .expect("absolute fallback cwd"), workspace_roots: None, current_date: None, timezone: None, @@ -475,7 +511,8 @@ mod tests { summary: codex_protocol::config_types::ReasoningSummary::Auto, }), "test-provider", - ); + ) + .expect("rollout item should apply"); assert_eq!(metadata.model.as_deref(), Some("gpt-5")); assert_eq!(metadata.reasoning_effort, Some(ReasoningEffort::High)); @@ -511,7 +548,8 @@ mod tests { git: None, }), "test-provider", - ); + ) + .expect("rollout item should apply"); assert_eq!(metadata.model, None); assert_eq!(metadata.reasoning_effort, None); diff --git a/codex-rs/state/src/runtime/threads.rs b/codex-rs/state/src/runtime/threads.rs index 6cebf27e874..81a490a0cb4 100644 --- a/codex-rs/state/src/runtime/threads.rs +++ b/codex-rs/state/src/runtime/threads.rs @@ -829,7 +829,7 @@ ON CONFLICT(id) DO UPDATE SET .unwrap_or_else(|| builder.build(&self.default_provider)); metadata.rollout_path = builder.rollout_path.clone(); for item in items { - apply_rollout_item(&mut metadata, item, &self.default_provider); + apply_rollout_item(&mut metadata, item, &self.default_provider)?; } if let Some(existing_metadata) = existing_metadata.as_ref() { metadata.prefer_existing_git_info(existing_metadata); diff --git a/codex-rs/thread-store/src/live_thread.rs b/codex-rs/thread-store/src/live_thread.rs index 88d347f57ba..0e81b165add 100644 --- a/codex-rs/thread-store/src/live_thread.rs +++ b/codex-rs/thread-store/src/live_thread.rs @@ -125,7 +125,7 @@ impl LiveThread { } } } - let metadata_sync = ThreadMetadataSync::for_resume(¶ms); + let metadata_sync = ThreadMetadataSync::for_resume(¶ms)?; Ok(Self { thread_id, thread_store, @@ -151,7 +151,7 @@ impl LiveThread { .metadata_sync .lock() .await - .observe_appended_items(canonical_items.as_slice()); + .observe_appended_items(canonical_items.as_slice())?; if let Some(update) = update { self.thread_store .update_thread_metadata(UpdateThreadMetadataParams { diff --git a/codex-rs/thread-store/src/thread_metadata_sync.rs b/codex-rs/thread-store/src/thread_metadata_sync.rs index 8eb23e3632c..d4bd8edb767 100644 --- a/codex-rs/thread-store/src/thread_metadata_sync.rs +++ b/codex-rs/thread-store/src/thread_metadata_sync.rs @@ -18,6 +18,8 @@ use crate::CreateThreadParams; use crate::GitInfoPatch; use crate::ResumeThreadParams; use crate::ThreadMetadataPatch; +use crate::ThreadStoreError; +use crate::ThreadStoreResult; const IMAGE_ONLY_USER_MESSAGE_PLACEHOLDER: &str = "[Image]"; #[cfg(not(test))] @@ -90,7 +92,7 @@ impl ThreadMetadataSync { } } - pub(crate) fn for_resume(params: &ResumeThreadParams) -> Self { + pub(crate) fn for_resume(params: &ResumeThreadParams) -> ThreadStoreResult { let mut sync = Self { thread_id: params.thread_id, cwd_seen: params @@ -108,11 +110,11 @@ impl ThreadMetadataSync { defer_resume_update_until_append: false, }; if let Some(history) = params.history.as_deref() { - let update = sync.observe_resume_history(history); + let update = sync.observe_resume_history(history)?; sync.merge_pending_update(update); sync.defer_resume_update_until_append = sync.pending_update.is_some(); } - sync + Ok(sync) } pub(crate) fn take_pending_update(&self) -> Option { @@ -148,7 +150,7 @@ impl ThreadMetadataSync { pub(crate) fn observe_appended_items( &mut self, items: &[RolloutItem], - ) -> Option { + ) -> ThreadStoreResult> { self.defer_create_update_until_history_exists = false; self.defer_resume_update_until_append = false; let affects_metadata = items @@ -157,7 +159,10 @@ impl ThreadMetadataSync { let update = if affects_metadata { self.observe_items(items)? } else { - thread_updated_at_touch() + Some(thread_updated_at_touch()) + }; + let Some(update) = update else { + return Ok(None); }; self.merge_pending_update(Some(update)); if !affects_metadata @@ -169,12 +174,15 @@ impl ThreadMetadataSync { Instant::now().duration_since(last_touch) < THREAD_UPDATED_AT_TOUCH_INTERVAL }) { - return None; + return Ok(None); } - self.take_pending_update() + Ok(self.take_pending_update()) } - fn observe_items(&mut self, items: &[RolloutItem]) -> Option { + fn observe_items( + &mut self, + items: &[RolloutItem], + ) -> ThreadStoreResult> { self.observe_items_with_update( items, ThreadMetadataPatch { @@ -184,7 +192,10 @@ impl ThreadMetadataSync { ) } - fn observe_resume_history(&mut self, items: &[RolloutItem]) -> Option { + fn observe_resume_history( + &mut self, + items: &[RolloutItem], + ) -> ThreadStoreResult> { self.observe_items_with_update(items, ThreadMetadataPatch::default()) } @@ -192,9 +203,9 @@ impl ThreadMetadataSync { &mut self, items: &[RolloutItem], mut update: ThreadMetadataPatch, - ) -> Option { + ) -> ThreadStoreResult> { if items.is_empty() { - return None; + return Ok(None); } for item in items { match item { @@ -227,14 +238,23 @@ impl ThreadMetadataSync { } } RolloutItem::TurnContext(turn_ctx) => { - if !self.cwd_seen && !turn_ctx.cwd.as_os_str().is_empty() { + if !self.cwd_seen + && let Ok(cwd) = turn_ctx.cwd.to_abs_path() + { self.cwd_seen = true; - update.cwd = Some(turn_ctx.cwd.clone()); + update.cwd = Some(cwd.into_path_buf()); } update.model = Some(turn_ctx.model.clone()); update.reasoning_effort = turn_ctx.effort.clone(); update.approval_mode = Some(turn_ctx.approval_policy); - update.permission_profile = Some(turn_ctx.permission_profile()); + update.permission_profile = + Some(turn_ctx.permission_profile().map_err(|err| { + ThreadStoreError::Internal { + message: format!( + "failed to hydrate turn permission profile: {err}" + ), + } + })?); } RolloutItem::EventMsg(EventMsg::UserMessage(user)) => { if let Some(preview) = user_message_preview(user) { @@ -276,7 +296,7 @@ impl ThreadMetadataSync { | RolloutItem::Compacted(_) => {} } } - Some(update) + Ok(Some(update)) } fn merge_pending_update(&mut self, update: Option) { @@ -394,7 +414,8 @@ mod tests { RolloutItem::SessionMeta(session_meta(thread_id)), RolloutItem::EventMsg(EventMsg::UserMessage(user_message("hello metadata"))), ], - )); + )) + .expect("resume metadata should hydrate"); let update = sync.take_pending_update().expect("pending metadata update"); assert_eq!( @@ -433,7 +454,8 @@ mod tests { ))), RolloutItem::EventMsg(EventMsg::UserMessage(user_message("first user text"))), ], - )); + )) + .expect("resume metadata should hydrate"); let update = sync.take_pending_update().expect("pending metadata update"); assert_eq!(update.patch.preview.as_deref(), Some("ship the refactor")); @@ -452,7 +474,8 @@ mod tests { vec![RolloutItem::EventMsg(EventMsg::UserMessage(user_message( "first user text", )))], - )); + )) + .expect("resume metadata should hydrate"); let pending = sync.take_pending_update().expect("pending resume metadata"); sync.mark_pending_update_applied(&pending); @@ -460,6 +483,7 @@ mod tests { .observe_appended_items(&[RolloutItem::EventMsg(EventMsg::UserMessage(user_message( "later user text", )))]) + .expect("appended metadata should hydrate") .expect("updated_at touch"); assert_eq!(update.patch.preview, None); @@ -471,7 +495,8 @@ mod tests { #[test] fn metadata_irrelevant_items_coalesce_updated_at_touches() { let thread_id = ThreadId::new(); - let mut sync = ThreadMetadataSync::for_resume(&resume_params(thread_id, Vec::new())); + let mut sync = ThreadMetadataSync::for_resume(&resume_params(thread_id, Vec::new())) + .expect("resume metadata should hydrate"); let item = RolloutItem::Compacted(CompactedItem { message: "compacted".to_string(), replacement_history: None, @@ -480,12 +505,14 @@ mod tests { let first = sync .observe_appended_items(std::slice::from_ref(&item)) + .expect("appended metadata should hydrate") .expect("first touch should apply immediately"); assert!(first.patch.updated_at.is_some()); sync.mark_pending_update_applied(&first); assert!( sync.observe_appended_items(std::slice::from_ref(&item)) + .expect("appended metadata should hydrate") .is_none(), "second touch inside the coalescing window should wait for a barrier" ); @@ -504,7 +531,8 @@ mod tests { RolloutItem::SessionMeta(session_meta(thread_id)), RolloutItem::EventMsg(EventMsg::UserMessage(user_message("hello metadata"))), ], - )); + )) + .expect("resume metadata should hydrate"); assert!( sync.take_pending_update_for_existing_history().is_none(), @@ -514,6 +542,7 @@ mod tests { sync.observe_appended_items(&[RolloutItem::EventMsg(EventMsg::UserMessage( user_message("new append"), ))]) + .expect("appended metadata should hydrate") .is_some(), "the first append should flush resume metadata together with append metadata" ); diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index b835530d5b4..7956b65cfaf 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -164,6 +164,19 @@ impl PathUri { } } + /// Renders this URI using the native path syntax inferred from its shape. + /// + /// This is independent of the current host: a Windows URI renders with + /// Windows separators on every host. If the convention cannot be inferred + /// or the URI cannot be represented using that convention, the canonical + /// URI string is returned instead. + pub fn inferred_native_path_string(&self) -> String { + self.infer_path_convention() + .and_then(|convention| ApiPathString::from_path_uri(self, convention).ok()) + .map(ApiPathString::into_string) + .unwrap_or_else(|| self.to_string()) + } + /// Returns the decoded final URI path segment, or `None` for the URI root /// or an opaque fallback URI created by [`Self::from_abs_path`]. /// diff --git a/codex-rs/utils/path-uri/src/tests.rs b/codex-rs/utils/path-uri/src/tests.rs index fe436476ddd..1992cc9c75b 100644 --- a/codex-rs/utils/path-uri/src/tests.rs +++ b/codex-rs/utils/path-uri/src/tests.rs @@ -100,6 +100,28 @@ fn drive_shaped_posix_uri_is_intentionally_inferred_as_windows() { assert_eq!(path.infer_path_convention(), Some(PathConvention::Windows)); } +#[test] +fn inferred_native_path_string_uses_the_inferred_convention() { + for (uri, expected) in [ + ("file:///home/alice/a%20file.rs", "/home/alice/a file.rs"), + ( + "file:///C:/Users/Alice%20Smith/main.rs", + r"C:\Users\Alice Smith\main.rs", + ), + ("file://server/share/main.rs", r"\\server\share\main.rs"), + ("file://server/", "file://server/"), + ("file:///%00/bad/path/YQ", "file:///%00/bad/path/YQ"), + ] { + let path = PathUri::parse(uri).expect("valid path URI"); + + assert_eq!( + path.inferred_native_path_string(), + expected, + "rendering {uri}" + ); + } +} + #[cfg(windows)] #[test] fn file_uri_falls_back_for_windows_prefixes_without_a_uri_representation() { From c88a7df5392544e3a6166c92fce22fc6ca2c4362 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Tue, 16 Jun 2026 21:53:24 +0000 Subject: [PATCH 4/4] codex: address PR review feedback (#28152) --- codex-rs/protocol/src/protocol.rs | 21 +++++---------------- codex-rs/utils/path-uri/src/lib.rs | 4 ++-- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 7275c0667fc..b3ebbff1711 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -53,7 +53,6 @@ use crate::request_permissions::RequestPermissionsResponse; use crate::request_user_input::RequestUserInputResponse; use crate::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; use schemars::JsonSchema; use serde::Deserialize; @@ -3034,28 +3033,18 @@ impl TurnContextItem { let file_system_sandbox_policy = match self.file_system_sandbox_policy.clone() { Some(file_system_sandbox_policy) => file_system_sandbox_policy, None => { - let convention = self.cwd.infer_path_convention().ok_or_else(|| { + let cwd = self.cwd.to_abs_path().map_err(|err| { std::io::Error::new( - std::io::ErrorKind::InvalidInput, + err.kind(), format!( - "cannot infer a path convention for legacy permission profile cwd {}", + "cannot hydrate legacy permission profile for cwd {}: {err}", self.cwd ), ) })?; - if convention != PathConvention::native() { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - format!( - "cannot hydrate legacy permission profile for {convention} cwd {} on a {} host", - self.cwd, - PathConvention::native(), - ), - )); - } FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( &self.sandbox_policy, - self.cwd.to_abs_path()?.as_path(), + cwd.as_path(), ) } }; @@ -5353,7 +5342,7 @@ mod tests { let foreign_cwd = if cfg!(windows) { "file:///tmp" } else { - "file:///C:/windows" + "file://server/share" }; let item: TurnContextItem = serde_json::from_value(json!({ "cwd": foreign_cwd, diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index 7956b65cfaf..18142c7b195 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -172,8 +172,8 @@ impl PathUri { /// URI string is returned instead. pub fn inferred_native_path_string(&self) -> String { self.infer_path_convention() - .and_then(|convention| ApiPathString::from_path_uri(self, convention).ok()) - .map(ApiPathString::into_string) + .and_then(|convention| LegacyAppPathString::from_path_uri(self, convention).ok()) + .map(LegacyAppPathString::into_string) .unwrap_or_else(|| self.to_string()) }