diff --git a/codex-rs/core/src/context/mod.rs b/codex-rs/core/src/context/mod.rs index da7db0b37389..20b35e7af3e0 100644 --- a/codex-rs/core/src/context/mod.rs +++ b/codex-rs/core/src/context/mod.rs @@ -62,6 +62,7 @@ pub(crate) use legacy_unified_exec_process_limit_warning::LegacyUnifiedExecProce pub(crate) use model_switch_instructions::ModelSwitchInstructions; pub(crate) use multi_agent_mode_instructions::MultiAgentModeInstructions; pub(crate) use network_rule_saved::NetworkRuleSaved; +pub use permissions_instructions::ApprovalPromptContext; pub use permissions_instructions::PermissionsInstructions; pub(crate) use personality_spec_instructions::PersonalitySpecInstructions; pub(crate) use plugin_instructions::PluginInstructions; diff --git a/codex-rs/core/src/context/permissions_instructions.rs b/codex-rs/core/src/context/permissions_instructions.rs index 73629a3d5302..3dd1bbac92f9 100644 --- a/codex-rs/core/src/context/permissions_instructions.rs +++ b/codex-rs/core/src/context/permissions_instructions.rs @@ -1 +1,2 @@ +pub use codex_prompts::ApprovalPromptContext; pub use codex_prompts::PermissionsInstructions; diff --git a/codex-rs/core/src/context_manager/updates.rs b/codex-rs/core/src/context_manager/updates.rs index a520e6f595b7..d4cf65e86ddf 100644 --- a/codex-rs/core/src/context_manager/updates.rs +++ b/codex-rs/core/src/context_manager/updates.rs @@ -1,3 +1,4 @@ +use crate::context::ApprovalPromptContext; use crate::context::CollaborationModeInstructions; use crate::context::ContextualUserFragment; use crate::context::ModelSwitchInstructions; @@ -30,6 +31,7 @@ fn build_permissions_update_item( let prev = previous?; if prev.permission_profile() == next.permission_profile() && prev.approval_policy == next.approval_policy.value() + && prev.model == next.model_info.slug { return None; } @@ -38,7 +40,13 @@ fn build_permissions_update_item( PermissionsInstructions::from_permission_profile( &next.permission_profile, next.approval_policy.value(), - next.config.approvals_reviewer, + ApprovalPromptContext::new( + next.config.approvals_reviewer, + next.model_info + .model_messages + .as_ref() + .and_then(|messages| messages.approvals.as_ref()), + ), exec_policy, #[allow(deprecated)] &next.cwd, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index ba8eacb395b8..836bb3710cdd 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -21,6 +21,7 @@ use crate::build_available_skills; use crate::compact; use crate::config::ManagedFeatures; use crate::config::resolve_tool_suggest_config_from_layer_stack; +use crate::context::ApprovalPromptContext; use crate::context::ApprovedCommandPrefixSaved; use crate::context::AvailableSkillsInstructions; use crate::context::CollaborationModeInstructions; @@ -3176,7 +3177,14 @@ impl Session { PermissionsInstructions::from_permission_profile( &turn_context.permission_profile, turn_context.approval_policy.value(), - turn_context.config.approvals_reviewer, + ApprovalPromptContext::new( + turn_context.config.approvals_reviewer, + turn_context + .model_info + .model_messages + .as_ref() + .and_then(|messages| messages.approvals.as_ref()), + ), self.services.exec_policy.current().as_ref(), #[allow(deprecated)] &turn_context.cwd, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 6cc223cd7d8e..1a8deb6c7d1c 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -9027,18 +9027,13 @@ async fn record_context_updates_and_set_reference_context_item_reinjects_full_co #[tokio::test] async fn record_context_updates_and_set_reference_context_item_persists_baseline_without_emitting_diffs() { - let (mut session, previous_context) = make_session_and_context().await; - let next_model = if previous_context.model_info.slug == "gpt-5.4" { - "gpt-5.2" - } else { - "gpt-5.4" - }; - let turn_context = previous_context - .with_model(next_model.to_string(), &session.services.models_manager) - .await; - let previous_context_item = previous_context.to_turn_context_item(); - let previous_context = Arc::new(previous_context); + let (mut session, turn_context) = make_session_and_context().await; + let previous_context_item = turn_context.to_turn_context_item(); + let previous_context = Arc::new(turn_context); let world_state = build_world_state_from_turn_context(&session, &previous_context).await; + let mut turn_context = Arc::try_unwrap(previous_context) + .unwrap_or_else(|_| panic!("previous turn context should have no remaining references")); + turn_context.sub_id = format!("{}-next", turn_context.sub_id); { let mut state = session.state.lock().await; state.set_reference_context_item(Some(previous_context_item.clone())); diff --git a/codex-rs/core/tests/suite/permissions_messages.rs b/codex-rs/core/tests/suite/permissions_messages.rs index e5a69962bc2e..7f42ba893c7f 100644 --- a/codex-rs/core/tests/suite/permissions_messages.rs +++ b/codex-rs/core/tests/suite/permissions_messages.rs @@ -2,10 +2,15 @@ use anyhow::Result; use codex_config::ConfigLayerStack; use codex_core::ForkSnapshot; use codex_core::config::Constrained; +use codex_core::context::ApprovalPromptContext; use codex_core::context::ContextualUserFragment; use codex_core::context::PermissionsInstructions; use codex_core::load_exec_policy; +use codex_models_manager::model_info::model_info_from_slug; use codex_protocol::models::PermissionProfile; +use codex_protocol::openai_models::ApprovalMessages; +use codex_protocol::openai_models::ModelMessages; +use codex_protocol::openai_models::ModelsResponse; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; @@ -33,6 +38,129 @@ fn permissions_texts(request: &ResponsesRequest) -> Vec { .collect() } +fn model_with_approval_messages( + slug: &str, + on_request: &str, + on_request_auto_review: &str, +) -> codex_protocol::openai_models::ModelInfo { + let mut model = model_info_from_slug(slug); + model.model_messages = Some(ModelMessages { + instructions_template: None, + instructions_variables: None, + approvals: Some(ApprovalMessages { + on_request: Some(on_request.to_string()), + on_request_auto_review: Some(on_request_auto_review.to_string()), + }), + }); + model +} + +async fn submit_text_turn( + test: &core_test_support::test_codex::TestCodex, + text: &str, +) -> Result<()> { + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: text.to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await?; + wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn catalog_approval_message_is_sent_in_initial_permissions() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let req = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + let model_slug = "catalog-approvals-model"; + let model = model_with_approval_messages( + model_slug, + "catalog user approval instructions", + "catalog auto-review approval instructions", + ); + let mut builder = test_codex() + .with_model(model_slug) + .with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + config.model_catalog = Some(ModelsResponse { + models: vec![model], + }); + }); + let test = builder.build(&server).await?; + + submit_text_turn(&test, "hello").await?; + + let permissions = permissions_texts(&req.single_request()); + assert_eq!(permissions.len(), 1); + assert!(permissions[0].contains("catalog user approval instructions")); + assert!(permissions[0].contains("Filesystem sandboxing defines")); + assert!(!permissions[0].contains("How to request escalation")); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn model_change_appends_new_catalog_approval_message() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let _req1 = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + let req2 = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]), + ) + .await; + let first_slug = "catalog-approvals-model-a"; + let second_slug = "catalog-approvals-model-b"; + let first = model_with_approval_messages(first_slug, "model A approvals", "model A auto"); + let second = model_with_approval_messages(second_slug, "model B approvals", "model B auto"); + let mut builder = test_codex() + .with_model(first_slug) + .with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + config.model_catalog = Some(ModelsResponse { + models: vec![first, second], + }); + }); + let test = builder.build(&server).await?; + submit_text_turn(&test, "first").await?; + + core_test_support::submit_thread_settings( + &test.codex, + codex_protocol::protocol::ThreadSettingsOverrides { + model: Some(second_slug.to_string()), + ..Default::default() + }, + ) + .await?; + submit_text_turn(&test, "second").await?; + + let permissions = permissions_texts(&req2.single_request()); + assert_eq!(permissions.len(), 2); + assert!( + permissions + .last() + .is_some_and(|text| text.contains("model B approvals")) + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn permissions_message_sent_once_on_start() -> Result<()> { skip_if_no_network!(Ok(())); @@ -563,7 +691,7 @@ async fn permissions_message_includes_writable_roots() -> Result<()> { let expected = PermissionsInstructions::from_permission_profile( &permission_profile, AskForApproval::OnRequest, - test.config.approvals_reviewer, + ApprovalPromptContext::new(test.config.approvals_reviewer, /*messages*/ None), &exec_policy, test.config.cwd.as_path(), /*exec_permission_approvals_enabled*/ false, diff --git a/codex-rs/core/tests/suite/personality.rs b/codex-rs/core/tests/suite/personality.rs index e74695b59d56..88733b466674 100644 --- a/codex-rs/core/tests/suite/personality.rs +++ b/codex-rs/core/tests/suite/personality.rs @@ -573,6 +573,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow personality_friendly: Some(friendly_personality_message.to_string()), personality_pragmatic: Some("Pragmatic variant".to_string()), }), + approvals: None, }), include_skills_usage_instructions: false, supports_reasoning_summaries: false, @@ -689,6 +690,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() - personality_friendly: Some(remote_friendly_message.to_string()), personality_pragmatic: Some(remote_pragmatic_message.to_string()), }), + approvals: None, }), include_skills_usage_instructions: false, supports_reasoning_summaries: false, diff --git a/codex-rs/models-manager/src/model_info.rs b/codex-rs/models-manager/src/model_info.rs index 573403b72860..75d333efe382 100644 --- a/codex-rs/models-manager/src/model_info.rs +++ b/codex-rs/models-manager/src/model_info.rs @@ -54,14 +54,24 @@ pub fn with_config_overrides(mut model: ModelInfo, config: &ModelsManagerConfig) if let Some(base_instructions) = &config.base_instructions { model.base_instructions = base_instructions.clone(); - model.model_messages = None; + clear_instruction_messages(&mut model); } else if !config.personality_enabled { - model.model_messages = None; + clear_instruction_messages(&mut model); } model } +fn clear_instruction_messages(model: &mut ModelInfo) { + if let Some(model_messages) = &mut model.model_messages { + model_messages.instructions_template = None; + model_messages.instructions_variables = None; + if model_messages.approvals.is_none() { + model.model_messages = None; + } + } +} + /// Build a minimal fallback model descriptor for missing/unknown slugs. pub fn model_info_from_slug(slug: &str) -> ModelInfo { warn!("Unknown model {slug} is used. This will use fallback model metadata."); @@ -119,6 +129,7 @@ fn local_personality_messages_for_slug(slug: &str) -> Option { personality_friendly: Some(LOCAL_FRIENDLY_TEMPLATE.to_string()), personality_pragmatic: Some(LOCAL_PRAGMATIC_TEMPLATE.to_string()), }), + approvals: None, }), _ => None, } diff --git a/codex-rs/models-manager/src/model_info_tests.rs b/codex-rs/models-manager/src/model_info_tests.rs index 70ad3da8dfb7..2240f0a39134 100644 --- a/codex-rs/models-manager/src/model_info_tests.rs +++ b/codex-rs/models-manager/src/model_info_tests.rs @@ -1,5 +1,6 @@ use super::*; use crate::ModelsManagerConfig; +use codex_protocol::openai_models::ApprovalMessages; use pretty_assertions::assert_eq; #[test] @@ -44,6 +45,68 @@ fn reasoning_summaries_override_false_is_noop_when_model_is_false() { assert_eq!(updated, model); } +#[test] +fn base_instruction_override_preserves_catalog_approval_messages() { + let mut model = model_info_from_slug("unknown-model"); + let approvals = ApprovalMessages { + on_request: Some("user approvals".to_string()), + on_request_auto_review: Some("auto approvals".to_string()), + }; + model.model_messages = Some(ModelMessages { + instructions_template: Some("template".to_string()), + instructions_variables: Some(ModelInstructionsVariables { + personality_default: Some("default".to_string()), + personality_friendly: Some("friendly".to_string()), + personality_pragmatic: Some("pragmatic".to_string()), + }), + approvals: Some(approvals.clone()), + }); + let config = ModelsManagerConfig { + base_instructions: Some("override".to_string()), + ..Default::default() + }; + + let updated = with_config_overrides(model, &config); + + assert_eq!( + updated.model_messages, + Some(ModelMessages { + instructions_template: None, + instructions_variables: None, + approvals: Some(approvals), + }) + ); +} + +#[test] +fn disabled_personality_preserves_catalog_approval_messages() { + let mut model = model_info_from_slug("unknown-model"); + let approvals = ApprovalMessages { + on_request: Some("user approvals".to_string()), + on_request_auto_review: None, + }; + model.model_messages = Some(ModelMessages { + instructions_template: Some("template".to_string()), + instructions_variables: None, + approvals: Some(approvals.clone()), + }); + let config = ModelsManagerConfig { + personality_enabled: false, + ..Default::default() + }; + + let updated = with_config_overrides(model, &config); + + assert_eq!( + updated.model_messages, + Some(ModelMessages { + instructions_template: None, + instructions_variables: None, + approvals: Some(approvals), + }) + ); +} + #[test] fn model_context_window_override_clamps_to_max_context_window() { let mut model = model_info_from_slug("unknown-model"); diff --git a/codex-rs/prompts/src/lib.rs b/codex-rs/prompts/src/lib.rs index 5a30358393d8..c598b2d765d9 100644 --- a/codex-rs/prompts/src/lib.rs +++ b/codex-rs/prompts/src/lib.rs @@ -12,6 +12,7 @@ pub use compact::SUMMARY_PREFIX; pub use goals::budget_limit_prompt; pub use goals::continuation_prompt; pub use goals::objective_updated_prompt; +pub use permissions_instructions::ApprovalPromptContext; pub use permissions_instructions::PermissionsInstructions; pub use realtime::BACKEND_PROMPT; pub use realtime::END_INSTRUCTIONS; diff --git a/codex-rs/prompts/src/permissions_instructions.rs b/codex-rs/prompts/src/permissions_instructions.rs index c382b804a0f8..1c19d008cf33 100644 --- a/codex-rs/prompts/src/permissions_instructions.rs +++ b/codex-rs/prompts/src/permissions_instructions.rs @@ -4,6 +4,7 @@ use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::SandboxMode; use codex_protocol::models::PermissionProfile; use codex_protocol::models::format_allow_prefixes; +use codex_protocol::openai_models::ApprovalMessages; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; @@ -47,6 +48,7 @@ static SANDBOX_MODE_READ_ONLY_TEMPLATE: LazyLock