diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst index ea1835a891..5222e1fd8c 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst differ diff --git a/codex-rs/app-server-protocol/src/protocol/v2/turn.rs b/codex-rs/app-server-protocol/src/protocol/v2/turn.rs index 3222bb38bb..9629a78410 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/turn.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/turn.rs @@ -47,6 +47,10 @@ pub enum TurnStatus { pub struct TurnSettingsUpdateParams { pub thread_id: String, pub turn_id: String, + /// Changes the active turn's reviewer without changing future thread settings. + /// Already captured steps and pending approvals retain their original reviewer. + #[ts(optional = nullable)] + pub approvals_reviewer: Option, /// Omission or `null` leaves the model unchanged. #[ts(optional = nullable)] pub model: Option, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 47dc3d3b4b..7ce5f5aab0 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -221,7 +221,7 @@ Example with notification opt-out: - `thread/revert` — replace a loaded paginated thread's durable history with the prefix strictly before `beforeTurnId` while preserving its thread id. The operation interrupts an active turn if needed, leaves older rollout files immutable, reloads the thread, returns updated thread metadata with empty `turns` plus pagination cursors, and emits `thread/reverted`. It does not revert local file changes. Parent-owned Multi-Agent V2 subagents reject direct revert requests. - `turn/start` — add user input or a named standalone function-call output to a thread and begin Codex generation; responds with the initial `turn` object and streams `turn/started`, `item/*`, and `turn/completed` notifications. For standalone outputs, provide `toolOutput` with an empty `input` array. Optional `turnTrigger` classifies who or what started a new turn and is sent as `turn_trigger` in Responses request metadata; it is ignored if the request steers an active turn. `clientUserMessageId` is optional; when supplied, the corresponding `userMessage` item echoes it as `clientId`. Experimental `runtimeWorkspaceRoots` supplies the default roots for newly resolved environment selections. Explicit `environments[].runtimeWorkspaceRoots` override that fallback with environment-native absolute paths. Prefer experimental `permissions` profile selection by id 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". Deprecated experimental `multiAgentMode` is ignored; Ultra reasoning effort selects proactive behavior. Parent-owned Multi-Agent V2 subagents reject direct turns. - `thread/inject_items` — append raw Responses API items to a loaded thread’s model-visible history without starting a turn; returns `{}` on success. Parent-owned Multi-Agent V2 subagents reject direct item injection. -- `turn/settings/update` — experimental; publish a narrow model-settings patch to the exact live task identified by `threadId` and `turnId`, regardless of task kind. Requires `step_model_switching`; returns `status: "applied"` or `status: "targetUnavailable"`, or a request error if rejected. Future-thread settings and already captured steps are unchanged. Parent-owned Multi-Agent V2 subagents reject direct settings updates. +- `turn/settings/update` — experimental; publish a reviewer or model-settings patch to the exact live task identified by `threadId` and `turnId`, regardless of task kind. Model-settings updates require `step_model_switching`; reviewer-only updates do not. Returns `status: "applied"` or `status: "targetUnavailable"`, or a request error if rejected. Future-thread settings and already captured steps are unchanged. Parent-owned Multi-Agent V2 subagents reject direct settings updates. - `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. `clientUserMessageId` is optional; when supplied, the corresponding `userMessage` item echoes it as `clientId`. Review and manual compaction turns reject `turn/steer`. Parent-owned Multi-Agent V2 subagents reject direct steering. - `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"`. Also available for parent-owned Multi-Agent V2 subagents. - `thread/realtime/start` — start a thread-scoped realtime session (experimental); pass `outputModality: "text"` or `outputModality: "audio"` to choose model output, optionally pass `model` and `version` to override configured realtime selection for this session only, pass `includeStartupContext: false` to omit Codex's generated startup context, and optionally pass `initialItems` to seed V3 with complete role-bearing text messages at session creation. Pass `realtimeStartInstructions` and `realtimeEndInstructions` to control the developer instructions given to the backing Codex model when this session starts and ends. Version `"v1"` uses legacy Bidi `conversation.handoff.*`, `"v2"` uses the Realtime Voice API, and `"v3"` preserves V1 Codex Voice behavior while using Frameless Bidi `delegation.*`. For V3 automatic Codex text, `codexResponseHandoffMode` accepts `"thinking"` (the default; all output uses channel-less thinking appends), `"commentary"` (all output uses the commentary channel), or `"bemTags"` (the raw BEM envelope selects the API channel: BEM `analysis` and `commentary` use `commentary`, while BEM `final` and unparsable output use `speakable`). The BEM envelope remains in the appended text for the frontend model to interpret. V1 and V2 ignore this setting. For V3, pass `delegationAckFiller: false` to suppress the Realtime API's delegation acknowledgement filler or `true` to restore it; omitting the field preserves the Realtime API's default. V1 and V2 ignore `delegationAckFiller`. V3 handoffs do not prepend the legacy `"Agent Final Message"` label. Pass `clientManagedHandoffs: true` to disable automatic Codex response delivery so only the client's explicit append calls produce handoffs. Pass `codexResponsesAsItems: true` to send automatic Codex responses as realtime conversation items instead, and optionally pass `codexResponseItemPrefix` to prepend experiment instructions to those items. Returns `{}` and streams `thread/realtime/*` notifications. Omit `transport` for the websocket transport, or pass `{ "type": "webrtc", "sdp": "..." }` to create a Bidi WebRTC session from a browser-generated SDP offer; the remote answer SDP is emitted as `thread/realtime/sdp`. Conversation `version: "v2"` requests remain unsupported for WebRTC. Parent-owned Multi-Agent V2 subagents reject this request. @@ -1351,8 +1351,8 @@ Use `thread/backgroundTerminals/terminate` to terminate one running background t ### Example: Update a running turn's settings (experimental) -Enable `capabilities.experimentalApi` and the disabled-by-default `step_model_switching` -feature. Supply the exact turn ID from `turn/start`, `turn/started`, `thread/read` with +Enable `capabilities.experimentalApi`; model-settings updates also require the +disabled-by-default `step_model_switching` feature. Supply the exact turn ID from `turn/start`, `turn/started`, `thread/read` with `includeTurns: true`, or `thread/turns/list`: ```json @@ -1362,10 +1362,21 @@ feature. Supply the exact turn ID from `turn/start`, `turn/started`, `thread/rea { "id": 42, "result": { "status": "applied" } } ``` -Only `model`, `effort`, `summary`, and `serviceTier` may change. Unknown fields are +Only `approvalsReviewer`, `model`, `effort`, `summary`, and `serviceTier` may change. Unknown fields are rejected. Omitted fields leave settings unchanged; `serviceTier: null` clears the requested tier, while `null` for model, effort, or summary leaves it unchanged. +A reviewer-only update does not require `step_model_switching`. Set +`approvalsReviewer` to `"auto_review"` or `"user"` to change review routing for +subsequently captured steps and newly initiated background approval requests. +Omission or `null` leaves the reviewer unchanged. Managed reviewer restrictions +and model-required auto review still apply. Existing captured steps and pending +approvals keep their original reviewer; this does not approve a pending request, +change sandbox permissions, or update child sessions. App/account-specific +reviewer overrides still take precedence. Future-thread defaults remain separate. +For compatibility, MCP keeps following refreshed thread defaults until an explicit +live reviewer update overrides them for that turn. + The response waits for core: `status: "applied"` means a settings snapshot was published for subsequent captures, even if its values were unchanged. Normal defaults and tier filtering still apply; publication does not guarantee another inference will run or use 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 2cd4c436f8..eb7cb98551 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -216,6 +216,9 @@ impl TurnRequestProcessor { Op::TurnSettings { turn_id: params.turn_id, update: TurnSettingsUpdate { + approvals_reviewer: params + .approvals_reviewer + .map(codex_app_server_protocol::ApprovalsReviewer::to_core), model: params.model, // Match thread/settings/update: public null does not clear effort. effort: params.effort.map(Some), diff --git a/codex-rs/app-server/tests/suite/v2/turn_settings_update.rs b/codex-rs/app-server/tests/suite/v2/turn_settings_update.rs index 24d1dc37d7..e494a38700 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_settings_update.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_settings_update.rs @@ -2,6 +2,8 @@ use anyhow::Context; use anyhow::Result; use app_test_support::MockResponsesConfig; use app_test_support::TestAppServer; +use codex_app_server_protocol::ApprovalsReviewer; +use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::JSONRPCErrorError; use codex_app_server_protocol::RequestId; @@ -25,6 +27,7 @@ use codex_protocol::config_types::ReasoningSummary; use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::ReasoningEffort; use core_test_support::responses; +use core_test_support::skip_if_wine_exec; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; @@ -132,6 +135,7 @@ async fn settings_updates_report_results_and_preserve_the_target_on_saved_thread effort: Some(ReasoningEffort::High), summary: Some(ReasoningSummary::Detailed), service_tier: Some(Some("priority".to_string())), + ..Default::default() }; match scenario { UpdateScenario::Future => { @@ -418,6 +422,121 @@ async fn disabled_turn_settings_update_reports_rejection_without_changing_future Ok(()) } +#[test_case(ApprovalsReviewer::User, ApprovalsReviewer::AutoReview; "enable auto review")] +#[test_case(ApprovalsReviewer::AutoReview, ApprovalsReviewer::User; "disable auto review")] +#[tokio::test] +async fn live_reviewer_updates_route_approvals_without_changing_future_turns( + initial: ApprovalsReviewer, + updated: ApprovalsReviewer, +) -> Result<()> { + skip_if_wine_exec!(Ok(()), "command approval requires host-native paths"); + + let server = responses::start_mock_server().await; + let mut replies = vec![responses::sse(vec![ + responses::ev_function_call( + "pause-turn", + "request_user_input", + &json!({"questions": [{ + "id": "continue", "header": "Continue", "question": "Continue?", + "options": [ + {"label": "Yes", "description": "Continue"}, + {"label": "No", "description": "Stop"} + ] + }]}) + .to_string(), + ), + responses::ev_completed("paused"), + ])]; + for (index, reviewer) in [&updated, &initial].into_iter().enumerate() { + replies.push(responses::sse(vec![ + responses::ev_function_call( + &format!("command-{index}"), + "exec_command", + r#"{"cmd":"echo reviewer-test","sandbox_permissions":"require_escalated","justification":"test approval routing"}"#, + ), + responses::ev_completed(&format!("command-response-{index}")), + ])); + if *reviewer == ApprovalsReviewer::AutoReview { + replies.push(responses::sse(vec![ + responses::ev_assistant_message("review", r#"{"outcome":"deny"}"#), + responses::ev_completed("review-response"), + ])); + } + replies.push(responses::sse_completed(&format!("done-{index}"))); + } + let requests = responses::mount_sse_sequence(&server, replies).await; + let codex_home = TempDir::new()?; + // Reviewer-only updates do not require the experimental model-switching feature. + mock_config(codex_home.path(), &server.uri())? + .enable_feature(Feature::GuardianApproval) + .write(codex_home.path())?; + let mut app = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .build_initialized_with_timeout(DEFAULT_TIMEOUT) + .await?; + let id = app + .send_thread_start_request_with_auto_env(ThreadStartParams { + approval_policy: Some(AskForApproval::OnRequest), + approvals_reviewer: Some(initial), + ..Default::default() + }) + .await?; + let ThreadStartResponse { thread, .. } = + timeout(DEFAULT_TIMEOUT, app.read_response(id)).await??; + let turn_id = start_turn(&mut app, &thread.id).await?; + let request = timeout(DEFAULT_TIMEOUT, app.read_stream_until_request_message()).await??; + let ServerRequest::ToolRequestUserInput { request_id, .. } = request else { + anyhow::bail!("expected paused turn, received {request:?}"); + }; + let result: TurnSettingsUpdateResponse = app + .request(|request_id| ClientRequest::TurnSettingsUpdate { + request_id, + params: TurnSettingsUpdateParams { + thread_id: thread.id.clone(), + turn_id, + approvals_reviewer: Some(updated), + ..Default::default() + }, + }) + .await?; + assert_eq!( + result, + TurnSettingsUpdateResponse { + status: TurnSettingsUpdateStatus::Applied, + } + ); + app.send_response( + request_id, + json!({"answers": {"continue": {"answers": ["Yes"]}}}), + ) + .await?; + + for (index, reviewer) in [updated, initial].into_iter().enumerate() { + if index != 0 { + start_turn(&mut app, &thread.id).await?; + } + if reviewer == ApprovalsReviewer::User { + let request = + timeout(DEFAULT_TIMEOUT, app.read_stream_until_request_message()).await??; + let ServerRequest::CommandExecutionRequestApproval { request_id, .. } = request else { + anyhow::bail!("expected manual approval, received {request:?}"); + }; + app.send_response(request_id, json!({"decision": "decline"})) + .await?; + } + let completed: TurnCompletedNotification = + timeout(DEFAULT_TIMEOUT, app.read_notification("turn/completed")).await??; + assert_eq!(completed.turn.status, TurnStatus::Completed); + } + let guardian_requests = requests + .requests() + .into_iter() + .filter(|request| request.body_json()["client_metadata"]["x-openai-subagent"] == "guardian") + .count(); + assert_eq!(guardian_requests, 1); + Ok(()) +} + async fn start_turn(app: &mut TestAppServer, thread_id: &str) -> Result { let request_id = app .send_turn_start_request(TurnStartParams { diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 4c6e9f1a04..a8b1e0b58b 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -1414,7 +1414,10 @@ async fn maybe_request_mcp_tool_approval( }; let approvals_reviewer = connectors::mcp_approvals_reviewer_from_layers( &config.config_layer_stack, - config.approvals_reviewer, + step_context + .settings + .mcp_approvals_reviewer_override + .unwrap_or(config.approvals_reviewer), Some(turn_context.model_info().slug.as_str()), &invocation.server, metadata.connector_id.as_deref(), diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 8920588d47..60ba58b169 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -724,6 +724,7 @@ async fn review_guardian_mcp_elicitation( let Some(mcp_config) = session.services.mcp_runtime.current_config() else { return Ok(None); }; + let step_settings = turn_context.current_settings.load_full(); // The invocation identifies the tool event, but a nested elicitation can // review a different action and connector than the enclosing JavaScript. @@ -893,7 +894,9 @@ async fn review_guardian_mcp_elicitation( let approvals_reviewer = crate::connectors::mcp_approvals_reviewer_from_layers( &mcp_config.config_layer_stack, - mcp_config.approvals_reviewer, + step_settings + .mcp_approvals_reviewer_override + .unwrap_or(mcp_config.approvals_reviewer), Some(turn_context.model_info().slug.as_str()), request.server_name.as_str(), elicitation_connector_id(&request.elicitation), diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index c8b816367f..1c3e2ff1c8 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -3479,11 +3479,14 @@ impl Session { if settings.selected().service_tier != root_service_tier { let mut selected = settings.selected().clone(); selected.service_tier = root_service_tier; - settings = Arc::new(ResolvedStepSettings::new( + let mut inherited_settings = ResolvedStepSettings::new( Arc::new(selected), Arc::clone(&settings.model_info), self.features.enabled(Feature::FastMode), - )); + ); + inherited_settings.mcp_approvals_reviewer_override = + settings.mcp_approvals_reviewer_override; + settings = Arc::new(inherited_settings); } } let token_budget = token_budget::resolve_token_budget( diff --git a/codex-rs/core/src/session/step_activation.rs b/codex-rs/core/src/session/step_activation.rs index 6bd3a5c530..5447fc3066 100644 --- a/codex-rs/core/src/session/step_activation.rs +++ b/codex-rs/core/src/session/step_activation.rs @@ -48,18 +48,13 @@ fn check_legacy_turn_safety( || ignored_models.is_some_and(|models| models.contains(&model.slug)) }; - // tools::approvals and guardian::review still route approvals using the - // originating turn's policy, reviewer, and required-model classification. + // Approval policy and required-model classification still have consumers + // using the originating turn. The reviewer is captured separately. if destination.constrained_approval_policy() != current.constrained_approval_policy() || destination.approval_policy() != turn_context.approval_policy() { return Err("the destination changes the admitted approval policy".to_string()); } - if destination.approvals_reviewer() != current.approvals_reviewer() - || destination.approvals_reviewer() != turn_context.config.approvals_reviewer - { - return Err("the destination changes the admitted approvals reviewer".to_string()); - } if required_review != requirements .auto_review_required_for_model(current.selected_collaboration_mode().model()) @@ -237,7 +232,12 @@ impl Session { turn_id: &str, update: TurnSettingsUpdate, ) -> TurnSettingsUpdateOutcome { - if !self.features.enabled(Feature::StepModelSwitching) { + let reviewer_only = update.approvals_reviewer.is_some() + && update.model.is_none() + && update.effort.is_none() + && update.summary.is_none() + && update.service_tier.is_none(); + if !reviewer_only && !self.features.enabled(Feature::StepModelSwitching) { return TurnSettingsUpdateOutcome::Rejected { reason: "turn settings updates require the step_model_switching feature" .to_string(), @@ -265,12 +265,14 @@ impl Session { return TurnSettingsUpdateOutcome::TargetUnavailable; }; let TurnSettingsUpdate { + approvals_reviewer, model, effort, summary, service_tier, } = update; let update = StepSettingsUpdate { + approvals_reviewer, model, effort, reasoning_summary: summary, @@ -313,6 +315,11 @@ impl Session { ) .map_err(|error| error.to_string()) .and_then(|()| { + // A reviewer-only patch cannot change any model-owned authority. + // Managed reviewer restrictions were checked above. + if reviewer_only { + return Ok(()); + } check_legacy_turn_safety( &turn_context, ¤t, diff --git a/codex-rs/core/src/session/step_activation_tests.rs b/codex-rs/core/src/session/step_activation_tests.rs index cd34233e98..ef63fb535a 100644 --- a/codex-rs/core/src/session/step_activation_tests.rs +++ b/codex-rs/core/src/session/step_activation_tests.rs @@ -607,6 +607,60 @@ enum ManagedAuthorizationChange { ApprovalsReviewer, } +#[test_case(false; "reviewer allow-list")] +#[test_case(true; "model-required review")] +#[tokio::test] +async fn reviewer_only_activation_enforces_managed_authority(required_review: bool) { + let ActivationFixture { session, turn, .. } = activation_fixture(activation_models()).await; + let original = turn.current_settings.load_full(); + let desired = desired_step_settings(&session).await; + let source = RequirementSource::Unknown; + let mut sourced = ConfigRequirementsWithSources::default(); + let reviewer = if required_review { + sourced.auto_review = Some(Sourced::new( + AutoReviewRequirementsToml { + required_on_models: Some(vec![turn.model_info().slug.clone()]), + ..Default::default() + }, + source, + )); + ApprovalsReviewer::User + } else { + sourced.allowed_approvals_reviewers = + Some(Sourced::new(vec![ApprovalsReviewer::User], source)); + ApprovalsReviewer::AutoReview + }; + { + let mut state = session.state.lock().await; + let config = Arc::make_mut(&mut state.session_configuration.original_config_do_not_use); + config.config_layer_stack = ConfigLayerStack::new( + config + .config_layer_stack + .all_layers_low_to_high() + .cloned() + .collect(), + ConfigRequirements::try_from(sourced.clone()).expect("managed requirements"), + sourced.into_toml(), + ) + .expect("managed config stack"); + } + assert!(matches!( + session + .apply_turn_settings( + &turn.sub_id, + TurnSettingsUpdate { + approvals_reviewer: Some(reviewer), + ..Default::default() + } + ) + .await, + TurnSettingsUpdateOutcome::Rejected { .. } + )); + assert!(Arc::ptr_eq(&turn.current_settings.load_full(), &original)); + assert_eq!(desired_step_settings(&session).await, desired); + session.abort_all_tasks(TurnAbortReason::Replaced).await; +} + #[test_case(ManagedAuthorizationChange::ApprovalPolicy; "approval policy refreshed during lookup")] #[test_case(ManagedAuthorizationChange::ApprovalsReviewer; "approvals reviewer refreshed during lookup")] #[tokio::test] diff --git a/codex-rs/core/src/session/step_settings.rs b/codex-rs/core/src/session/step_settings.rs index cba7033064..49502e8a6e 100644 --- a/codex-rs/core/src/session/step_settings.rs +++ b/codex-rs/core/src/session/step_settings.rs @@ -47,6 +47,9 @@ pub(crate) struct ResolvedStepSettings { pub(crate) reasoning_summary: ReasoningSummary, /// Tier sent on requests, filtered by feature and model support. pub(crate) service_tier: Option, + /// MCP historically follows refreshed thread defaults. Preserve that behavior + /// for older clients until this turn receives an explicit live reviewer update. + pub(crate) mcp_approvals_reviewer_override: Option, } impl ResolvedStepSettings { @@ -69,6 +72,7 @@ impl ResolvedStepSettings { model_info, reasoning_summary, service_tier, + mcp_approvals_reviewer_override: None, } } @@ -135,7 +139,11 @@ impl ResolvedStepSettings { .await, ) }; - Ok(Self::new(Arc::new(selected), model_info, fast_mode_enabled)) + let mut next = Self::new(Arc::new(selected), model_info, fast_mode_enabled); + next.mcp_approvals_reviewer_override = update + .approvals_reviewer + .or(self.mcp_approvals_reviewer_override); + Ok(next) } /// Rechecks the retained selection against current managed constraints. diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 3e631df6b2..d6c7010365 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -9141,8 +9141,12 @@ async fn refreshed_mcp_binding_captures_current_approval_authority() { ); } +#[test_case(false; "legacy thread reviewer")] +#[test_case(true; "explicit live reviewer")] #[tokio::test] -async fn mcp_elicitation_reviewer_uses_latest_runtime_authority() { +async fn mcp_elicitation_reviewer_uses_active_reviewer_and_latest_runtime_policy( + live_update: bool, +) { let guardian_server = start_mock_server().await; mount_sse_once( &guardian_server, @@ -9182,17 +9186,34 @@ async fn mcp_elicitation_reviewer_uses_latest_runtime_authority() { ) .await; - session - .update_settings(SessionSettingsUpdate { - step_settings: StepSettingsUpdate { - approvals_reviewer: Some(ApprovalsReviewer::AutoReview), - ..Default::default() - }, - ..Default::default() - }) - .await - .expect("reviewer settings should update"); + session.mark_mcp_runtime_dirty(); session.refresh_mcp_if_dirty().await; + if live_update { + assert_eq!( + session + .apply_turn_settings( + &old_turn.sub_id, + codex_protocol::protocol::TurnSettingsUpdate { + approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + ..Default::default() + }, + ) + .await, + codex_protocol::protocol::TurnSettingsUpdateOutcome::Applied + ); + } else { + session + .update_settings(SessionSettingsUpdate { + step_settings: StepSettingsUpdate { + approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + ..Default::default() + }, + ..Default::default() + }) + .await + .expect("reviewer settings should update"); + session.refresh_mcp_if_dirty().await; + } let request = codex_mcp::ElicitationReviewRequest { server_name: "browser-use".to_string(), diff --git a/codex-rs/core/tests/suite/step_settings.rs b/codex-rs/core/tests/suite/step_settings.rs index bf5651d64c..ce58e29931 100644 --- a/codex-rs/core/tests/suite/step_settings.rs +++ b/codex-rs/core/tests/suite/step_settings.rs @@ -247,6 +247,7 @@ async fn settings_updates_preserve_turn_identity_and_target(target: SettingsTarg effort: Some(Some(ReasoningEffort::High)), summary: Some(ReasoningSummary::Detailed), service_tier: Some(Some(ServiceTier::Fast.request_value().to_string())), + ..Default::default() }; assert_eq!( submit_turn_settings(&test.codex, "different-turn", update.clone()).await?, diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 9c59568b0b..248a1b1551 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -490,6 +490,8 @@ pub struct ConversationSpeechParams { /// task kind. Child sessions and consumers of frozen initial settings are unchanged. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct TurnSettingsUpdate { + /// Changes the reviewer for subsequent approval requests, not pending reviews. + pub approvals_reviewer: Option, pub model: Option, /// `None` preserves the selection; `Some(None)` clears it. pub effort: Option>,