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" );