From f95c19c33b6963542c192c82fd90b64e1ae923e3 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 12 May 2026 23:22:45 -0700 Subject: [PATCH] app-server: select permission profiles by id --- codex-rs/app-server/README.md | 26 +- codex-rs/app-server/src/in_process.rs | 46 +- codex-rs/app-server/src/message_processor.rs | 51 +- codex-rs/app-server/src/request_processors.rs | 8 + .../legacy_sandbox_compat.rs | 317 ++++++++++++ .../request_processors/thread_processor.rs | 486 +++++++++++++++++- .../thread_processor_tests.rs | 178 +++++++ .../src/request_processors/turn_processor.rs | 130 +++-- .../tests/suite/v2/thread_resume.rs | 119 +++++ .../app-server/tests/suite/v2/thread_start.rs | 51 ++ .../app-server/tests/suite/v2/turn_start.rs | 254 ++++++++- .../tests/suite/v2/turn_start_zsh_fork.rs | 10 +- 12 files changed, 1547 insertions(+), 129 deletions(-) create mode 100644 codex-rs/app-server/src/request_processors/legacy_sandbox_compat.rs diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 2be4694bf6..9e7324ee5a 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -76,7 +76,7 @@ Use the thread APIs to create, list, or archive conversations. Drive a conversat - Initialize once per connection: Immediately after opening a transport connection, send an `initialize` request with your client metadata, then emit an `initialized` notification. Any other request on that connection before this handshake gets rejected. - Start (or resume) a thread: Call `thread/start` to open a fresh conversation. The response returns the thread object and you’ll also get a `thread/started` notification. If you’re continuing an existing conversation, call `thread/resume` with its ID instead. If you want to branch from an existing conversation, call `thread/fork` to create a new thread id with copied history. Like `thread/start`, `thread/fork` also accepts `ephemeral: true` for an in-memory temporary thread. The returned `thread.ephemeral` flag tells you whether the session is intentionally in-memory only; when it is `true`, `thread.path` is `null`. -- Begin a turn: To send user input, call `turn/start` with the target `threadId` and the user's input. Optional fields let you override model, cwd, sandbox policy or experimental `permissions` profile selection, approval policy, approvals reviewer, etc. This immediately returns the new turn object. The app-server emits `turn/started` when that turn actually begins running. +- Begin a turn: To send user input, call `turn/start` with the target `threadId` and the user's input. Optional fields let you override model, cwd, workspace roots, approval policy, approvals reviewer, and the active permission profile name. Supplying `permissions` reselects one of the thread's named permission profiles; clients cannot submit an arbitrary permission profile value through `turn/start`. This immediately returns the new turn object. The app-server emits `turn/started` when that turn actually begins running. - Stream events: After `turn/start`, keep reading JSON-RPC notifications on stdout. You’ll see `item/started`, `item/completed`, deltas like `item/agentMessage/delta`, tool progress, etc. These represent streaming model output plus any side effects (commands, tool calls, reasoning notes). - Finish the turn: When the model is done (or the turn is interrupted via making the `turn/interrupt` call), the server sends `turn/completed` with the final turn state and token usage. @@ -131,9 +131,9 @@ Example with notification opt-out: ## API Overview - `thread/start` — create a new thread; emits `thread/started` (including the current `thread.status`) and auto-subscribes you to turn/item events for that thread. When the request includes a `cwd` and the resolved sandbox is `workspace-write` or full access, app-server also marks that project as trusted in the user `config.toml`. Pass `sessionStartSource: "clear"` when starting a replacement thread after clearing the current session so `SessionStart` hooks receive `source: "clear"` instead of the default `"startup"`. For permissions, prefer experimental `permissions` profile selection; the legacy `sandbox` shorthand is still accepted but cannot be combined with `permissions`. Experimental `environments` selects the sticky execution environments for turns on the thread; omit it to use the server default, pass `[]` to disable environments, or pass explicit environment ids with per-environment `cwd`. -- `thread/resume` — reopen an existing thread by id so subsequent `turn/start` calls append to it. Accepts the same permission override rules as `thread/start`. -- `thread/fork` — fork an existing thread into a new thread id by copying the stored history; if the source thread is currently mid-turn, the fork records the same interruption marker as `turn/interrupt` instead of inheriting an unmarked partial turn suffix. The returned `thread.forkedFromId` points at the source thread when known. Accepts `ephemeral: true` for an in-memory temporary fork, emits `thread/started` (including the current `thread.status`), and auto-subscribes you to turn/item events for the new thread. Experimental clients can pass `excludeTurns: true` when they plan to page fork history via `thread/turns/list` instead of receiving the full turn array immediately. Accepts the same permission override rules as `thread/start`. -- `thread/start`, `thread/resume`, and `thread/fork` responses include the legacy `sandbox` compatibility projection. Experimental clients can read response `permissionProfile` for the exact active runtime permissions and `activePermissionProfile` for the named or implicit built-in profile identity/provenance when known. +- `thread/resume` — reopen an existing thread by id so subsequent `turn/start` calls append to it. Omitted permission fields preserve the stored permission profile; clients may replace `workspaceRoots` or select a server-defined profile by id with `permissions`, but they cannot send arbitrary `PermissionProfile` bodies. The legacy `sandbox` shorthand is accepted as a compatibility spelling only when it maps to a named permissions profile and cannot be combined with `permissions`. +- `thread/fork` — fork an existing thread into a new thread id by copying the stored history; if the source thread is currently mid-turn, the fork records the same interruption marker as `turn/interrupt` instead of inheriting an unmarked partial turn suffix. The returned `thread.forkedFromId` points at the source thread when known. Accepts `ephemeral: true` for an in-memory temporary fork, emits `thread/started` (including the current `thread.status`), and auto-subscribes you to turn/item events for the new thread. Experimental clients can pass `excludeTurns: true` when they plan to page fork history via `thread/turns/list` instead of receiving the full turn array immediately. Like resume, omitted permission fields preserve the source permission profile while allowing explicit `workspaceRoots`, named `permissions` profile selection, or legacy `sandbox` compatibility selection. +- `thread/start`, `thread/resume`, and `thread/fork` responses include the legacy `sandbox` compatibility projection. Experimental clients can read response `activePermissionProfile` for the named or implicit built-in profile identity/provenance when known; the full `PermissionProfile` value is not exposed by thread lifecycle responses. - `thread/list` — page through stored rollouts; supports cursor-based pagination and optional `modelProviders`, `sourceKinds`, `archived`, `cwd`, and `searchTerm` filters. Each returned `thread` includes `status` (`ThreadStatus`), defaulting to `notLoaded` when the thread is not currently loaded. - `thread/loaded/list` — list the thread ids currently loaded in memory. - `thread/read` — read a stored thread by id without resuming it; optionally include turns via `includeTurns`. The returned `thread` includes `status` (`ThreadStatus`), defaulting to `notLoaded` when the thread is not currently loaded. @@ -156,7 +156,7 @@ Example with notification opt-out: - `thread/shellCommand` — run a user-initiated `!` shell command against a thread; this runs unsandboxed with full access rather than inheriting the thread sandbox policy. Returns `{}` immediately while progress streams through standard turn/item notifications and any active turn receives the formatted output in its message stream. - `thread/backgroundTerminals/clean` — terminate all running background terminals for a thread (experimental; requires `capabilities.experimentalApi`); returns `{}` when the cleanup request is accepted. - `thread/rollback` — drop the last N turns from the agent’s in-memory context and persist a rollback marker in the rollout so future resumes see the pruned history; returns the updated `thread` (with `turns` populated) on success. -- `turn/start` — add user input to a thread and begin Codex generation; responds with the initial `turn` object and streams `turn/started`, `item/*`, and `turn/completed` notifications. Prefer experimental `permissions` profile selection for permission overrides; the legacy `sandboxPolicy` field is still accepted but cannot be combined with `permissions`. For `collaborationMode`, `settings.developer_instructions: null` means "use built-in instructions for the selected mode". +- `turn/start` — add user input to a thread and begin Codex generation; responds with the initial `turn` object and streams `turn/started`, `item/*`, and `turn/completed` notifications. It can update `workspaceRoots` and use experimental `permissions` to select a server-defined profile by id, but it cannot replace the thread's permission profile with arbitrary values. The legacy `sandboxPolicy` field is accepted as a compatibility spelling only when it is a no-op or maps to a named permissions profile. For `collaborationMode`, `settings.developer_instructions: null` means "use built-in instructions for the selected mode". - `thread/inject_items` — append raw Responses API items to a loaded thread’s model-visible history without starting a user turn; returns `{}` on success. - `turn/steer` — add user input to an already in-flight regular turn without starting a new turn; returns the active `turnId` that accepted the input. Review and manual compaction turns reject `turn/steer`. - `turn/interrupt` — request cancellation of an in-flight turn by `(thread_id, turn_id)`; success is an empty `{}` response and the turn finishes with `status: "interrupted"`. @@ -231,10 +231,11 @@ Start a fresh thread when you need a new Codex conversation. // current config settings. "model": "gpt-5.1-codex", "cwd": "/Users/me/project", + "workspaceRoots": ["/Users/me/project"], "approvalPolicy": "never", "sandbox": "workspaceWrite", // Prefer experimental profile selection: - // "permissions": { "type": "profile", "id": ":workspace" } + // "permissions": ":workspace" // Do not send both "sandbox" and "permissions". "personality": "friendly", "serviceName": "my_app_server_client", // optional metrics tag (`service_name`) @@ -268,7 +269,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`. +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 pass non-permission configuration overrides such as `approvalsReviewer`; omitted permission fields preserve the stored permission profile. Use `workspaceRoots` to replace the thread root list, and use `permissions` to select a server-defined profile by id. Older clients may still send `sandbox` when it maps to the same named profile selection. 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. In that mode the server also skips replaying restored `thread/tokenUsage/updated`, which avoids rebuilding turns just to attribute historical usage. @@ -636,19 +637,14 @@ You can optionally specify config overrides on the new turn. If specified, these "input": [ { "type": "text", "text": "Run tests" } ], // Below are optional config overrides "cwd": "/Users/me/project", + "workspaceRoots": ["/Users/me/project", "/Users/me/project/packages/api"], // Experimental: turn-scoped environment selection. "environments": [ { "environmentId": "local", "cwd": "/Users/me/project" } ], "approvalPolicy": "unlessTrusted", - "sandboxPolicy": { - "type": "workspaceWrite", - "writableRoots": ["/Users/me/project"], - "networkAccess": true - }, - // Prefer experimental profile selection: - // "permissions": { "type": "profile", "id": ":workspace" } - // Do not send both "sandboxPolicy" and "permissions". + // Optional: select a named permission profile for this and later turns. + // "permissions": ":workspace" "model": "gpt-5.1-codex", "effort": "medium", "summary": "concise", diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 969ae95746..354f8ce7a4 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -194,18 +194,24 @@ pub struct InProcessClientSender { } impl InProcessClientSender { - pub async fn request(&self, request: ClientRequest) -> IoResult { + pub fn request( + &self, + request: ClientRequest, + ) -> impl std::future::Future> + '_ { let (response_tx, response_rx) = oneshot::channel(); - self.try_send_client_message(InProcessClientMessage::Request { + let send_result = self.try_send_client_message(InProcessClientMessage::Request { request: Box::new(request), response_tx, - })?; - response_rx.await.map_err(|err| { - IoError::new( - ErrorKind::BrokenPipe, - format!("in-process request response channel closed: {err}"), - ) - }) + }); + async move { + send_result?; + response_rx.await.map_err(|err| { + IoError::new( + ErrorKind::BrokenPipe, + format!("in-process request response channel closed: {err}"), + ) + }) + } } pub fn notify(&self, notification: ClientNotification) -> IoResult<()> { @@ -266,8 +272,11 @@ impl InProcessClientHandle { /// request IDs unique among concurrent requests; reusing an in-flight ID /// produces an `INVALID_REQUEST` response and can make request routing /// ambiguous in the caller. - pub async fn request(&self, request: ClientRequest) -> IoResult { - self.client.request(request).await + pub fn request( + &self, + request: ClientRequest, + ) -> impl std::future::Future> + '_ { + self.client.request(request) } /// Sends a typed client notification into the in-process runtime. @@ -444,12 +453,12 @@ async fn start_uninitialized(args: InProcessStartArgs) -> IoResult IoResult { match message { Some(InProcessClientMessage::Request { request, response_tx }) => { - let request = *request; let request_id = request.id().clone(); match pending_request_responses.entry(request_id.clone()) { Entry::Vacant(entry) => { @@ -534,7 +542,7 @@ async fn start_uninitialized(args: InProcessStartArgs) -> IoResult {} Err(mpsc::error::TrySendError::Full(_)) => { if let Some(response_tx) = diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 0581d385ca..1e7af9d706 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -567,7 +567,7 @@ impl MessageProcessor { pub(crate) fn process_client_request<'a>( self: &'a Arc, connection_id: ConnectionId, - request: ClientRequest, + request: Box, session: Arc, outbound_initialized: &'a AtomicBool, ) -> Pin + Send + 'a>> { @@ -575,8 +575,11 @@ impl MessageProcessor { connection_id, request_id: request.id().clone(), }; - let request_span = - crate::app_server_tracing::typed_request_span(&request, connection_id, &session); + let request_span = crate::app_server_tracing::typed_request_span( + request.as_ref(), + connection_id, + &session, + ); let request_context = RequestContext::new(request_id.clone(), request_span, /*parent_trace*/ None); tracing::trace!( @@ -592,15 +595,14 @@ impl MessageProcessor { // In-process clients do not have the websocket transport loop that performs // post-initialize bookkeeping, so they still finalize outbound readiness in // the shared request handler. - let result = self - .handle_client_request( - request_id.clone(), - request, - Arc::clone(&session), - Some(outbound_initialized), - request_context.clone(), - ) - .await; + let result = Box::pin(self.handle_client_request( + request_id.clone(), + *request, + Arc::clone(&session), + Some(outbound_initialized), + request_context.clone(), + )) + .await; if let Err(error) = result { self.outgoing.send_error(request_id.clone(), error).await; } @@ -623,10 +625,10 @@ impl MessageProcessor { tracing::info!("<- typed notification: {:?}", notification); } - async fn run_request_with_context<'a>( + async fn run_request_with_context( outgoing: Arc, request_context: RequestContext, - request_fut: Pin + Send + 'a>>, + request_fut: Pin + Send + '_>>, ) { outgoing .register_request_context(request_context.clone()) @@ -765,12 +767,12 @@ impl MessageProcessor { return Ok(()); } - self.dispatch_initialized_client_request( + Box::pin(self.dispatch_initialized_client_request( connection_request_id, codex_request, session, request_context, - ) + )) .await } @@ -808,15 +810,14 @@ impl MessageProcessor { rpc_gate, async move { let processor_for_request = Arc::clone(&processor); - let result = processor_for_request - .handle_initialized_client_request( - connection_request_id, - codex_request, - request_context, - app_server_client_name, - client_version, - ) - .await; + let result = Box::pin(processor_for_request.handle_initialized_client_request( + connection_request_id, + codex_request, + request_context, + app_server_client_name, + client_version, + )) + .await; if let Err(error) = result { processor.outgoing.send_error(error_request_id, error).await; } diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index 5b94aa4bbf..e119fe3286 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -352,6 +352,8 @@ use codex_protocol::error::CodexErr; use codex_protocol::error::Result as CodexResult; #[cfg(test)] use codex_protocol::items::TurnItem; +use codex_protocol::models::ActivePermissionProfile; +use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItem; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::protocol::AgentStatus; @@ -428,6 +430,11 @@ use uuid::Uuid; #[cfg(test)] use codex_app_server_protocol::ServerRequest; +struct ResolvedPermissionProfileSelection { + permission_profile: PermissionProfile, + active_permission_profile: ActivePermissionProfile, +} + mod account_processor; mod apps_processor; mod catalog_processor; @@ -439,6 +446,7 @@ mod feedback_processor; mod fs_processor; mod git_processor; mod initialize_processor; +mod legacy_sandbox_compat; mod marketplace_processor; mod mcp_processor; mod plugins; diff --git a/codex-rs/app-server/src/request_processors/legacy_sandbox_compat.rs b/codex-rs/app-server/src/request_processors/legacy_sandbox_compat.rs new file mode 100644 index 0000000000..80caad51a5 --- /dev/null +++ b/codex-rs/app-server/src/request_processors/legacy_sandbox_compat.rs @@ -0,0 +1,317 @@ +use super::*; + +const WORKSPACE_PERMISSION_PROFILE_ID: &str = ":workspace"; +const READ_ONLY_PERMISSION_PROFILE_ID: &str = ":read-only"; +const DANGER_NO_SANDBOX_PERMISSION_PROFILE_ID: &str = ":danger-no-sandbox"; + +pub(super) struct CurrentPermissionProfile<'a> { + pub(super) permission_profile: &'a PermissionProfile, + pub(super) workspace_roots: &'a [AbsolutePathBuf], +} + +pub(super) struct LegacySandboxProfileSelection { + pub(super) permissions: String, + pub(super) workspace_roots: Option>, + expected_sandbox_policy: codex_protocol::protocol::SandboxPolicy, +} + +pub(super) enum LegacySandboxResolution { + Noop { + workspace_roots: Option>, + }, + Selection(LegacySandboxProfileSelection), +} + +pub(super) fn resolve_legacy_sandbox_profile_selection( + sandbox_policy: &codex_app_server_protocol::SandboxPolicy, + current: Option>, + cwd: &AbsolutePathBuf, + explicit_workspace_roots: Option<&[AbsolutePathBuf]>, + field_name: &str, +) -> Result { + let legacy_workspace_roots = + workspace_roots_from_implicit_legacy_sandbox(cwd, sandbox_policy, explicit_workspace_roots); + if let Some(current_match) = legacy_sandbox_current_match( + sandbox_policy, + current, + cwd, + explicit_workspace_roots, + legacy_workspace_roots.as_deref(), + ) { + return Ok(LegacySandboxResolution::Noop { + workspace_roots: current_match.workspace_roots, + }); + } + + let expected_sandbox_policy = sandbox_policy.to_core(); + match sandbox_policy { + codex_app_server_protocol::SandboxPolicy::DangerFullAccess => Ok( + LegacySandboxResolution::Selection(LegacySandboxProfileSelection { + permissions: DANGER_NO_SANDBOX_PERMISSION_PROFILE_ID.to_string(), + workspace_roots: None, + expected_sandbox_policy, + }), + ), + codex_app_server_protocol::SandboxPolicy::ReadOnly { network_access: _ } => Ok( + LegacySandboxResolution::Selection(LegacySandboxProfileSelection { + permissions: READ_ONLY_PERMISSION_PROFILE_ID.to_string(), + workspace_roots: None, + expected_sandbox_policy, + }), + ), + codex_app_server_protocol::SandboxPolicy::WorkspaceWrite { .. } => Ok( + LegacySandboxResolution::Selection(LegacySandboxProfileSelection { + permissions: WORKSPACE_PERMISSION_PROFILE_ID.to_string(), + workspace_roots: legacy_workspace_roots, + expected_sandbox_policy, + }), + ), + codex_app_server_protocol::SandboxPolicy::ExternalSandbox { .. } => { + Err(invalid_request(format!( + "`{field_name}` externalSandbox cannot be mapped to a named permissions profile" + ))) + } + } +} + +pub(super) fn sandbox_policy_from_legacy_mode( + sandbox_mode: SandboxMode, +) -> codex_app_server_protocol::SandboxPolicy { + match sandbox_mode { + SandboxMode::ReadOnly => codex_protocol::protocol::SandboxPolicy::new_read_only_policy(), + SandboxMode::WorkspaceWrite => { + codex_protocol::protocol::SandboxPolicy::new_workspace_write_policy() + } + SandboxMode::DangerFullAccess => codex_protocol::protocol::SandboxPolicy::DangerFullAccess, + } + .into() +} + +pub(super) fn validate_legacy_sandbox_profile_selection( + legacy_selection: &LegacySandboxProfileSelection, + resolved_selection: &ResolvedPermissionProfileSelection, + cwd: &AbsolutePathBuf, + workspace_roots: Option<&[AbsolutePathBuf]>, + field_name: &str, +) -> Result<(), JSONRPCErrorError> { + let workspace_roots = workspace_roots + .or(legacy_selection.workspace_roots.as_deref()) + .unwrap_or(&[]); + let materialized_profile = resolved_selection + .permission_profile + .materialize_project_roots_with_workspace_roots(workspace_roots); + let file_system_policy = materialized_profile.file_system_sandbox_policy(); + let active_sandbox = codex_sandboxing::compatibility_sandbox_policy_for_permission_profile( + &materialized_profile, + &file_system_policy, + materialized_profile.network_sandbox_policy(), + cwd.as_path(), + ); + if active_sandbox != legacy_selection.expected_sandbox_policy { + return Err(invalid_request(format!( + "`{field_name}` does not match permissions profile `{}`", + legacy_selection.permissions + ))); + } + Ok(()) +} + +pub(super) fn resolve_cwd_against_fallback( + cwd: Option<&Path>, + fallback_cwd: &AbsolutePathBuf, +) -> AbsolutePathBuf { + match cwd { + Some(cwd) => { + if let Ok(path) = AbsolutePathBuf::try_from(cwd) { + path + } else { + AbsolutePathBuf::resolve_path_against_base(cwd, fallback_cwd.as_path()) + } + } + None => fallback_cwd.clone(), + } +} + +struct LegacySandboxCurrentMatch { + workspace_roots: Option>, +} + +fn legacy_sandbox_current_match( + sandbox_policy: &codex_app_server_protocol::SandboxPolicy, + current: Option>, + cwd: &AbsolutePathBuf, + explicit_workspace_roots: Option<&[AbsolutePathBuf]>, + legacy_workspace_roots: Option<&[AbsolutePathBuf]>, +) -> Option { + let current = current?; + + let projection_workspace_roots = explicit_workspace_roots + .or(legacy_workspace_roots) + .unwrap_or(current.workspace_roots); + let materialized_profile = current + .permission_profile + .materialize_project_roots_with_workspace_roots(projection_workspace_roots); + let file_system_policy = materialized_profile.file_system_sandbox_policy(); + let active_sandbox = codex_sandboxing::compatibility_sandbox_policy_for_permission_profile( + &materialized_profile, + &file_system_policy, + materialized_profile.network_sandbox_policy(), + cwd.as_path(), + ); + if active_sandbox != sandbox_policy.to_core() { + return None; + } + + let workspace_roots = legacy_workspace_roots + .filter(|roots| *roots != current.workspace_roots) + .map(<[AbsolutePathBuf]>::to_vec); + + Some(LegacySandboxCurrentMatch { workspace_roots }) +} + +fn workspace_roots_from_implicit_legacy_sandbox( + cwd: &AbsolutePathBuf, + sandbox_policy: &codex_app_server_protocol::SandboxPolicy, + explicit_workspace_roots: Option<&[AbsolutePathBuf]>, +) -> Option> { + if explicit_workspace_roots.is_some() + || !matches!( + sandbox_policy, + codex_app_server_protocol::SandboxPolicy::WorkspaceWrite { .. } + ) + { + None + } else { + Some(workspace_roots_from_legacy_sandbox(cwd, sandbox_policy)) + } +} + +fn workspace_roots_from_legacy_sandbox( + cwd: &AbsolutePathBuf, + sandbox_policy: &codex_app_server_protocol::SandboxPolicy, +) -> Vec { + let mut roots = Vec::with_capacity(1 + sandbox_policy.legacy_writable_roots().len()); + push_unique_root(&mut roots, cwd.clone()); + for root in sandbox_policy.legacy_writable_roots() { + push_unique_root(&mut roots, root.clone()); + } + roots +} + +fn push_unique_root(roots: &mut Vec, root: AbsolutePathBuf) { + if !roots.iter().any(|existing| existing == &root) { + roots.push(root); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn abs_test_path(name: &str) -> AbsolutePathBuf { + AbsolutePathBuf::from_absolute_path(std::env::temp_dir().join(name)) + .expect("temp dir path should be absolute") + } + + fn workspace_write_policy() -> codex_app_server_protocol::SandboxPolicy { + codex_app_server_protocol::SandboxPolicy::WorkspaceWrite { + network_access: false, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + legacy_writable_roots: Vec::new(), + } + } + + #[test] + fn legacy_workspace_sandbox_updates_roots_when_current_profile_matches_new_cwd() { + let old_root = abs_test_path("codex-old-workspace-root"); + let cwd = abs_test_path("codex-new-workspace-root"); + let policy = workspace_write_policy(); + + let resolution = resolve_legacy_sandbox_profile_selection( + &policy, + Some(CurrentPermissionProfile { + permission_profile: &PermissionProfile::workspace_write(), + workspace_roots: std::slice::from_ref(&old_root), + }), + &cwd, + /*explicit_workspace_roots*/ None, + "sandboxPolicy", + ) + .expect("legacy sandbox should resolve"); + + match resolution { + LegacySandboxResolution::Noop { + workspace_roots: Some(workspace_roots), + } => assert_eq!(workspace_roots, vec![cwd]), + _ => panic!("expected workspace-roots-only resolution, got unexpected selection"), + } + } + + #[test] + fn legacy_workspace_sandbox_is_noop_when_current_workspace_roots_match() { + let cwd = abs_test_path("codex-current-workspace-root"); + let policy = workspace_write_policy(); + + let resolution = resolve_legacy_sandbox_profile_selection( + &policy, + Some(CurrentPermissionProfile { + permission_profile: &PermissionProfile::workspace_write(), + workspace_roots: std::slice::from_ref(&cwd), + }), + &cwd, + /*explicit_workspace_roots*/ None, + "sandboxPolicy", + ) + .expect("legacy sandbox should resolve"); + + match resolution { + LegacySandboxResolution::Noop { + workspace_roots: None, + } => {} + _ => panic!("expected no-op resolution, got unexpected workspace roots or selection"), + } + } + + #[test] + fn legacy_workspace_selection_validation_requires_exact_projection() { + let cwd = abs_test_path("codex-projection-mismatch-root"); + let policy = workspace_write_policy(); + let resolution = resolve_legacy_sandbox_profile_selection( + &policy, + /*current*/ None, + &cwd, + /*explicit_workspace_roots*/ None, + "sandboxPolicy", + ) + .expect("legacy sandbox should resolve"); + let LegacySandboxResolution::Selection(selection) = resolution else { + panic!("expected named profile selection"); + }; + let resolved_selection = ResolvedPermissionProfileSelection { + permission_profile: PermissionProfile::workspace_write_with( + &[], + codex_protocol::permissions::NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ false, + ), + active_permission_profile: codex_protocol::models::ActivePermissionProfile::new( + ":workspace", + ), + }; + + let err = validate_legacy_sandbox_profile_selection( + &selection, + &resolved_selection, + &cwd, + selection.workspace_roots.as_deref(), + "sandboxPolicy", + ) + .expect_err("mismatched workspace profile should be rejected"); + + assert_eq!( + err.message, + "`sandboxPolicy` does not match permissions profile `:workspace`" + ); + } +} 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 89ea960fc4..fdccae1e66 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -1,3 +1,4 @@ +use super::legacy_sandbox_compat::*; use super::*; use crate::error_code::method_not_found; @@ -17,6 +18,106 @@ struct ThreadListFilters { use_state_db_only: bool, } +#[derive(Clone)] +struct PersistedThreadPermissionState { + permission_profile: PermissionProfile, + active_permission_profile: Option, + workspace_roots: Vec, +} + +fn absolute_path_from_history_path( + path: &Path, + base: Option<&AbsolutePathBuf>, +) -> Option { + if let Ok(path) = AbsolutePathBuf::try_from(path) { + Some(path) + } else if let Some(base) = base { + Some(AbsolutePathBuf::resolve_path_against_base( + path, + base.as_path(), + )) + } else { + AbsolutePathBuf::relative_to_current_dir(path).ok() + } +} + +fn effective_cwd_for_legacy_sandbox( + request_cwd: Option<&str>, + history_cwd: Option<&Path>, + persisted_permission_state: Option<&PersistedThreadPermissionState>, +) -> Option { + let history_cwd = + history_cwd.and_then(|cwd| absolute_path_from_history_path(cwd, /*base*/ None)); + request_cwd + .and_then(|cwd| absolute_path_from_history_path(Path::new(cwd), history_cwd.as_ref())) + .or_else(|| history_cwd.clone()) + .or_else(|| { + persisted_permission_state + .and_then(|state| state.workspace_roots.first()) + .cloned() + }) +} + +fn persisted_thread_permission_state( + history: &InitialHistory, + fallback_cwd: Option<&Path>, + fallback_sandbox_policy: Option<&codex_protocol::protocol::SandboxPolicy>, +) -> Option { + let mut cwd = + fallback_cwd.and_then(|cwd| absolute_path_from_history_path(cwd, /*base*/ None)); + let mut workspace_roots = None; + let mut permission_profile = None; + let mut active_permission_profile = None; + + for item in history.get_rollout_items() { + match item { + RolloutItem::SessionMeta(meta_line) => { + cwd = absolute_path_from_history_path(meta_line.meta.cwd.as_path(), cwd.as_ref()) + .or(cwd); + workspace_roots = Some(meta_line.meta.workspace_roots); + } + RolloutItem::TurnContext(context) => { + cwd = absolute_path_from_history_path(context.cwd.as_path(), cwd.as_ref()).or(cwd); + workspace_roots = Some(context.workspace_roots); + let context_cwd = cwd + .as_ref() + .map(AbsolutePathBuf::as_path) + .unwrap_or(context.cwd.as_path()); + permission_profile = Some(context.permission_profile.unwrap_or_else(|| { + PermissionProfile::from_legacy_sandbox_policy_for_cwd( + &context.sandbox_policy, + context_cwd, + ) + })); + active_permission_profile = context.active_permission_profile; + } + RolloutItem::EventMsg(EventMsg::SessionConfigured(event)) => { + cwd = Some(event.cwd.clone()); + workspace_roots = Some(event.workspace_roots); + permission_profile = Some(event.permission_profile); + active_permission_profile = event.active_permission_profile; + } + RolloutItem::ResponseItem(_) | RolloutItem::Compacted(_) | RolloutItem::EventMsg(_) => { + } + } + } + + if permission_profile.is_none() { + let cwd = cwd.as_ref()?; + let fallback_sandbox_policy = fallback_sandbox_policy?; + permission_profile = Some(PermissionProfile::from_legacy_sandbox_policy_for_cwd( + fallback_sandbox_policy, + cwd.as_path(), + )); + } + + Some(PersistedThreadPermissionState { + permission_profile: permission_profile?, + active_permission_profile, + workspace_roots: workspace_roots.unwrap_or_else(|| cwd.into_iter().collect()), + }) +} + fn collect_resume_override_mismatches( request: &ThreadResumeParams, config_snapshot: &ThreadConfigSnapshot, @@ -98,12 +199,6 @@ fn collect_resume_override_mismatches( )); } } - if request.permissions.is_some() { - mismatch_details.push(format!( - "permissions override was provided and ignored while running; active={:?}", - config_snapshot.active_permission_profile - )); - } if let Some(requested_personality) = request.personality.as_ref() && config_snapshot.personality.as_ref() != Some(requested_personality) { @@ -977,7 +1072,7 @@ impl ThreadRequestProcessor { let requested_permissions_trust_project = requested_permissions_trust_project(&typesafe_overrides, config.cwd.as_path()); let effective_permissions_trust_project = permission_profile_trusts_project( - &config.permissions.permission_profile(), + config.permissions.permission_profile_ref(), config.cwd.as_path(), ); @@ -1247,6 +1342,47 @@ impl ThreadRequestProcessor { overrides } + async fn validate_active_permission_profile_selection( + &self, + permissions: String, + request_overrides: Option>, + cwd: Option, + fallback_cwd: Option, + ) -> Result { + let mut overrides = ConfigOverrides { + cwd, + codex_linux_sandbox_exe: self.arg0_paths.codex_linux_sandbox_exe.clone(), + main_execve_wrapper_exe: self.arg0_paths.main_execve_wrapper_exe.clone(), + ..Default::default() + }; + apply_permission_profile_selection_to_config_overrides(&mut overrides, Some(permissions)); + let config = self + .config_manager + .load_for_cwd(request_overrides, overrides, fallback_cwd) + .await + .map_err(|err| config_load_error(&err))?; + if let Some(warning) = config.startup_warnings.iter().find(|warning| { + warning.contains("Configured value for `permission_profile` is disallowed") + }) { + return Err(invalid_request(format!( + "invalid permission profile selection: {warning}" + ))); + } + let active_permission_profile = + config + .permissions + .active_permission_profile() + .ok_or_else(|| { + invalid_request( + "permission profile selection did not resolve to a named profile", + ) + })?; + Ok(ResolvedPermissionProfileSelection { + permission_profile: config.permissions.permission_profile(), + active_permission_profile, + }) + } + fn parse_environment_selections( &self, environments: Option>, @@ -2368,6 +2504,7 @@ impl ThreadRequestProcessor { persist_extended_history: _persist_extended_history, } = params; let include_turns = !exclude_turns; + let mut workspace_roots = workspace_roots; let (thread_history, resume_source_thread) = match if let Some(history) = history { self.resume_thread_from_history(history.as_slice()) @@ -2386,6 +2523,118 @@ impl ThreadRequestProcessor { }; let history_cwd = thread_history.session_cwd(); + let persisted_permission_state = persisted_thread_permission_state( + &thread_history, + history_cwd.as_deref(), + resume_source_thread + .as_ref() + .map(|thread| &thread.sandbox_policy), + ); + let permission_profile_selection = if let Some(permissions) = permissions { + match self + .validate_active_permission_profile_selection( + permissions, + request_overrides.clone(), + cwd.clone().map(PathBuf::from), + history_cwd.clone(), + ) + .await + { + Ok(selection) => Some(selection), + Err(error) => { + self.outgoing.send_error(request_id, error).await; + return Ok(()); + } + } + } else if let Some(sandbox_mode) = sandbox { + let sandbox_policy = sandbox_policy_from_legacy_mode(sandbox_mode); + let Some(effective_cwd) = effective_cwd_for_legacy_sandbox( + cwd.as_deref(), + history_cwd.as_deref(), + persisted_permission_state.as_ref(), + ) else { + self.outgoing + .send_error( + request_id, + invalid_request("`sandbox` requires a cwd to resolve legacy permissions"), + ) + .await; + return Ok(()); + }; + match resolve_legacy_sandbox_profile_selection( + &sandbox_policy, + persisted_permission_state + .as_ref() + .map(|state| CurrentPermissionProfile { + permission_profile: &state.permission_profile, + workspace_roots: &state.workspace_roots, + }), + &effective_cwd, + workspace_roots.as_deref(), + "sandbox", + ) { + Ok(LegacySandboxResolution::Noop { + workspace_roots: legacy_workspace_roots, + }) => { + if workspace_roots.is_none() { + workspace_roots = legacy_workspace_roots; + } + None + } + Ok(LegacySandboxResolution::Selection(legacy_selection)) => { + if workspace_roots.is_none() { + workspace_roots = legacy_selection.workspace_roots.clone(); + } + match self + .validate_active_permission_profile_selection( + legacy_selection.permissions.clone(), + request_overrides.clone(), + cwd.clone().map(PathBuf::from), + history_cwd.clone(), + ) + .await + .and_then(|selection| { + validate_legacy_sandbox_profile_selection( + &legacy_selection, + &selection, + &effective_cwd, + workspace_roots.as_deref(), + "sandbox", + ) + .map(|()| selection) + }) { + Ok(selection) => Some(selection), + Err(error) => { + self.outgoing.send_error(request_id, error).await; + return Ok(()); + } + } + } + Err(error) => { + self.outgoing.send_error(request_id, error).await; + return Ok(()); + } + } + } else { + None + }; + let active_permission_profile = permission_profile_selection + .as_ref() + .map(|selection| selection.active_permission_profile.clone()) + .or_else(|| { + persisted_permission_state + .as_ref() + .and_then(|state| state.active_permission_profile.clone()) + }); + let expected_active_permission_profile_source = permission_profile_selection + .as_ref() + .map(|selection| selection.permission_profile.clone()) + .or_else(|| { + persisted_permission_state + .as_ref() + .map(|state| state.permission_profile.clone()) + }); + let workspace_roots_were_explicit = workspace_roots.is_some(); let mut typesafe_overrides = self.build_thread_config_overrides( model, model_provider, @@ -2394,12 +2643,29 @@ impl ThreadRequestProcessor { workspace_roots, approval_policy, approvals_reviewer, - sandbox, - permissions, + /*sandbox*/ None, + /*permissions*/ None, base_instructions, developer_instructions, personality, ); + typesafe_overrides.permission_profile = expected_active_permission_profile_source.clone(); + if !workspace_roots_were_explicit { + if let Some(persisted_permission_state) = persisted_permission_state.as_ref() { + typesafe_overrides.workspace_roots = Some( + persisted_permission_state + .workspace_roots + .iter() + .map(codex_utils_absolute_path::AbsolutePathBuf::to_path_buf) + .collect(), + ); + } else if let Some(root) = history_cwd + .as_deref() + .and_then(|cwd| absolute_path_from_history_path(cwd, /*base*/ None)) + { + typesafe_overrides.workspace_roots = Some(vec![root.to_path_buf()]); + } + } self.load_and_apply_persisted_resume_metadata( &thread_history, &mut request_overrides, @@ -2408,7 +2674,7 @@ impl ThreadRequestProcessor { .await; // Derive a Config using the same logic as new conversation, honoring overrides if provided. - let config = match self + let mut config = match self .config_manager .load_for_cwd(request_overrides, typesafe_overrides, history_cwd) .await @@ -2420,6 +2686,12 @@ impl ThreadRequestProcessor { return Ok(()); } }; + config + .permissions + .set_active_permission_profile_for_current_profile( + active_permission_profile, + expected_active_permission_profile_source.as_ref(), + ); let instruction_sources = Self::instruction_sources_from_config(&config).await; let response_history = thread_history.clone(); @@ -2688,6 +2960,96 @@ impl ThreadRequestProcessor { ) .await?; + let config_snapshot = existing_thread.config_snapshot().await; + let mut workspace_roots = params.workspace_roots.clone(); + let permission_profile_selection = if let Some(permissions) = params.permissions.clone() + { + Some( + self.validate_active_permission_profile_selection( + permissions, + /*request_overrides*/ None, + /*cwd*/ None, + Some(config_snapshot.cwd.to_path_buf()), + ) + .await?, + ) + } else if let Some(sandbox_mode) = params.sandbox { + let sandbox_policy = sandbox_policy_from_legacy_mode(sandbox_mode); + match resolve_legacy_sandbox_profile_selection( + &sandbox_policy, + Some(CurrentPermissionProfile { + permission_profile: &config_snapshot.permission_profile, + workspace_roots: &config_snapshot.workspace_roots, + }), + &config_snapshot.cwd, + workspace_roots.as_deref(), + "sandbox", + )? { + LegacySandboxResolution::Noop { + workspace_roots: legacy_workspace_roots, + } => { + if workspace_roots.is_none() { + workspace_roots = legacy_workspace_roots; + } + None + } + LegacySandboxResolution::Selection(legacy_selection) => { + if workspace_roots.is_none() { + workspace_roots = legacy_selection.workspace_roots.clone(); + } + let selection = self + .validate_active_permission_profile_selection( + legacy_selection.permissions.clone(), + /*request_overrides*/ None, + /*cwd*/ None, + Some(config_snapshot.cwd.to_path_buf()), + ) + .await?; + validate_legacy_sandbox_profile_selection( + &legacy_selection, + &selection, + &config_snapshot.cwd, + workspace_roots.as_deref(), + "sandbox", + )?; + Some(selection) + } + } + } else { + None + }; + if workspace_roots.is_some() || permission_profile_selection.is_some() { + existing_thread + .update_turn_context_overrides(CodexThreadTurnContextOverrides { + cwd: None, + workspace_roots: workspace_roots.map(|roots| { + roots + .into_iter() + .map(AbsolutePathBuf::into_path_buf) + .collect() + }), + approval_policy: None, + approvals_reviewer: None, + sandbox_policy: None, + permission_profile: permission_profile_selection + .as_ref() + .map(|selection| selection.permission_profile.clone()), + active_permission_profile: permission_profile_selection + .map(|selection| selection.active_permission_profile), + windows_sandbox_level: None, + model: None, + effort: None, + summary: None, + service_tier: None, + collaboration_mode: None, + personality: None, + }) + .await + .map_err(|err| { + invalid_request(format!("invalid thread resume override: {err}")) + })?; + } + let config_snapshot = existing_thread.config_snapshot().await; let mismatch_details = collect_resume_override_mismatches(params, &config_snapshot); if !mismatch_details.is_empty() { @@ -3055,6 +3417,88 @@ impl ThreadRequestProcessor { } else { Some(cli_overrides) }; + let fork_history = InitialHistory::Forked(history_items.clone()); + let persisted_permission_state = persisted_thread_permission_state( + &fork_history, + history_cwd.as_deref(), + Some(&source_thread.sandbox_policy), + ) + .ok_or_else(|| { + invalid_request("thread history is missing persisted permission configuration") + })?; + let mut workspace_roots = workspace_roots; + let permission_profile_selection = if let Some(permissions) = permissions { + Some( + self.validate_active_permission_profile_selection( + permissions, + request_overrides.clone(), + cwd.clone().map(PathBuf::from), + history_cwd.clone(), + ) + .await?, + ) + } else if let Some(sandbox_mode) = sandbox { + let sandbox_policy = sandbox_policy_from_legacy_mode(sandbox_mode); + let effective_cwd = effective_cwd_for_legacy_sandbox( + cwd.as_deref(), + history_cwd.as_deref(), + Some(&persisted_permission_state), + ) + .ok_or_else(|| { + invalid_request("`sandbox` requires a cwd to resolve legacy permissions") + })?; + match resolve_legacy_sandbox_profile_selection( + &sandbox_policy, + Some(CurrentPermissionProfile { + permission_profile: &persisted_permission_state.permission_profile, + workspace_roots: &persisted_permission_state.workspace_roots, + }), + &effective_cwd, + workspace_roots.as_deref(), + "sandbox", + )? { + LegacySandboxResolution::Noop { + workspace_roots: legacy_workspace_roots, + } => { + if workspace_roots.is_none() { + workspace_roots = legacy_workspace_roots; + } + None + } + LegacySandboxResolution::Selection(legacy_selection) => { + if workspace_roots.is_none() { + workspace_roots = legacy_selection.workspace_roots.clone(); + } + let selection = self + .validate_active_permission_profile_selection( + legacy_selection.permissions.clone(), + request_overrides.clone(), + cwd.clone().map(PathBuf::from), + history_cwd.clone(), + ) + .await?; + validate_legacy_sandbox_profile_selection( + &legacy_selection, + &selection, + &effective_cwd, + workspace_roots.as_deref(), + "sandbox", + )?; + Some(selection) + } + } + } else { + None + }; + let active_permission_profile = permission_profile_selection + .as_ref() + .map(|selection| selection.active_permission_profile.clone()) + .or_else(|| persisted_permission_state.active_permission_profile.clone()); + let expected_active_permission_profile_source = permission_profile_selection + .as_ref() + .map(|selection| selection.permission_profile.clone()) + .or_else(|| Some(persisted_permission_state.permission_profile.clone())); + let workspace_roots_were_explicit = workspace_roots.is_some(); let mut typesafe_overrides = self.build_thread_config_overrides( model, model_provider, @@ -3063,19 +3507,35 @@ impl ThreadRequestProcessor { workspace_roots, approval_policy, approvals_reviewer, - sandbox, - permissions, + /*sandbox*/ None, + /*permissions*/ None, base_instructions, developer_instructions, /*personality*/ None, ); + typesafe_overrides.permission_profile = expected_active_permission_profile_source.clone(); + if !workspace_roots_were_explicit { + typesafe_overrides.workspace_roots = Some( + persisted_permission_state + .workspace_roots + .iter() + .map(codex_utils_absolute_path::AbsolutePathBuf::to_path_buf) + .collect(), + ); + } typesafe_overrides.ephemeral = ephemeral.then_some(true); // Derive a Config using the same logic as new conversation, honoring overrides if provided. - let config = self + let mut config = self .config_manager .load_for_cwd(request_overrides, typesafe_overrides, history_cwd) .await .map_err(|err| config_load_error(&err))?; + config + .permissions + .set_active_permission_profile_for_current_profile( + active_permission_profile, + expected_active_permission_profile_source.as_ref(), + ); let fallback_model_provider = config.model_provider_id.clone(); let instruction_sources = Self::instruction_sources_from_config(&config).await; 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 429ed88a99..9bfec7627a 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 @@ -495,6 +495,184 @@ mod thread_processor_behavior_tests { )); } + #[test] + fn persisted_thread_permission_state_uses_latest_turn_active_profile() { + let cwd = test_path_buf("/tmp/project").abs(); + let workspace_root = test_path_buf("/tmp/workspace").abs(); + let active_permission_profile = + codex_protocol::models::ActivePermissionProfile::new(":workspace"); + let permission_profile = codex_protocol::models::PermissionProfile::workspace_write(); + + let history = codex_protocol::protocol::InitialHistory::Forked(vec![ + codex_protocol::protocol::RolloutItem::TurnContext( + codex_protocol::protocol::TurnContextItem { + turn_id: Some("turn-1".to_string()), + trace_id: None, + cwd: cwd.to_path_buf(), + workspace_roots: vec![workspace_root.clone()], + current_date: None, + timezone: None, + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + permission_profile: Some(permission_profile.clone()), + active_permission_profile: Some(active_permission_profile.clone()), + network: None, + file_system_sandbox_policy: None, + model: "gpt-5".to_string(), + personality: None, + collaboration_mode: None, + realtime_active: None, + effort: None, + summary: codex_protocol::config_types::ReasoningSummary::Auto, + user_instructions: None, + developer_instructions: None, + final_output_json_schema: None, + truncation_policy: None, + }, + ), + ]); + + let persisted = persisted_thread_permission_state( + &history, + Some(cwd.as_path()), + /*fallback_sandbox_policy*/ None, + ) + .expect("permission state should be reconstructed"); + + assert_eq!(persisted.permission_profile, permission_profile); + assert_eq!( + persisted.active_permission_profile, + Some(active_permission_profile) + ); + assert_eq!(persisted.workspace_roots, vec![workspace_root]); + } + + #[test] + fn persisted_thread_permission_state_clears_stale_active_profile() { + let cwd = test_path_buf("/tmp/project").abs(); + let workspace_root = test_path_buf("/tmp/workspace").abs(); + let active_permission_profile = + codex_protocol::models::ActivePermissionProfile::new(":workspace"); + let workspace_profile = codex_protocol::models::PermissionProfile::workspace_write(); + let read_only_profile = codex_protocol::models::PermissionProfile::read_only(); + + let first_context = codex_protocol::protocol::TurnContextItem { + turn_id: Some("turn-1".to_string()), + trace_id: None, + cwd: cwd.to_path_buf(), + workspace_roots: vec![workspace_root.clone()], + current_date: None, + timezone: None, + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + permission_profile: Some(workspace_profile), + active_permission_profile: Some(active_permission_profile), + network: None, + file_system_sandbox_policy: None, + model: "gpt-5".to_string(), + personality: None, + collaboration_mode: None, + realtime_active: None, + effort: None, + summary: codex_protocol::config_types::ReasoningSummary::Auto, + user_instructions: None, + developer_instructions: None, + final_output_json_schema: None, + truncation_policy: None, + }; + let mut second_context = first_context.clone(); + second_context.turn_id = Some("turn-2".to_string()); + second_context.permission_profile = Some(read_only_profile.clone()); + second_context.active_permission_profile = None; + + let history = codex_protocol::protocol::InitialHistory::Forked(vec![ + codex_protocol::protocol::RolloutItem::TurnContext(first_context), + codex_protocol::protocol::RolloutItem::TurnContext(second_context), + ]); + + let persisted = persisted_thread_permission_state( + &history, + Some(cwd.as_path()), + /*fallback_sandbox_policy*/ None, + ) + .expect("permission state should be reconstructed"); + + assert_eq!(persisted.permission_profile, read_only_profile); + assert_eq!(persisted.active_permission_profile, None); + assert_eq!(persisted.workspace_roots, vec![workspace_root]); + } + + #[test] + fn persisted_thread_permission_state_preserves_empty_workspace_roots_from_event_roundtrip() { + let cwd = test_path_buf("/tmp/project").abs(); + let thread_id = ThreadId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8").unwrap(); + let permission_profile = codex_protocol::models::PermissionProfile::workspace_write(); + let event = codex_protocol::protocol::EventMsg::SessionConfigured( + codex_protocol::protocol::SessionConfiguredEvent { + session_id: thread_id.into(), + thread_id, + forked_from_id: None, + thread_source: None, + thread_name: None, + model: "gpt-5".to_string(), + model_provider_id: "mock_provider".to_string(), + service_tier: None, + approval_policy: AskForApproval::Never, + approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer::User, + permission_profile: permission_profile.clone(), + active_permission_profile: None, + cwd: cwd.clone(), + workspace_roots: Vec::new(), + reasoning_effort: None, + initial_messages: None, + network_proxy: None, + rollout_path: None, + }, + ); + let event = serde_json::from_value(serde_json::to_value(event).unwrap()).unwrap(); + let history = codex_protocol::protocol::InitialHistory::Forked(vec![ + codex_protocol::protocol::RolloutItem::EventMsg(event), + ]); + + let persisted = persisted_thread_permission_state( + &history, + Some(cwd.as_path()), + /*fallback_sandbox_policy*/ None, + ) + .expect("permission state should be reconstructed"); + + assert_eq!(persisted.permission_profile, permission_profile); + assert_eq!(persisted.workspace_roots, Vec::::new()); + } + + #[test] + fn persisted_thread_permission_state_preserves_empty_workspace_roots_from_session_meta() { + let cwd = test_path_buf("/tmp/project").abs(); + let thread_id = ThreadId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8").unwrap(); + let history = codex_protocol::protocol::InitialHistory::Forked(vec![ + codex_protocol::protocol::RolloutItem::SessionMeta( + codex_protocol::protocol::SessionMetaLine { + meta: codex_protocol::protocol::SessionMeta { + id: thread_id, + cwd: cwd.clone().into_path_buf(), + workspace_roots: Vec::new(), + ..Default::default() + }, + git: None, + }, + ), + ]); + + let persisted = persisted_thread_permission_state( + &history, + Some(cwd.as_path()), + Some(&SandboxPolicy::new_workspace_write_policy()), + ) + .expect("permission state should be reconstructed"); + + assert_eq!(persisted.workspace_roots, Vec::::new()); + } + #[test] fn config_load_error_marks_cloud_requirements_failures_for_relogin() { let err = std::io::Error::other(CloudRequirementsLoadError::new( diff --git a/codex-rs/app-server/src/request_processors/turn_processor.rs b/codex-rs/app-server/src/request_processors/turn_processor.rs index 07e8298770..bcf1b292bf 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -1,3 +1,4 @@ +use super::legacy_sandbox_compat::*; use super::*; #[derive(Clone)] @@ -376,48 +377,72 @@ impl TurnRequestProcessor { } let cwd = params.cwd; - let workspace_roots = params.workspace_roots; + let mut workspace_roots = params.workspace_roots; let approval_policy = params.approval_policy.map(AskForApproval::to_core); let approvals_reviewer = params .approvals_reviewer .map(codex_app_server_protocol::ApprovalsReviewer::to_core); - let sandbox_policy = params.sandbox_policy.map(|p| p.to_core()); + let legacy_sandbox_policy = params.sandbox_policy; + let sandbox_policy = None; let (permission_profile, active_permission_profile) = if let Some(permissions) = params.permissions { let snapshot = thread.config_snapshot().await; - let mut overrides = ConfigOverrides { - cwd: cwd.clone(), - codex_linux_sandbox_exe: self.arg0_paths.codex_linux_sandbox_exe.clone(), - main_execve_wrapper_exe: self.arg0_paths.main_execve_wrapper_exe.clone(), - ..Default::default() - }; - apply_permission_profile_selection_to_config_overrides( - &mut overrides, - Some(permissions), - ); - let config = self - .config_manager - .load_for_cwd( - /*request_overrides*/ None, - overrides, + let selection = self + .validate_active_permission_profile_selection( + permissions, + cwd.clone(), Some(snapshot.cwd.to_path_buf()), ) - .await - .map_err(|err| config_load_error(&err))?; - // Startup config is allowed to fall back when requirements - // disallow a configured profile. An explicit turn request - // is different: reject it before accepting user input. - if let Some(warning) = config.startup_warnings.iter().find(|warning| { - warning.contains("Configured value for `permission_profile` is disallowed") - }) { - return Err(invalid_request(format!( - "invalid turn context override: {warning}" - ))); - } + .await?; ( - Some(config.permissions.permission_profile()), - config.permissions.active_permission_profile(), + Some(selection.permission_profile), + Some(selection.active_permission_profile), ) + } else if let Some(legacy_sandbox_policy) = legacy_sandbox_policy.as_ref() { + let snapshot = thread.config_snapshot().await; + let effective_cwd = resolve_cwd_against_fallback(cwd.as_deref(), &snapshot.cwd); + match resolve_legacy_sandbox_profile_selection( + legacy_sandbox_policy, + Some(CurrentPermissionProfile { + permission_profile: &snapshot.permission_profile, + workspace_roots: &snapshot.workspace_roots, + }), + &effective_cwd, + workspace_roots.as_deref(), + "sandboxPolicy", + )? { + LegacySandboxResolution::Noop { + workspace_roots: legacy_workspace_roots, + } => { + if workspace_roots.is_none() { + workspace_roots = legacy_workspace_roots; + } + (None, None) + } + LegacySandboxResolution::Selection(legacy_selection) => { + if workspace_roots.is_none() { + workspace_roots = legacy_selection.workspace_roots.clone(); + } + let selection = self + .validate_active_permission_profile_selection( + legacy_selection.permissions.clone(), + cwd.clone(), + Some(snapshot.cwd.to_path_buf()), + ) + .await?; + validate_legacy_sandbox_profile_selection( + &legacy_selection, + &selection, + &effective_cwd, + workspace_roots.as_deref(), + "sandboxPolicy", + )?; + ( + Some(selection.permission_profile), + Some(selection.active_permission_profile), + ) + } + } } else { (None, None) }; @@ -522,6 +547,49 @@ impl TurnRequestProcessor { Ok(TurnStartResponse { turn }) } + async fn validate_active_permission_profile_selection( + &self, + permissions: String, + cwd: Option, + fallback_cwd: Option, + ) -> Result { + let mut overrides = ConfigOverrides { + cwd, + codex_linux_sandbox_exe: self.arg0_paths.codex_linux_sandbox_exe.clone(), + main_execve_wrapper_exe: self.arg0_paths.main_execve_wrapper_exe.clone(), + ..Default::default() + }; + apply_permission_profile_selection_to_config_overrides(&mut overrides, Some(permissions)); + let config = self + .config_manager + .load_for_cwd(/*request_overrides*/ None, overrides, fallback_cwd) + .await + .map_err(|err| config_load_error(&err))?; + // Startup config is allowed to fall back when requirements disallow a + // configured profile. An explicit turn request is different: reject it + // before accepting user input. + if let Some(warning) = config.startup_warnings.iter().find(|warning| { + warning.contains("Configured value for `permission_profile` is disallowed") + }) { + return Err(invalid_request(format!( + "invalid turn context override: {warning}" + ))); + } + let active_permission_profile = + config + .permissions + .active_permission_profile() + .ok_or_else(|| { + invalid_request( + "permission profile selection did not resolve to a named profile", + ) + })?; + Ok(ResolvedPermissionProfileSelection { + permission_profile: config.permissions.permission_profile(), + active_permission_profile, + }) + } + async fn thread_inject_items_response_inner( &self, params: ThreadInjectItemsParams, 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 c42e455718..2444ce8699 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -26,6 +26,7 @@ use codex_app_server_protocol::JSONRPCResponse; 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::ServerNotification; use codex_app_server_protocol::ServerRequest; use codex_app_server_protocol::SessionSource; @@ -2236,6 +2237,124 @@ async fn thread_resume_rejoins_running_thread_even_with_override_mismatch() -> R Ok(()) } +#[tokio::test] +async fn thread_resume_running_applies_workspace_roots_and_active_profile_name() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let first_response = responses::sse_response(responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_assistant_message("msg-1", "Done"), + responses::ev_completed("resp-1"), + ])); + let second_response = responses::sse_response(responses::sse(vec![ + responses::ev_response_created("resp-2"), + responses::ev_assistant_message("msg-2", "Done"), + responses::ev_completed("resp-2"), + ])) + .set_delay(std::time::Duration::from_millis(500)); + let _response_mock = + responses::mount_response_sequence(&server, vec![first_response, second_response]).await; + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + let workspace_root = codex_home.path().join("replacement-root"); + std::fs::create_dir_all(&workspace_root)?; + let workspace_root = AbsolutePathBuf::from_absolute_path(workspace_root.canonicalize()?)?; + + let mut primary = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, primary.initialize()).await??; + + let start_id = primary + .send_thread_start_request(ThreadStartParams { + model: Some("gpt-5.4".to_string()), + ..Default::default() + }) + .await?; + let start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + primary.read_stream_until_response_message(RequestId::Integer(start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + + let seed_turn_id = primary + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![UserInput::Text { + text: "seed history".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + primary.read_stream_until_response_message(RequestId::Integer(seed_turn_id)), + ) + .await??; + timeout( + DEFAULT_READ_TIMEOUT, + primary.read_stream_until_notification_message("turn/completed"), + ) + .await??; + primary.clear_message_buffer(); + + let running_turn_id = primary + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![UserInput::Text { + text: "keep running".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + primary.read_stream_until_response_message(RequestId::Integer(running_turn_id)), + ) + .await??; + timeout( + DEFAULT_READ_TIMEOUT, + primary.read_stream_until_notification_message("turn/started"), + ) + .await??; + + let resume_id = primary + .send_thread_resume_request(ThreadResumeParams { + thread_id: thread.id.clone(), + workspace_roots: Some(vec![workspace_root.clone()]), + sandbox: Some(SandboxMode::WorkspaceWrite), + ..Default::default() + }) + .await?; + let resume_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + primary.read_stream_until_response_message(RequestId::Integer(resume_id)), + ) + .await??; + let ThreadResumeResponse { + workspace_roots, + active_permission_profile, + .. + } = to_response::(resume_resp)?; + assert_eq!(workspace_roots, vec![workspace_root]); + assert_eq!( + active_permission_profile + .as_ref() + .map(|profile| profile.id.as_str()), + Some(":workspace") + ); + + timeout( + DEFAULT_READ_TIMEOUT, + primary.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + Ok(()) +} + #[tokio::test] async fn thread_resume_can_skip_turns_when_thread_is_running() -> Result<()> { let server = responses::start_mock_server().await; 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 78155d8c9a..88af61a730 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -14,6 +14,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::ThreadSource; use codex_app_server_protocol::ThreadStartParams; @@ -91,6 +92,56 @@ async fn thread_start_deprecates_persist_extended_history_true() -> Result<()> { Ok(()) } +#[tokio::test] +async fn thread_start_selects_permission_profile_by_id() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + create_config_toml_without_approval_policy(codex_home.path(), &server.uri())?; + let workspace = TempDir::new()?; + let workspace_root = workspace.path().to_path_buf().abs(); + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let req_id = mcp + .send_thread_start_request(ThreadStartParams { + cwd: Some(workspace.path().display().to_string()), + permissions: Some(":workspace".to_string()), + ..Default::default() + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(req_id)), + ) + .await??; + let ThreadStartResponse { + active_permission_profile, + workspace_roots, + sandbox, + .. + } = to_response::(response)?; + + assert_eq!( + active_permission_profile + .as_ref() + .map(|profile| profile.id.as_str()), + Some(":workspace") + ); + assert_eq!(workspace_roots, vec![workspace_root]); + assert_eq!( + sandbox, + SandboxPolicy::WorkspaceWrite { + network_access: false, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + legacy_writable_roots: Vec::new(), + } + ); + + Ok(()) +} + #[tokio::test] async fn thread_start_creates_thread_and_emits_started() -> Result<()> { // Provide a mock server and config so model wiring is valid. diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index d21fae4e10..a015d1f43b 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -290,7 +290,9 @@ async fn turn_start_emits_thread_scoped_warning_notification_for_trimmed_skills( write_test_skill(codex_home.path(), "alpha-skill")?; write_test_skill(codex_home.path(), "beta-skill")?; - let mut mcp = McpProcess::new(codex_home.path()).await?; + let home_dir = codex_home.path().display().to_string(); + let mut mcp = + McpProcess::new_with_env(codex_home.path(), &[("HOME", Some(home_dir.as_str()))]).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let thread_req = mcp @@ -826,6 +828,77 @@ async fn turn_start_rejects_invalid_permission_selection_before_starting_turn() Ok(()) } +#[tokio::test] +async fn turn_start_rejects_unknown_permission_selection_before_starting_turn() -> Result<()> { + let codex_home = TempDir::new()?; + let server = responses::start_mock_server().await; + create_config_toml_with_extra_top_level_config( + codex_home.path(), + &server.uri(), + "never", + &BTreeMap::from([(Feature::Personality, true)]), + "read-only", + r#" +default_permissions = "workspace" + +[permissions.workspace.filesystem] +":minimal" = "read" +"#, + )?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let thread_req = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let thread_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; + let turn_req = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id, + input: vec![V2UserInput::Text { + text: "Hello".to_string(), + text_elements: Vec::new(), + }], + permissions: Some("missing-profile".to_string()), + ..Default::default() + }) + .await?; + let err: JSONRPCError = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_error_message(RequestId::Integer(turn_req)), + ) + .await??; + + assert_eq!(err.error.code, INVALID_REQUEST_ERROR_CODE); + assert!( + err.error + .message + .contains("default_permissions refers to undefined profile `missing-profile`"), + "unexpected error message: {}", + err.error.message + ); + let turn_started = tokio::time::timeout( + std::time::Duration::from_millis(250), + mcp.read_stream_until_notification_message("turn/started"), + ) + .await; + assert!( + turn_started.is_err(), + "did not expect a turn/started notification after rejected permissions selection" + ); + + Ok(()) +} + #[tokio::test] async fn turn_start_rejects_unknown_environment_before_starting_turn() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; @@ -1671,7 +1744,6 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> { text_elements: Vec::new(), }], approval_policy: Some(codex_app_server_protocol::AskForApproval::Never), - sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::DangerFullAccess), model: Some("mock-model".to_string()), effort: Some(ReasoningEffort::Medium), summary: Some(ReasoningSummary::Auto), @@ -1839,7 +1911,7 @@ async fn turn_start_exec_approval_decline_v2() -> Result<()> { } #[tokio::test] -async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { +async fn turn_start_updates_cwd_without_replacing_workspace_roots_v2() -> Result<()> { skip_if_no_network!(Ok(())); let tmp = TempDir::new()?; @@ -1868,12 +1940,14 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { )?, create_final_assistant_message_sse_response("done second")?, ]; - let server = create_mock_responses_server_sequence(responses).await; - create_config_toml( + let server = responses::start_mock_server().await; + let response_mock = responses::mount_sse_sequence(&server, responses).await; + create_config_toml_with_sandbox( &codex_home, &server.uri(), "untrusted", &BTreeMap::default(), + "read-only", )?; let mut mcp = McpProcess::new(&codex_home).await?; @@ -1893,7 +1967,7 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { .await??; let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; - // first turn with workspace-write sandbox and first_cwd + // first turn with first_cwd as the thread's workspace root let first_turn = mcp .send_turn_start_request(TurnStartParams { environments: None, @@ -1904,16 +1978,11 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { }], responsesapi_client_metadata: None, cwd: Some(first_cwd.clone()), - workspace_roots: Some(vec![first_cwd.try_into()?]), + workspace_roots: Some(vec![first_cwd.clone().try_into()?]), approval_policy: Some(codex_app_server_protocol::AskForApproval::Never), approvals_reviewer: None, - sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::WorkspaceWrite { - network_access: false, - exclude_tmpdir_env_var: false, - exclude_slash_tmp: false, - legacy_writable_roots: Vec::new(), - }), - permissions: None, + sandbox_policy: None, + permissions: Some(":workspace".to_string()), model: Some("mock-model".to_string()), effort: Some(ReasoningEffort::Medium), summary: Some(ReasoningSummary::Auto), @@ -1935,7 +2004,8 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { .await??; mcp.clear_message_buffer(); - // second turn with workspace-write and second_cwd, ensure exec begins in second_cwd + // second turn changes cwd only; workspace roots stay on first_cwd while + // exec begins in second_cwd. let second_turn = mcp .send_turn_start_request(TurnStartParams { environments: None, @@ -1949,7 +2019,7 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { workspace_roots: None, approval_policy: Some(codex_app_server_protocol::AskForApproval::Never), approvals_reviewer: None, - sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::DangerFullAccess), + sandbox_policy: None, permissions: None, model: Some("mock-model".to_string()), effort: Some(ReasoningEffort::Medium), @@ -1997,6 +2067,22 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { assert_eq!(command, expected_command); assert_eq!(status, CommandExecutionStatus::InProgress); + let requests = response_mock.requests(); + assert!( + requests.len() >= 3, + "expected at least 3 model requests, got {}", + requests.len() + ); + let second_turn_developer_text = requests[2].message_input_texts("developer").join("\n"); + let first_cwd_name = first_cwd + .file_name() + .expect("first cwd should have a final path component") + .to_string_lossy(); + assert!( + second_turn_developer_text.contains(first_cwd_name.as_ref()), + "second turn developer instructions should retain first_cwd as a workspace root; got {second_turn_developer_text:?}" + ); + timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("turn/completed"), @@ -2006,6 +2092,112 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { Ok(()) } +#[tokio::test] +async fn turn_start_legacy_workspace_sandbox_updates_workspace_roots_for_cwd() -> Result<()> { + skip_if_no_network!(Ok(())); + + let tmp = TempDir::new()?; + let codex_home = tmp.path().join("codex_home"); + std::fs::create_dir(&codex_home)?; + let first_cwd = tmp.path().join("turn1"); + let second_cwd = tmp.path().join("turn2"); + std::fs::create_dir(&first_cwd)?; + std::fs::create_dir(&second_cwd)?; + + let server = responses::start_mock_server().await; + let response_mock = responses::mount_sse_sequence( + &server, + vec![ + create_final_assistant_message_sse_response("done first")?, + create_final_assistant_message_sse_response("done second")?, + ], + ) + .await; + create_config_toml(&codex_home, &server.uri(), "never", &BTreeMap::default())?; + + let mut mcp = McpProcess::new(&codex_home).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".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 first_turn = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![V2UserInput::Text { + text: "first turn".to_string(), + text_elements: Vec::new(), + }], + cwd: Some(first_cwd.clone()), + workspace_roots: Some(vec![first_cwd.clone().try_into()?]), + permissions: Some(":workspace".to_string()), + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(first_turn)), + ) + .await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let second_turn = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id, + input: vec![V2UserInput::Text { + text: "second turn".to_string(), + text_elements: Vec::new(), + }], + cwd: Some(second_cwd.clone()), + sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::WorkspaceWrite { + network_access: false, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + legacy_writable_roots: Vec::new(), + }), + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(second_turn)), + ) + .await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let requests = response_mock.requests(); + assert_eq!(requests.len(), 2); + let second_turn_developer_text = requests[1].message_input_texts("developer").join("\n"); + let second_cwd_name = second_cwd + .file_name() + .expect("second cwd should have a final path component") + .to_string_lossy(); + assert!( + second_turn_developer_text.contains(second_cwd_name.as_ref()), + "legacy sandboxPolicy should rebind workspace roots to second_cwd; got {second_turn_developer_text:?}" + ); + + Ok(()) +} + #[tokio::test] async fn turn_start_resolves_sticky_thread_local_environment_and_turn_overrides() -> Result<()> { let tmp = TempDir::new()?; @@ -3322,7 +3514,6 @@ async fn command_execution_notifications_include_process_id() -> Result<()> { text: "run a command".to_string(), text_elements: Vec::new(), }], - sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::DangerFullAccess), ..Default::default() }) .await?; @@ -3418,7 +3609,8 @@ async fn command_execution_notifications_include_process_id() -> Result<()> { } #[tokio::test] -async fn turn_start_with_elevated_override_does_not_persist_project_trust() -> Result<()> { +async fn turn_start_accepts_legacy_sandbox_policy_and_does_not_persist_project_trust() -> Result<()> +{ let responses = vec![create_final_assistant_message_sse_response("Done")?]; let server = create_mock_responses_server_sequence_unchecked(responses).await; @@ -3452,7 +3644,12 @@ async fn turn_start_with_elevated_override_does_not_persist_project_trust() -> R .send_turn_start_request(TurnStartParams { thread_id: thread.id, cwd: Some(workspace.path().to_path_buf()), - sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::DangerFullAccess), + sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::WorkspaceWrite { + network_access: false, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + legacy_writable_roots: Vec::new(), + }), input: vec![V2UserInput::Text { text: "Hello".to_string(), text_elements: Vec::new(), @@ -3500,6 +3697,24 @@ fn create_config_toml_with_sandbox( approval_policy: &str, feature_flags: &BTreeMap, sandbox_mode: &str, +) -> std::io::Result<()> { + create_config_toml_with_extra_top_level_config( + codex_home, + server_uri, + approval_policy, + feature_flags, + sandbox_mode, + "", + ) +} + +fn create_config_toml_with_extra_top_level_config( + codex_home: &Path, + server_uri: &str, + approval_policy: &str, + feature_flags: &BTreeMap, + sandbox_mode: &str, + extra_top_level_config: &str, ) -> std::io::Result<()> { let mut features = BTreeMap::new(); for (feature, enabled) in feature_flags { @@ -3527,6 +3742,7 @@ approval_policy = "{approval_policy}" sandbox_mode = "{sandbox_mode}" model_provider = "mock_provider" +{extra_top_level_config} [features] {feature_entries} diff --git a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs index c14263318e..c20eb58883 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs @@ -104,6 +104,7 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> { .send_thread_start_request(ThreadStartParams { model: Some("mock-model".to_string()), cwd: Some(workspace.to_string_lossy().into_owned()), + sandbox: Some(codex_app_server_protocol::SandboxMode::DangerFullAccess), ..Default::default() }) .await?; @@ -123,7 +124,6 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> { }], cwd: Some(workspace.clone()), approval_policy: Some(codex_app_server_protocol::AskForApproval::Never), - sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::DangerFullAccess), model: Some("mock-model".to_string()), effort: Some(codex_protocol::openai_models::ReasoningEffort::Medium), summary: Some(codex_protocol::config_types::ReasoningSummary::Auto), @@ -515,6 +515,7 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() .send_thread_start_request(ThreadStartParams { model: Some("mock-model".to_string()), cwd: Some(workspace.to_string_lossy().into_owned()), + sandbox: Some(codex_app_server_protocol::SandboxMode::WorkspaceWrite), ..Default::default() }) .await?; @@ -535,12 +536,7 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() cwd: Some(workspace.clone()), workspace_roots: Some(vec![workspace.clone().try_into()?]), approval_policy: Some(codex_app_server_protocol::AskForApproval::UnlessTrusted), - sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::WorkspaceWrite { - network_access: false, - exclude_tmpdir_env_var: false, - exclude_slash_tmp: false, - legacy_writable_roots: Vec::new(), - }), + sandbox_policy: None, model: Some("mock-model".to_string()), effort: Some(codex_protocol::openai_models::ReasoningEffort::Medium), summary: Some(codex_protocol::config_types::ReasoningSummary::Auto),