diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index bf3e6b08c5..77e28b85e5 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -13,7 +13,6 @@ use crate::hook_runtime::PreCompactHookOutcome; use crate::hook_runtime::run_post_compact_hooks; use crate::hook_runtime::run_pre_compact_hooks; use crate::responses_metadata::CodexResponsesMetadata; -use crate::responses_metadata::CodexResponsesRequestKind; use crate::responses_metadata::CompactionTurnMetadata; use crate::session::RequestEffortUsage; use crate::session::session::Session; @@ -258,10 +257,7 @@ async fn run_compact_task_inner_impl( // request tracking) // survives retries within this compact turn. let responses_metadata = sess - .responses_metadata( - turn_context.as_ref(), - CodexResponsesRequestKind::Compaction(compaction_metadata), - ) + .compaction_responses_metadata(turn_context.as_ref(), compaction_metadata) .await; let compaction_response_id = loop { diff --git a/codex-rs/core/src/compact_remote_v2_attempt.rs b/codex-rs/core/src/compact_remote_v2_attempt.rs index 28e0f2d55e..f770f0712b 100644 --- a/codex-rs/core/src/compact_remote_v2_attempt.rs +++ b/codex-rs/core/src/compact_remote_v2_attempt.rs @@ -6,7 +6,6 @@ use crate::Prompt; use crate::client::ModelClientSession; use crate::compact::CompactionAnalyticsDetails; use crate::compact_remote_history::trim_function_call_history_to_fit_context_window; -use crate::responses_metadata::CodexResponsesRequestKind; use crate::responses_metadata::CompactionTurnMetadata; use crate::session::session::Session; use crate::session::step_context::StepContext; @@ -87,12 +86,10 @@ pub(super) async fn run_remote_compact_v2_attempt( cyber_access_program: turn_context.cyber_access_program, }; - let responses_metadata = sess - .responses_metadata( - turn_context.as_ref(), - CodexResponsesRequestKind::Compaction(compaction_metadata), - ) + let mut responses_metadata = sess + .compaction_responses_metadata(turn_context.as_ref(), compaction_metadata) .await; + responses_metadata.tool_namespaces_info = tool_router.tool_namespaces_info().cloned(); let trace_attempt = compaction_trace.start_attempt(&serde_json::json!({ "model": turn_context.model_info().slug.as_str(), "instructions": prompt.base_instructions.text.as_str(), diff --git a/codex-rs/core/src/guardian/input_budget.rs b/codex-rs/core/src/guardian/input_budget.rs index 13a9c27a12..5a775632f4 100644 --- a/codex-rs/core/src/guardian/input_budget.rs +++ b/codex-rs/core/src/guardian/input_budget.rs @@ -98,7 +98,7 @@ pub(crate) async fn finalize( ReasoningSummary::None, /*service_tier*/ None, &session - .responses_metadata(&step.turn, CodexResponsesRequestKind::Turn) + .responses_metadata(step, CodexResponsesRequestKind::Turn) .await, )?; let mut existing = super::request_budget::estimate_request_tokens(&request) diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index 4046cc8826..4e276868ce 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -61,6 +61,7 @@ use tracing::instrument; use crate::context::ContextualUserFragment; use crate::context::HookAdditionalContext; use crate::event_mapping::parse_turn_item; +use crate::guardian::GuardianReviewContext; use crate::session::TurnInput; use crate::session::session::Session; use crate::session::step_context::StepContext; @@ -68,7 +69,7 @@ use crate::session::turn_context::TurnContext; use crate::state::TurnState; use crate::tools::hook_names::HookToolName; use crate::tools::sandboxing::PermissionRequestPayload; -use crate::turn_metadata::McpTurnMetadataContext; +use crate::turn_metadata::ExecutionMetadata; pub(crate) struct HookRuntimeOutcome { pub should_stop: bool, @@ -155,7 +156,7 @@ pub(crate) async fn run_pending_session_start_hooks( cwd: turn_context.cwd.clone(), transcript_path: sess.hook_transcript_path().await, model: turn_context.model_info().slug.clone(), - permission_mode: hook_permission_mode(turn_context), + permission_mode: hook_permission_mode(turn_context.approval_policy()), target, }; let hooks = sess.hooks(); @@ -184,11 +185,12 @@ pub(crate) async fn run_pending_session_start_hooks( /// handlers. pub(crate) async fn run_pre_tool_use_hooks( sess: &Arc, - turn_context: &Arc, + step_context: &StepContext, tool_use_id: String, tool_name: &HookToolName, tool_input: &Value, ) -> PreToolUseHookResult { + let turn_context = &step_context.turn; let request = PreToolUseRequest { session_id: sess.session_id().into(), turn_id: turn_context.sub_id.clone(), @@ -196,8 +198,8 @@ pub(crate) async fn run_pre_tool_use_hooks( #[allow(deprecated)] cwd: turn_context.cwd.clone(), transcript_path: sess.hook_transcript_path().await, - model: turn_context.model_info().slug.clone(), - permission_mode: hook_permission_mode(turn_context), + model: step_context.settings.model_info.slug.clone(), + permission_mode: hook_permission_mode(step_context.settings.approval_policy()), tool_name: tool_name.name().to_string(), matcher_aliases: tool_name.matcher_aliases().to_vec(), tool_use_id, @@ -246,10 +248,11 @@ pub(crate) async fn run_pre_tool_use_hooks( // tool input or post-run state. pub(crate) async fn run_permission_request_hooks( sess: &Arc, - turn_context: &Arc, + review_context: &GuardianReviewContext, run_id_suffix: &str, payload: PermissionRequestPayload, ) -> Option { + let turn_context = review_context.turn(); let request = PermissionRequestRequest { session_id: sess.session_id().into(), turn_id: turn_context.sub_id.clone(), @@ -257,8 +260,8 @@ pub(crate) async fn run_permission_request_hooks( #[allow(deprecated)] cwd: turn_context.cwd.to_path_buf(), transcript_path: sess.hook_transcript_path().await, - model: turn_context.model_info().slug.clone(), - permission_mode: hook_permission_mode(turn_context), + model: review_context.model_info.slug.clone(), + permission_mode: hook_permission_mode(review_context.approval_policy), tool_name: payload.tool_name.name().to_string(), matcher_aliases: payload.tool_name.matcher_aliases().to_vec(), run_id_suffix: run_id_suffix.to_string(), @@ -285,13 +288,14 @@ pub(crate) async fn run_permission_request_hooks( /// matchers and hook logs. pub(crate) async fn run_post_tool_use_hooks( sess: &Arc, - turn_context: &Arc, + step_context: &StepContext, tool_use_id: String, tool_name: String, matcher_aliases: Vec, tool_input: Value, tool_response: Value, ) -> PostToolUseOutcome { + let turn_context = &step_context.turn; let request = PostToolUseRequest { session_id: sess.session_id().into(), turn_id: turn_context.sub_id.clone(), @@ -299,8 +303,8 @@ pub(crate) async fn run_post_tool_use_hooks( #[allow(deprecated)] cwd: turn_context.cwd.clone(), transcript_path: sess.hook_transcript_path().await, - model: turn_context.model_info().slug.clone(), - permission_mode: hook_permission_mode(turn_context), + model: step_context.settings.model_info.slug.clone(), + permission_mode: hook_permission_mode(step_context.settings.approval_policy()), tool_name, matcher_aliases, tool_use_id, @@ -367,11 +371,7 @@ fn build_request_metadata( .unwrap_or(turn_context.initial_settings.as_ref()); turn_context .turn_metadata_state - .current_meta_value_for_mcp_request(McpTurnMetadataContext { - model: settings.model_info.slug.as_str(), - reasoning_effort: settings.effective_reasoning_effort(), - node_repl_disabled: settings.model_info.node_repl_disabled, - }) + .current_meta_value_for_mcp_request(ExecutionMetadata::from_settings(settings)) .map(|turn_metadata| { Map::from_iter([( crate::X_CODEX_TURN_METADATA_HEADER.to_string(), @@ -445,7 +445,7 @@ pub(crate) async fn run_turn_stop_hooks( cwd: turn_context.cwd.clone(), transcript_path, model: turn_context.model_info().slug.clone(), - permission_mode: hook_permission_mode(turn_context), + permission_mode: hook_permission_mode(turn_context.approval_policy()), request_metadata: (!request_metadata.is_empty()).then_some(request_metadata), stop_hook_active, last_assistant_message, @@ -522,7 +522,7 @@ pub(crate) async fn run_turn_interrupt_hooks( cwd: turn_context.cwd.clone(), transcript_path: sess.hook_transcript_path().await, model: turn_context.model_info().slug.clone(), - permission_mode: hook_permission_mode(turn_context), + permission_mode: hook_permission_mode(turn_context.approval_policy()), request_metadata: (!request_metadata.is_empty()).then_some(request_metadata), }; if let Err(err) = sess.flush_rollout().await { @@ -682,7 +682,7 @@ pub(crate) async fn inspect_pending_input( cwd: turn_context.cwd.clone(), transcript_path: sess.hook_transcript_path().await, model: turn_context.model_info().slug.clone(), - permission_mode: hook_permission_mode(turn_context), + permission_mode: hook_permission_mode(turn_context.approval_policy()), prompt: UserMessageItem::new(content).message(), }; let hooks = sess.hooks(); @@ -1020,8 +1020,8 @@ fn hook_run_metric_tags(run: &HookRunSummary) -> [(&'static str, &'static str); ] } -fn hook_permission_mode(turn_context: &TurnContext) -> String { - match turn_context.approval_policy() { +fn hook_permission_mode(approval_policy: AskForApproval) -> String { + match approval_policy { AskForApproval::Never => "bypassPermissions", AskForApproval::UnlessTrusted | AskForApproval::OnRequest | AskForApproval::Granular(_) => { "default" diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 50244fd34b..178c147dcf 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -20,7 +20,7 @@ use crate::tools::hook_names::HookToolName; use crate::tools::lifecycle::process_mcp_tool_result; use crate::tools::sandboxing::ApprovalAction; use crate::tools::sandboxing::ToolError; -use crate::turn_metadata::McpTurnMetadataContext; +use crate::turn_metadata::ExecutionMetadata; use codex_analytics::AppInvocation; use codex_analytics::InvocationType; use codex_analytics::build_track_events_context; @@ -1252,11 +1252,9 @@ fn build_mcp_tool_call_request_meta( if let Some(turn_metadata) = step_context .turn .turn_metadata_state - .current_meta_value_for_mcp_request(McpTurnMetadataContext { - model: step_context.settings.model_info.slug.as_str(), - reasoning_effort: step_context.settings.effective_reasoning_effort(), - node_repl_disabled: step_context.settings.model_info.node_repl_disabled, - }) + .current_meta_value_for_mcp_request(ExecutionMetadata::from_settings( + &step_context.settings, + )) { request_meta.insert( crate::X_CODEX_TURN_METADATA_HEADER.to_string(), diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 3233728975..8afe24ef34 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -14,7 +14,7 @@ use crate::session::turn_context::TurnEnvironment; use crate::state::ActiveTurn; use crate::test_support::models_manager_with_provider; use crate::tools::hook_names::HookToolName; -use crate::turn_metadata::McpTurnMetadataContext; +use crate::turn_metadata::ExecutionMetadata; use codex_config::CONFIG_TOML_FILE; use codex_config::config_toml::ConfigToml; use codex_config::types::AppConfig; @@ -189,11 +189,16 @@ fn non_apps_tool_does_not_require_account_metadata(meta: JsonValue) { ); } -fn mcp_turn_metadata_context(turn_context: &TurnContext) -> McpTurnMetadataContext<'_> { - McpTurnMetadataContext { +fn mcp_turn_metadata_context(turn_context: &TurnContext) -> ExecutionMetadata<'_> { + ExecutionMetadata { model: turn_context.model_info().slug.as_str(), reasoning_effort: turn_context.effective_reasoning_effort(), node_repl_disabled: turn_context.model_info().node_repl_disabled, + auto_review_enabled: crate::guardian::routes_approval_policy_to_guardian( + turn_context.approval_policy(), + turn_context.config.approvals_reviewer, + ), + node_repl_auto_review_required: turn_context.model_info().node_repl_auto_review_required, } } @@ -1160,21 +1165,27 @@ async fn mcp_tool_call_request_meta_includes_turn_metadata_for_custom_server() { turn_metadata .get("model") .and_then(serde_json::Value::as_str), - Some(turn_context.model_info().slug.as_str()) + Some(step_context.settings.model_info.slug.as_str()) ); assert_eq!( turn_metadata["node_repl_auto_review_required"], - serde_json::Value::Bool(turn_context.model_info().node_repl_auto_review_required), + serde_json::Value::Bool( + step_context + .settings + .model_info + .node_repl_auto_review_required + ), ); assert_eq!( turn_metadata["node_repl_disabled"], - serde_json::Value::Bool(turn_context.model_info().node_repl_disabled), + serde_json::Value::Bool(step_context.settings.model_info.node_repl_disabled), ); assert_eq!( turn_metadata .get("reasoning_effort") .and_then(serde_json::Value::as_str), - turn_context + step_context + .settings .effective_reasoning_effort() .map(|effort| effort.to_string()) .as_deref() @@ -1214,7 +1225,7 @@ async fn mcp_tool_call_request_meta_uses_the_issuing_step( model.slug = "model-b".to_string(); model.default_reasoning_level = Some(ReasoningEffortConfig::High); model.node_repl_disabled = true; - // This separate policy remains turn-owned in this migration. + // Node review requirements are captured with the issuing step. model.node_repl_auto_review_required = !turn_context.model_info().node_repl_auto_review_required; @@ -1222,6 +1233,8 @@ async fn mcp_tool_call_request_meta_uses_the_issuing_step( expected["model"] = serde_json::json!("model-b"); expected["reasoning_effort"] = serde_json::json!(expected_effort); expected["node_repl_disabled"] = serde_json::json!(true); + expected["node_repl_auto_review_required"] = + serde_json::json!(step_b.settings.model_info.node_repl_auto_review_required); assert_eq!( build_mcp_tool_call_request_meta(&step_b, "node_repl", "call-b", /*metadata*/ None), Some(serde_json::json!({ @@ -1234,18 +1247,6 @@ async fn mcp_tool_call_request_meta_uses_the_issuing_step( build_mcp_tool_call_request_meta(&step_a, "node_repl", "call-a", /*metadata*/ None), original_meta, ); - assert_eq!( - turn_context - .turn_metadata_state - .to_responses_metadata( - "installation".to_string(), - "window".to_string(), - crate::responses_metadata::CodexResponsesRequestKind::Turn, - ) - .turn_metadata_value() - .expect("Responses turn metadata")["node_repl_disabled"], - serde_json::json!(false), - ); } #[tokio::test] diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 3bb68a8839..3e0a931b43 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -3812,20 +3812,6 @@ impl Session { ) .or_cancel(cancellation_token) .await??; - // Publish inventory only after the step's tool plan has been finalized. - if turn_context - .config - .tool_registry - .turn_metadata_includes_tool_info - && turn_context.model_info().use_responses_lite - { - turn_context.turn_metadata_state.set_tool_namespaces_info( - tool_router - .tool_namespaces_info() - .cloned() - .unwrap_or_default(), - ); - } Ok(Arc::new(StepContext { settings, token_budget, diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index b3abc3d5c9..3f9a02f0d7 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1,5 +1,6 @@ use super::input_queue::InputQueue; use super::mcp_refresh::McpRefresh; +use super::step_context::StepContext; use super::step_settings::ModelInfoOverrides; use super::step_settings::StepSettings; use super::step_settings::StepSettingsConstraints; @@ -15,9 +16,11 @@ use crate::hook_mcp_executor::CoreHookMcpExecutor; use crate::mcp_tool_call::McpToolApprovalMetadata; use crate::responses_metadata::CodexResponsesMetadata; use crate::responses_metadata::CodexResponsesRequestKind; +use crate::responses_metadata::CompactionTurnMetadata; use crate::shell_snapshot::ShellSnapshot; use crate::shell_snapshot::SnapshotCredentialBrokerState; use crate::state::ActiveTurn; +use crate::turn_metadata::ExecutionMetadata; use codex_extension_api::ExtensionDataInit; use codex_http_client::ClientRouteClass; use codex_http_client::RouteAwareClientPool; @@ -644,15 +647,64 @@ impl Session { pub(crate) async fn responses_metadata( &self, - turn_context: &TurnContext, + step_context: &StepContext, request_kind: CodexResponsesRequestKind, + ) -> CodexResponsesMetadata { + let (window_id, window_number, context_window_id) = self.current_window().await; + let mut responses_metadata = step_context.turn.turn_metadata_state.to_responses_metadata( + self.installation_id.clone(), + window_id, + request_kind, + ); + ExecutionMetadata::from_settings(&step_context.settings).apply_to(&mut responses_metadata); + responses_metadata.tool_namespaces_info = if step_context + .turn + .config + .tool_registry + .turn_metadata_includes_tool_info + && step_context.settings.model_info.use_responses_lite + { + step_context.tool_router.tool_namespaces_info().cloned() + } else { + None + }; + self.with_window_and_fork_metadata( + &step_context.turn, + responses_metadata, + window_number, + context_window_id, + ) + } + + // TODO(CDXENT-454): Build the compaction request and metadata from the captured execution. + // Remote compaction currently attaches only finalized tool inventory because the rest of the + // request remains turn-backed; local compaction does not have a finalized request inventory. + pub(crate) async fn compaction_responses_metadata( + &self, + turn_context: &TurnContext, + compaction_metadata: CompactionTurnMetadata, ) -> CodexResponsesMetadata { let (window_id, window_number, context_window_id) = self.current_window().await; let responses_metadata = turn_context.turn_metadata_state.to_responses_metadata( self.installation_id.clone(), window_id, - request_kind, + CodexResponsesRequestKind::Compaction(compaction_metadata), ); + self.with_window_and_fork_metadata( + turn_context, + responses_metadata, + window_number, + context_window_id, + ) + } + + fn with_window_and_fork_metadata( + &self, + turn_context: &TurnContext, + responses_metadata: CodexResponsesMetadata, + window_number: u64, + context_window_id: uuid::Uuid, + ) -> CodexResponsesMetadata { CodexResponsesMetadata { window_number: Some(window_number), context_window_id: Some(context_window_id), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 1178d2a0d1..de620e2265 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -5862,16 +5862,17 @@ pub(crate) async fn build_world_state_from_turn_context( #[tokio::test] async fn responses_metadata_uses_selected_harness_analytics_client() { - let (mut session, mut turn_context) = make_session_and_context().await; for enabled in [true, false] { + let (mut session, mut turn_context) = make_session_and_context().await; session.services.analytics_events_client = AnalyticsEventsClient::new( Arc::clone(&session.services.auth_manager), turn_context.config.chatgpt_base_url.clone(), Some(enabled), ); Arc::make_mut(&mut turn_context.config).analytics_enabled = Some(!enabled); + let step_context = StepContext::for_test(Arc::new(turn_context)); let metadata = session - .responses_metadata(&turn_context, CodexResponsesRequestKind::Turn) + .responses_metadata(&step_context, CodexResponsesRequestKind::Turn) .await; assert_eq!(metadata.analytics_enabled, Some(enabled)); } diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 777dc8963b..c28add68e3 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -517,7 +517,7 @@ pub(crate) async fn run_turn( .await; let responses_metadata = sess - .responses_metadata(turn_context.as_ref(), CodexResponsesRequestKind::Turn) + .responses_metadata(step_context.as_ref(), CodexResponsesRequestKind::Turn) .await; run_sampling_request( Arc::clone(&sess), diff --git a/codex-rs/core/src/session_startup_prewarm.rs b/codex-rs/core/src/session_startup_prewarm.rs index 2c04f88a8f..9074162ea4 100644 --- a/codex-rs/core/src/session_startup_prewarm.rs +++ b/codex-rs/core/src/session_startup_prewarm.rs @@ -316,7 +316,7 @@ async fn schedule_startup_prewarm_inner( /*status*/ None, ); let responses_metadata = session - .responses_metadata(&startup_turn_context, CodexResponsesRequestKind::Prewarm) + .responses_metadata(step_context.as_ref(), CodexResponsesRequestKind::Prewarm) .await; let mut client_session = session.services.model_client.new_session(); let websocket_warmup_started_at = Instant::now(); diff --git a/codex-rs/core/src/tools/approvals.rs b/codex-rs/core/src/tools/approvals.rs index 8675ead36d..8786c55040 100644 --- a/codex-rs/core/src/tools/approvals.rs +++ b/codex-rs/core/src/tools/approvals.rs @@ -495,7 +495,7 @@ impl Session { // 2. If StrictAutoReview || Guardian enabled, then Guardian. Else, user. let resolution = match run_permission_request_hooks( self, - ctx.review_context.turn(), + &ctx.review_context, &permission_request_run_id, action.permission_request_payload(), ) diff --git a/codex-rs/core/src/tools/handlers/extension_tools.rs b/codex-rs/core/src/tools/handlers/extension_tools.rs index 2e209a8807..92ac4bdb81 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -26,7 +26,7 @@ use crate::tools::handlers::apply_granted_turn_permissions; use crate::tools::lifecycle::extension_tool_call_source; use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::turn_metadata::McpTurnMetadataContext; +use crate::turn_metadata::ExecutionMetadata; pub(crate) struct ExtensionToolAdapter( Arc codex_tools::ToolExecutor>>, @@ -170,11 +170,9 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall<'_> let codex_turn_metadata = invocation .turn .turn_metadata_state - .current_meta_value_for_mcp_request(McpTurnMetadataContext { - model: settings.model_info.slug.as_str(), - reasoning_effort: settings.effective_reasoning_effort(), - node_repl_disabled: settings.model_info.node_repl_disabled, - }) + .current_meta_value_for_mcp_request(ExecutionMetadata::from_settings( + &invocation.step_context.settings, + )) .and_then(|metadata| to_ascii_json_string(&metadata).ok()); let mut environments = Vec::new(); for environment in invocation.step_context.environments.turn_environments() { diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 7cc8c128c3..e4949a87d4 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -567,7 +567,7 @@ impl ToolRegistry { if let Some(pre_tool_use_payload) = tool.pre_tool_use_payload(&invocation) { match run_pre_tool_use_hooks( &invocation.session, - &invocation.turn, + invocation.step_context.as_ref(), invocation.call_id.clone(), &pre_tool_use_payload.tool_name, &pre_tool_use_payload.tool_input, @@ -683,7 +683,7 @@ impl ToolRegistry { Some( run_post_tool_use_hooks( &invocation.session, - &invocation.turn, + invocation.step_context.as_ref(), post_tool_use_payload.tool_use_id, post_tool_use_payload.tool_name.name().to_string(), post_tool_use_payload.tool_name.matcher_aliases().to_vec(), diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index c77936fc90..c0125ad996 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -45,7 +45,6 @@ use serde_json::json; use crate::WaitForEnvironmentToolConfig; use crate::config::CurrentTimeReminderConfig; use crate::environment_selection::TurnEnvironmentState; -use crate::responses_metadata::CodexResponsesRequestKind; use crate::responses_metadata::TurnToolFunctionInfo; use crate::responses_metadata::TurnToolNamespacesInfo; use crate::responses_metadata::TurnToolSource; @@ -1596,13 +1595,6 @@ async fn candidate_model_plan_leaves_selected_model_and_inventory_unchanged() { turn.model_info(), ToolPlanInputs::default(), )); - let selected_inventory = selected - .tool_namespaces_info - .clone() - .expect("selected plan inventory"); - turn.turn_metadata_state - .set_tool_namespaces_info(selected_inventory.clone()); - let mut candidate_model = selected_model.as_ref().clone(); candidate_model.tool_mode = Some(ToolMode::CodeModeOnly); candidate_model.shell_type = ConfigShellToolType::UnifiedExec; @@ -1633,12 +1625,6 @@ async fn candidate_model_plan_leaves_selected_model_and_inventory_unchanged() { source: TurnToolSource::Harness, } ); - let metadata = turn.turn_metadata_state.to_responses_metadata( - "installation".to_string(), - "window".to_string(), - CodexResponsesRequestKind::Turn, - ); - assert_eq!(metadata.tool_namespaces_info, Some(selected_inventory)); assert_eq!(turn.model_info(), &selected_model); assert_eq!( ToolPlanProbe::from_router(plan_with_model( diff --git a/codex-rs/core/src/turn_metadata.rs b/codex-rs/core/src/turn_metadata.rs index 662b754552..9b278da054 100644 --- a/codex-rs/core/src/turn_metadata.rs +++ b/codex-rs/core/src/turn_metadata.rs @@ -20,12 +20,12 @@ use crate::responses_metadata::MAX_EXTRA_METADATA_VALUE_BYTES; use crate::responses_metadata::PARENT_TURN_ID_KEY; use crate::responses_metadata::ROOT_TURN_ID_KEY; use crate::responses_metadata::TurnMetadataWorkspace; -use crate::responses_metadata::TurnToolNamespacesInfo; use crate::responses_metadata::filter_extra_metadata; use crate::responses_metadata::subagent_header_value; use crate::responses_metadata::subagent_metadata_kind; use crate::sandbox_tags::SandboxTags; use crate::sandbox_tags::record_policy_metadata; +use crate::session::step_settings::ResolvedStepSettings; use codex_git_utils::SanitizedGitUrl; use codex_git_utils::get_git_remote_urls_assume_git_repo; use codex_git_utils::get_has_changes_in_repo; @@ -50,10 +50,48 @@ const REASONING_EFFORT_KEY: &str = "reasoning_effort"; const USER_INPUT_REQUESTED_DURING_TURN_KEY: &str = "user_input_requested_during_turn"; const WORKSPACE_KIND_KEY: &str = "workspace_kind"; -pub(crate) struct McpTurnMetadataContext<'a> { +/// Captured execution settings shared by Responses and MCP metadata. +pub(crate) struct ExecutionMetadata<'a> { pub(crate) model: &'a str, pub(crate) reasoning_effort: Option, pub(crate) node_repl_disabled: bool, + pub(crate) auto_review_enabled: bool, + pub(crate) node_repl_auto_review_required: bool, +} + +impl<'a> ExecutionMetadata<'a> { + pub(crate) fn from_settings(settings: &'a ResolvedStepSettings) -> Self { + Self { + model: settings.model_info.slug.as_str(), + reasoning_effort: settings.effective_reasoning_effort(), + node_repl_disabled: settings.model_info.node_repl_disabled, + auto_review_enabled: crate::guardian::routes_approval_policy_to_guardian( + settings.approval_policy(), + settings.approvals_reviewer(), + ), + node_repl_auto_review_required: settings.model_info.computer_use_review_required(), + } + } + + pub(crate) fn apply_to(&self, metadata: &mut CodexResponsesMetadata) { + metadata.auto_review_enabled = Some(self.auto_review_enabled); + metadata.node_repl_auto_review_required = Some(self.node_repl_auto_review_required); + metadata.node_repl_disabled = Some(self.node_repl_disabled); + metadata + .extra + .insert(MODEL_KEY.to_string(), self.model.to_string()); + match &self.reasoning_effort { + Some(reasoning_effort) => { + metadata.extra.insert( + REASONING_EFFORT_KEY.to_string(), + reasoning_effort.to_string(), + ); + } + None => { + metadata.extra.remove(REASONING_EFFORT_KEY); + } + } + } } #[derive(Clone, Debug, Default)] @@ -136,7 +174,6 @@ pub(crate) struct TurnMetadataState { node_repl_auto_review_required: bool, node_repl_disabled: bool, enriched_workspaces: RwLock>>, - tool_namespaces_info: RwLock>, turn_started_at_unix_ms: RwLock>, responses_api_metadata: RwLock>, responsesapi_client_metadata: RwLock>, @@ -222,7 +259,6 @@ impl TurnMetadataState { node_repl_auto_review_required: model_info.computer_use_review_required(), node_repl_disabled: model_info.node_repl_disabled, enriched_workspaces: RwLock::new(None), - tool_namespaces_info: RwLock::new(None), turn_started_at_unix_ms: RwLock::new(None), responses_api_metadata: RwLock::new(BTreeMap::new()), responsesapi_client_metadata: RwLock::new(BTreeMap::new()), @@ -234,11 +270,10 @@ impl TurnMetadataState { pub(crate) fn current_meta_value_for_mcp_request( &self, - context: McpTurnMetadataContext<'_>, + execution: ExecutionMetadata<'_>, ) -> Option { let mut responses_metadata = self.mcp_metadata_template(); - // Use the issuing step's Node REPL restriction. - responses_metadata.node_repl_disabled = Some(context.node_repl_disabled); + execution.apply_to(&mut responses_metadata); // Never serialize harness-owned tool inventory for external MCP servers. responses_metadata.tool_namespaces_info = None; let Value::Object(mut metadata) = responses_metadata.turn_metadata_value()? else { @@ -247,25 +282,10 @@ impl TurnMetadataState { metadata.remove(AGENT_NAME_KEY); metadata.remove(PARENT_TURN_ID_KEY); metadata.remove(ROOT_TURN_ID_KEY); - metadata.insert( - MODEL_KEY.to_string(), - Value::String(context.model.to_string()), - ); metadata.insert( CODEX_VERSION_KEY.to_string(), Value::String(env!("CARGO_PKG_VERSION").to_string()), ); - match context.reasoning_effort { - Some(reasoning_effort) => { - metadata.insert( - REASONING_EFFORT_KEY.to_string(), - Value::String(reasoning_effort.to_string()), - ); - } - None => { - metadata.remove(REASONING_EFFORT_KEY); - } - } if self .user_input_requested_during_turn .load(Ordering::Relaxed) @@ -299,14 +319,6 @@ impl TurnMetadataState { .store(true, Ordering::Relaxed); } - pub(crate) fn set_tool_namespaces_info(&self, tool_namespaces_info: TurnToolNamespacesInfo) { - *self - .tool_namespaces_info - .write() - .unwrap_or_else(std::sync::PoisonError::into_inner) = - (!tool_namespaces_info.is_empty()).then_some(tool_namespaces_info); - } - pub(crate) fn set_parent_turn_id(&self, parent_turn_id: String) { if parent_turn_id.trim().is_empty() { return; @@ -430,11 +442,7 @@ impl TurnMetadataState { node_repl_auto_review_required: Some(self.node_repl_auto_review_required), node_repl_disabled: Some(self.node_repl_disabled), workspaces: self.current_workspaces(), - tool_namespaces_info: self - .tool_namespaces_info - .read() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .clone(), + tool_namespaces_info: None, turn_started_at_unix_ms: self.current_turn_started_at_unix_ms(), extra, ..CodexResponsesMetadata::new( diff --git a/codex-rs/core/src/turn_metadata_tests.rs b/codex-rs/core/src/turn_metadata_tests.rs index 01de9c70df..85c9279946 100644 --- a/codex-rs/core/src/turn_metadata_tests.rs +++ b/codex-rs/core/src/turn_metadata_tests.rs @@ -3,6 +3,7 @@ use super::*; use crate::responses_metadata::ANALYTICS_ENABLED_KEY; use crate::responses_metadata::AUTO_REVIEW_ENABLED_KEY; use crate::responses_metadata::CONTEXT_WINDOW_ID_KEY; +use crate::responses_metadata::CodexResponsesMetadata; use crate::responses_metadata::CodexResponsesRequestKind; use crate::responses_metadata::CompactionTurnMetadata; use crate::responses_metadata::FORKED_FROM_ORDINAL_EXCLUSIVE_KEY; @@ -15,12 +16,12 @@ use crate::responses_metadata::ROOT_TURN_ID_KEY; use crate::responses_metadata::SANDBOX_MODE_KEY; use crate::responses_metadata::TOOL_NAMESPACES_INFO_KEY; use crate::responses_metadata::TURN_TRIGGER_KEY; -use crate::responses_metadata::TurnToolFunctionInfo; -use crate::responses_metadata::TurnToolNamespaceInfo; -use crate::responses_metadata::TurnToolSource; use crate::responses_metadata::WINDOW_ID_KEY; use crate::responses_metadata::WINDOW_NUMBER_KEY; use crate::responses_metadata::validate_extra_metadata; +use crate::session::step_context::StepContext; +use crate::session::tests::make_session_and_context; +use crate::session::tests::update_turn_settings_for_test; use codex_analytics::CompactionImplementation; use codex_analytics::CompactionPhase; use codex_analytics::CompactionReason; @@ -46,31 +47,31 @@ use std::time::Duration; use tempfile::TempDir; use tokio::process::Command; -fn test_mcp_turn_metadata_context() -> McpTurnMetadataContext<'static> { - McpTurnMetadataContext { +fn test_mcp_turn_metadata_context() -> ExecutionMetadata<'static> { + ExecutionMetadata { model: "gpt-5.4", reasoning_effort: Some(ReasoningEffortConfig::High), node_repl_disabled: false, + auto_review_enabled: true, + node_repl_auto_review_required: true, } } -fn test_responses_metadata_json( +fn test_turn_responses_metadata( state: &TurnMetadataState, window_id: &str, - request_kind: CodexResponsesRequestKind, -) -> String { - state - .to_responses_metadata( - "installation-a".to_string(), - window_id.to_string(), - request_kind, - ) - .turn_metadata_json() - .expect("turn metadata json") +) -> CodexResponsesMetadata { + state.to_responses_metadata( + "installation-a".to_string(), + window_id.to_string(), + CodexResponsesRequestKind::Turn, + ) } fn test_turn_responses_metadata_json(state: &TurnMetadataState, window_id: &str) -> String { - test_responses_metadata_json(state, window_id, CodexResponsesRequestKind::Turn) + test_turn_responses_metadata(state, window_id) + .turn_metadata_json() + .expect("turn metadata json") } fn test_compaction_responses_metadata_json( @@ -78,11 +79,14 @@ fn test_compaction_responses_metadata_json( window_id: &str, compaction: CompactionTurnMetadata, ) -> String { - test_responses_metadata_json( - state, - window_id, - CodexResponsesRequestKind::Compaction(compaction), - ) + state + .to_responses_metadata( + "installation-a".to_string(), + window_id.to_string(), + CodexResponsesRequestKind::Compaction(compaction), + ) + .turn_metadata_json() + .expect("turn metadata json") } fn test_turn_metadata_header(state: &TurnMetadataState) -> String { @@ -547,12 +551,19 @@ fn turn_metadata_state_includes_model_and_reasoning_effort_only_in_request_meta( assert!(meta.get("request_kind").is_none()); assert_eq!(meta["model"].as_str(), Some("gpt-5.4")); assert_eq!(meta["reasoning_effort"].as_str(), Some("high")); + assert_eq!(meta[AUTO_REVIEW_ENABLED_KEY].as_bool(), Some(true)); + assert_eq!( + meta[NODE_REPL_AUTO_REVIEW_REQUIRED_KEY].as_bool(), + Some(true) + ); let meta_without_reasoning_effort = state - .current_meta_value_for_mcp_request(McpTurnMetadataContext { + .current_meta_value_for_mcp_request(ExecutionMetadata { model: "gpt-5.4", reasoning_effort: None, node_repl_disabled: false, + auto_review_enabled: false, + node_repl_auto_review_required: false, }) .expect("turn metadata should be present"); assert_eq!( @@ -566,6 +577,35 @@ fn turn_metadata_state_includes_model_and_reasoning_effort_only_in_request_meta( ); } +#[tokio::test] +async fn execution_metadata_applies_captured_settings_to_responses_metadata() { + let (_session, mut turn) = make_session_and_context().await; + let config = Arc::make_mut(&mut turn.config); + config.approvals_reviewer = codex_protocol::config_types::ApprovalsReviewer::AutoReview; + config.permissions.approval_policy = + crate::config::Constrained::allow_any(codex_protocol::protocol::AskForApproval::OnRequest); + update_turn_settings_for_test(&mut turn, |settings| { + let model_info = Arc::make_mut(&mut settings.model_info); + model_info.slug = "captured-step-model".to_string(); + model_info.node_repl_auto_review_required = true; + model_info.node_repl_disabled = true; + }); + let step_context = StepContext::for_test(Arc::new(turn)); + + let mut metadata = step_context.turn.turn_metadata_state.to_responses_metadata( + "installation-a".to_string(), + "window-a".to_string(), + CodexResponsesRequestKind::Turn, + ); + ExecutionMetadata::from_settings(&step_context.settings).apply_to(&mut metadata); + let metadata = metadata.turn_metadata_value().expect("turn metadata"); + + assert_eq!(metadata["model"], "captured-step-model"); + assert_eq!(metadata[AUTO_REVIEW_ENABLED_KEY], true); + assert_eq!(metadata[NODE_REPL_AUTO_REVIEW_REQUIRED_KEY], true); + assert_eq!(metadata[NODE_REPL_DISABLED_KEY], true); +} + #[test] fn turn_metadata_state_marks_user_input_requested_during_turn_only_for_mcp_request_meta() { let temp_dir = TempDir::new().expect("temp dir"); @@ -816,25 +856,6 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields( ), ])); state.set_turn_started_at_unix_ms(/*turn_started_at_unix_ms*/ 1_700_000_000_123); - state.set_tool_namespaces_info(BTreeMap::from([( - "mcp__calendar".to_string(), - TurnToolNamespaceInfo { - name: "mcp__calendar".to_string(), - functions: BTreeMap::from([( - "lookup".to_string(), - TurnToolFunctionInfo { - name: "lookup".to_string(), - direct: true, - code_mode_name: Some("mcp__calendar__lookup".to_string()), - deferred: false, - source: TurnToolSource::Mcp { - server_name: "calendar".to_string(), - }, - }, - )]), - }, - )])); - let header = test_turn_metadata_header(&state); assert!(header.is_ascii()); assert!(!header.contains("東京")); @@ -851,26 +872,7 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields( assert_eq!(json["thread_id"].as_str(), Some("thread-a")); assert!(json.get(LEGACY_CODE_MODE_TOOL_NAMES_KEY).is_none()); assert_eq!(json["agent_name"].as_str(), Some("/root")); - assert_eq!( - json[TOOL_NAMESPACES_INFO_KEY], - serde_json::json!({ - "mcp__calendar": { - "name": "mcp__calendar", - "functions": { - "lookup": { - "name": "lookup", - "direct": true, - "code_mode_name": "mcp__calendar__lookup", - "deferred": false, - "source": { - "kind": "mcp", - "server_name": "calendar", - }, - }, - }, - }, - }) - ); + assert!(json.get(TOOL_NAMESPACES_INFO_KEY).is_none()); assert!(json.get(INSTALLATION_ID_KEY).is_none()); assert!(json.get("x-codex-installation-id").is_none()); assert!(json.get("x-codex-parent-thread-id").is_none()); @@ -921,13 +923,8 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields( Some("thread-a:1") ); - let compatibility_headers = state - .to_responses_metadata( - "installation-a".to_string(), - "thread-a:1".to_string(), - CodexResponsesRequestKind::Turn, - ) - .compatibility_headers(); + let compatibility_headers = + test_turn_responses_metadata(&state, "thread-a:1").compatibility_headers(); let compatibility_metadata: Value = serde_json::from_str( compatibility_headers .get("x-codex-turn-metadata") diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 9ec46330f7..0b5dd9b925 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -1,4 +1,3 @@ -use std::collections::HashMap; use std::fs; use std::path::Path; @@ -20,6 +19,7 @@ use codex_history::InitialHistory; use codex_history::RolloutItem; use codex_model_provider_info::ModelProviderInfo; use codex_model_provider_info::built_in_model_providers; +use codex_models_manager::bundled_models_response; use codex_plugin::PluginHookSource; use codex_plugin::PluginId; use codex_protocol::items::parse_hook_prompt_fragment; @@ -28,6 +28,8 @@ use codex_protocol::models::ContentItem; use codex_protocol::models::NetworkPermissions; use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItem; +use codex_protocol::openai_models::ModelsResponse; +use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; @@ -36,9 +38,13 @@ use codex_protocol::protocol::Op; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; use codex_protocol::protocol::ThreadSettingsOverrides; +use codex_protocol::protocol::TurnSettingsUpdate; +use codex_protocol::protocol::TurnSettingsUpdateOutcome; use codex_protocol::request_permissions::PermissionGrantScope; use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsResponse; +use codex_protocol::request_user_input::RequestUserInputAnswer; +use codex_protocol::request_user_input::RequestUserInputResponse; use codex_protocol::user_input::UserInput; use codex_thread_store::InMemoryThreadStore; use codex_utils_absolute_path::AbsolutePathBuf; @@ -70,10 +76,12 @@ use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; use core_test_support::test_target_os; use core_test_support::wait_for_event; +use core_test_support::wait_for_event_match; use core_test_support::wait_for_mcp_server; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; +use std::collections::HashMap; use std::sync::Arc; use std::time::Duration; use tempfile::TempDir; @@ -3605,6 +3613,143 @@ async fn pre_tool_use_json_deny_blocks_exec_command_before_execution() -> Result Ok(()) } +#[tokio::test] +async fn pre_tool_use_hook_model_tracks_step_after_a_turn_update() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let request_user_input_call_id = "pause-before-pretooluse-exec-command"; + let call_id = "pretooluse-exec-command"; + let model_a = "pretooluse-attribution-a"; + let model_b = "pretooluse-attribution-b"; + let command = "echo attribution"; + let args = serde_json::json!({ "cmd": command }); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call( + request_user_input_call_id, + "request_user_input", + r#"{"questions":[{"id":"continue","header":"Continue","question":"Continue?","options":[{"label":"Yes (Recommended)","description":"Run it."},{"label":"No","description":"Stop."}]}]}"#, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + core_test_support::responses::ev_function_call( + call_id, + "exec_command", + &serde_json::to_string(&args)?, + ), + ev_completed("resp-2"), + ]), + sse(vec![ + ev_response_created("resp-3"), + ev_assistant_message("msg-1", "hook blocked it"), + ev_completed("resp-3"), + ]), + ], + ) + .await; + + let base_model = bundled_models_response() + .expect("bundled models should parse") + .models + .into_iter() + .find(|model| model.slug == "gpt-5.4") + .expect("bundled gpt-5.4 model"); + let models = [model_a, model_b] + .into_iter() + .map(|slug| { + let mut model = base_model.clone(); + model.slug = slug.to_string(); + model + }) + .collect(); + let mut builder = test_codex() + .with_model(model_a) + .with_pre_build_hook(|home| { + write_pre_tool_use_hook(home, Some("^Bash$"), "json_deny", "blocked by pre hook") + .expect("failed to write pre tool use hook test fixture"); + }) + .with_config(move |config| { + trust_discovered_hooks(config); + for feature in [ + Feature::StepModelSwitching, + Feature::DefaultModeRequestUserInput, + ] { + config + .features + .enable(feature) + .expect("test config should allow feature update"); + } + config.model_catalog = Some(ModelsResponse { models }); + }); + let test = builder.build(&server).await?; + + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path()); + test.codex + .start_or_steer_turn( + TurnInputRequest::user_input(vec![UserInput::Text { + text: "run the blocked shell command".to_string(), + text_elements: Vec::new(), + }]) + .with_thread_settings(ThreadSettingsOverrides { + approval_policy: Some(AskForApproval::Never), + sandbox_policy: Some(sandbox_policy), + permission_profile, + ..Default::default() + }), + ) + .await?; + + let request = wait_for_event_match(&test.codex, |event| match event { + EventMsg::RequestUserInput(request) => Some(request.clone()), + _ => None, + }) + .await; + assert_eq!(request.call_id, request_user_input_call_id); + let (reply, outcome) = oneshot::channel(); + test.codex + .submit(Op::TurnSettings { + turn_id: request.turn_id.clone(), + update: TurnSettingsUpdate { + model: Some(model_b.to_string()), + effort: Some(Some(ReasoningEffort::High)), + ..Default::default() + }, + reply, + }) + .await?; + assert_eq!(outcome.await?, TurnSettingsUpdateOutcome::Applied); + test.codex + .submit(Op::UserInputAnswer { + id: request.turn_id, + response: RequestUserInputResponse { + answers: HashMap::from([( + "continue".to_string(), + RequestUserInputAnswer { + answers: vec!["Yes (Recommended)".to_string()], + }, + )]), + }, + }) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + assert_eq!(responses.requests().len(), 3); + let hook_inputs = read_pre_tool_use_hook_inputs(test.codex_home_path())?; + assert_eq!(hook_inputs.len(), 1); + assert_eq!(hook_inputs[0]["model"], model_b); + + Ok(()) +} + #[tokio::test] async fn pre_tool_use_records_additional_context_for_exec_command() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/core/tests/suite/mcp_turn_metadata.rs b/codex-rs/core/tests/suite/mcp_turn_metadata.rs index f1edbdcea5..9b5eedb7d9 100644 --- a/codex-rs/core/tests/suite/mcp_turn_metadata.rs +++ b/codex-rs/core/tests/suite/mcp_turn_metadata.rs @@ -8,6 +8,7 @@ use codex_core::TurnInputRequest; use codex_core::config::Config; use codex_features::Feature; use codex_history::RolloutItem; +use codex_models_manager::bundled_models_response; use codex_protocol::approvals::ElicitationRequest; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::CollaborationMode; @@ -17,12 +18,16 @@ use codex_protocol::items::McpToolCallStatus; use codex_protocol::items::TurnItem; use codex_protocol::models::NetworkPermissions; use codex_protocol::models::PermissionProfile; +use codex_protocol::openai_models::ModelsResponse; +use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ElicitationAction; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::protocol::ThreadSettingsOverrides; use codex_protocol::protocol::TurnEnvironmentSelections; +use codex_protocol::protocol::TurnSettingsUpdate; +use codex_protocol::protocol::TurnSettingsUpdateOutcome; use codex_protocol::request_permissions::PermissionGrantScope; use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsResponse; @@ -325,6 +330,65 @@ async fn submit_user_turn( Ok(()) } +fn attribution_models(model_slugs: [&str; 2]) -> Vec { + let base_model = bundled_models_response() + .expect("bundled models should parse") + .models + .into_iter() + .find(|model| model.slug == "gpt-5.4") + .expect("bundled gpt-5.4 model"); + model_slugs + .into_iter() + .map(|slug| { + let mut model = base_model.clone(); + model.slug = slug.to_string(); + model + }) + .collect() +} + +async fn apply_turn_attribution_update( + test: &TestCodex, + request_user_input_call_id: &str, + model: &str, +) -> Result<()> { + let request = wait_for_event_match(&test.codex, |event| match event { + EventMsg::RequestUserInput(request) => Some(request.clone()), + _ => None, + }) + .await; + assert_eq!(request.call_id, request_user_input_call_id); + + let (reply, outcome) = tokio::sync::oneshot::channel(); + test.codex + .submit(Op::TurnSettings { + turn_id: request.turn_id.clone(), + update: TurnSettingsUpdate { + model: Some(model.to_string()), + effort: Some(Some(ReasoningEffort::High)), + ..Default::default() + }, + reply, + }) + .await?; + assert_eq!(outcome.await?, TurnSettingsUpdateOutcome::Applied); + + test.codex + .submit(Op::UserInputAnswer { + id: request.turn_id, + response: RequestUserInputResponse { + answers: HashMap::from([( + "confirm_path".to_string(), + RequestUserInputAnswer { + answers: vec!["Yes (Recommended)".to_string()], + }, + )]), + }, + }) + .await?; + Ok(()) +} + async fn wait_for_mcp_tool_call_item( test: &TestCodex, call_id: &str, @@ -1311,7 +1375,7 @@ async fn apps_default_writes_prompts_for_writes_but_not_reads() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn mcp_tool_call_metadata_records_prior_request_user_input_tool() -> Result<()> { +async fn mcp_tool_call_metadata_uses_captured_step_after_request_user_input() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -1368,16 +1432,26 @@ async fn mcp_tool_call_metadata_records_prior_request_user_input_tool() -> Resul ) .await; + let model_a = "mcp-metadata-a"; + let model_b = "mcp-metadata-b"; + let models = attribution_models([model_a, model_b]); let mut builder = search_capable_apps_builder(apps_server.chatgpt_base_url.clone()) - .with_config(|config| { + .with_model(model_a) + .with_config(move |config| { set_calendar_approval_mode(config, AppToolApproval::Approve); + config + .features + .enable(Feature::StepModelSwitching) + .expect("test config should allow feature update"); + config.model_catalog = Some(ModelsResponse { models }); + config.model_reasoning_effort = Some(ReasoningEffort::Low); }); let test = builder.build(&server).await?; submit_user_turn( &test, "Ask for confirmation, then create a calendar event.", - AskForApproval::Never, + AskForApproval::OnRequest, PermissionProfile::Disabled, Some(CollaborationMode { mode: ModeKind::Plan, @@ -1390,26 +1464,7 @@ async fn mcp_tool_call_metadata_records_prior_request_user_input_tool() -> Resul ) .await?; - let request = wait_for_event_match(&test.codex, |event| match event { - EventMsg::RequestUserInput(request) => Some(request.clone()), - _ => None, - }) - .await; - assert_eq!(request.call_id, request_user_input_call_id); - - test.codex - .submit(Op::UserInputAnswer { - id: request.turn_id, - response: RequestUserInputResponse { - answers: HashMap::from([( - "confirm_path".to_string(), - RequestUserInputAnswer { - answers: vec!["Yes (Recommended)".to_string()], - }, - )]), - }, - }) - .await?; + apply_turn_attribution_update(&test, request_user_input_call_id, model_b).await?; let EventMsg::McpToolCallBegin(begin) = wait_for_event(&test.codex, |event| { matches!(event, EventMsg::McpToolCallBegin(_)) @@ -1433,6 +1488,14 @@ async fn mcp_tool_call_metadata_records_prior_request_user_input_tool() -> Resul .pointer("/params/_meta/x-codex-turn-metadata/user_input_requested_during_turn"), Some(&json!(true)) ); + assert_eq!( + apps_tool_call.pointer("/params/_meta/x-codex-turn-metadata/model"), + Some(&json!(model_b)) + ); + assert_eq!( + apps_tool_call.pointer("/params/_meta/x-codex-turn-metadata/reasoning_effort"), + Some(&json!("high")) + ); Ok(()) } diff --git a/codex-rs/core/tests/suite/step_settings.rs b/codex-rs/core/tests/suite/step_settings.rs index 055e1f1780..afda72bf8f 100644 --- a/codex-rs/core/tests/suite/step_settings.rs +++ b/codex-rs/core/tests/suite/step_settings.rs @@ -169,14 +169,6 @@ fn direct_tool_settings_test() -> TestCodexBuilder { }) } -fn advertises_apply_patch(request: &Value) -> bool { - request["tools"] - .as_array() - .expect("provider tools") - .iter() - .any(|tool| tool["type"] == "custom" && tool["name"] == "apply_patch") -} - fn paused_response(response_id: &str, call_id: &str) -> String { sse(vec![ ev_response_created(response_id), @@ -288,6 +280,15 @@ fn request_turn_id(request: &ResponsesRequest) -> String { .to_string() } +fn request_turn_metadata(request: &ResponsesRequest) -> Value { + serde_json::from_str( + request.body_json()["client_metadata"]["x-codex-turn-metadata"] + .as_str() + .expect("request should include turn metadata"), + ) + .expect("valid turn metadata") +} + // Dynamic tools return the original payload, so handler truncation cannot hide a recorder bug. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn tool_result_history_keeps_originating_model_across_switch_and_replay() -> Result<()> { @@ -1569,7 +1570,9 @@ async fn captured_model_replans_tools_and_retains_the_issuing_request_router() - for feature in [Feature::ShellTool, Feature::UnifiedExec] { config.features.enable(feature).expect("enable shell tools"); } + config.tool_registry.turn_metadata_includes_tool_info = true; for model in &mut config.model_catalog.as_mut().expect("models").models { + model.use_responses_lite = true; model.shell_type = if model.slug == MODEL_B { ConfigShellToolType::Disabled } else { @@ -1635,7 +1638,35 @@ async fn captured_model_replans_tools_and_retains_the_issuing_request_router() - assert_eq!( requests .iter() - .map(advertises_apply_patch) + .map(|request| { + core_test_support::responses::namespace_child_tool( + &request["input"][0], + "functions", + "apply_patch", + ) + .is_some() + }) + .collect::>(), + vec![false, true, false], + ); + let metadata = requests + .iter() + .map(|request| { + serde_json::from_str::( + request["client_metadata"]["x-codex-turn-metadata"] + .as_str() + .expect("request metadata"), + ) + }) + .collect::, _>>()?; + assert_eq!( + metadata + .iter() + .map(|metadata| { + metadata["tool_namespaces_info"]["functions"]["functions"] + .get("apply_patch") + .is_some() + }) .collect::>(), vec![false, true, false], ); @@ -1643,12 +1674,16 @@ async fn captured_model_replans_tools_and_retains_the_issuing_request_router() - requests .iter() .map(|request| { - request["tools"] - .as_array() - .expect("provider tools") - .iter() - .filter_map(|tool| tool["name"].as_str()) - .filter(|name| matches!(*name, "exec_command" | "write_stdin")) + ["exec_command", "write_stdin"] + .into_iter() + .filter(|name| { + core_test_support::responses::namespace_child_tool( + &request["input"][0], + "functions", + name, + ) + .is_some() + }) .collect::>() }) .collect::>(), @@ -1779,7 +1814,7 @@ async fn captured_model_enables_and_executes_code_mode() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn response_metadata_uses_the_captured_model_after_a_turn_update() -> Result<()> { +async fn response_metadata_uses_the_captured_step_after_a_turn_update() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -1800,15 +1835,19 @@ async fn response_metadata_uses_the_captured_model_after_a_turn_update() -> Resu .await; let test = step_settings_test().build_with_auto_env(&server).await?; let request = start_paused_turn(&test.codex).await?; - apply_turn_settings( - &test.codex, - &request.turn_id, - TurnSettingsUpdate { - model: Some(MODEL_B.to_string()), - ..Default::default() - }, - ) - .await?; + assert_eq!( + submit_turn_settings( + &test.codex, + &request.turn_id, + TurnSettingsUpdate { + model: Some(MODEL_B.to_string()), + effort: Some(Some(ReasoningEffort::High)), + ..Default::default() + }, + ) + .await?, + TurnSettingsUpdateOutcome::Applied + ); answer_paused_turn(&test.codex, &request.turn_id).await?; let mut reroutes = Vec::new(); @@ -1836,6 +1875,29 @@ async fn response_metadata_uses_the_captured_model_after_a_turn_update() -> Resu .collect::>(), vec![json!(MODEL_A), json!(MODEL_B)] ); + assert_eq!( + response_mock + .requests() + .iter() + .map(request_turn_metadata) + .map(|metadata| { + json!({ + "model": metadata["model"], + "reasoning_effort": metadata["reasoning_effort"], + }) + }) + .collect::>(), + vec![ + json!({ + "model": MODEL_A, + "reasoning_effort": "low", + }), + json!({ + "model": MODEL_B, + "reasoning_effort": "high", + }), + ] + ); // B's matching response header is not a reroute from the turn's initial A. // Buffering metadata likewise belongs to the captured B step. assert_eq!(