diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index f3983e31f4..1049cb33a4 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -3717,12 +3717,45 @@ }, "ThreadGoalSetParams": { "properties": { + "approvalPolicy": { + "anyOf": [ + { + "$ref": "#/definitions/AskForApproval" + }, + { + "type": "null" + } + ], + "description": "Override the approval policy before a goal continuation can start." + }, + "approvalsReviewer": { + "anyOf": [ + { + "$ref": "#/definitions/ApprovalsReviewer" + }, + { + "type": "null" + } + ], + "description": "Override where approval requests are routed before a goal continuation can start." + }, "objective": { "type": [ "string", "null" ] }, + "sandboxPolicy": { + "anyOf": [ + { + "$ref": "#/definitions/SandboxPolicy" + }, + { + "type": "null" + } + ], + "description": "Override the sandbox policy before a goal continuation can start." + }, "status": { "anyOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index b77b512b36..890873a39e 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -17531,12 +17531,45 @@ "ThreadGoalSetParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { + "approvalPolicy": { + "anyOf": [ + { + "$ref": "#/definitions/v2/AskForApproval" + }, + { + "type": "null" + } + ], + "description": "Override the approval policy before a goal continuation can start." + }, + "approvalsReviewer": { + "anyOf": [ + { + "$ref": "#/definitions/v2/ApprovalsReviewer" + }, + { + "type": "null" + } + ], + "description": "Override where approval requests are routed before a goal continuation can start." + }, "objective": { "type": [ "string", "null" ] }, + "sandboxPolicy": { + "anyOf": [ + { + "$ref": "#/definitions/v2/SandboxPolicy" + }, + { + "type": "null" + } + ], + "description": "Override the sandbox policy before a goal continuation can start." + }, "status": { "anyOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index bdd61d6b94..03a25b619c 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -15310,12 +15310,45 @@ "ThreadGoalSetParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { + "approvalPolicy": { + "anyOf": [ + { + "$ref": "#/definitions/AskForApproval" + }, + { + "type": "null" + } + ], + "description": "Override the approval policy before a goal continuation can start." + }, + "approvalsReviewer": { + "anyOf": [ + { + "$ref": "#/definitions/ApprovalsReviewer" + }, + { + "type": "null" + } + ], + "description": "Override where approval requests are routed before a goal continuation can start." + }, "objective": { "type": [ "string", "null" ] }, + "sandboxPolicy": { + "anyOf": [ + { + "$ref": "#/definitions/SandboxPolicy" + }, + { + "type": "null" + } + ], + "description": "Override the sandbox policy before a goal continuation can start." + }, "status": { "anyOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadGoalSetParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadGoalSetParams.json index 0087827c21..5ad4caea87 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadGoalSetParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadGoalSetParams.json @@ -1,6 +1,174 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "AbsolutePathBuf": { + "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", + "type": "string" + }, + "ApprovalsReviewer": { + "description": "Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `auto_review` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request. The legacy value `guardian_subagent` is accepted for compatibility.", + "enum": [ + "user", + "auto_review", + "guardian_subagent" + ], + "type": "string" + }, + "AskForApproval": { + "oneOf": [ + { + "enum": [ + "untrusted", + "on-request", + "never" + ], + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "granular": { + "properties": { + "mcp_elicitations": { + "type": "boolean" + }, + "request_permissions": { + "default": false, + "type": "boolean" + }, + "rules": { + "type": "boolean" + }, + "sandbox_approval": { + "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + } + }, + "required": [ + "granular" + ], + "title": "GranularAskForApproval", + "type": "object" + } + ] + }, + "NetworkAccess": { + "enum": [ + "restricted", + "enabled" + ], + "type": "string" + }, + "SandboxPolicy": { + "oneOf": [ + { + "properties": { + "type": { + "enum": [ + "dangerFullAccess" + ], + "title": "DangerFullAccessSandboxPolicyType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "DangerFullAccessSandboxPolicy", + "type": "object" + }, + { + "properties": { + "networkAccess": { + "default": false, + "type": "boolean" + }, + "type": { + "enum": [ + "readOnly" + ], + "title": "ReadOnlySandboxPolicyType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "ReadOnlySandboxPolicy", + "type": "object" + }, + { + "properties": { + "networkAccess": { + "allOf": [ + { + "$ref": "#/definitions/NetworkAccess" + } + ], + "default": "restricted" + }, + "type": { + "enum": [ + "externalSandbox" + ], + "title": "ExternalSandboxSandboxPolicyType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "ExternalSandboxSandboxPolicy", + "type": "object" + }, + { + "properties": { + "excludeSlashTmp": { + "default": false, + "type": "boolean" + }, + "excludeTmpdirEnvVar": { + "default": false, + "type": "boolean" + }, + "networkAccess": { + "default": false, + "type": "boolean" + }, + "type": { + "enum": [ + "workspaceWrite" + ], + "title": "WorkspaceWriteSandboxPolicyType", + "type": "string" + }, + "writableRoots": { + "default": [], + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + } + }, + "required": [ + "type" + ], + "title": "WorkspaceWriteSandboxPolicy", + "type": "object" + } + ] + }, "ThreadGoalStatus": { "enum": [ "active", @@ -14,12 +182,45 @@ } }, "properties": { + "approvalPolicy": { + "anyOf": [ + { + "$ref": "#/definitions/AskForApproval" + }, + { + "type": "null" + } + ], + "description": "Override the approval policy before a goal continuation can start." + }, + "approvalsReviewer": { + "anyOf": [ + { + "$ref": "#/definitions/ApprovalsReviewer" + }, + { + "type": "null" + } + ], + "description": "Override where approval requests are routed before a goal continuation can start." + }, "objective": { "type": [ "string", "null" ] }, + "sandboxPolicy": { + "anyOf": [ + { + "$ref": "#/definitions/SandboxPolicy" + }, + { + "type": "null" + } + ], + "description": "Override the sandbox policy before a goal continuation can start." + }, "status": { "anyOf": [ { diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadGoalSetParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadGoalSetParams.ts index b92720c178..233d97474c 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadGoalSetParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadGoalSetParams.ts @@ -1,6 +1,18 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { ApprovalsReviewer } from "./ApprovalsReviewer"; +import type { AskForApproval } from "./AskForApproval"; +import type { SandboxPolicy } from "./SandboxPolicy"; import type { ThreadGoalStatus } from "./ThreadGoalStatus"; -export type ThreadGoalSetParams = { threadId: string, objective?: string | null, status?: ThreadGoalStatus | null, tokenBudget?: number | null, }; +export type ThreadGoalSetParams = {threadId: string, objective?: string | null, status?: ThreadGoalStatus | null, /** + * Override the approval policy before a goal continuation can start. + */ +approvalPolicy?: AskForApproval | null, /** + * Override where approval requests are routed before a goal continuation can start. + */ +approvalsReviewer?: ApprovalsReviewer | null, /** + * Override the sandbox policy before a goal continuation can start. + */ +sandboxPolicy?: SandboxPolicy | null, tokenBudget?: number | null}; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index d8db99d4ff..602282a78b 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -532,6 +532,7 @@ client_request_definitions! { }, ThreadGoalSet => "thread/goal/set" { params: v2::ThreadGoalSetParams, + inspect_params: true, serialization: thread_id(params.thread_id), response: v2::ThreadGoalSetResponse, }, @@ -1976,6 +1977,10 @@ mod tests { thread_id: "goal-thread".to_string(), objective: Some("ship it".to_string()), status: None, + approval_policy: None, + approvals_reviewer: None, + sandbox_policy: None, + permissions: None, token_budget: None, }, }; @@ -3544,6 +3549,10 @@ mod tests { thread_id: "thr_123".to_string(), objective: Some("ship goal mode".to_string()), status: Some(v2::ThreadGoalStatus::Active), + approval_policy: None, + approvals_reviewer: None, + sandbox_policy: None, + permissions: None, token_budget: Some(Some(10_000)), }, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/thread.rs b/codex-rs/app-server-protocol/src/protocol/v2/thread.rs index 7cd03930d8..c58a186440 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/thread.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/thread.rs @@ -775,7 +775,9 @@ impl From for ThreadGoal { } } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema, TS)] +#[derive( + Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema, TS, ExperimentalApi, +)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct ThreadGoalSetParams { @@ -784,6 +786,21 @@ pub struct ThreadGoalSetParams { pub objective: Option, #[ts(optional = nullable)] pub status: Option, + /// Override the approval policy before a goal continuation can start. + #[experimental(nested)] + #[ts(optional = nullable)] + pub approval_policy: Option, + /// Override where approval requests are routed before a goal continuation can start. + #[ts(optional = nullable)] + pub approvals_reviewer: Option, + /// Override the sandbox policy before a goal continuation can start. + #[ts(optional = nullable)] + pub sandbox_policy: Option, + /// Select a named permissions profile id before a goal continuation can + /// start. Cannot be combined with `sandboxPolicy`. + #[experimental("thread/goal/set.permissions")] + #[ts(optional = nullable)] + pub permissions: Option, #[serde( default, deserialize_with = "crate::protocol::serde_helpers::deserialize_double_option", diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index c9ac2f5fb2..8b2de2bd76 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -150,7 +150,7 @@ Example with notification opt-out: - `thread/settings/update` — experimental; queue a partial update to a loaded thread’s next-turn settings without starting a turn or adding transcript items. Omitted fields leave settings unchanged; `serviceTier: null` clears the tier; deprecated `multiAgentMode` is ignored, while Ultra reasoning effort enables proactive multi-agent behavior; `sandboxPolicy` and `permissions` cannot be combined. Returns `{}` when the update is accepted and emits `thread/settings/updated` with the full effective settings only if they actually change. `turn/start` settings overrides emit the same notification when they change the stored settings. - `thread/memoryMode/set` — experimental; set a thread’s persisted memory eligibility to `"enabled"` or `"disabled"` for either a loaded thread or a stored rollout; returns `{}` on success. - `memory/reset` — experimental; clear the current `CODEX_HOME/memories` directory and reset persisted memory stage data in sqlite while preserving existing thread memory modes; returns `{}` on success. -- `thread/goal/set` — create or update the single persisted goal for a materialized thread; returns the current goal and emits `thread/goal/updated`. +- `thread/goal/set` — create or update the single persisted goal for a materialized thread; returns the current goal and emits `thread/goal/updated`. Because an active goal can immediately wake a continuation, clients should include the current `approvalPolicy`, `approvalsReviewer`, and either `sandboxPolicy` or `permissions`; the server applies those settings before runtime effects run. - `thread/goal/get` — fetch the current persisted goal for a materialized thread; returns `goal: null` when no goal exists. - `thread/goal/clear` — clear the current persisted goal for a materialized thread; returns whether a goal was removed and emits `thread/goal/cleared` when state changes. - `thread/goal/updated` — notification emitted whenever a thread goal changes; includes the full current goal. @@ -575,12 +575,15 @@ Experimental: use `memory/reset` to clear local memory artifacts and sqlite-back ### Example: Set and update a thread goal -Use `thread/goal/set` to create or update the current goal for a materialized thread. Clients can set `budgetLimited` when they stop because a token budget is exhausted or nearly exhausted, `blocked` when progress is waiting on outside intervention, and `usageLimited` when usage availability stops further work. The system also sets `budgetLimited` when accounting crosses a configured token budget and `usageLimited` when a turn ends on a hard usage-limit error. +Use `thread/goal/set` to create or update the current goal for a materialized thread. Clients can set `budgetLimited` when they stop because a token budget is exhausted or nearly exhausted, `blocked` when progress is waiting on outside intervention, and `usageLimited` when usage availability stops further work. The system also sets `budgetLimited` when accounting crosses a configured token budget and `usageLimited` when a turn ends on a hard usage-limit error. Because an active goal can immediately wake a continuation, include the current permission context on the same request; `permissions` selects a named profile and cannot be combined with `sandboxPolicy`. ```json { "method": "thread/goal/set", "id": 27, "params": { "threadId": "thr_123", "objective": "Keep improving the benchmark until p95 latency is under 120ms", + "approvalPolicy": "never", + "approvalsReviewer": "user", + "permissions": ":danger-full-access", "tokenBudget": 200000 } } { "id": 27, "result": { "goal": { diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 375307e804..aaf1d52444 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -474,7 +474,9 @@ impl MessageProcessor { let thread_goal_processor = ThreadGoalRequestProcessor::new( Arc::clone(&thread_manager), outgoing.clone(), + arg0_paths.clone(), Arc::clone(&config), + config_manager.clone(), thread_state_manager.clone(), state_db.clone(), Arc::clone(&goal_service), diff --git a/codex-rs/app-server/src/request_processors/thread_goal_processor.rs b/codex-rs/app-server/src/request_processors/thread_goal_processor.rs index 5878047237..d9758eb209 100644 --- a/codex-rs/app-server/src/request_processors/thread_goal_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_goal_processor.rs @@ -9,17 +9,22 @@ use codex_goal_extension::GoalTokenBudgetUpdate; pub(crate) struct ThreadGoalRequestProcessor { thread_manager: Arc, outgoing: Arc, + arg0_paths: Arg0DispatchPaths, config: Arc, + config_manager: ConfigManager, thread_state_manager: ThreadStateManager, state_db: Option, goal_service: Arc, } impl ThreadGoalRequestProcessor { + #[allow(clippy::too_many_arguments)] pub(crate) fn new( thread_manager: Arc, outgoing: Arc, + arg0_paths: Arg0DispatchPaths, config: Arc, + config_manager: ConfigManager, thread_state_manager: ThreadStateManager, state_db: Option, goal_service: Arc, @@ -27,7 +32,9 @@ impl ThreadGoalRequestProcessor { Self { thread_manager, outgoing, + arg0_paths, config, + config_manager, thread_state_manager, state_db, goal_service, @@ -102,6 +109,11 @@ impl ThreadGoalRequestProcessor { if !self.config.features.enabled(Feature::Goals) { return Err(invalid_request("goals feature is disabled")); } + if params.sandbox_policy.is_some() && params.permissions.is_some() { + return Err(invalid_request( + "`permissions` cannot be combined with `sandboxPolicy`", + )); + } let thread_id = parse_thread_id_for_request(params.thread_id.as_str())?; let state_db = self.state_db_for_materialized_thread(thread_id).await?; @@ -115,6 +127,15 @@ impl ThreadGoalRequestProcessor { }; let status = params.status.map(ThreadGoalStatus::to_core); let objective = params.objective.as_deref(); + let token_budget = params.token_budget; + + if let Ok(thread) = self.thread_manager.get_thread(thread_id).await { + // Apply before writing the goal. Once an active goal is visible, + // an already-queued idle lifecycle hook can wake a continuation + // independently of `apply_runtime_effects`. + self.apply_goal_permission_context(thread.as_ref(), ¶ms) + .await?; + } let outcome = self .goal_service @@ -126,7 +147,7 @@ impl ThreadGoalRequestProcessor { .map(GoalObjectiveUpdate::Set) .unwrap_or(GoalObjectiveUpdate::Keep), status, - token_budget: match params.token_budget { + token_budget: match token_budget { Some(token_budget) => GoalTokenBudgetUpdate::Set(token_budget), None => GoalTokenBudgetUpdate::Keep, }, @@ -162,6 +183,78 @@ impl ThreadGoalRequestProcessor { Ok(()) } + async fn apply_goal_permission_context( + &self, + thread: &CodexThread, + params: &ThreadGoalSetParams, + ) -> Result<(), JSONRPCErrorError> { + let has_permission_context = params.approval_policy.is_some() + || params.approvals_reviewer.is_some() + || params.sandbox_policy.is_some() + || params.permissions.is_some(); + if !has_permission_context { + return Ok(()); + } + + let approval_policy = params + .approval_policy + .map(codex_app_server_protocol::AskForApproval::to_core); + let approvals_reviewer = params + .approvals_reviewer + .map(codex_app_server_protocol::ApprovalsReviewer::to_core); + let sandbox_policy = params.sandbox_policy.clone().map(|policy| policy.to_core()); + let (permission_profile, active_permission_profile, profile_workspace_roots) = + if let Some(permissions) = params.permissions.clone() { + let snapshot = thread.config_snapshot().await; + let overrides = ConfigOverrides { + workspace_roots: Some(snapshot.workspace_roots.clone()), + default_permissions: Some(permissions), + 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() + }; + let config = self + .config_manager + .load_for_cwd( + /*request_overrides*/ None, + overrides, + 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 goal update is + // different: reject it before a continuation can start. + 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 goal continuation settings: {warning}" + ))); + } + ( + Some(config.permissions.permission_profile().clone()), + config.permissions.active_permission_profile(), + Some(config.permissions.profile_workspace_roots().to_vec()), + ) + } else { + (None, None, None) + }; + + thread + .apply_thread_settings_overrides(CodexThreadSettingsOverrides { + approval_policy, + approvals_reviewer, + sandbox_policy, + permission_profile, + active_permission_profile, + profile_workspace_roots, + ..Default::default() + }) + .await + .map_err(|err| invalid_request(format!("invalid goal continuation settings: {err}"))) + } + async fn thread_goal_get_inner( &self, params: ThreadGoalGetParams, diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index bdb03271d0..8cbf9ce75f 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -65,6 +65,7 @@ mod sleep; mod thread_archive; mod thread_delete; mod thread_fork; +mod thread_goal_permissions; mod thread_inject_items; mod thread_list; mod thread_loaded_list; diff --git a/codex-rs/app-server/tests/suite/v2/thread_goal_permissions.rs b/codex-rs/app-server/tests/suite/v2/thread_goal_permissions.rs new file mode 100644 index 0000000000..54b375afee --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/thread_goal_permissions.rs @@ -0,0 +1,314 @@ +use anyhow::Context; +use anyhow::Result; +use app_test_support::TestAppServer; +use app_test_support::create_fake_rollout; +use app_test_support::to_response; +use codex_app_server_protocol::JSONRPCError; +use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ThreadGoalSetResponse; +use codex_app_server_protocol::ThreadStartParams; +use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::UserInput; +use core_test_support::responses; +use pretty_assertions::assert_eq; +use serde_json::json; +use std::time::Duration; +use tempfile::TempDir; +use tokio::time::timeout; + +#[cfg(windows)] +const DEFAULT_TIMEOUT: Duration = Duration::from_secs(25); +#[cfg(not(windows))] +const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10); + +#[tokio::test] +async fn thread_goal_set_applies_named_permissions_before_continuation() -> Result<()> { + let server = responses::start_mock_server().await; + let response_mock = responses::mount_sse_sequence( + &server, + vec![ + responses::sse(vec![ + responses::ev_response_created("materialize-thread"), + responses::ev_assistant_message("materialize-message", "done"), + responses::ev_completed("materialize-thread"), + ]), + responses::sse(vec![ + responses::ev_response_created("goal-continuation"), + responses::ev_assistant_message("goal-message", "done"), + responses::ev_completed("goal-continuation"), + ]), + ], + ) + .await; + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + + let mut mcp = TestAppServer::new_with_auto_env(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let start_id = mcp + .send_thread_start_request_with_auto_env(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let start_response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response(start_response)?; + + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![UserInput::Text { + text: "materialize this thread".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + let _: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let goal_id = mcp + .send_raw_request( + "thread/goal/set", + Some(json!({ + "threadId": thread.id, + "objective": "keep improving", + "approvalPolicy": "never", + "approvalsReviewer": "user", + "permissions": "dev", + })), + ) + .await?; + let goal_response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(goal_id)), + ) + .await??; + let _: ThreadGoalSetResponse = to_response(goal_response)?; + timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let requests = response_mock.requests(); + assert_eq!( + requests.len(), + 2, + "expected materialization and continuation" + ); + let latest_permissions_instructions = requests[1] + .message_input_texts("developer") + .into_iter() + .rev() + .find(|text| text.contains("")) + .context("goal continuation should include permissions instructions")?; + assert!( + latest_permissions_instructions.contains("Approval policy is currently never"), + "goal continuation should use the current approval policy: {latest_permissions_instructions}" + ); + assert!( + latest_permissions_instructions.contains("workspace-write"), + "goal continuation should use the selected named profile: {latest_permissions_instructions}" + ); + assert!( + !latest_permissions_instructions.contains("read-only"), + "goal continuation should not reuse the stale sandbox: {latest_permissions_instructions}" + ); + + Ok(()) +} + +#[tokio::test] +async fn thread_goal_set_applies_explicit_sandbox_before_continuation() -> Result<()> { + let server = responses::start_mock_server().await; + let response_mock = responses::mount_sse_sequence( + &server, + vec![ + responses::sse(vec![ + responses::ev_response_created("materialize-thread"), + responses::ev_assistant_message("materialize-message", "done"), + responses::ev_completed("materialize-thread"), + ]), + responses::sse(vec![ + responses::ev_response_created("goal-continuation"), + responses::ev_assistant_message("goal-message", "done"), + responses::ev_completed("goal-continuation"), + ]), + ], + ) + .await; + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + + let mut mcp = TestAppServer::new_with_auto_env(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let start_id = mcp + .send_thread_start_request_with_auto_env(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let start_response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response(start_response)?; + + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![UserInput::Text { + text: "materialize this thread".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + let _: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let goal_id = mcp + .send_raw_request( + "thread/goal/set", + Some(json!({ + "threadId": thread.id, + "objective": "keep improving", + "approvalPolicy": "never", + "approvalsReviewer": "user", + "sandboxPolicy": { "type": "dangerFullAccess" }, + })), + ) + .await?; + let goal_response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(goal_id)), + ) + .await??; + let _: ThreadGoalSetResponse = to_response(goal_response)?; + timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let requests = response_mock.requests(); + assert_eq!( + requests.len(), + 2, + "expected materialization and continuation" + ); + let latest_permissions_instructions = requests[1] + .message_input_texts("developer") + .into_iter() + .rev() + .find(|text| text.contains("")) + .context("goal continuation should include permissions instructions")?; + assert!( + latest_permissions_instructions.contains("Approval policy is currently never"), + "goal continuation should use the current approval policy: {latest_permissions_instructions}" + ); + assert!( + latest_permissions_instructions.contains("danger-full-access"), + "goal continuation should use the explicit sandbox: {latest_permissions_instructions}" + ); + assert!( + !latest_permissions_instructions.contains("read-only"), + "goal continuation should not reuse the stale sandbox: {latest_permissions_instructions}" + ); + + Ok(()) +} + +#[tokio::test] +async fn thread_goal_set_rejects_permissions_with_sandbox_for_unloaded_thread() -> Result<()> { + let server = responses::start_mock_server().await; + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + let thread_id = create_fake_rollout( + codex_home.path(), + "2025-01-05T12-00-00", + "2025-01-05T12:00:00Z", + "", + Some("mock_provider"), + /*git_info*/ None, + )?; + + let mut mcp = TestAppServer::new_with_auto_env(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_raw_request( + "thread/goal/set", + Some(json!({ + "threadId": thread_id, + "objective": "keep improving", + "sandboxPolicy": { "type": "dangerFullAccess" }, + "permissions": "dev", + })), + ) + .await?; + let error: JSONRPCError = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_error_message(RequestId::Integer(request_id)), + ) + .await??; + + assert_eq!( + error.error.message, + "`permissions` cannot be combined with `sandboxPolicy`" + ); + Ok(()) +} + +fn create_config_toml(codex_home: &std::path::Path, server_uri: &str) -> std::io::Result<()> { + std::fs::write( + codex_home.join("config.toml"), + format!( + r#" +model = "mock-model" +approval_policy = "on-request" +sandbox_mode = "read-only" +model_provider = "mock_provider" + +[features] +goals = true + +[model_providers.mock_provider] +name = "Mock provider for test" +base_url = "{server_uri}/v1" +wire_api = "responses" +request_max_retries = 0 +stream_max_retries = 0 + +[permissions.dev.filesystem.":workspace_roots"] +"." = "write" +"# + ), + ) +} diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 4a562092f1..d2a8e0f19d 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -355,6 +355,15 @@ impl CodexThread { self.codex.session.preview_settings(&updates).await } + /// Apply persistent thread settings overrides immediately. + pub async fn apply_thread_settings_overrides( + &self, + overrides: CodexThreadSettingsOverrides, + ) -> ConstraintResult<()> { + let updates = self.thread_settings_update(overrides).await; + self.codex.session.update_settings(updates).await + } + async fn thread_settings_update( &self, overrides: CodexThreadSettingsOverrides, diff --git a/codex-rs/tui/src/app/thread_goal_actions.rs b/codex-rs/tui/src/app/thread_goal_actions.rs index a6f1ed5965..0688cbd945 100644 --- a/codex-rs/tui/src/app/thread_goal_actions.rs +++ b/codex-rs/tui/src/app/thread_goal_actions.rs @@ -2,6 +2,7 @@ use super::App; use crate::app_event::AppEvent; use crate::app_event::ThreadGoalSetMode; use crate::app_server_session::AppServerSession; +use crate::app_server_session::TurnPermissionsOverride; use crate::bottom_pane::SelectionAction; use crate::bottom_pane::SelectionItem; use crate::bottom_pane::SelectionViewParams; @@ -14,6 +15,14 @@ use crate::text_formatting::truncate_text; use codex_app_server_protocol::ThreadGoal; use codex_app_server_protocol::ThreadGoalStatus; use codex_protocol::ThreadId; +use codex_protocol::config_types::ApprovalsReviewer; + +struct GoalPermissionContext { + approval_policy: codex_app_server_protocol::AskForApproval, + approvals_reviewer: ApprovalsReviewer, + permissions_override: TurnPermissionsOverride, + cwd: codex_utils_absolute_path::AbsolutePathBuf, +} const EPHEMERAL_THREAD_GOAL_ERROR_MESSAGE: &str = concat!( "Goals need a saved session. This session is temporary.\n", @@ -132,6 +141,10 @@ impl App { draft: goal_files::GoalDraft, mode: ThreadGoalSetMode, ) { + let Some(permission_context) = self.goal_permission_context(thread_id) else { + tracing::debug!(%thread_id, "skipping stale goal update for non-visible thread"); + return; + }; let codex_home = app_server.codex_home_path(&self.config.codex_home); let mode = if matches!(mode, ThreadGoalSetMode::ConfirmIfExists) { let result = app_server.thread_goal_get(thread_id).await; @@ -199,8 +212,27 @@ impl App { } => (status, Some(token_budget)), }; + if self.current_displayed_thread_id() != Some(thread_id) { + cleanup_materialized_goal_files(app_server, output_dir).await; + return; + } + let GoalPermissionContext { + approval_policy, + approvals_reviewer, + permissions_override, + cwd, + } = permission_context; let result = app_server - .thread_goal_set(thread_id, Some(objective), Some(status), token_budget) + .thread_goal_set( + thread_id, + Some(objective), + Some(status), + approval_policy, + approvals_reviewer, + permissions_override, + cwd.as_path(), + token_budget, + ) .await; match result { @@ -232,11 +264,25 @@ impl App { thread_id: ThreadId, status: ThreadGoalStatus, ) { + let Some(GoalPermissionContext { + approval_policy, + approvals_reviewer, + permissions_override, + cwd, + }) = self.goal_permission_context(thread_id) + else { + tracing::debug!(%thread_id, "skipping stale goal status update for non-visible thread"); + return; + }; let result = app_server .thread_goal_set( thread_id, /*objective*/ None, Some(status), + approval_policy, + approvals_reviewer, + permissions_override, + cwd.as_path(), /*token_budget*/ None, ) .await; @@ -255,6 +301,28 @@ impl App { } } + fn goal_permission_context(&self, thread_id: ThreadId) -> Option { + if self.current_displayed_thread_id() != Some(thread_id) { + return None; + } + + let config = self.chat_widget.config_ref(); + let permissions = &config.permissions; + let permissions_override = + if let Some(active_permission_profile) = permissions.active_permission_profile() { + TurnPermissionsOverride::ActiveProfile(active_permission_profile) + } else { + TurnPermissionsOverride::LegacySandbox(permissions.effective_permission_profile()) + }; + + Some(GoalPermissionContext { + approval_policy: permissions.approval_policy.value().into(), + approvals_reviewer: config.approvals_reviewer, + permissions_override, + cwd: config.cwd.clone(), + }) + } + pub(super) async fn clear_thread_goal( &mut self, app_server: &mut AppServerSession, diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index aeca17091e..e781ede156 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -936,14 +936,20 @@ impl AppServerSession { .wrap_err("thread/goal/get failed in TUI") } + #[allow(clippy::too_many_arguments)] pub(crate) async fn thread_goal_set( &mut self, thread_id: ThreadId, objective: Option, status: Option, + approval_policy: AskForApproval, + approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer, + permissions_override: TurnPermissionsOverride, + cwd: &std::path::Path, token_budget: Option>, ) -> Result { let request_id = self.next_request_id(); + let (sandbox_policy, permissions) = turn_permissions_overrides(permissions_override, cwd); self.client .request_typed(ClientRequest::ThreadGoalSet { request_id, @@ -951,6 +957,10 @@ impl AppServerSession { thread_id: thread_id.to_string(), objective, status, + approval_policy: Some(approval_policy), + approvals_reviewer: Some(approvals_reviewer.into()), + sandbox_policy, + permissions, token_budget, }, })