From fc49471ee225a285ec8d33188769db7aaddcfd91 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 9 Jul 2026 16:30:46 -0700 Subject: [PATCH] app-server: preserve environment root compatibility --- .../request_processors/thread_lifecycle.rs | 3 +- .../request_processors/thread_processor.rs | 25 +++++---------- .../src/request_processors/thread_summary.rs | 22 ++++--------- .../app-server/tests/suite/v2/thread_start.rs | 31 ++++++++++++++++--- codex-rs/core/src/session/session.rs | 2 +- 5 files changed, 43 insertions(+), 40 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/thread_lifecycle.rs b/codex-rs/app-server/src/request_processors/thread_lifecycle.rs index 0fc9194210..25346de7ee 100644 --- a/codex-rs/app-server/src/request_processors/thread_lifecycle.rs +++ b/codex-rs/app-server/src/request_processors/thread_lifecycle.rs @@ -619,6 +619,7 @@ pub(super) async fn handle_pending_thread_resume_request( } let config_snapshot = pending.config_snapshot; + let sandbox = config_snapshot.sandbox_policy().into(); let cwd = config_snapshot.cwd().clone(); let ThreadConfigSnapshot { model, @@ -626,7 +627,6 @@ pub(super) async fn handle_pending_thread_resume_request( service_tier, approval_policy, approvals_reviewer, - permission_profile, active_permission_profile, workspace_roots, reasoning_effort, @@ -634,7 +634,6 @@ pub(super) async fn handle_pending_thread_resume_request( .. } = config_snapshot; let instruction_sources = pending.instruction_sources; - let sandbox = thread_response_sandbox_policy(&permission_profile, cwd.as_path()); let active_permission_profile = thread_response_active_permission_profile(active_permission_profile); let session_id = conversation.session_configured().session_id.to_string(); 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 93e467b079..f361dd6087 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -1105,6 +1105,10 @@ impl ThreadRequestProcessor { .load_with_overrides(config_overrides.clone(), typesafe_overrides.clone()) .await .map_err(|err| config_load_error(&err))?; + let environment_selections = resolve_turn_environment_selections( + listener_task_context.thread_manager.as_ref(), + environments, + )?; // The user may have requested WorkspaceWrite or DangerFullAccess via // the command line, though in the process of deriving the Config, it @@ -1186,11 +1190,7 @@ impl ThreadRequestProcessor { } } - let environments = resolve_turn_environment_selections( - listener_task_context.thread_manager.as_ref(), - environments, - )? - .unwrap_or_else(|| { + let environments = environment_selections.unwrap_or_else(|| { listener_task_context .thread_manager .default_environment_selections(&config.cwd, &config.workspace_roots) @@ -1318,10 +1318,7 @@ impl ThreadRequestProcessor { /*has_in_progress_turn*/ false, ); - let sandbox = thread_response_sandbox_policy( - &config_snapshot.permission_profile, - config_snapshot.cwd().as_path(), - ); + let sandbox = config_snapshot.sandbox_policy().into(); let cwd = config_snapshot.cwd().clone(); let active_permission_profile = thread_response_active_permission_profile(config_snapshot.active_permission_profile); @@ -2895,10 +2892,7 @@ impl ThreadRequestProcessor { /*has_live_in_progress_turn*/ false, ); let config_snapshot = codex_thread.config_snapshot().await; - let sandbox = thread_response_sandbox_policy( - &config_snapshot.permission_profile, - config_snapshot.cwd().as_path(), - ); + let sandbox = config_snapshot.sandbox_policy().into(); let active_permission_profile = thread_response_active_permission_profile( config_snapshot.active_permission_profile, ); @@ -3657,10 +3651,7 @@ impl ThreadRequestProcessor { /*has_in_progress_turn*/ false, ); let config_snapshot = forked_thread.config_snapshot().await; - let sandbox = thread_response_sandbox_policy( - &config_snapshot.permission_profile, - config_snapshot.cwd().as_path(), - ); + let sandbox = config_snapshot.sandbox_policy().into(); let active_permission_profile = thread_response_active_permission_profile(config_snapshot.active_permission_profile); let thread_originator = config_snapshot.originator.clone(); diff --git a/codex-rs/app-server/src/request_processors/thread_summary.rs b/codex-rs/app-server/src/request_processors/thread_summary.rs index 139da67bca..caf662d642 100644 --- a/codex-rs/app-server/src/request_processors/thread_summary.rs +++ b/codex-rs/app-server/src/request_processors/thread_summary.rs @@ -172,17 +172,6 @@ pub(crate) fn thread_response_active_permission_profile( active_permission_profile.map(Into::into) } -pub(crate) fn thread_response_sandbox_policy( - permission_profile: &codex_protocol::models::PermissionProfile, - cwd: &Path, -) -> codex_app_server_protocol::SandboxPolicy { - let sandbox_policy = codex_sandboxing::compatibility_sandbox_policy_for_permission_profile( - permission_profile, - cwd, - ); - sandbox_policy.into() -} - pub(crate) fn thread_settings_from_config_snapshot( config_snapshot: &ThreadConfigSnapshot, ) -> ThreadSettings { @@ -190,10 +179,7 @@ pub(crate) fn thread_settings_from_config_snapshot( cwd: config_snapshot.cwd().clone(), approval_policy: config_snapshot.approval_policy.into(), approvals_reviewer: config_snapshot.approvals_reviewer.into(), - sandbox_policy: thread_response_sandbox_policy( - &config_snapshot.permission_profile, - config_snapshot.cwd().as_path(), - ), + sandbox_policy: config_snapshot.sandbox_policy().into(), active_permission_profile: thread_response_active_permission_profile( config_snapshot.active_permission_profile.clone(), ), @@ -225,7 +211,11 @@ pub(crate) fn thread_settings_from_core_snapshot( personality, collaboration_mode, } = snapshot; - let sandbox_policy = thread_response_sandbox_policy(&permission_profile, cwd.as_path()); + let sandbox_policy = codex_sandboxing::compatibility_sandbox_policy_for_permission_profile( + &permission_profile, + cwd.as_path(), + ) + .into(); ThreadSettings { sandbox_policy, cwd, diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index 0ee560ec5f..95407f8a68 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -16,6 +16,7 @@ use codex_app_server_protocol::McpServerStartupState; use codex_app_server_protocol::McpServerStatusUpdatedNotification; 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::TextPosition; use codex_app_server_protocol::TextRange; @@ -533,6 +534,7 @@ async fn thread_start_accepts_absolute_runtime_workspace_roots() -> Result<()> { .send_thread_start_request(ThreadStartParams { cwd: Some(cwd.to_string_lossy().to_string()), runtime_workspace_roots: Some(vec![extra_root.abs()]), + sandbox: Some(SandboxMode::WorkspaceWrite), ..Default::default() }) .await?; @@ -545,11 +547,19 @@ async fn thread_start_accepts_absolute_runtime_workspace_roots() -> Result<()> { let ThreadStartResponse { cwd: response_cwd, runtime_workspace_roots, + sandbox, .. } = to_response::(resp)?; assert_eq!(response_cwd, cwd.abs()); assert_eq!(runtime_workspace_roots, vec![extra_root.abs()]); + let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = sandbox else { + panic!("expected workspace-write sandbox"); + }; + assert!( + writable_roots.contains(&extra_root.abs().canonicalize()?), + "legacy sandbox projection should include the runtime workspace root" + ); let environment_root = cwd.join("environment-root"); std::fs::create_dir_all(&environment_root)?; @@ -559,6 +569,7 @@ async fn thread_start_accepts_absolute_runtime_workspace_roots() -> Result<()> { .send_thread_start_request(ThreadStartParams { runtime_workspace_roots: Some(vec![extra_root.abs()]), environments: Some(vec![environment]), + sandbox: Some(SandboxMode::WorkspaceWrite), ..Default::default() }) .await?; @@ -569,9 +580,17 @@ async fn thread_start_accepts_absolute_runtime_workspace_roots() -> Result<()> { .await??; let ThreadStartResponse { runtime_workspace_roots, + sandbox, .. } = to_response::(resp)?; assert_eq!(runtime_workspace_roots, vec![environment_root.abs()]); + let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = sandbox else { + panic!("expected workspace-write sandbox"); + }; + assert!( + writable_roots.contains(&environment_root.abs().canonicalize()?), + "legacy sandbox projection should include the environment workspace root" + ); Ok(()) } @@ -626,6 +645,10 @@ async fn thread_start_rejects_unknown_environment_as_invalid_request() -> Result let codex_home = TempDir::new()?; create_config_toml_without_approval_policy(codex_home.path(), &server.uri())?; + let config_path = codex_home.path().join("config.toml"); + let config_before = std::fs::read_to_string(&config_path)?; + let workspace = TempDir::new()?; + let workspace = workspace.path().to_path_buf().abs(); let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) @@ -635,12 +658,11 @@ async fn thread_start_rejects_unknown_environment_as_invalid_request() -> Result let request_id = mcp .send_thread_start_request(ThreadStartParams { + cwd: Some(workspace.to_string_lossy().into_owned()), + sandbox: Some(SandboxMode::WorkspaceWrite), environments: Some(vec![TurnEnvironmentParams { environment_id: "missing".to_string(), - cwd: codex_utils_absolute_path::AbsolutePathBuf::try_from( - codex_home.path().to_path_buf(), - )? - .into(), + cwd: workspace.into(), runtime_workspace_roots: None, }]), ..Default::default() @@ -656,6 +678,7 @@ async fn thread_start_rejects_unknown_environment_as_invalid_request() -> Result assert_eq!(error.id, RequestId::Integer(request_id)); assert_eq!(error.error.code, INVALID_REQUEST_ERROR_CODE); assert_eq!(error.error.message, "unknown turn environment id `missing`"); + assert_eq!(std::fs::read_to_string(config_path)?, config_before); Ok(()) } diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 7247d267c4..4453e908cd 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -198,7 +198,7 @@ impl SessionConfiguration { service_tier: self.service_tier.clone(), approval_policy: self.approval_policy.value(), approvals_reviewer: self.approvals_reviewer, - permission_profile: self.permission_profile(), + permission_profile: self.materialized_permission_profile(), active_permission_profile: self.active_permission_profile(), environments: self.environments.clone(), workspace_roots: self.primary_workspace_roots(),