From 3d02c443bccd5fc61a22947a05b75b673442fd87 Mon Sep 17 00:00:00 2001 From: "Adam Perry @ OpenAI" Date: Tue, 16 Jun 2026 19:05:26 -0700 Subject: [PATCH] [codex] core: restore absolute turn context cwd (#28629) ## Why #28152 jumped the gun on moving the rollout format to store URIs, and would likely break compat with some features that don't go through the same types as the core logic. ## What Make `TurnContextItem.cwd` an `AbsolutePathBuf` again, remove test added for `PathUri` serialization in rollouts. Also drops a bunch of error paths that are no longer needed. --- codex-rs/core/src/codex_thread.rs | 2 +- .../core/src/context/environment_context.rs | 35 ++++----- .../src/context/environment_context_tests.rs | 6 +- .../core/src/context_manager/history_tests.rs | 4 +- 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 | 55 ++++++-------- codex-rs/core/src/session/turn.rs | 12 +--- codex-rs/core/src/session/turn_context.rs | 10 +-- codex-rs/core/tests/suite/resume_warning.rs | 3 +- codex-rs/exec/src/lib.rs | 6 +- codex-rs/protocol/src/protocol.rs | 71 ++++--------------- codex-rs/rollout/src/metadata.rs | 2 +- codex-rs/rollout/src/metadata_tests.rs | 3 +- codex-rs/rollout/src/recorder.rs | 1 - codex-rs/state/src/extract.rs | 55 +++++--------- 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 ++++++------------ 21 files changed, 142 insertions(+), 282 deletions(-) diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index e19bad0eb..451b49782 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 fe5f9b1d0..e93ffb88f 100644 --- a/codex-rs/core/src/context/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -380,12 +380,12 @@ impl EnvironmentContext { pub(crate) fn diff_from_turn_context_item( before: &TurnContextItem, after: &EnvironmentContext, - ) -> std::io::Result { + ) -> Self { 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 before.cwd != environment.cwd { + if PathUri::from_abs_path(&before.cwd) != environment.cwd { EnvironmentContextEnvironments::Single(EnvironmentContextEnvironment::legacy( environment.cwd.clone(), environment.shell.clone(), @@ -409,14 +409,14 @@ impl EnvironmentContext { } else { before_filesystem }; - Ok(EnvironmentContext::new_with_environments( + 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 { @@ -440,18 +440,18 @@ impl EnvironmentContext { pub(crate) fn from_turn_context_item( turn_context_item: &TurnContextItem, shell: String, - ) -> std::io::Result { - Ok(Self::new_with_environments( + ) -> Self { + Self::new_with_environments( EnvironmentContextEnvironments::from_vec(vec![EnvironmentContextEnvironment::legacy( - turn_context_item.cwd.clone(), + PathUri::from_abs_path(&turn_context_item.cwd), 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 { @@ -498,11 +498,11 @@ impl EnvironmentContext { fn filesystem_from_turn_context_item( turn_context_item: &TurnContextItem, - ) -> std::io::Result> { - Ok(Some(FileSystemContext::from_permission_profile( - &turn_context_item.permission_profile()?, + ) -> Option { + Some(FileSystemContext::from_permission_profile( + &turn_context_item.permission_profile(), &workspace_roots_from_turn_context_item(turn_context_item), - ))) + )) } } @@ -513,12 +513,7 @@ fn workspace_roots_from_turn_context_item( return workspace_roots.clone(); } - // Older rollout items did not persist workspace roots. Fall back to the - // legacy cwd binding only when reconstructing that historical context. - match turn_context_item.cwd.to_abs_path() { - Ok(cwd) => vec![cwd], - Err(_) => Vec::new(), - } + vec![turn_context_item.cwd.clone()] } impl ContextualUserFragment for EnvironmentContext { diff --git a/codex-rs/core/src/context/environment_context_tests.rs b/codex-rs/core/src/context/environment_context_tests.rs index 383a38b55..1204a1d16 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: PathUri::from_abs_path(&test_abs_path("/not-the-workspace")), + cwd: test_abs_path("/not-the-workspace"), workspace_roots: Some(vec![repo.clone(), other_repo.clone()]), current_date: None, timezone: None, @@ -209,9 +209,7 @@ 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()) - .expect("turn context should hydrate") - .render(); + let context = EnvironmentContext::from_turn_context_item(&item, fake_shell_name()).render(); assert!( context.contains(&format!( diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index 0b538be11..908cb286f 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -21,9 +21,9 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::InterAgentCommunication; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::TurnContextItem; +use codex_utils_absolute_path::AbsolutePathBuf; 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; @@ -127,7 +127,7 @@ fn developer_msg_with_fragments(texts: &[&str]) -> ResponseItem { fn reference_context_item() -> TurnContextItem { TurnContextItem { turn_id: Some("reference-turn".to_string()), - cwd: PathUri::from_path( + cwd: AbsolutePathBuf::try_from( std::env::current_dir() .expect("current directory") .join("reference-cwd"), diff --git a/codex-rs/core/src/context_manager/updates.rs b/codex-rs/core/src/context_manager/updates.rs index d9c0b2f03..0ce02240b 100644 --- a/codex-rs/core/src/context_manager/updates.rs +++ b/codex-rs/core/src/context_manager/updates.rs @@ -22,44 +22,40 @@ fn build_environment_update_item( previous: Option<&TurnContextItem>, next: &TurnContext, shell: &Shell, -) -> std::io::Result> { +) -> Option { if !next.config.include_environment_context { - return Ok(None); + return None; } - let Some(prev) = previous else { - return Ok(None); - }; - let prev_context = EnvironmentContext::from_turn_context_item(prev, shell.name().to_string())?; + let prev = previous?; + 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 Ok(None); + return None; } - Ok(Some(ContextualUserFragment::into( - EnvironmentContext::diff_from_turn_context_item(prev, &next_context)?, - ))) + 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, -) -> std::io::Result> { +) -> Option { if !next.config.include_permissions_instructions { - return Ok(None); + return None; } - let Some(prev) = previous else { - return Ok(None); - }; - if prev.permission_profile()? == next.permission_profile() + let prev = previous?; + if prev.permission_profile() == next.permission_profile() && prev.approval_policy == next.approval_policy.value() { - return Ok(None); + return None; } - Ok(Some( + Some( PermissionsInstructions::from_permission_profile( &next.permission_profile, next.approval_policy.value(), @@ -71,7 +67,7 @@ fn build_permissions_update_item( next.features.enabled(Feature::RequestPermissionsTool), ) .render(), - )) + ) } fn build_collaboration_mode_update_item( @@ -218,17 +214,17 @@ pub(crate) fn build_settings_update_items( shell: &Shell, exec_policy: &Policy, personality_feature_enabled: bool, -) -> std::io::Result> { +) -> Vec { // 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), @@ -244,5 +240,5 @@ pub(crate) fn build_settings_update_items( if let Some(contextual_user_message) = contextual_user_message { items.push(contextual_user_message); } - Ok(items) + items } diff --git a/codex-rs/core/src/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs index f3da9b93f..0da42d8a4 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 5a2e33bbf..d632609c0 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1621,7 +1621,7 @@ impl Session { &self, reference_context_item: Option<&TurnContextItem>, current_context: &TurnContext, - ) -> CodexResult> { + ) -> Vec { // 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 @@ -1632,15 +1632,13 @@ impl Session { }; let shell = self.user_shell(); let exec_policy = self.services.exec_policy.current(); - 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), - )?, + 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), ) } @@ -3188,7 +3186,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() @@ -3199,7 +3197,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() { @@ -3215,7 +3213,6 @@ 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 67632aa53..084553f15 100644 --- a/codex-rs/core/src/session/rollout_reconstruction_tests.rs +++ b/codex-rs/core/src/session/rollout_reconstruction_tests.rs @@ -9,7 +9,6 @@ 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; @@ -89,7 +88,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: PathUri::from_abs_path(&turn_context.cwd), + cwd: turn_context.cwd.clone(), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -129,7 +128,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: PathUri::from_abs_path(&turn_context.cwd), + cwd: turn_context.cwd.clone(), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -990,7 +989,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: PathUri::from_abs_path(&turn_context.cwd), + cwd: turn_context.cwd.clone(), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -1072,7 +1071,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: PathUri::from_abs_path(&turn_context.cwd), + cwd: turn_context.cwd.clone(), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -1102,7 +1101,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: PathUri::from_abs_path(&turn_context.cwd), + cwd: turn_context.cwd.clone(), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -1224,7 +1223,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: PathUri::from_abs_path(&turn_context.cwd), + cwd: turn_context.cwd.clone(), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -1344,7 +1343,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: PathUri::from_abs_path(&turn_context.cwd), + cwd: turn_context.cwd.clone(), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -1507,7 +1506,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: PathUri::from_abs_path(&turn_context.cwd), + cwd: turn_context.cwd.clone(), 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 96044ed3a..b7ddb90b1 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -1874,8 +1874,7 @@ async fn resumed_history_injects_initial_context_on_first_context_update_only() session .record_context_updates_and_set_reference_context_item(&turn_context) - .await - .expect("context updates should hydrate"); + .await; let initial_context = session.build_initial_context(&turn_context).await; expected.extend(initial_context); let history_after_seed = session.clone_history().await; @@ -1883,8 +1882,7 @@ async fn resumed_history_injects_initial_context_on_first_context_update_only() session .record_context_updates_and_set_reference_context_item(&turn_context) - .await - .expect("context updates should hydrate"); + .await; let history_after_second_seed = session.clone_history().await; assert_eq!( history_after_seed.raw_items(), @@ -2675,7 +2673,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: PathUri::from_abs_path(&turn_context.cwd), + cwd: turn_context.cwd.clone(), workspace_roots: None, current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), @@ -7292,8 +7290,7 @@ 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 - .expect("settings updates should hydrate"); + .await; let environment_update = user_input_texts(&update_items) .into_iter() @@ -7363,8 +7360,7 @@ 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 - .expect("settings updates should hydrate"); + .await; let environment_update = user_input_texts(&update_items) .into_iter() @@ -7392,8 +7388,7 @@ 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 - .expect("settings updates should hydrate"); + .await; let user_texts = user_input_texts(&update_items); assert!( @@ -7421,8 +7416,7 @@ async fn build_settings_update_items_emits_realtime_start_when_session_becomes_l Some(&previous_context.to_turn_context_item()), ¤t_context, ) - .await - .expect("settings updates should hydrate"); + .await; let developer_texts = developer_input_texts(&update_items); assert!( @@ -7450,8 +7444,7 @@ async fn build_settings_update_items_emits_realtime_end_when_session_stops_being Some(&previous_context.to_turn_context_item()), ¤t_context, ) - .await - .expect("settings updates should hydrate"); + .await; let developer_texts = developer_input_texts(&update_items); assert!( @@ -7485,8 +7478,7 @@ 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 - .expect("settings updates should hydrate"); + .await; let developer_texts = developer_input_texts(&update_items); assert!( @@ -8139,18 +8131,20 @@ async fn turn_context_item_uses_turn_context_comp_hash_snapshot() { } #[tokio::test] -async fn turn_context_item_stores_primary_environment_cwd_uri() { +async fn turn_context_item_stores_local_cwd() { 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(), + cwd, environment.shell, ); - assert_eq!(turn_context.to_turn_context_item().cwd, cwd); + #[allow(deprecated)] + let local_cwd = turn_context.cwd.clone(); + assert_eq!(turn_context.to_turn_context_item().cwd, local_cwd); } #[tokio::test] @@ -8194,8 +8188,7 @@ 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 - .expect("context updates should hydrate"); + .await; 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); @@ -8226,8 +8219,7 @@ 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 - .expect("context updates should hydrate"); + .await; { let mut state = session.state.lock().await; state.set_reference_context_item(/*item*/ None); @@ -8241,8 +8233,7 @@ 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 - .expect("context updates should hydrate"); + .await; let history = session.clone_history().await; let mut expected_history = vec![compacted_summary]; @@ -8272,14 +8263,12 @@ 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 - .expect("settings updates should hydrate"); + .await; assert_eq!(update_items, Vec::new()); session .record_context_updates_and_set_reference_context_item(&turn_context) - .await - .expect("context updates should hydrate"); + .await; assert_eq!( session.clone_history().await.raw_items().to_vec(), @@ -8326,8 +8315,7 @@ 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 - .expect("context updates should hydrate"); + .await; session.ensure_rollout_materialized().await; session.flush_rollout().await.expect("rollout should flush"); @@ -8411,8 +8399,7 @@ 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 - .expect("context updates should hydrate"); + .await; 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 d70a9cc8d..4b052cb65 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -159,16 +159,8 @@ pub(crate) async fn run_turn( return None; } - 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; - } + sess.record_context_updates_and_set_reference_context_item(turn_context.as_ref()) + .await; 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 25ad962fd..526a16bac 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -396,14 +396,8 @@ 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) - }); + #[allow(deprecated)] + let cwd = self.cwd.clone(); TurnContextItem { turn_id: Some(self.sub_id.clone()), cwd, diff --git a/codex-rs/core/tests/suite/resume_warning.rs b/codex-rs/core/tests/suite/resume_warning.rs index 001c11299..546219ce9 100644 --- a/codex-rs/core/tests/suite/resume_warning.rs +++ b/codex-rs/core/tests/suite/resume_warning.rs @@ -14,7 +14,6 @@ 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; @@ -28,7 +27,7 @@ fn resume_history( let turn_id = "resume-warning-seed-turn".to_string(); let turn_ctx = TurnContextItem { turn_id: Some(turn_id.clone()), - cwd: PathUri::from_abs_path(&config.cwd), + cwd: config.cwd.clone(), 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 c7ab8cfa0..482785c5c 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -1423,11 +1423,7 @@ async fn parse_latest_turn_context_cwd(path: &Path) -> Option { continue; }; if let RolloutItem::TurnContext(item) = rollout_line.item { - return item - .cwd - .to_abs_path() - .ok() - .map(AbsolutePathBuf::into_path_buf); + return Some(item.cwd.into_path_buf()); } } None diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index b3ebbff17..0a9271c9f 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -2988,7 +2988,7 @@ pub struct TurnContextNetworkItem { pub struct TurnContextItem { #[serde(default, skip_serializing_if = "Option::is_none")] pub turn_id: Option, - pub cwd: PathUri, + pub cwd: AbsolutePathBuf, /// Effective workspace roots used to materialize symbolic /// `:workspace_roots` filesystem permissions in `permission_profile`. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -3026,35 +3026,21 @@ pub struct TurnContextItem { } impl TurnContextItem { - 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 cwd = self.cwd.to_abs_path().map_err(|err| { - std::io::Error::new( - err.kind(), - format!( - "cannot hydrate legacy permission profile for cwd {}: {err}", - self.cwd - ), + 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.as_path(), ) - })?; - FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( - &self.sandbox_policy, - cwd.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), - ), - ) + ) + }) } } @@ -5315,50 +5301,21 @@ mod tests { } #[test] - fn turn_context_item_accepts_legacy_cwd_and_serializes_path_uri() -> Result<()> { - let legacy_cwd = test_path_buf("/tmp"); + fn turn_context_item_deserializes_without_network() -> Result<()> { let item: TurnContextItem = serde_json::from_value(json!({ - "cwd": legacy_cwd, + "cwd": test_path_buf("/tmp"), "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://server/share" - }; - 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")?; @@ -5396,7 +5353,7 @@ mod tests { fn turn_context_item_serializes_network_when_present() -> Result<()> { let item = TurnContextItem { turn_id: None, - cwd: PathUri::from_abs_path(&test_path_buf("/tmp").abs()), + cwd: 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 61d89f09d..dcb38b6d2 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 e28ae84df..01e56f569 100644 --- a/codex-rs/rollout/src/metadata_tests.rs +++ b/codex-rs/rollout/src/metadata_tests.rs @@ -69,8 +69,7 @@ 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") - .expect("rollout item should apply"); + apply_rollout_item(&mut expected, &rollout_line.item, "openai"); expected.updated_at = file_modified_time_utc(&path).await.expect("mtime"); expected.recency_at = expected.updated_at; diff --git a/codex-rs/rollout/src/recorder.rs b/codex-rs/rollout/src/recorder.rs index 113ee372e..7ba111e94 100644 --- a/codex-rs/rollout/src/recorder.rs +++ b/codex-rs/rollout/src/recorder.rs @@ -1827,7 +1827,6 @@ async fn resume_candidate_matches_cwd( | 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.as_path(), cwd); } diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index f2745a553..4d25965c4 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,7 +28,6 @@ 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. @@ -73,21 +72,15 @@ fn apply_session_meta_from_item(metadata: &mut ThreadMetadata, meta_line: &Sessi } } -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(); +fn apply_turn_context(metadata: &mut ThreadMetadata, turn_ctx: &TurnContextItem) { + if metadata.cwd.as_os_str().is_empty() { + metadata.cwd = turn_ctx.cwd.clone().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) { @@ -201,8 +194,7 @@ mod tests { metadata: None, }); - apply_rollout_item(&mut metadata, &item, "test-provider") - .expect("rollout item should apply"); + apply_rollout_item(&mut metadata, &item, "test-provider"); assert_eq!(metadata.first_user_message, None); assert_eq!(metadata.preview, None); @@ -221,8 +213,7 @@ mod tests { ..Default::default() })); - apply_rollout_item(&mut metadata, &item, "test-provider") - .expect("rollout item should apply"); + apply_rollout_item(&mut metadata, &item, "test-provider"); assert_eq!( metadata.first_user_message.as_deref(), @@ -244,8 +235,7 @@ mod tests { ..Default::default() })); - apply_rollout_item(&mut metadata, &item, "test-provider") - .expect("rollout item should apply"); + apply_rollout_item(&mut metadata, &item, "test-provider"); assert_eq!( metadata.first_user_message.as_deref(), @@ -270,8 +260,7 @@ mod tests { ..Default::default() })); - apply_rollout_item(&mut metadata, &item, "test-provider") - .expect("rollout item should apply"); + apply_rollout_item(&mut metadata, &item, "test-provider"); assert_eq!(metadata.first_user_message, None); assert_eq!(metadata.preview, None); @@ -297,8 +286,7 @@ mod tests { }, })); - apply_rollout_item(&mut metadata, &goal_item, "test-provider") - .expect("rollout item should apply"); + apply_rollout_item(&mut metadata, &goal_item, "test-provider"); assert_eq!(metadata.preview.as_deref(), Some("optimize the benchmark")); assert_eq!(metadata.first_user_message, None); @@ -313,8 +301,7 @@ mod tests { ..Default::default() })); - apply_rollout_item(&mut metadata, &user_item, "test-provider") - .expect("rollout item should apply"); + apply_rollout_item(&mut metadata, &user_item, "test-provider"); assert_eq!(metadata.preview.as_deref(), Some("optimize the benchmark")); assert_eq!( @@ -357,8 +344,7 @@ mod tests { git: None, }), "test-provider", - ) - .expect("rollout item should apply"); + ); apply_rollout_item( &mut metadata, &RolloutItem::TurnContext(TurnContextItem { @@ -387,8 +373,7 @@ 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; @@ -432,8 +417,7 @@ mod tests { summary: codex_protocol::config_types::ReasoningSummary::Auto, }), "test-provider", - ) - .expect("rollout item should apply"); + ); assert_eq!( metadata.sandbox_policy, @@ -473,8 +457,7 @@ mod tests { summary: codex_protocol::config_types::ReasoningSummary::Auto, }), "test-provider", - ) - .expect("rollout item should apply"); + ); assert_eq!(metadata.cwd, fallback_cwd); } @@ -511,8 +494,7 @@ 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)); @@ -548,8 +530,7 @@ 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 7bf354437..576f76026 100644 --- a/codex-rs/state/src/runtime/threads.rs +++ b/codex-rs/state/src/runtime/threads.rs @@ -883,7 +883,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 0e81b165a..88d347f57 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 6705f4574..b2953d7ef 100644 --- a/codex-rs/thread-store/src/thread_metadata_sync.rs +++ b/codex-rs/thread-store/src/thread_metadata_sync.rs @@ -18,8 +18,6 @@ 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))] @@ -92,7 +90,7 @@ impl ThreadMetadataSync { } } - pub(crate) fn for_resume(params: &ResumeThreadParams) -> ThreadStoreResult { + pub(crate) fn for_resume(params: &ResumeThreadParams) -> Self { let mut sync = Self { thread_id: params.thread_id, cwd_seen: params @@ -110,11 +108,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(); } - Ok(sync) + sync } pub(crate) fn take_pending_update(&self) -> Option { @@ -150,7 +148,7 @@ impl ThreadMetadataSync { pub(crate) fn observe_appended_items( &mut self, items: &[RolloutItem], - ) -> ThreadStoreResult> { + ) -> Option { self.defer_create_update_until_history_exists = false; self.defer_resume_update_until_append = false; let affects_metadata = items @@ -162,10 +160,7 @@ impl ThreadMetadataSync { let mut update = if affects_metadata { self.observe_items(items)? } else { - Some(thread_updated_at_touch()) - }; - let Some(update) = update else { - return Ok(None); + thread_updated_at_touch() }; if advances_recency { update.advance_recency_at = Some(Utc::now()); @@ -180,15 +175,12 @@ impl ThreadMetadataSync { Instant::now().duration_since(last_touch) < THREAD_UPDATED_AT_TOUCH_INTERVAL }) { - return Ok(None); + return None; } - Ok(self.take_pending_update()) + self.take_pending_update() } - fn observe_items( - &mut self, - items: &[RolloutItem], - ) -> ThreadStoreResult> { + fn observe_items(&mut self, items: &[RolloutItem]) -> Option { self.observe_items_with_update( items, ThreadMetadataPatch { @@ -198,10 +190,7 @@ impl ThreadMetadataSync { ) } - fn observe_resume_history( - &mut self, - items: &[RolloutItem], - ) -> ThreadStoreResult> { + fn observe_resume_history(&mut self, items: &[RolloutItem]) -> Option { self.observe_items_with_update(items, ThreadMetadataPatch::default()) } @@ -209,9 +198,9 @@ impl ThreadMetadataSync { &mut self, items: &[RolloutItem], mut update: ThreadMetadataPatch, - ) -> ThreadStoreResult> { + ) -> Option { if items.is_empty() { - return Ok(None); + return None; } for item in items { match item { @@ -244,23 +233,14 @@ impl ThreadMetadataSync { } } RolloutItem::TurnContext(turn_ctx) => { - if !self.cwd_seen - && let Ok(cwd) = turn_ctx.cwd.to_abs_path() - { + if !self.cwd_seen { self.cwd_seen = true; - update.cwd = Some(cwd.into_path_buf()); + update.cwd = Some(turn_ctx.cwd.clone().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().map_err(|err| { - ThreadStoreError::Internal { - message: format!( - "failed to hydrate turn permission profile: {err}" - ), - } - })?); + update.permission_profile = Some(turn_ctx.permission_profile()); } RolloutItem::EventMsg(EventMsg::UserMessage(user)) => { if let Some(preview) = user_message_preview(user) { @@ -302,7 +282,7 @@ impl ThreadMetadataSync { | RolloutItem::Compacted(_) => {} } } - Ok(Some(update)) + Some(update) } fn merge_pending_update(&mut self, update: Option) { @@ -422,8 +402,7 @@ 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!( @@ -462,8 +441,7 @@ 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")); @@ -482,8 +460,7 @@ 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); @@ -491,7 +468,6 @@ 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); @@ -503,8 +479,7 @@ 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())) - .expect("resume metadata should hydrate"); + let mut sync = ThreadMetadataSync::for_resume(&resume_params(thread_id, Vec::new())); let item = RolloutItem::Compacted(CompactedItem { message: "compacted".to_string(), replacement_history: None, @@ -513,14 +488,12 @@ 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" ); @@ -560,8 +533,7 @@ 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(), @@ -571,7 +543,6 @@ 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" );