From 0746e8a34574b4bf4721672c97fc6a94fd8bfad8 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 8 Jul 2026 17:58:28 -0700 Subject: [PATCH] [codex] Preserve reviewer when resuming threads (#30278) ## Why A thread resumed without an explicit reviewer could pick up the reviewer from the current config instead of preserving the reviewer already in use by the thread. After an app restart, this meant a thread running with auto review could silently switch back to user review, and the next turn could continue under the wrong reviewer. ## What changed Persist the effective reviewer with each turn and restore the latest persisted value when the thread resumes. If the resume request explicitly provides a reviewer, that value still takes precedence. ## Test plan - Added a regression test that starts a thread with auto review, records a turn, restarts with user review in config, resumes without an override, and verifies that auto review is preserved. - `just test -p codex-protocol` - `just test -p codex-state` - `just test -p codex-rollout` - `just test -p codex-app-server thread_resume_preserves_persisted_approvals_reviewer` - Clippy for the affected crates --- .../request_processors/thread_processor.rs | 33 +++ .../tests/suite/v2/thread_resume.rs | 189 ++++++++++++++++++ .../core/src/context_manager/history_tests.rs | 1 + codex-rs/core/src/session/handlers.rs | 29 ++- codex-rs/core/src/session/mod.rs | 24 ++- .../session/rollout_reconstruction_tests.rs | 8 + codex-rs/core/src/session/tests.rs | 1 + codex-rs/core/src/session/turn_context.rs | 1 + codex-rs/core/tests/suite/resume_warning.rs | 1 + codex-rs/exec/src/lib.rs | 29 ++- codex-rs/exec/src/lib_tests.rs | 45 ++++- codex-rs/protocol/src/protocol.rs | 3 + codex-rs/rollout/src/policy.rs | 2 +- codex-rs/rollout/src/recorder_tests.rs | 1 + codex-rs/state/src/extract.rs | 4 + 15 files changed, 347 insertions(+), 24 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index af8525873f..88c8625642 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -164,6 +164,34 @@ fn merge_persisted_resume_metadata( } } +fn merge_persisted_approvals_reviewer( + thread_history: &InitialHistory, + request_overrides: Option<&HashMap>, + typesafe_overrides: &mut ConfigOverrides, +) { + if typesafe_overrides.approvals_reviewer.is_some() + || request_overrides.is_some_and(|overrides| overrides.contains_key("approvals_reviewer")) + { + return; + } + + let InitialHistory::Resumed(resumed_history) = thread_history else { + return; + }; + typesafe_overrides.approvals_reviewer = + resumed_history + .history + .iter() + .rev() + .find_map(|item| match item { + RolloutItem::TurnContext(turn_context) => turn_context.approvals_reviewer, + RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => { + Some(event.thread_settings.approvals_reviewer) + } + _ => None, + }); +} + fn normalize_thread_list_cwd_filters( cwd: Option, ) -> Result>, JSONRPCErrorError> { @@ -2957,6 +2985,11 @@ impl ThreadRequestProcessor { request_overrides: &mut Option>, typesafe_overrides: &mut ConfigOverrides, ) -> Option { + merge_persisted_approvals_reviewer( + thread_history, + request_overrides.as_ref(), + typesafe_overrides, + ); let InitialHistory::Resumed(resumed_history) = thread_history else { return None; }; diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index a4ac03ee7d..8337575e52 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -14,6 +14,7 @@ use app_test_support::test_absolute_path; use app_test_support::to_response; use app_test_support::write_chatgpt_auth; use chrono::Utc; +use codex_app_server_protocol::ApprovalsReviewer; use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::ClientInfo; use codex_app_server_protocol::CommandExecutionApprovalDecision; @@ -42,6 +43,8 @@ use codex_app_server_protocol::ThreadReadResponse; use codex_app_server_protocol::ThreadResumeInitialTurnsPageParams; use codex_app_server_protocol::ThreadResumeParams; use codex_app_server_protocol::ThreadResumeResponse; +use codex_app_server_protocol::ThreadSettingsUpdateParams; +use codex_app_server_protocol::ThreadSettingsUpdateResponse; use codex_app_server_protocol::ThreadSource; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; @@ -436,6 +439,192 @@ async fn turn_start_updates_runtime_workspace_roots_for_loaded_thread() -> Resul Ok(()) } +#[tokio::test] +async fn thread_resume_preserves_persisted_approvals_reviewer() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + + let thread_id = { + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build() + .await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("gpt-5.4".to_string()), + approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + ..Default::default() + }) + .await?; + let start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + client_user_message_id: None, + input: vec![UserInput::Text { + text: "materialize this thread".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + thread.id + }; + + let config_path = codex_home.path().join("config.toml"); + let config = std::fs::read_to_string(&config_path)?; + std::fs::write( + config_path, + config.replace( + "approval_policy = \"never\"\n", + "approval_policy = \"never\"\napprovals_reviewer = \"user\"\n", + ), + )?; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build() + .await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + let resume_id = mcp + .send_thread_resume_request(ThreadResumeParams { + thread_id, + ..Default::default() + }) + .await?; + let resume_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), + ) + .await??; + let ThreadResumeResponse { + approvals_reviewer, .. + } = to_response::(resume_resp)?; + + assert_eq!(approvals_reviewer, ApprovalsReviewer::AutoReview); + + Ok(()) +} + +#[tokio::test] +async fn thread_resume_preserves_acknowledged_approvals_reviewer_update() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + + let thread_id = { + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build() + .await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("gpt-5.4".to_string()), + ..Default::default() + }) + .await?; + let start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + client_user_message_id: None, + input: vec![UserInput::Text { + text: "materialize this thread".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let update_id = mcp + .send_thread_settings_update_request(ThreadSettingsUpdateParams { + thread_id: thread.id.clone(), + approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + ..Default::default() + }) + .await?; + let update_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(update_id)), + ) + .await??; + let _: ThreadSettingsUpdateResponse = to_response(update_resp)?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("thread/settings/updated"), + ) + .await??; + + thread.id + }; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build() + .await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + let resume_id = mcp + .send_thread_resume_request(ThreadResumeParams { + thread_id, + ..Default::default() + }) + .await?; + let resume_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), + ) + .await??; + let ThreadResumeResponse { + approvals_reviewer, .. + } = to_response::(resume_resp)?; + + assert_eq!(approvals_reviewer, ApprovalsReviewer::AutoReview); + + Ok(()) +} + #[tokio::test] async fn thread_goal_get_rejects_unmaterialized_thread() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index 679e1a69f7..68b3134810 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -223,6 +223,7 @@ fn reference_context_item() -> TurnContextItem { current_date: Some("2026-03-23".to_string()), timezone: Some("America/Los_Angeles".to_string()), approval_policy: AskForApproval::OnRequest, + approvals_reviewer: None, sandbox_policy: SandboxPolicy::new_read_only_policy(), permission_profile: None, network: None, diff --git a/codex-rs/core/src/session/handlers.rs b/codex-rs/core/src/session/handlers.rs index 924750bc34..cc21755524 100644 --- a/codex-rs/core/src/session/handlers.rs +++ b/codex-rs/core/src/session/handlers.rs @@ -95,14 +95,25 @@ pub async fn update_thread_settings( thread_settings: ThreadSettingsOverrides, ) { let updates = thread_settings_update(sess, thread_settings).await; - let msg = match sess.update_settings(updates).await { - Ok(()) => thread_settings_applied_event(sess).await, - Err(err) => EventMsg::Error(ErrorEvent { - message: format!("invalid thread settings override: {err}"), - codex_error_info: Some(CodexErrorInfo::BadRequest), - }), - }; - sess.send_event_raw(Event { id: sub_id, msg }).await; + match sess.update_settings(updates).await { + Ok(()) => { + sess.send_event_raw_without_materializing_rollout(Event { + id: sub_id, + msg: thread_settings_applied_event(sess).await, + }) + .await; + } + Err(err) => { + sess.send_event_raw(Event { + id: sub_id, + msg: EventMsg::Error(ErrorEvent { + message: format!("invalid thread settings override: {err}"), + codex_error_info: Some(CodexErrorInfo::BadRequest), + }), + }) + .await; + } + } } async fn thread_settings_update( @@ -209,7 +220,7 @@ pub(super) async fn user_input_or_turn_inner( return; }; if emit_thread_settings_applied { - sess.send_event_raw(Event { + sess.send_event_raw_without_materializing_rollout(Event { id: sub_id.clone(), msg: thread_settings_applied_event(sess).await, }) diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 1efdc44189..b4f68443e2 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1941,9 +1941,29 @@ impl Session { } pub(crate) async fn send_event_raw(&self, event: Event) { + self.send_event_raw_with_persistence(event, /*persist*/ true) + .await; + } + + /// Delivers an event without creating a local rollout for a thread that has not materialized. + pub(crate) async fn send_event_raw_without_materializing_rollout(&self, event: Event) { + let persist = match self.current_rollout_path().await { + Ok(Some(path)) => codex_rollout::existing_rollout_path(&path).await.is_some(), + Ok(None) => true, + Err(err) => { + warn!("failed to check whether thread persistence is materialized: {err}"); + true + } + }; + self.send_event_raw_with_persistence(event, persist).await; + } + + async fn send_event_raw_with_persistence(&self, event: Event, persist: bool) { // Persist the event into rollout storage (the store filters as needed). - let rollout_items = vec![RolloutItem::EventMsg(event.msg.clone())]; - self.persist_rollout_items(&rollout_items).await; + if persist { + let rollout_items = vec![RolloutItem::EventMsg(event.msg.clone())]; + self.persist_rollout_items(&rollout_items).await; + } self.services .rollout_thread_trace .record_protocol_event(&event.msg); diff --git a/codex-rs/core/src/session/rollout_reconstruction_tests.rs b/codex-rs/core/src/session/rollout_reconstruction_tests.rs index a7d2a7efc1..460ae21d1c 100644 --- a/codex-rs/core/src/session/rollout_reconstruction_tests.rs +++ b/codex-rs/core/src/session/rollout_reconstruction_tests.rs @@ -172,6 +172,7 @@ async fn record_initial_history_resumed_bare_turn_context_does_not_hydrate_previ current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), approval_policy: turn_context.approval_policy.value(), + approvals_reviewer: None, sandbox_policy: turn_context.sandbox_policy(), permission_profile: None, network: None, @@ -218,6 +219,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), approval_policy: turn_context.approval_policy.value(), + approvals_reviewer: None, sandbox_policy: turn_context.sandbox_policy(), permission_profile: None, network: None, @@ -1252,6 +1254,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), approval_policy: turn_context.approval_policy.value(), + approvals_reviewer: None, sandbox_policy: turn_context.sandbox_policy(), permission_profile: None, network: None, @@ -1338,6 +1341,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), approval_policy: turn_context.approval_policy.value(), + approvals_reviewer: None, sandbox_policy: turn_context.sandbox_policy(), permission_profile: None, network: None, @@ -1369,6 +1373,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), approval_policy: turn_context.approval_policy.value(), + approvals_reviewer: None, sandbox_policy: turn_context.sandbox_policy(), permission_profile: None, network: None, @@ -1495,6 +1500,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), approval_policy: turn_context.approval_policy.value(), + approvals_reviewer: None, sandbox_policy: turn_context.sandbox_policy(), permission_profile: None, network: None, @@ -1616,6 +1622,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), approval_policy: turn_context.approval_policy.value(), + approvals_reviewer: None, sandbox_policy: turn_context.sandbox_policy(), permission_profile: None, network: None, @@ -1783,6 +1790,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), approval_policy: turn_context.approval_policy.value(), + approvals_reviewer: None, sandbox_policy: turn_context.sandbox_policy(), permission_profile: None, network: None, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 7274d583e8..9828b71e76 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -3082,6 +3082,7 @@ async fn record_initial_history_forked_hydrates_previous_turn_settings() { current_date: turn_context.current_date.clone(), timezone: turn_context.timezone.clone(), approval_policy: turn_context.approval_policy.value(), + approvals_reviewer: None, sandbox_policy: turn_context.sandbox_policy(), permission_profile: None, network: None, diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index b9bf42073b..309ac8401e 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -361,6 +361,7 @@ impl TurnContext { current_date: self.current_date.clone(), timezone: self.timezone.clone(), approval_policy: self.approval_policy.value(), + approvals_reviewer: Some(self.config.approvals_reviewer), sandbox_policy: self.sandbox_policy(), permission_profile: Some(self.permission_profile()), network: self.turn_context_network_item(), diff --git a/codex-rs/core/tests/suite/resume_warning.rs b/codex-rs/core/tests/suite/resume_warning.rs index 50395b2ecb..3e135abf4d 100644 --- a/codex-rs/core/tests/suite/resume_warning.rs +++ b/codex-rs/core/tests/suite/resume_warning.rs @@ -34,6 +34,7 @@ fn resume_history( current_date: None, timezone: None, approval_policy: config.permissions.approval_policy.value(), + approvals_reviewer: None, sandbox_policy: config.legacy_sandbox_policy(), permission_profile: None, network: None, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 7ea8461b00..1d9fd2e56d 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -206,6 +206,7 @@ struct ExecRunArgs { state_db: Option, command: Option, config: Config, + resume_approvals_reviewer_override: Option, dangerously_bypass_approvals_and_sandbox: bool, exec_span: tracing::Span, images: Vec, @@ -463,6 +464,10 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result build_config, ) .await?; + let resume_approvals_reviewer_override = cli_kv_overrides + .iter() + .any(|(key, _)| key == "approvals_reviewer") + .then(|| config.approvals_reviewer.into()); #[allow(clippy::print_stderr)] match check_execpolicy_for_warnings(&config.config_layer_stack).await { @@ -576,6 +581,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result state_db, command, config, + resume_approvals_reviewer_override, dangerously_bypass_approvals_and_sandbox, exec_span: exec_span.clone(), images, @@ -673,6 +679,7 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { state_db, command, config, + resume_approvals_reviewer_override, dangerously_bypass_approvals_and_sandbox, exec_span, images, @@ -804,7 +811,11 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { &client, ClientRequest::ThreadResume { request_id: request_ids.next(), - params: thread_resume_params_from_config(&config, thread_id), + params: thread_resume_params_from_config( + &config, + thread_id, + resume_approvals_reviewer_override, + ), }, "thread/resume", ) @@ -1066,7 +1077,7 @@ fn thread_start_params_from_config(config: &Config) -> ThreadStartParams { cwd: Some(config.cwd.to_string_lossy().to_string()), runtime_workspace_roots: Some(config.workspace_roots.clone()), approval_policy: Some(config.permissions.approval_policy.value().into()), - approvals_reviewer: approvals_reviewer_override_from_config(config), + approvals_reviewer: Some(config.approvals_reviewer.into()), sandbox: sandbox.flatten(), permissions, config: thread_config_overrides_from_config(config), @@ -1076,7 +1087,11 @@ fn thread_start_params_from_config(config: &Config) -> ThreadStartParams { } } -fn thread_resume_params_from_config(config: &Config, thread_id: String) -> ThreadResumeParams { +fn thread_resume_params_from_config( + config: &Config, + thread_id: String, + approvals_reviewer_override: Option, +) -> ThreadResumeParams { let permissions = permissions_selection_from_config(config); let sandbox = permissions.is_none().then(|| { sandbox_mode_from_permission_profile( @@ -1091,7 +1106,7 @@ fn thread_resume_params_from_config(config: &Config, thread_id: String) -> Threa cwd: Some(config.cwd.to_string_lossy().to_string()), runtime_workspace_roots: Some(config.workspace_roots.clone()), approval_policy: Some(config.permissions.approval_policy.value().into()), - approvals_reviewer: approvals_reviewer_override_from_config(config), + approvals_reviewer: approvals_reviewer_override, sandbox: sandbox.flatten(), permissions, config: thread_config_overrides_from_config(config), @@ -1141,12 +1156,6 @@ fn sandbox_mode_from_permission_profile( } } -fn approvals_reviewer_override_from_config( - config: &Config, -) -> Option { - Some(config.approvals_reviewer.into()) -} - async fn send_request_with_response( client: &InProcessAppServerClient, request: ClientRequest, diff --git a/codex-rs/exec/src/lib_tests.rs b/codex-rs/exec/src/lib_tests.rs index 97ded50525..fc1b713c92 100644 --- a/codex-rs/exec/src/lib_tests.rs +++ b/codex-rs/exec/src/lib_tests.rs @@ -491,6 +491,39 @@ async fn thread_start_params_include_review_policy_when_auto_review_is_enabled() ); } +#[tokio::test] +async fn thread_resume_params_only_include_explicit_review_policy_override() { + let codex_home = tempdir().expect("create temp codex home"); + let cwd = tempdir().expect("create temp cwd"); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .harness_overrides(ConfigOverrides { + approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + ..Default::default() + }) + .fallback_cwd(Some(cwd.path().to_path_buf())) + .build() + .await + .expect("build config with guardian review policy"); + + let params_without_override = thread_resume_params_from_config( + &config, + "thread-id".to_string(), + /*approvals_reviewer_override*/ None, + ); + let params_with_override = thread_resume_params_from_config( + &config, + "thread-id".to_string(), + Some(codex_app_server_protocol::ApprovalsReviewer::AutoReview), + ); + + assert_eq!(params_without_override.approvals_reviewer, None); + assert_eq!( + params_with_override.approvals_reviewer, + Some(codex_app_server_protocol::ApprovalsReviewer::AutoReview) + ); +} + #[tokio::test] async fn build_exec_config_retries_without_invalid_headless_policy_for_auto_review() { let codex_home = tempdir().expect("create temp codex home"); @@ -616,7 +649,11 @@ async fn thread_lifecycle_params_preserve_hook_trust_bypass() { )])); let start_params = thread_start_params_from_config(&config); - let resume_params = thread_resume_params_from_config(&config, "thread-id".to_string()); + let resume_params = thread_resume_params_from_config( + &config, + "thread-id".to_string(), + /*approvals_reviewer_override*/ None, + ); assert_eq!(start_params.config, expected_config); assert_eq!(resume_params.config, expected_config); @@ -648,7 +685,11 @@ async fn thread_lifecycle_params_include_legacy_sandbox_when_no_active_profile() .expect("build config with legacy sandbox override"); let start_params = thread_start_params_from_config(&config); - let resume_params = thread_resume_params_from_config(&config, "thread-id".to_string()); + let resume_params = thread_resume_params_from_config( + &config, + "thread-id".to_string(), + /*approvals_reviewer_override*/ None, + ); assert_eq!(config.permissions.active_permission_profile(), None); assert_eq!( diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 9eb9cb6de4..894c9696bc 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -3194,6 +3194,8 @@ pub struct TurnContextItem { #[serde(default, skip_serializing_if = "Option::is_none")] pub timezone: Option, pub approval_policy: AskForApproval, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub approvals_reviewer: Option, pub sandbox_policy: SandboxPolicy, #[serde(default, skip_serializing_if = "Option::is_none")] pub permission_profile: Option, @@ -5939,6 +5941,7 @@ mod tests { current_date: None, timezone: None, approval_policy: AskForApproval::Never, + approvals_reviewer: None, sandbox_policy: SandboxPolicy::DangerFullAccess, permission_profile: None, network: Some(TurnContextNetworkItem { diff --git a/codex-rs/rollout/src/policy.rs b/codex-rs/rollout/src/policy.rs index 2390a0fc61..33103f54c9 100644 --- a/codex-rs/rollout/src/policy.rs +++ b/codex-rs/rollout/src/policy.rs @@ -95,6 +95,7 @@ pub fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::TurnAborted(_) | EventMsg::TurnStarted(_) | EventMsg::TurnComplete(_) + | EventMsg::ThreadSettingsApplied(_) | EventMsg::WebSearchEnd(_) | EventMsg::ImageGenerationEnd(_) | EventMsg::SubAgentActivity(_) => true, @@ -132,7 +133,6 @@ pub fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::AgentReasoningSectionBreak(_) | EventMsg::RawResponseItem(_) | EventMsg::SessionConfigured(_) - | EventMsg::ThreadSettingsApplied(_) | EventMsg::McpToolCallBegin(_) | EventMsg::ExecCommandBegin(_) | EventMsg::TerminalInteraction(_) diff --git a/codex-rs/rollout/src/recorder_tests.rs b/codex-rs/rollout/src/recorder_tests.rs index 7fcaff08ab..ba07a4f5c0 100644 --- a/codex-rs/rollout/src/recorder_tests.rs +++ b/codex-rs/rollout/src/recorder_tests.rs @@ -1212,6 +1212,7 @@ async fn resume_candidate_matches_cwd_reads_latest_turn_context() -> std::io::Re current_date: None, timezone: None, approval_policy: AskForApproval::Never, + approvals_reviewer: None, sandbox_policy: SandboxPolicy::new_read_only_policy(), permission_profile: None, network: None, diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index 48e308f627..e98ef0d763 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -369,6 +369,7 @@ mod tests { current_date: None, timezone: None, approval_policy: AskForApproval::Never, + approvals_reviewer: None, sandbox_policy: SandboxPolicy::DangerFullAccess, permission_profile: None, network: None, @@ -414,6 +415,7 @@ mod tests { current_date: None, timezone: None, approval_policy: AskForApproval::OnRequest, + approvals_reviewer: None, sandbox_policy: SandboxPolicy::DangerFullAccess, permission_profile: Some(permission_profile.clone()), network: None, @@ -455,6 +457,7 @@ mod tests { current_date: None, timezone: None, approval_policy: AskForApproval::OnRequest, + approvals_reviewer: None, sandbox_policy: SandboxPolicy::new_read_only_policy(), permission_profile: None, network: None, @@ -493,6 +496,7 @@ mod tests { current_date: None, timezone: None, approval_policy: AskForApproval::OnRequest, + approvals_reviewer: None, sandbox_policy: SandboxPolicy::new_read_only_policy(), permission_profile: None, network: None,