diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index bbd0476e5c..67642f9e4d 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -358,7 +358,7 @@ Start a fresh thread when you need a new Codex conversation. Valid `personality` values are `"friendly"`, `"pragmatic"`, and `"none"`. When `"none"` is selected, the personality placeholder is replaced with an empty string. -To continue a stored session, call `thread/resume` with the `thread.id` you previously recorded. The response shape matches `thread/start`. When the stored session includes persisted token usage, the server emits `thread/tokenUsage/updated` immediately after the response so clients can render restored usage before the next turn starts. You can also pass the same configuration overrides supported by `thread/start`, including `approvalsReviewer`. On cold resume, approval policy uses the first allowed value in this order: request override, latest persisted thread setting, current configured default. +To continue a stored session, call `thread/resume` with the `thread.id` you previously recorded. The response shape matches `thread/start`. When the stored session includes persisted token usage, the server emits `thread/tokenUsage/updated` immediately after the response so clients can render restored usage before the next turn starts. You can also pass the same configuration overrides supported by `thread/start`, including `approvalsReviewer`. On cold resume, approval policy and the active permission-profile ID select a source in this order: request override, latest persisted thread setting, current configured default. The persisted profile ID is resolved through the same config and requirements path as a `permissions` override. Threads without an active profile ID use current config instead of restoring their concrete historical permissions. By default, `thread/resume` includes the reconstructed turn history in `thread.turns`. Experimental clients can pass `excludeTurns: true` to return only thread metadata and live resume state, then call `thread/turns/list` separately if they want to page the turn history over the network. A cold paginated resume can still replay persisted `thread/tokenUsage/updated` when it can identify the corresponding stored turn; resuming an already-loaded thread waits for the next live update. diff --git a/codex-rs/app-server/src/request_processors/persisted_resume_settings.rs b/codex-rs/app-server/src/request_processors/persisted_resume_settings.rs index 55e28953fb..5e408b02c9 100644 --- a/codex-rs/app-server/src/request_processors/persisted_resume_settings.rs +++ b/codex-rs/app-server/src/request_processors/persisted_resume_settings.rs @@ -1,10 +1,14 @@ use codex_protocol::config_types::ApprovalsReviewer; +use codex_protocol::models::ActivePermissionProfile; +use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_rollout::RolloutItem; #[derive(Debug, PartialEq, Eq)] pub(super) struct PersistedResumeSettings { + pub(super) approval_policy: AskForApproval, pub(super) approvals_reviewer: Option, + pub(super) active_permission_profile: Option, } pub(super) fn latest_persisted_resume_settings( @@ -16,6 +20,7 @@ pub(super) fn latest_persisted_resume_settings( .rev() .find_map(|(index, item)| match item { RolloutItem::TurnContext(turn_context) => Some(PersistedResumeSettings { + approval_policy: turn_context.approval_policy, approvals_reviewer: turn_context.approvals_reviewer.or_else(|| { history[..index].iter().rev().find_map(|item| match item { RolloutItem::TurnContext(turn_context) => turn_context.approvals_reviewer, @@ -25,10 +30,16 @@ pub(super) fn latest_persisted_resume_settings( _ => None, }) }), + active_permission_profile: turn_context.active_permission_profile.clone(), }), RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => { Some(PersistedResumeSettings { + approval_policy: event.thread_settings.approval_policy, approvals_reviewer: Some(event.thread_settings.approvals_reviewer), + active_permission_profile: event + .thread_settings + .active_permission_profile + .clone(), }) } _ => None, diff --git a/codex-rs/app-server/src/request_processors/persisted_resume_settings_tests.rs b/codex-rs/app-server/src/request_processors/persisted_resume_settings_tests.rs index 48fc65cd0c..ed96867eb1 100644 --- a/codex-rs/app-server/src/request_processors/persisted_resume_settings_tests.rs +++ b/codex-rs/app-server/src/request_processors/persisted_resume_settings_tests.rs @@ -4,6 +4,7 @@ use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; +use codex_protocol::models::ActivePermissionProfile; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; @@ -20,17 +21,21 @@ fn cwd() -> AbsolutePathBuf { .expect("absolute current directory") } -fn settings_item(approvals_reviewer: ApprovalsReviewer) -> RolloutItem { +fn settings_item( + approval_policy: AskForApproval, + approvals_reviewer: ApprovalsReviewer, + active_permission_profile: Option, +) -> RolloutItem { RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied( ThreadSettingsAppliedEvent { thread_settings: ThreadSettingsSnapshot { model: "gpt-5".to_string(), model_provider_id: "openai".to_string(), service_tier: None, - approval_policy: AskForApproval::OnRequest, + approval_policy, approvals_reviewer, permission_profile: PermissionProfile::read_only(), - active_permission_profile: None, + active_permission_profile, cwd: cwd(), reasoning_effort: None, reasoning_summary: None, @@ -48,18 +53,23 @@ fn settings_item(approvals_reviewer: ApprovalsReviewer) -> RolloutItem { )) } -fn turn_context_item(turn_id: &str, approvals_reviewer: Option) -> RolloutItem { +fn turn_context_item( + turn_id: &str, + approval_policy: AskForApproval, + approvals_reviewer: Option, + active_permission_profile: Option, +) -> RolloutItem { RolloutItem::TurnContext(TurnContextItem { turn_id: Some(turn_id.to_string()), cwd: cwd(), - workspace_roots: None, + workspace_roots: Some(vec![cwd()]), current_date: None, timezone: None, - approval_policy: AskForApproval::OnRequest, + approval_policy, approvals_reviewer, sandbox_policy: SandboxPolicy::new_read_only_policy(), - permission_profile: None, - active_permission_profile: None, + permission_profile: Some(PermissionProfile::read_only()), + active_permission_profile, network: None, file_system_sandbox_policy: None, model: "gpt-5".to_string(), @@ -76,45 +86,74 @@ fn turn_context_item(turn_id: &str, approvals_reviewer: Option Option { - history - .iter() - .enumerate() - .rev() - .find_map(|(index, item)| match item { - RolloutItem::TurnContext(turn_context) => { - let updated_policy = turn_context.turn_id.as_ref().and_then(|turn_id| { - let turn_start = history[..index].iter().rposition(|item| { - matches!( - item, - RolloutItem::EventMsg(EventMsg::TurnStarted(event)) - if &event.turn_id == turn_id - ) - })?; - history[turn_start + 1..index] - .iter() - .rev() - .find_map(|item| match item { - RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => { - Some(event.thread_settings.approval_policy) - } - _ => None, - }) - }); - Some(updated_policy.unwrap_or(turn_context.approval_policy)) - } - RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => { - Some(event.thread_settings.approval_policy) - } - _ => None, - }) -} - fn normalize_thread_list_cwd_filters( cwd: Option, ) -> Result>, JSONRPCErrorError> { @@ -297,6 +262,18 @@ fn has_model_resume_override( .is_some_and(|overrides| overrides.contains_key("model_reasoning_effort")) } +fn has_permission_override( + request_overrides: Option<&HashMap>, + typesafe_overrides: &ConfigOverrides, +) -> bool { + typesafe_overrides.sandbox_mode.is_some() + || typesafe_overrides.permission_profile.is_some() + || typesafe_overrides.default_permissions.is_some() + || request_overrides.is_some_and(|overrides| { + overrides.contains_key("sandbox_mode") || overrides.contains_key("default_permissions") + }) +} + fn validate_dynamic_tools(tools: &[DynamicToolSpec]) -> Result<(), String> { const DYNAMIC_TOOL_NAME_MAX_LEN: usize = 128; const DYNAMIC_TOOL_NAMESPACE_MAX_LEN: usize = 64; @@ -3931,18 +3908,23 @@ impl ThreadRequestProcessor { let InitialHistory::Resumed(resumed_history) = thread_history else { return None; }; - if typesafe_overrides.approvals_reviewer.is_none() - && !request_overrides - .as_ref() - .is_some_and(|overrides| overrides.contains_key("approvals_reviewer")) + if let Some(persisted_settings) = latest_persisted_resume_settings(&resumed_history.history) { - typesafe_overrides.approvals_reviewer = - latest_persisted_resume_settings(&resumed_history.history) - .and_then(|settings| settings.approvals_reviewer); - } - if typesafe_overrides.approval_policy.is_none() { - typesafe_overrides.approval_policy = - latest_persisted_approval_policy(&resumed_history.history); + if typesafe_overrides.approval_policy.is_none() { + typesafe_overrides.approval_policy = Some(persisted_settings.approval_policy); + } + if typesafe_overrides.approvals_reviewer.is_none() + && !request_overrides + .as_ref() + .is_some_and(|overrides| overrides.contains_key("approvals_reviewer")) + { + typesafe_overrides.approvals_reviewer = persisted_settings.approvals_reviewer; + } + if !has_permission_override(request_overrides.as_ref(), typesafe_overrides) { + typesafe_overrides.persisted_permission_profile_id = persisted_settings + .active_permission_profile + .map(|profile| profile.id); + } } let state_db_ctx = self.state_db.clone()?; let persisted_metadata = state_db_ctx @@ -4669,44 +4651,66 @@ impl ThreadRequestProcessor { /*personality*/ None, ); typesafe_overrides.ephemeral = ephemeral.then_some(true); - let latest_context = if paginated_source - && typesafe_overrides.approvals_reviewer.is_none() + let restore_approval_policy = typesafe_overrides.approval_policy.is_none(); + let restore_approvals_reviewer = typesafe_overrides.approvals_reviewer.is_none() && !request_overrides .as_ref() - .is_some_and(|overrides| overrides.contains_key("approvals_reviewer")) - { + .is_some_and(|overrides| overrides.contains_key("approvals_reviewer")); + let restore_permission_profile = + !has_permission_override(request_overrides.as_ref(), &typesafe_overrides); + let needs_latest_settings = + restore_approval_policy || restore_approvals_reviewer || restore_permission_profile; + let loaded_parent_settings = if paginated_source && needs_latest_settings { if let Ok(parent) = self.thread_manager.get_thread(source_thread_id).await { - typesafe_overrides.approvals_reviewer = - Some(parent.config_snapshot().await.approvals_reviewer); - None - } else if last_turn_id.is_some() || before_turn_id.is_some() { - Some( - self.thread_store - .load_latest_model_context(StoreLoadThreadHistoryParams { - thread_id: source_thread_id, - include_archived: true, - }) - .await - .map_err(thread_store_resume_read_error)? - .items, - ) + let snapshot = parent.config_snapshot().await; + Some(PersistedResumeSettings { + approval_policy: snapshot.approval_policy, + approvals_reviewer: Some(snapshot.approvals_reviewer), + active_permission_profile: snapshot.active_permission_profile, + }) } else { None } } else { None }; - if typesafe_overrides.approvals_reviewer.is_none() - && !request_overrides - .as_ref() - .is_some_and(|overrides| overrides.contains_key("approvals_reviewer")) + let latest_context = if paginated_source + && needs_latest_settings + && loaded_parent_settings.is_none() + && (last_turn_id.is_some() || before_turn_id.is_some()) { - typesafe_overrides.approvals_reviewer = latest_persisted_resume_settings( + Some( + self.thread_store + .load_latest_model_context(StoreLoadThreadHistoryParams { + thread_id: source_thread_id, + include_archived: true, + }) + .await + .map_err(thread_store_resume_read_error)? + .items, + ) + } else { + None + }; + let persisted_settings = loaded_parent_settings.or_else(|| { + latest_persisted_resume_settings( latest_context .as_deref() .unwrap_or_else(|| source_history_items.as_ref()), ) - .and_then(|settings| settings.approvals_reviewer); + }); + if let Some(persisted_settings) = persisted_settings { + if restore_approval_policy { + typesafe_overrides.approval_policy = Some(persisted_settings.approval_policy); + } + if restore_approvals_reviewer { + typesafe_overrides.approvals_reviewer = persisted_settings.approvals_reviewer; + } + if restore_permission_profile { + typesafe_overrides.persisted_permission_profile_id = persisted_settings + .active_permission_profile + .map(|profile| profile.id); + } } // Derive a Config using the same logic as new conversation, honoring overrides if provided. let config = self diff --git a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs index f3bb53d46b..ffba9ba44f 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs @@ -36,137 +36,6 @@ mod thread_list_cwd_filter_tests { } } -mod persisted_resume_approval_policy_tests { - use super::super::latest_persisted_approval_policy; - use codex_protocol::config_types::ApprovalsReviewer; - use codex_protocol::config_types::CollaborationMode; - use codex_protocol::config_types::ModeKind; - use codex_protocol::config_types::Settings; - use codex_protocol::models::PermissionProfile; - use codex_protocol::protocol::AskForApproval; - use codex_protocol::protocol::EventMsg; - use codex_protocol::protocol::SandboxPolicy; - use codex_protocol::protocol::ThreadSettingsAppliedEvent; - use codex_protocol::protocol::ThreadSettingsSnapshot; - use codex_protocol::protocol::TurnContextItem; - use codex_protocol::protocol::TurnStartedEvent; - use codex_rollout::RolloutItem; - use codex_utils_absolute_path::AbsolutePathBuf; - use pretty_assertions::assert_eq; - - fn cwd() -> AbsolutePathBuf { - AbsolutePathBuf::try_from(std::env::current_dir().expect("current directory")) - .expect("absolute current directory") - } - - fn settings_item(approval_policy: AskForApproval) -> RolloutItem { - RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied( - ThreadSettingsAppliedEvent { - thread_settings: ThreadSettingsSnapshot { - model: "gpt-5".to_string(), - model_provider_id: "openai".to_string(), - service_tier: None, - approval_policy, - approvals_reviewer: ApprovalsReviewer::User, - permission_profile: PermissionProfile::read_only(), - active_permission_profile: None, - cwd: cwd(), - reasoning_effort: None, - reasoning_summary: None, - personality: None, - collaboration_mode: CollaborationMode { - mode: ModeKind::Default, - settings: Settings { - model: "gpt-5".to_string(), - reasoning_effort: None, - developer_instructions: None, - }, - }, - }, - }, - )) - } - - fn turn_started_item(turn_id: &str) -> RolloutItem { - RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent { - turn_id: turn_id.to_string(), - trace_id: None, - started_at: None, - model_context_window: None, - collaboration_mode_kind: ModeKind::Default, - })) - } - - fn turn_context_item(turn_id: &str, approval_policy: AskForApproval) -> RolloutItem { - RolloutItem::TurnContext(TurnContextItem { - turn_id: Some(turn_id.to_string()), - cwd: cwd(), - workspace_roots: None, - current_date: None, - timezone: None, - approval_policy, - approvals_reviewer: None, - sandbox_policy: SandboxPolicy::new_read_only_policy(), - permission_profile: None, - active_permission_profile: None, - network: None, - file_system_sandbox_policy: None, - model: "gpt-5".to_string(), - comp_hash: None, - personality: None, - collaboration_mode: None, - multi_agent_version: None, - multi_agent_mode: None, - realtime_active: None, - effort: None, - summary: codex_protocol::config_types::ReasoningSummary::Auto, - }) - } - - #[test] - fn latest_settings_snapshot_wins() { - let history = vec![ - settings_item(AskForApproval::Never), - settings_item(AskForApproval::OnRequest), - ]; - - assert_eq!( - latest_persisted_approval_policy(&history), - Some(AskForApproval::OnRequest) - ); - } - - #[test] - fn settings_applied_during_turn_wins_over_stale_compaction_context() { - let history = vec![ - turn_started_item("turn-1"), - settings_item(AskForApproval::Never), - turn_context_item("turn-1", AskForApproval::OnRequest), - ]; - - assert_eq!( - latest_persisted_approval_policy(&history), - Some(AskForApproval::Never) - ); - } - - #[test] - fn later_turn_context_wins_over_earlier_settings_update() { - let history = vec![ - turn_started_item("turn-1"), - settings_item(AskForApproval::Never), - turn_context_item("turn-1", AskForApproval::OnRequest), - turn_started_item("turn-2"), - turn_context_item("turn-2", AskForApproval::OnRequest), - ]; - - assert_eq!( - latest_persisted_approval_policy(&history), - Some(AskForApproval::OnRequest) - ); - } -} - mod background_terminal_pagination_tests { use super::super::paginate_background_terminals; use codex_app_server_protocol::ThreadBackgroundTerminal; diff --git a/codex-rs/app-server/tests/suite/v2/model_auto_review.rs b/codex-rs/app-server/tests/suite/v2/model_auto_review.rs index 9d95eb21c9..2a5b6fc98e 100644 --- a/codex-rs/app-server/tests/suite/v2/model_auto_review.rs +++ b/codex-rs/app-server/tests/suite/v2/model_auto_review.rs @@ -346,7 +346,12 @@ async fn thread_resume_and_fork_upgrade_legacy_protected_model_settings() -> Res )) .await?; let fork: ForkResponse = timeout(TIMEOUT, server.read_response(id)).await??; - assert_protected(&fork.model, fork.approval_policy, fork.approvals_reviewer); + assert_protected_with_policy( + &fork.model, + fork.approval_policy, + fork.approvals_reviewer, + Never, + ); let id = server .send_thread_resume_request(params!( ResumeParams, diff --git a/codex-rs/app-server/tests/suite/v2/thread_fork.rs b/codex-rs/app-server/tests/suite/v2/thread_fork.rs index 4175230d59..f27dd8d554 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_fork.rs @@ -10,12 +10,16 @@ use app_test_support::create_mock_responses_server_sequence_unchecked; use app_test_support::rollout_path; use app_test_support::to_response; use app_test_support::write_chatgpt_auth; +use codex_app_server_protocol::ActivePermissionProfile; use codex_app_server_protocol::ApprovalsReviewer; +use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCMessage; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::SandboxMode; +use codex_app_server_protocol::SandboxPolicy; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::SessionSource; use codex_app_server_protocol::ThreadForkParams; @@ -293,6 +297,115 @@ async fn paginated_thread_fork_preserves_persisted_approvals_reviewer() -> Resul assert_thread_fork_preserves_persisted_approvals_reviewer(ThreadHistoryMode::Paginated).await } +#[tokio::test] +async fn thread_fork_preserves_persisted_permission_profile_and_honors_overrides() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + + for history_mode in [ThreadHistoryMode::Legacy, ThreadHistoryMode::Paginated] { + let codex_home = TempDir::new()?; + MockResponsesConfig::new(&server.uri()) + .with_root_config("default_permissions = \":danger-full-access\"") + .with_extra_config("[permissions.dev]\nextends = \":read-only\"") + .write(codex_home.path())?; + + let source_thread_id = { + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + let start_id = mcp + .send_thread_start_request_with_auto_env(ThreadStartParams { + history_mode: Some(history_mode), + approval_policy: Some(AskForApproval::OnRequest), + permissions: Some("dev".to_string()), + ..Default::default() + }) + .await?; + let ThreadStartResponse { thread, .. } = + timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(start_id)).await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.start_turn_and_wait_for_completion(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![UserInput::Text { + text: "persist permission profile".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }), + ) + .await??; + thread.id + }; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + let fork_id = mcp + .send_thread_fork_request(ThreadForkParams { + thread_id: source_thread_id.clone(), + ..Default::default() + }) + .await?; + let ThreadForkResponse { + approval_policy, + sandbox, + active_permission_profile, + .. + } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(fork_id)).await??; + assert!(matches!(sandbox, SandboxPolicy::ReadOnly { .. })); + assert_eq!(approval_policy, AskForApproval::OnRequest); + assert_eq!( + active_permission_profile, + Some(ActivePermissionProfile { + id: "dev".to_string(), + extends: Some(":read-only".to_string()), + }) + ); + + let fork_id = mcp + .send_thread_fork_request(ThreadForkParams { + thread_id: source_thread_id.clone(), + approval_policy: Some(AskForApproval::Never), + sandbox: Some(SandboxMode::DangerFullAccess), + ..Default::default() + }) + .await?; + let ThreadForkResponse { + approval_policy, + sandbox, + active_permission_profile, + .. + } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(fork_id)).await??; + assert!(matches!(sandbox, SandboxPolicy::DangerFullAccess)); + assert_eq!(approval_policy, AskForApproval::Never); + assert_eq!(active_permission_profile, None); + + let fork_id = mcp + .send_thread_fork_request(ThreadForkParams { + thread_id: source_thread_id, + permissions: Some(":workspace".to_string()), + ..Default::default() + }) + .await?; + let ThreadForkResponse { + sandbox, + active_permission_profile, + .. + } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(fork_id)).await??; + assert!(matches!(sandbox, SandboxPolicy::WorkspaceWrite { .. })); + assert_eq!( + active_permission_profile, + Some(ActivePermissionProfile::new(":workspace")) + ); + } + + Ok(()) +} + async fn assert_thread_fork_preserves_persisted_approvals_reviewer( history_mode: ThreadHistoryMode, ) -> Result<()> { @@ -309,6 +422,7 @@ async fn assert_thread_fork_preserves_persisted_approvals_reviewer( let start_id = mcp .send_thread_start_request_with_auto_env(ThreadStartParams { history_mode: Some(history_mode), + permissions: Some(":workspace".to_string()), ..Default::default() }) .await?; @@ -348,7 +462,9 @@ async fn assert_thread_fork_preserves_persisted_approvals_reviewer( text: "switch to auto-review".to_string(), text_elements: Vec::new(), }], + approval_policy: Some(AskForApproval::OnRequest), approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + permissions: Some(":read-only".to_string()), ..Default::default() }) .await?; @@ -372,9 +488,17 @@ async fn assert_thread_fork_preserves_persisted_approvals_reviewer( }) .await?; let ThreadForkResponse { - approvals_reviewer, .. + approval_policy, + approvals_reviewer, + active_permission_profile, + .. } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(fork_id)).await??; + assert_eq!(approval_policy, AskForApproval::OnRequest); assert_eq!(approvals_reviewer, ApprovalsReviewer::AutoReview); + assert_eq!( + active_permission_profile, + Some(ActivePermissionProfile::new(":read-only")) + ); } (thread.id, turn.id) @@ -397,24 +521,41 @@ async fn assert_thread_fork_preserves_persisted_approvals_reviewer( ) .await??; let ThreadForkResponse { - approvals_reviewer, .. + approval_policy, + approvals_reviewer, + active_permission_profile, + .. } = to_response(fork_resp)?; + assert_eq!(approval_policy, AskForApproval::OnRequest); assert_eq!(approvals_reviewer, ApprovalsReviewer::AutoReview); + assert_eq!( + active_permission_profile, + Some(ActivePermissionProfile::new(":read-only")) + ); if matches!(history_mode, ThreadHistoryMode::Paginated) { let fork_id = mcp .send_thread_fork_request(ThreadForkParams { thread_id: source_thread_id, last_turn_id: Some(source_turn_id), + approval_policy: Some(AskForApproval::Never), approvals_reviewer: Some(ApprovalsReviewer::User), ..Default::default() }) .await?; let ThreadForkResponse { - approvals_reviewer, .. + approval_policy, + approvals_reviewer, + active_permission_profile, + .. } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(fork_id)).await??; + assert_eq!(approval_policy, AskForApproval::Never); assert_eq!(approvals_reviewer, ApprovalsReviewer::User); + assert_eq!( + active_permission_profile, + Some(ActivePermissionProfile::new(":read-only")) + ); } Ok(()) 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 a751490666..5dbcba6eef 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -16,6 +16,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::ActivePermissionProfile; use codex_app_server_protocol::ApprovalsReviewer; use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::ClientInfo; @@ -30,6 +31,8 @@ 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::SandboxMode; +use codex_app_server_protocol::SandboxPolicy as AppSandboxPolicy; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequest; use codex_app_server_protocol::SessionSource; @@ -75,6 +78,9 @@ use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Personality; use codex_protocol::config_types::Settings; use codex_protocol::mcp::CallToolResult; +use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS; +use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY; +use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ReasoningEffort; @@ -1126,6 +1132,358 @@ async fn thread_resume_preserves_acknowledged_model_effort_and_approvals_reviewe Ok(()) } +#[tokio::test] +async fn cold_resume_reresolves_persisted_active_permission_profile() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + for history_mode in [ThreadHistoryMode::Legacy, ThreadHistoryMode::Paginated] { + let codex_home = TempDir::new()?; + let previous_workspace_root = TempDir::new()?; + write_dev_permission_config(&server.uri(), codex_home.path(), ":workspace")?; + let thread_id = { + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + let thread_id = materialize_dev_permission_thread(&mut mcp, history_mode).await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.start_turn_and_wait_for_completion(TurnStartParams { + thread_id: thread_id.clone(), + runtime_workspace_roots: Some(vec![AbsolutePathBuf::from_absolute_path( + previous_workspace_root.path(), + )?]), + input: vec![UserInput::Text { + text: "update runtime workspace roots".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }), + ) + .await??; + thread_id + }; + + write_dev_permission_config(&server.uri(), codex_home.path(), ":read-only")?; + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + let resume_id = mcp + .send_thread_resume_request(ThreadResumeParams { + thread_id, + ..Default::default() + }) + .await?; + let ThreadResumeResponse { + sandbox, + active_permission_profile, + runtime_workspace_roots, + .. + } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??; + + assert!(matches!(sandbox, AppSandboxPolicy::ReadOnly { .. })); + assert_eq!( + active_permission_profile, + Some(ActivePermissionProfile { + id: "dev".to_string(), + extends: Some(BUILT_IN_PERMISSION_PROFILE_READ_ONLY.to_string()), + }) + ); + assert!( + !runtime_workspace_roots.contains(&AbsolutePathBuf::from_absolute_path( + previous_workspace_root.path(), + )?) + ); + } + Ok(()) +} + +#[tokio::test] +async fn cold_resume_with_removed_permission_profile_uses_configured_default() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + for history_mode in [ThreadHistoryMode::Legacy, ThreadHistoryMode::Paginated] { + let codex_home = TempDir::new()?; + write_dev_permission_config(&server.uri(), codex_home.path(), ":workspace")?; + let thread_id = { + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + materialize_dev_permission_thread(&mut mcp, history_mode).await? + }; + + MockResponsesConfig::new(&server.uri()) + .with_root_config(&format!( + "default_permissions = \"{BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS}\"" + )) + .write(codex_home.path())?; + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + let resume_id = mcp + .send_thread_resume_request(ThreadResumeParams { + thread_id, + ..Default::default() + }) + .await?; + let ThreadResumeResponse { + sandbox, + active_permission_profile, + .. + } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??; + + assert!(matches!(sandbox, AppSandboxPolicy::DangerFullAccess)); + assert_eq!( + active_permission_profile, + Some(ActivePermissionProfile::new( + BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS, + )) + ); + } + Ok(()) +} + +#[tokio::test] +async fn cold_resume_permission_overrides_win_over_persisted_profile() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + write_dev_permission_config(&server.uri(), codex_home.path(), ":workspace")?; + let thread_id = { + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + materialize_dev_permission_thread(&mut mcp, ThreadHistoryMode::Legacy).await? + }; + + for params in [ + ThreadResumeParams { + thread_id: thread_id.clone(), + sandbox: Some(SandboxMode::ReadOnly), + ..Default::default() + }, + ThreadResumeParams { + thread_id: thread_id.clone(), + permissions: Some(BUILT_IN_PERMISSION_PROFILE_READ_ONLY.to_string()), + ..Default::default() + }, + ThreadResumeParams { + thread_id: thread_id.clone(), + config: Some(std::collections::HashMap::from([( + "default_permissions".to_string(), + json!(BUILT_IN_PERMISSION_PROFILE_READ_ONLY), + )])), + ..Default::default() + }, + ] { + let expected_active_permission_profile = params + .sandbox + .is_none() + .then(ActivePermissionProfile::read_only); + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + let resume_id = mcp.send_thread_resume_request(params).await?; + let ThreadResumeResponse { + sandbox, + active_permission_profile, + .. + } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??; + + assert!(matches!(sandbox, AppSandboxPolicy::ReadOnly { .. })); + assert_eq!( + active_permission_profile, + expected_active_permission_profile + ); + } + Ok(()) +} + +#[tokio::test] +async fn cold_resume_without_active_permission_profile_uses_current_config() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + mock_responses_config(&server.uri()).write(codex_home.path())?; + let thread_id = { + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + let ThreadStartResponse { thread, .. } = mcp + .start_thread(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.start_turn_and_wait_for_completion(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![UserInput::Text { + text: "persist full access".to_string(), + text_elements: Vec::new(), + }], + sandbox_policy: Some(AppSandboxPolicy::DangerFullAccess), + ..Default::default() + }), + ) + .await??; + thread.id + }; + + MockResponsesConfig::new(&server.uri()) + .with_root_config(&format!( + "default_permissions = \"{BUILT_IN_PERMISSION_PROFILE_WORKSPACE}\"" + )) + .write(codex_home.path())?; + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + let resume_id = mcp + .send_thread_resume_request(ThreadResumeParams { + thread_id, + ..Default::default() + }) + .await?; + let ThreadResumeResponse { + sandbox, + active_permission_profile, + .. + } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??; + + assert!(matches!(sandbox, AppSandboxPolicy::WorkspaceWrite { .. })); + assert_eq!( + active_permission_profile, + Some(ActivePermissionProfile::new( + BUILT_IN_PERMISSION_PROFILE_WORKSPACE + )) + ); + Ok(()) +} + +#[tokio::test] +async fn cold_resume_restores_profile_selected_by_settings_update() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + mock_responses_config(&server.uri()).write(codex_home.path())?; + let thread_id = { + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + let ThreadStartResponse { thread, .. } = mcp + .start_thread(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.start_turn_and_wait_for_completion(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![UserInput::Text { + text: "persist permission profile".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }), + ) + .await??; + let update_id = mcp + .send_thread_settings_update_request(ThreadSettingsUpdateParams { + thread_id: thread.id.clone(), + permissions: Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string()), + ..Default::default() + }) + .await?; + let _: ThreadSettingsUpdateResponse = + timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(update_id)).await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("thread/settings/updated"), + ) + .await??; + thread.id + }; + + mock_responses_config(&server.uri()).write(codex_home.path())?; + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .build_initialized() + .await?; + let resume_id = mcp + .send_thread_resume_request(ThreadResumeParams { + thread_id, + ..Default::default() + }) + .await?; + let ThreadResumeResponse { + sandbox, + active_permission_profile, + .. + } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??; + + assert!(matches!(sandbox, AppSandboxPolicy::WorkspaceWrite { .. })); + assert_eq!( + active_permission_profile, + Some(ActivePermissionProfile::new( + BUILT_IN_PERMISSION_PROFILE_WORKSPACE + )) + ); + Ok(()) +} + +async fn materialize_dev_permission_thread( + mcp: &mut TestAppServer, + history_mode: ThreadHistoryMode, +) -> Result { + let ThreadStartResponse { thread, .. } = mcp + .start_thread(ThreadStartParams { + model: Some("mock-model".to_string()), + history_mode: Some(history_mode), + permissions: Some("dev".to_string()), + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.start_turn_and_wait_for_completion(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![UserInput::Text { + text: "persist permission profile".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }), + ) + .await??; + Ok(thread.id) +} + +fn write_dev_permission_config( + server_uri: &str, + codex_home: &Path, + dev_extends: &str, +) -> std::io::Result<()> { + MockResponsesConfig::new(server_uri) + .with_root_config("default_permissions = \":danger-full-access\"") + .with_extra_config(&format!("[permissions.dev]\nextends = \"{dev_extends}\"")) + .write(codex_home) +} + #[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/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 0f4332c30c..d3e147cd60 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -2360,6 +2360,196 @@ async fn permission_profile_override_populates_runtime_permissions() -> std::io: Ok(()) } +#[tokio::test] +async fn persisted_permission_profile_id_wins_over_configured_default() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + let config_toml = toml::from_str( + r#" +default_permissions = ":read-only" + +[permissions.dev] +extends = ":workspace" +"#, + ) + .expect("permission profile config should deserialize"); + + let config = Config::load_from_base_config_with_overrides( + config_toml, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + persisted_permission_profile_id: Some("dev".to_string()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + assert_eq!( + config.permissions.active_permission_profile(), + Some(ActivePermissionProfile { + id: "dev".to_string(), + extends: Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string()), + }) + ); + assert_eq!( + config.legacy_sandbox_policy(), + SandboxPolicy::new_workspace_write_policy() + ); + Ok(()) +} + +#[tokio::test] +async fn missing_persisted_permission_profile_id_uses_configured_default() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + let project_key = cwd.path().to_string_lossy().to_string(); + + for (configured_default, configured_sandbox, expected_active_profile) in [ + ( + Some(BUILT_IN_PERMISSION_PROFILE_READ_ONLY.to_string()), + None, + Some(ActivePermissionProfile::read_only()), + ), + (None, Some(SandboxMode::ReadOnly), None), + ] { + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + default_permissions: configured_default, + sandbox_mode: configured_sandbox, + projects: Some(HashMap::from([( + project_key.clone(), + ProjectConfig { + trust_level: Some(TrustLevel::Trusted), + }, + )])), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + persisted_permission_profile_id: Some("removed-profile".to_string()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + assert_eq!( + config.permissions.active_permission_profile(), + expected_active_profile + ); + assert_eq!( + config.permissions.effective_permission_profile(), + PermissionProfile::read_only() + ); + } + Ok(()) +} + +#[tokio::test] +async fn invalid_persisted_permission_profile_uses_configured_default() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + for invalid_profile in [ + "extends = \"removed-parent\"", + "extends = \":read-only\"\n\n[permissions.dev.filesystem]\nglob_scan_max_depth = 0", + ] { + let config_toml = toml::from_str(&format!( + "default_permissions = \":danger-full-access\"\n\n[permissions.dev]\n{invalid_profile}" + )) + .expect("permission profile config should deserialize"); + + let config = Config::load_from_base_config_with_overrides( + config_toml, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + persisted_permission_profile_id: Some("dev".to_string()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + assert_eq!( + config.permissions.active_permission_profile(), + Some(ActivePermissionProfile::new( + BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS, + )) + ); + assert_eq!( + config.permissions.effective_permission_profile(), + PermissionProfile::Disabled + ); + } + Ok(()) +} + +#[tokio::test] +async fn persisted_profile_cycle_uses_configured_default() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + let config_toml = toml::from_str( + r#" +default_permissions = ":danger-full-access" + +[permissions.dev] +extends = "base" + +[permissions.base] +extends = "dev" +"#, + ) + .expect("permission profile config should deserialize"); + + let config = Config::load_from_base_config_with_overrides( + config_toml, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + persisted_permission_profile_id: Some("dev".to_string()), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + assert_eq!( + config.permissions.active_permission_profile(), + Some(ActivePermissionProfile::new( + BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS, + )) + ); + assert_eq!( + config.permissions.effective_permission_profile(), + PermissionProfile::Disabled + ); + Ok(()) +} + +#[tokio::test] +async fn missing_explicit_default_permissions_remains_an_error() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + + let error = Config::load_from_base_config_with_overrides( + ConfigToml::default(), + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + default_permissions: Some("removed-profile".to_string()), + ..Default::default() + }, + codex_home.abs(), + ) + .await + .expect_err("explicit missing profile should be rejected"); + + assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); + assert_eq!( + error.to_string(), + "default_permissions requires a `[permissions]` table" + ); + Ok(()) +} + #[test] fn permission_snapshot_setter_preserves_permission_constraints() { let initial_profile = PermissionProfile::read_only(); @@ -10383,32 +10573,46 @@ async fn permission_profile_override_falls_back_when_disallowed_by_requirements( #[tokio::test] async fn active_profile_is_cleared_when_requirements_force_fallback() -> std::io::Result<()> { let codex_home = TempDir::new()?; - let config = ConfigBuilder::without_managed_config_for_tests() - .codex_home(codex_home.path().to_path_buf()) - .fallback_cwd(Some(codex_home.path().to_path_buf())) - .harness_overrides(ConfigOverrides { + for overrides in [ + ConfigOverrides { default_permissions: Some(BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS.to_string()), ..Default::default() - }) - .cloud_config_bundle( - CloudConfigBundleFixture::loader_with_enterprise_requirement( - r#"allowed_sandbox_modes = ["read-only"]"#, + }, + ConfigOverrides { + persisted_permission_profile_id: Some( + BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS.to_string(), ), - ) - .build() - .await?; + ..Default::default() + }, + ] { + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .harness_overrides(overrides) + .cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement( + r#"allowed_sandbox_modes = ["read-only"]"#, + ), + ) + .build() + .await?; - assert_eq!( - config.permissions.effective_permission_profile(), - PermissionProfile::read_only() - ); - assert_eq!(config.permissions.active_permission_profile(), None); - assert!( - config.startup_warnings.iter().any(|warning| warning - .contains("Configured value for `permission_profile` is disallowed by requirements")), - "{:?}", - config.startup_warnings - ); + assert_eq!( + config.permissions.effective_permission_profile(), + PermissionProfile::read_only() + ); + assert_eq!(config.permissions.active_permission_profile(), None); + assert!( + config + .startup_warnings + .iter() + .any(|warning| warning.contains( + "Configured value for `permission_profile` is disallowed by requirements" + )), + "{:?}", + config.startup_warnings + ); + } Ok(()) } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 10f4c0aa00..7cfa392daf 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2388,6 +2388,7 @@ struct PermissionSelectionToml { struct EffectivePermissionSelection<'a> { profiles: Option, selected_profile_id: Option<&'a str>, + persisted_profile_id_was_provided: bool, requirements_force_profile_selection: bool, } @@ -2403,7 +2404,8 @@ impl EffectivePermissionSelection<'_> { default_permissions_override: Option<&str>, permission_config_syntax: Option, ) -> bool { - self.requirements_force_profile_selection + self.persisted_profile_id_was_provided + || self.requirements_force_profile_selection || default_permissions_override.is_some() || matches!( permission_config_syntax, @@ -2506,6 +2508,10 @@ pub struct ConfigOverrides { pub sandbox_mode: Option, pub permission_profile: Option, pub default_permissions: Option, + /// Permission profile ID recovered from persisted thread state. Explicit + /// permission overrides take precedence, and stale profile IDs fall back + /// to the configured default. + pub persisted_permission_profile_id: Option, pub model_provider: Option, pub service_tier: Option>, pub codex_self_exe: Option, @@ -3153,6 +3159,7 @@ impl Config { sandbox_mode, permission_profile, default_permissions: default_permissions_override, + persisted_permission_profile_id, model_provider, service_tier: service_tier_override, codex_self_exe, @@ -3297,9 +3304,23 @@ impl Config { sandbox_mode, ); let requirements_toml = config_layer_stack.requirements_toml(); + let windows_sandbox_level = match effective_windows_sandbox_mode { + Some(WindowsSandboxModeToml::Elevated) => WindowsSandboxLevel::Elevated, + Some(WindowsSandboxModeToml::Unelevated) => WindowsSandboxLevel::RestrictedToken, + None => WindowsSandboxLevel::Disabled, + }; + let persisted_permission_profile_id = if sandbox_mode.is_some() + || permission_profile.is_some() + || default_permissions_override.is_some() + { + None + } else { + persisted_permission_profile_id.as_deref() + }; let effective_permission_selection = resolve_effective_permission_selection( cfg.permissions.as_ref(), default_permissions_override.as_deref(), + persisted_permission_profile_id, cfg.default_permissions.as_deref(), requirements_toml, &mut startup_warnings, @@ -3318,11 +3339,6 @@ impl Config { )); } - let windows_sandbox_level = match effective_windows_sandbox_mode { - Some(WindowsSandboxModeToml::Elevated) => WindowsSandboxLevel::Elevated, - Some(WindowsSandboxModeToml::Unelevated) => WindowsSandboxLevel::RestrictedToken, - None => WindowsSandboxLevel::Disabled, - }; let memories_config: MemoriesConfig = cfg.memories.clone().unwrap_or_default().into(); let memories_root = memory_root(&codex_home); @@ -3330,7 +3346,9 @@ impl Config { default_permissions_override.as_deref(), permission_config_syntax, ); - let explicit_permission_profile_mode = default_permissions_override.is_some() + let explicit_permission_profile_mode = effective_permission_selection + .persisted_profile_id_was_provided + || default_permissions_override.is_some() || matches!( permission_config_syntax, Some(PermissionConfigSyntax::Profiles) @@ -3342,7 +3360,9 @@ impl Config { .into_iter() .filter(|profile| !is_builtin_permission_profile_name(&profile.id)) .collect(); - let using_implicit_builtin_profile = permission_config_syntax.is_none() + let using_implicit_builtin_profile = !effective_permission_selection + .persisted_profile_id_was_provided + && permission_config_syntax.is_none() && effective_permission_selection.selected_profile_id.is_none(); let should_seed_legacy_workspace_roots = effective_permission_selection .selected_profile_id @@ -4387,18 +4407,31 @@ fn merge_managed_permission_profiles( } fn resolve_effective_permission_selection<'a>( - configured_permissions: Option<&PermissionsToml>, + configured_profiles: Option<&PermissionsToml>, default_permissions_override: Option<&'a str>, - configured_default_permissions: Option<&'a str>, + persisted_profile_id: Option<&'a str>, + configured_default_profile_id: Option<&'a str>, requirements_toml: &'a ConfigRequirementsToml, startup_warnings: &mut Vec, ) -> std::io::Result> { - let profiles = merge_managed_permission_profiles(configured_permissions, requirements_toml)?; + let profiles = merge_managed_permission_profiles(configured_profiles, requirements_toml)?; validate_user_permission_profile_names(profiles.as_ref())?; validate_required_permission_profile_catalog(requirements_toml, profiles.as_ref())?; + let valid_persisted_profile_id = persisted_profile_id.filter(|profile_id| { + is_builtin_permission_profile_name(profile_id) + || profiles.as_ref().is_some_and(|profiles| { + compile_permission_profile_selection( + Some(profiles), + profile_id, + /*workspace_write*/ None, + &mut Vec::new(), + ) + .is_ok() + }) + }); let selected_profile_id = resolve_default_permissions( - default_permissions_override, - configured_default_permissions, + default_permissions_override.or(valid_persisted_profile_id), + configured_default_profile_id, requirements_toml, startup_warnings, )?; @@ -4406,6 +4439,8 @@ fn resolve_effective_permission_selection<'a>( Ok(EffectivePermissionSelection { profiles, selected_profile_id, + persisted_profile_id_was_provided: default_permissions_override.is_none() + && valid_persisted_profile_id.is_some(), requirements_force_profile_selection: requirements_toml .allowed_permission_profiles .is_some(), diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 8143b72ff7..68bf5cea15 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -413,6 +413,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result sandbox_mode, permission_profile: None, default_permissions: None, + persisted_permission_profile_id: None, cwd: resolved_cwd, workspace_roots: None, model_provider: model_provider.clone(),