From 71aee97dfb9bfd6c81e4e48a4ef15726ce329909 Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Tue, 23 Jun 2026 09:42:37 -0700 Subject: [PATCH] Preserve resume permissions from rollout --- .../request_processors/thread_processor.rs | 112 +++++++++++++++ .../tests/suite/v2/thread_resume.rs | 133 ++++++++++++++++++ codex-rs/rollout/src/policy.rs | 4 +- 3 files changed, 247 insertions(+), 2 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 a59bb530d1..a5cff33d01 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -160,6 +160,111 @@ fn merge_persisted_resume_metadata( } } +struct PersistedResumeThreadSettings { + approval_policy: codex_protocol::protocol::AskForApproval, + approvals_reviewer: Option, + permission_profile: codex_protocol::models::PermissionProfile, +} + +fn merge_persisted_resume_thread_settings( + request_overrides: Option<&HashMap>, + typesafe_overrides: &mut ConfigOverrides, + settings: PersistedResumeThreadSettings, +) { + if !has_config_override(request_overrides, "approval_policy") + && typesafe_overrides.approval_policy.is_none() + { + typesafe_overrides.approval_policy = Some(settings.approval_policy); + } + + if let Some(approvals_reviewer) = settings.approvals_reviewer + && !has_config_override(request_overrides, "approvals_reviewer") + && typesafe_overrides.approvals_reviewer.is_none() + { + typesafe_overrides.approvals_reviewer = Some(approvals_reviewer); + } + + if !has_permission_resume_override(request_overrides, typesafe_overrides) { + typesafe_overrides.permission_profile = Some(settings.permission_profile); + } +} + +fn has_config_override( + request_overrides: Option<&HashMap>, + key: &str, +) -> bool { + request_overrides.is_some_and(|overrides| overrides.contains_key(key)) +} + +fn has_permission_resume_override( + request_overrides: Option<&HashMap>, + typesafe_overrides: &ConfigOverrides, +) -> bool { + typesafe_overrides.permission_profile.is_some() + || typesafe_overrides.default_permissions.is_some() + || typesafe_overrides.sandbox_mode.is_some() + || has_config_override(request_overrides, "permission_profile") + || has_config_override(request_overrides, "default_permissions") + || has_config_override(request_overrides, "sandbox_mode") +} + +async fn latest_persisted_resume_thread_settings( + resumed_history: &ResumedHistory, +) -> Option { + if let Some(rollout_path) = resumed_history.rollout_path.as_ref() + && let Ok((items, _, _)) = + codex_rollout::RolloutRecorder::load_rollout_items(rollout_path).await + && let Some(settings) = latest_persisted_resume_thread_settings_from_items(&items) + { + return Some(settings); + } + + latest_persisted_resume_thread_settings_from_items(&resumed_history.history) +} + +fn latest_persisted_resume_thread_settings_from_items( + rollout_items: &[RolloutItem], +) -> Option { + let mut settings = None; + + for item in rollout_items { + match item { + RolloutItem::EventMsg(EventMsg::SessionConfigured(event)) => { + settings = Some(PersistedResumeThreadSettings { + approval_policy: event.approval_policy, + approvals_reviewer: Some(event.approvals_reviewer), + permission_profile: event.permission_profile.clone(), + }); + } + RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => { + let snapshot = &event.thread_settings; + settings = Some(PersistedResumeThreadSettings { + approval_policy: snapshot.approval_policy, + approvals_reviewer: Some(snapshot.approvals_reviewer), + permission_profile: snapshot.permission_profile.clone(), + }); + } + RolloutItem::TurnContext(turn_context) => { + let approvals_reviewer = settings + .as_ref() + .and_then(|settings| settings.approvals_reviewer); + settings = Some(PersistedResumeThreadSettings { + approval_policy: turn_context.approval_policy, + approvals_reviewer, + permission_profile: turn_context.permission_profile(), + }); + } + RolloutItem::SessionMeta(_) + | RolloutItem::EventMsg(_) + | RolloutItem::ResponseItem(_) + | RolloutItem::InterAgentCommunication(_) + | RolloutItem::Compacted(_) => {} + } + } + + settings +} + fn normalize_thread_list_cwd_filters( cwd: Option, ) -> Result>, JSONRPCErrorError> { @@ -2840,6 +2945,13 @@ impl ThreadRequestProcessor { let InitialHistory::Resumed(resumed_history) = thread_history else { return None; }; + if let Some(settings) = latest_persisted_resume_thread_settings(resumed_history).await { + merge_persisted_resume_thread_settings( + request_overrides.as_ref(), + typesafe_overrides, + settings, + ); + } let state_db_ctx = self.state_db.clone()?; let persisted_metadata = state_db_ctx .get_thread(resumed_history.conversation_id) 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 ea7a1a9d46..e5c2d61b16 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; @@ -27,6 +28,7 @@ use codex_app_server_protocol::McpToolCallAppContext; use codex_app_server_protocol::PatchApplyStatus; use codex_app_server_protocol::PatchChangeKind; use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::SandboxPolicy; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequest; use codex_app_server_protocol::SessionSource; @@ -58,6 +60,8 @@ use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR; use codex_protocol::ThreadId; use codex_protocol::config_types::Personality; use codex_protocol::mcp::CallToolResult; +use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS; +use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::AgentMessageEvent; @@ -112,6 +116,7 @@ use super::analytics::wait_for_goal_event; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(25); #[cfg(not(windows))] const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); +const PERMISSIONS_RESUME_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); const CODEX_5_2_INSTRUCTIONS_TEMPLATE_DEFAULT: &str = "You are Codex, a coding agent based on GPT-5. You and the user share the same workspace and collaborate to achieve the user's goals."; fn normalized_existing_path(path: impl AsRef) -> Result { @@ -148,6 +153,50 @@ async fn wait_for_responses_request_count( Ok(()) } +async fn start_materialized_workspace_auto_review_thread( + mcp: &mut TestAppServer, +) -> Result { + let start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("gpt-5.4".to_string()), + approval_policy: Some(AskForApproval::OnRequest), + approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + permissions: Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string()), + ..Default::default() + }) + .await?; + let start_resp: JSONRPCResponse = timeout( + PERMISSIONS_RESUME_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: "seed history".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + timeout( + PERMISSIONS_RESUME_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + timeout( + PERMISSIONS_RESUME_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + Ok(thread.id) +} + #[tokio::test] async fn thread_resume_rejects_unmaterialized_thread() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; @@ -195,6 +244,90 @@ async fn thread_resume_rejects_unmaterialized_thread() -> Result<()> { Ok(()) } +#[tokio::test] +async fn thread_resume_preserves_persisted_permissions_without_overrides() -> 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 mut primary = TestAppServer::new(codex_home.path()).await?; + timeout(PERMISSIONS_RESUME_READ_TIMEOUT, primary.initialize()).await??; + let thread_id = start_materialized_workspace_auto_review_thread(&mut primary).await?; + drop(primary); + + let mut secondary = TestAppServer::new(codex_home.path()).await?; + timeout(PERMISSIONS_RESUME_READ_TIMEOUT, secondary.initialize()).await??; + + let resume_id = secondary + .send_thread_resume_request(ThreadResumeParams { + thread_id, + ..Default::default() + }) + .await?; + let resume_resp: JSONRPCResponse = timeout( + PERMISSIONS_RESUME_READ_TIMEOUT, + secondary.read_stream_until_response_message(RequestId::Integer(resume_id)), + ) + .await??; + let ThreadResumeResponse { + approval_policy, + approvals_reviewer, + sandbox, + .. + } = to_response::(resume_resp)?; + + assert_eq!(approval_policy, AskForApproval::OnRequest); + assert_eq!(approvals_reviewer, ApprovalsReviewer::AutoReview); + assert!( + matches!(sandbox, SandboxPolicy::WorkspaceWrite { .. }), + "expected persisted workspace-write permissions, got {sandbox:?}" + ); + + Ok(()) +} + +#[tokio::test] +async fn thread_resume_permission_overrides_win_over_persisted_permissions() -> 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 mut primary = TestAppServer::new(codex_home.path()).await?; + timeout(PERMISSIONS_RESUME_READ_TIMEOUT, primary.initialize()).await??; + let thread_id = start_materialized_workspace_auto_review_thread(&mut primary).await?; + drop(primary); + + let mut secondary = TestAppServer::new(codex_home.path()).await?; + timeout(PERMISSIONS_RESUME_READ_TIMEOUT, secondary.initialize()).await??; + + let resume_id = secondary + .send_thread_resume_request(ThreadResumeParams { + thread_id, + approval_policy: Some(AskForApproval::Never), + approvals_reviewer: Some(ApprovalsReviewer::User), + permissions: Some(BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS.to_string()), + ..Default::default() + }) + .await?; + let resume_resp: JSONRPCResponse = timeout( + PERMISSIONS_RESUME_READ_TIMEOUT, + secondary.read_stream_until_response_message(RequestId::Integer(resume_id)), + ) + .await??; + let ThreadResumeResponse { + approval_policy, + approvals_reviewer, + sandbox, + .. + } = to_response::(resume_resp)?; + + assert_eq!(approval_policy, AskForApproval::Never); + assert_eq!(approvals_reviewer, ApprovalsReviewer::User); + assert_eq!(sandbox, SandboxPolicy::DangerFullAccess); + + Ok(()) +} + #[tokio::test] async fn thread_resume_with_empty_path_uses_running_thread_id() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; diff --git a/codex-rs/rollout/src/policy.rs b/codex-rs/rollout/src/policy.rs index 4db26576b0..d9f4de57e1 100644 --- a/codex-rs/rollout/src/policy.rs +++ b/codex-rs/rollout/src/policy.rs @@ -85,6 +85,8 @@ pub fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::PatchApplyEnd(_) | EventMsg::TokenCount(_) | EventMsg::ThreadGoalUpdated(_) + | EventMsg::SessionConfigured(_) + | EventMsg::ThreadSettingsApplied(_) | EventMsg::ContextCompacted(_) | EventMsg::EnteredReviewMode(_) | EventMsg::ExitedReviewMode(_) @@ -129,8 +131,6 @@ pub fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::TurnModerationMetadata(_) | EventMsg::AgentReasoningSectionBreak(_) | EventMsg::RawResponseItem(_) - | EventMsg::SessionConfigured(_) - | EventMsg::ThreadSettingsApplied(_) | EventMsg::McpToolCallBegin(_) | EventMsg::ExecCommandBegin(_) | EventMsg::TerminalInteraction(_)