From 132c73917181b88d9c6959e000c7f6e78efa4f51 Mon Sep 17 00:00:00 2001 From: rhan-oai Date: Sat, 12 Sep 2026 00:10:56 +0000 Subject: [PATCH] Retire Friendly and Pragmatic personality selection (#44946) ## What changed - Use literal model instruction templates and the standard fallback prompt, ignoring legacy personality variables. Retain catalog decoding compatibility and report `supports_personality` as `false` for generated model presets. - Stop emitting `` developer messages and assigning an implicit Pragmatic configuration default. - Filter Friendly/Pragmatic overrides from TUI requests while preserving explicit `personality = "none"`. With `features.personality` enabled, this opt-out still strips the model's personality section. - Refresh inherited model instructions for agent roles only when the personality opt-out changes, preserving custom instructions. ## Testing Update coverage for literal legacy templates, deprecated overrides on turn start and resume, role instruction refresh, and forwarding the explicit opt-out through thread start, resume, and fork requests. GitOrigin-RevId: 1391db4e565e010569f0885f80013d1f86b45825 --- .../tests/suite/v2/thread_resume.rs | 15 +- .../app-server/tests/suite/v2/turn_start.rs | 37 ++- codex-rs/core/src/agent/role.rs | 9 +- codex-rs/core/src/agent/role_tests.rs | 43 ++- codex-rs/core/src/config/edit.rs | 7 - codex-rs/core/src/config/mod.rs | 8 +- codex-rs/core/src/context/mod.rs | 2 - .../context/personality_spec_instructions.rs | 42 --- codex-rs/core/src/context/world_state/mod.rs | 2 - .../src/context/world_state/personality.rs | 101 ------ .../context/world_state/personality_tests.rs | 106 ------ .../core/src/session/step_settings_tests.rs | 15 +- codex-rs/core/src/session/tests.rs | 49 +-- codex-rs/core/src/session/world_state.rs | 59 +--- codex-rs/core/tests/suite/model_switching.rs | 5 +- codex-rs/core/tests/suite/personality.rs | 310 +----------------- codex-rs/core/tests/suite/step_settings.rs | 26 +- codex-rs/models-manager/src/model_info.rs | 98 ++---- .../models-manager/src/model_info_tests.rs | 94 +----- codex-rs/protocol/src/openai_models.rs | 236 ++----------- codex-rs/tui/src/app/tests.rs | 7 +- codex-rs/tui/src/app/thread_settings.rs | 4 +- codex-rs/tui/src/app_server_session.rs | 55 +++- 23 files changed, 213 insertions(+), 1117 deletions(-) delete mode 100644 codex-rs/core/src/context/personality_spec_instructions.rs delete mode 100644 codex-rs/core/src/context/world_state/personality.rs delete mode 100644 codex-rs/core/src/context/world_state/personality_tests.rs diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 75cf2dce5d..52f24eb001 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -159,7 +159,6 @@ use super::analytics::wait_for_matching_analytics_event; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(25); #[cfg(not(windows))] const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); -const CODEX_5_2_INSTRUCTIONS_TEMPLATE_DEFAULT: &str = "You are Codex, a coding agent based on GPT-5. You and the user share the same workspace and collaborate to achieve the user's goals."; #[tokio::test] async fn thread_resume_paginated_model_context_preserves_original_metadata() -> Result<()> { @@ -6060,7 +6059,7 @@ async fn start_materialized_thread_and_restart( } #[tokio::test] -async fn thread_resume_accepts_personality_override() -> Result<()> { +async fn thread_resume_accepts_deprecated_personality_override() -> Result<()> { skip_if_no_network!(Ok(())); let server = responses::start_mock_server().await; @@ -6157,6 +6156,8 @@ async fn thread_resume_accepts_personality_override() -> Result<()> { .await??; let requests = response_mock.requests(); + assert_eq!(requests.len(), 2, "expected initial and resumed turns"); + let initial_instructions_text = requests[0].instructions_text(); let request = requests .last() .expect("expected request for resumed thread turn"); @@ -6164,13 +6165,13 @@ async fn thread_resume_accepts_personality_override() -> Result<()> { assert!( developer_texts .iter() - .any(|text| text.contains("")), - "expected a personality update message in developer input, got {developer_texts:?}" + .all(|text| !text.contains("")), + "deprecated personality override emitted a developer update: {developer_texts:?}" ); let instructions_text = request.instructions_text(); - assert!( - instructions_text.contains(CODEX_5_2_INSTRUCTIONS_TEMPLATE_DEFAULT), - "expected default base instructions from history, got {instructions_text:?}" + assert_eq!( + instructions_text, initial_instructions_text, + "resume should retain the original base instructions" ); Ok(()) diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index a8881ddd7e..f9488fbec6 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -77,6 +77,7 @@ use codex_core::test_support::all_model_presets; use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_features::Feature; use codex_login::AuthCredentialsStoreMode; +use codex_models_manager::model_info::BASE_INSTRUCTIONS; use codex_models_manager::model_info::model_info_from_slug; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; @@ -2433,8 +2434,29 @@ async fn turn_start_uses_thread_feature_overrides_for_request_user_input_tool_de Ok(()) } +fn assert_fallback_model_instructions(request: &responses::ResponsesRequest) { + let instructions = request.instructions_text(); + let expected_intro = BASE_INSTRUCTIONS + .lines() + .next() + .expect("fallback prompt has an opening sentence"); + let expected_personality = BASE_INSTRUCTIONS + .lines() + .find(|line| line.starts_with("Your default personality and tone")) + .expect("fallback prompt has a Friendly personality section"); + + assert!( + instructions.contains(expected_intro), + "expected fallback model identity instructions in the request" + ); + assert!( + instructions.contains(expected_personality), + "expected baked Friendly instructions in the request" + ); +} + #[tokio::test] -async fn turn_start_accepts_personality_override_v2() -> Result<()> { +async fn turn_start_accepts_deprecated_personality_override_v2() -> Result<()> { skip_if_no_network!(Ok(())); let server = responses::start_mock_server().await; @@ -2485,6 +2507,7 @@ async fn turn_start_accepts_personality_override_v2() -> Result<()> { .await??; let request = response_mock.single_request(); + assert_fallback_model_instructions(&request); let developer_texts = request.message_input_texts("developer"); if developer_texts.is_empty() { eprintln!("request body: {}", request.body_json()); @@ -2493,8 +2516,8 @@ async fn turn_start_accepts_personality_override_v2() -> Result<()> { assert!( developer_texts .iter() - .any(|text| text.contains("")), - "expected personality update message in developer input, got {developer_texts:?}" + .all(|text| !text.contains("")), + "deprecated personality override emitted a developer update: {developer_texts:?}" ); Ok(()) @@ -2641,7 +2664,7 @@ async fn thread_start_ignores_deprecated_multi_agent_mode() -> Result<()> { } #[tokio::test] -async fn turn_start_change_personality_mid_thread_v2() -> Result<()> { +async fn turn_start_ignores_personality_change_mid_thread_v2() -> Result<()> { skip_if_no_network!(Ok(())); let server = responses::start_mock_server().await; @@ -2722,6 +2745,7 @@ async fn turn_start_change_personality_mid_thread_v2() -> Result<()> { assert_eq!(requests.len(), 2, "expected two requests"); let first_developer_texts = requests[0].message_input_texts("developer"); + assert_fallback_model_instructions(&requests[0]); assert!( first_developer_texts .iter() @@ -2730,11 +2754,12 @@ async fn turn_start_change_personality_mid_thread_v2() -> Result<()> { ); let second_developer_texts = requests[1].message_input_texts("developer"); + assert_fallback_model_instructions(&requests[1]); assert!( second_developer_texts .iter() - .any(|text| text.contains("")), - "expected personality update message in second request, got {second_developer_texts:?}" + .all(|text| !text.contains("")), + "deprecated personality change emitted a developer update: {second_developer_texts:?}" ); Ok(()) diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index cb203776ed..18c153b32f 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -222,10 +222,11 @@ mod role_overrides { { next_config.include_skill_instructions = false; } - let personality_changed = config.personality != next_config.personality - || config.features.enabled(Feature::Personality) - != next_config.features.enabled(Feature::Personality); - if personality_changed + let strips_baked_personality = |config: &Config| { + config.features.enabled(Feature::Personality) + && config.personality == Some(Personality::None) + }; + if strips_baked_personality(config) != strips_baked_personality(&next_config) && matches!( config.base_instructions_provenance, Some(BaseInstructionsProvenance::Model { .. }) diff --git a/codex-rs/core/src/agent/role_tests.rs b/codex-rs/core/src/agent/role_tests.rs index 6abe3610d8..ba8a5de67a 100644 --- a/codex-rs/core/src/agent/role_tests.rs +++ b/codex-rs/core/src/agent/role_tests.rs @@ -232,26 +232,51 @@ async fn apply_role_preserves_unspecified_keys() { } #[tokio::test] -async fn apply_role_regenerates_model_instructions_when_personality_changes() { - for (role_contents, provenance) in [ +async fn apply_role_refreshes_model_instructions_only_when_personality_opt_out_changes() { + for (parent_personality, role_contents, provenance, should_refresh) in [ ( + "friendly", "personality = \"none\"", BaseInstructionsProvenance::Model { model: "parent-model".to_string(), }, + true, ), ( + "friendly", "[features]\npersonality = false", BaseInstructionsProvenance::Model { model: "parent-model".to_string(), }, + false, + ), + ( + "friendly", + "personality = \"pragmatic\"", + BaseInstructionsProvenance::Model { + model: "parent-model".to_string(), + }, + false, + ), + ( + "none", + "personality = \"friendly\"", + BaseInstructionsProvenance::Model { + model: "parent-model".to_string(), + }, + true, + ), + ( + "friendly", + "personality = \"none\"", + BaseInstructionsProvenance::Custom, + false, ), - ("personality = \"none\"", BaseInstructionsProvenance::Custom), ] { let (home, mut config) = test_config_with_cli_overrides(vec![ ( "personality".to_string(), - TomlValue::String("friendly".to_string()), + TomlValue::String(parent_personality.to_string()), ), ("features.personality".to_string(), TomlValue::Boolean(true)), ]) @@ -272,12 +297,10 @@ async fn apply_role_regenerates_model_instructions_when_personality_changes() { .await .expect("custom role should apply"); - let expected = match provenance { - BaseInstructionsProvenance::Model { .. } => (None, None), - BaseInstructionsProvenance::Custom => ( - Some("inherited instructions".to_string()), - Some(BaseInstructionsProvenance::Custom), - ), + let expected = if should_refresh { + (None, None) + } else { + (Some("inherited instructions".to_string()), Some(provenance)) }; assert_eq!( ( diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index 4fc7ff06bf..82ad54d530 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -8,7 +8,6 @@ use codex_config::types::ResumeCwdMode; use codex_config::types::SessionPickerViewMode; use codex_config::types::ToolSuggestDisabledTool; use codex_features::FEATURES; -use codex_protocol::config_types::Personality; use codex_protocol::config_types::ServiceTier; use codex_protocol::config_types::TrustLevel; use codex_protocol::openai_models::ReasoningEffort; @@ -39,8 +38,6 @@ pub enum ConfigEdit { }, /// Update the service tier preference for future turns. SetServiceTier { service_tier: Option }, - /// Update the active (or default) model personality. - SetModelPersonality { personality: Option }, /// Toggle the acknowledgement flag under `[notice]`. SetNoticeHideFullAccessWarning(bool), /// Toggle the Windows world-writable directories warning acknowledgement flag. @@ -245,10 +242,6 @@ impl ConfigDocument { value(config_value) }), )), - ConfigEdit::SetModelPersonality { personality } => Ok(self.write_optional_value( - &["personality"], - personality.map(|personality| value(personality.to_string())), - )), ConfigEdit::SetNoticeHideFullAccessWarning(acknowledged) => Ok(self.write_value( &[NOTICE_TABLE_KEY, "hide_full_access_warning"], value(*acknowledged), diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 1ae78cdae3..5ae1f366a0 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -3931,13 +3931,7 @@ impl Config { auto_review.policy.as_deref(), )) }); - let personality = personality - .or(cfg.personality) - .or_else(|| { - features - .enabled(Feature::Personality) - .then_some(Personality::Pragmatic) - }); + let personality = personality.or(cfg.personality); let experimental_compact_prompt_path = cfg.experimental_compact_prompt_file.as_ref(); let file_compact_prompt = Self::try_read_non_empty_file( diff --git a/codex-rs/core/src/context/mod.rs b/codex-rs/core/src/context/mod.rs index 4becfb308d..a4c42b3688 100644 --- a/codex-rs/core/src/context/mod.rs +++ b/codex-rs/core/src/context/mod.rs @@ -35,7 +35,6 @@ mod multi_agent_usage_hint; mod network_rule_saved; mod node_repl_review_evidence; mod permissions_instructions; -mod personality_spec_instructions; mod plugin_instructions; mod realtime_delegation; mod realtime_end_instructions; @@ -103,7 +102,6 @@ pub use node_repl_review_evidence::NodeReplReviewEvidenceMode; pub(crate) use node_repl_review_evidence::node_repl_review_evidence_mode; pub use permissions_instructions::ApprovalPromptContext; pub use permissions_instructions::PermissionsInstructions; -pub(crate) use personality_spec_instructions::PersonalitySpecInstructions; pub(crate) use plugin_instructions::PluginInstructions; pub(crate) use realtime_delegation::RealtimeDelegation; pub(crate) use realtime_delegation::RealtimeDelegationSource; diff --git a/codex-rs/core/src/context/personality_spec_instructions.rs b/codex-rs/core/src/context/personality_spec_instructions.rs deleted file mode 100644 index 856a4e0b54..0000000000 --- a/codex-rs/core/src/context/personality_spec_instructions.rs +++ /dev/null @@ -1,42 +0,0 @@ -use super::ContextualUserFragment; -use codex_protocol::models::ContentItemKind; - -#[derive(Debug, Clone, PartialEq)] -pub(crate) struct PersonalitySpecInstructions { - spec: String, -} - -impl PersonalitySpecInstructions { - pub(crate) fn new(spec: impl Into) -> Self { - Self { spec: spec.into() } - } -} - -impl ContextualUserFragment for PersonalitySpecInstructions { - fn content_kind(&self) -> ContentItemKind { - ContentItemKind("personality.spec_instructions".to_string()) - } - - fn role(&self) -> &'static str { - "developer" - } - - fn requires_separate_message(&self) -> bool { - true - } - - fn markers(&self) -> (&'static str, &'static str) { - Self::type_markers() - } - - fn type_markers() -> (&'static str, &'static str) { - ("", "") - } - - fn body(&self) -> String { - format!( - " The user has requested a new communication style. Future messages should adhere to the following personality: \n{} ", - self.spec - ) - } -} diff --git a/codex-rs/core/src/context/world_state/mod.rs b/codex-rs/core/src/context/world_state/mod.rs index 249e4c80f8..ef03b15503 100644 --- a/codex-rs/core/src/context/world_state/mod.rs +++ b/codex-rs/core/src/context/world_state/mod.rs @@ -11,7 +11,6 @@ mod multi_agent_mode; mod multi_agent_usage_hint; mod permissions; mod persistent_mode; -mod personality; mod plugins_instructions; mod realtime; #[cfg(test)] @@ -51,7 +50,6 @@ pub(crate) use multi_agent_mode::MultiAgentModeState; pub(crate) use multi_agent_usage_hint::MultiAgentUsageHintState; pub(crate) use permissions::PermissionsState; pub(crate) use persistent_mode::PersistentModeState; -pub(crate) use personality::PersonalityState; pub(crate) use plugins_instructions::PluginsInstructionsState; pub(crate) use realtime::RealtimeState; pub(crate) use tools::ToolsState; diff --git a/codex-rs/core/src/context/world_state/personality.rs b/codex-rs/core/src/context/world_state/personality.rs deleted file mode 100644 index b8672327de..0000000000 --- a/codex-rs/core/src/context/world_state/personality.rs +++ /dev/null @@ -1,101 +0,0 @@ -use super::PreviousSectionState; -use super::WorldStateSection; -use crate::context::ContextualUserFragment; -use crate::context::PersonalitySpecInstructions; -use codex_protocol::config_types::Personality; -use serde::Deserialize; -use serde::Serialize; - -/// Personality instructions currently visible to the model. -#[derive(Clone, Debug)] -pub(crate) struct PersonalityState { - snapshot: PersonalitySnapshot, - previous: Option, - instructions: Option, - personality_is_baked: bool, -} - -#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] -pub(crate) struct PersonalitySnapshot { - model: String, - #[serde(default, skip_serializing_if = "Option::is_none")] - personality: Option, -} - -impl PersonalityState { - pub(crate) fn new( - model: &str, - personality: Option, - previous_model: Option<&str>, - previous_personality: Option, - instructions: Option, - personality_is_baked: bool, - ) -> Self { - Self { - snapshot: PersonalitySnapshot { - model: model.to_string(), - personality, - }, - previous: previous_model.map(|model| PersonalitySnapshot { - model: model.to_string(), - personality: previous_personality, - }), - instructions, - personality_is_baked, - } - } - - fn render_change( - &self, - previous: &PersonalitySnapshot, - ) -> Option> { - (previous.model == self.snapshot.model && previous.personality != self.snapshot.personality) - .then_some(self.instructions.as_ref()) - .flatten() - .map(|instructions| { - Box::new(PersonalitySpecInstructions::new(instructions.clone())) - as Box - }) - } -} - -impl WorldStateSection for PersonalityState { - const ID: &'static str = "personality"; - type Snapshot = PersonalitySnapshot; - - fn snapshot(&self) -> Self::Snapshot { - self.snapshot.clone() - } - - fn matches_legacy_fragment(role: &str, text: &str) -> bool { - role == "developer" && PersonalitySpecInstructions::matches_text(text) - } - - fn render_diff( - &self, - previous: PreviousSectionState<'_, Self::Snapshot>, - ) -> Option> { - match previous { - PreviousSectionState::Known(previous) => self.render_change(previous), - PreviousSectionState::Unknown => self - .previous - .as_ref() - .and_then(|previous| self.render_change(previous)), - PreviousSectionState::Absent => (!self.personality_is_baked - && self - .previous - .as_ref() - .is_none_or(|previous| previous.model == self.snapshot.model)) - .then_some(self.instructions.as_ref()) - .flatten() - .map(|instructions| { - Box::new(PersonalitySpecInstructions::new(instructions.clone())) - as Box - }), - } - } -} - -#[cfg(test)] -#[path = "personality_tests.rs"] -mod tests; diff --git a/codex-rs/core/src/context/world_state/personality_tests.rs b/codex-rs/core/src/context/world_state/personality_tests.rs deleted file mode 100644 index 57970497c1..0000000000 --- a/codex-rs/core/src/context/world_state/personality_tests.rs +++ /dev/null @@ -1,106 +0,0 @@ -use super::*; -use crate::context::world_state::WorldState; -use pretty_assertions::assert_eq; - -fn state( - model: &str, - personality: Option, - previous: Option<(&str, Option)>, - personality_is_baked: bool, -) -> PersonalityState { - PersonalityState::new( - model, - personality, - previous.map(|(model, _)| model), - previous.and_then(|(_, personality)| personality), - personality.map(|personality| format!("instructions for {personality:?}")), - personality_is_baked, - ) -} - -#[test] -fn initial_personality_renders_only_when_missing_from_base_instructions() { - let separate = state( - "gpt-test", - Some(Personality::Friendly), - /*previous*/ None, - /*personality_is_baked*/ false, - ); - let baked = state( - "gpt-test", - Some(Personality::Friendly), - /*previous*/ None, - /*personality_is_baked*/ true, - ); - - assert_eq!( - separate - .render_diff(PreviousSectionState::Absent) - .expect("separate personality should render") - .markers(), - PersonalitySpecInstructions::type_markers() - ); - assert!(baked.render_diff(PreviousSectionState::Absent).is_none()); -} - -#[test] -fn personality_changes_render_without_repeating_model_changes() { - let previous = PersonalitySnapshot { - model: "gpt-test".to_string(), - personality: Some(Personality::Friendly), - }; - let changed = state( - "gpt-test", - Some(Personality::Pragmatic), - Some(("gpt-test", Some(Personality::Friendly))), - /*personality_is_baked*/ true, - ); - let model_changed = state( - "gpt-next", - Some(Personality::Pragmatic), - Some(("gpt-test", Some(Personality::Friendly))), - /*personality_is_baked*/ true, - ); - - assert_eq!( - changed - .render_diff(PreviousSectionState::Known(&previous)) - .expect("changed personality should render") - .markers(), - PersonalitySpecInstructions::type_markers() - ); - assert!( - model_changed - .render_diff(PreviousSectionState::Known(&previous)) - .is_none() - ); - assert!( - model_changed - .render_diff(PreviousSectionState::Unknown) - .is_none() - ); - assert!( - model_changed - .render_diff(PreviousSectionState::Absent) - .is_none() - ); -} - -#[test] -fn persisted_personality_does_not_require_a_retained_update() { - let state = state( - "gpt-test", - Some(Personality::Friendly), - Some(("gpt-test", Some(Personality::Friendly))), - /*personality_is_baked*/ false, - ); - let mut world_state = WorldState::default(); - world_state.add_section(state); - let snapshot = world_state.snapshot(); - - assert!( - world_state - .render_history_diff(Some(&snapshot), &[]) - .is_empty() - ); -} diff --git a/codex-rs/core/src/session/step_settings_tests.rs b/codex-rs/core/src/session/step_settings_tests.rs index ba04038947..72584a146e 100644 --- a/codex-rs/core/src/session/step_settings_tests.rs +++ b/codex-rs/core/src/session/step_settings_tests.rs @@ -16,7 +16,6 @@ use codex_protocol::models::BaseInstructionsProvenance; use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::AutoReviewMessages; use codex_protocol::openai_models::GuardianV2ModelConfig; -use codex_protocol::openai_models::ModelInstructionsVariables; use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::TruncationPolicyConfig; use codex_protocol::permissions::FileSystemAccessMode; @@ -407,7 +406,7 @@ fn collaboration_replacement_wins_and_effort_clear_remains_sparse() { async fn model_resolution_preserves_startup_overrides_and_instruction_provenance( provenance: BaseInstructionsProvenance, ) { - let configured_instructions = "explicit {{ personality }}"; + let configured_instructions = "explicit instructions"; let auto_review = AutoReviewMessages { policy: Some("catalog review policy".to_string()), policy_template: Some("catalog review template".to_string()), @@ -430,12 +429,8 @@ async fn model_resolution_preserves_startup_overrides_and_instruction_provenance .as_mut() .expect("test model should have instruction metadata"); messages.instructions_template = - Some("Catalog B.\n# Personality\n{{ personality }}\n# Rules\nKeep the rules.".to_string()); - messages.instructions_variables = Some(ModelInstructionsVariables { - personality_default: Some("default".to_string()), - personality_friendly: Some("friendly".to_string()), - personality_pragmatic: Some("pragmatic".to_string()), - }); + Some("Catalog B.\n# Personality\nfixed\n# Rules\nKeep the rules.".to_string()); + messages.instructions_variables = None; messages.auto_review = Some(auto_review); messages.guardian_v2 = Some(guardian_v2); let catalog = ModelsResponse { @@ -460,7 +455,7 @@ async fn model_resolution_preserves_startup_overrides_and_instruction_provenance ( Personality::Friendly, true, - "Catalog B.\n# Personality\nfriendly\n# Rules\nKeep the rules.", + "Catalog B.\n# Personality\nfixed\n# Rules\nKeep the rules.", ), ( Personality::None, @@ -470,7 +465,7 @@ async fn model_resolution_preserves_startup_overrides_and_instruction_provenance ( Personality::Friendly, false, - "Catalog B.\n# Personality\ndefault\n# Rules\nKeep the rules.", + "Catalog B.\n# Personality\nfixed\n# Rules\nKeep the rules.", ), ] { config.personality = Some(personality); diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index b3303d3fc3..1178d2a0d1 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -76,7 +76,6 @@ use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ImageDetail; use codex_protocol::models::PermissionProfile; use codex_protocol::models::SandboxEnforcement; -use codex_protocol::openai_models::ModelInstructionsVariables; use codex_protocol::openai_models::ModelServiceTier; use codex_protocol::openai_models::ToolMode; use codex_protocol::permissions::FileSystemAccessMode; @@ -10130,9 +10129,7 @@ async fn build_initial_context_uses_retained_step_after_model_change() { CodexAuth::from_api_key("Test API Key"), Vec::new(), |config| { - config.features.enable(Feature::Personality).unwrap(); config.features.enable(Feature::TokenBudget).unwrap(); - config.personality = Some(Personality::Friendly); }, ) .await; @@ -10153,12 +10150,8 @@ async fn build_initial_context_uses_retained_step_after_model_change() { model_info.context_window = None; model_info.max_context_window = None; let messages = model_info.model_messages.as_mut().unwrap(); - messages.instructions_template = Some("A instructions: {{ personality }}".to_string()); - messages.instructions_variables = Some(ModelInstructionsVariables { - personality_default: Some("default".to_string()), - personality_friendly: Some("friendly".to_string()), - personality_pragmatic: Some("pragmatic".to_string()), - }); + messages.instructions_template = Some("A instructions".to_string()); + messages.instructions_variables = None; }); session .set_previous_turn_settings(Some(PreviousTurnSettings { @@ -10181,7 +10174,6 @@ async fn build_initial_context_uses_retained_step_after_model_change() { let mut selected_b = step_a.settings.selected().clone(); selected_b.collaboration_mode.settings.model = "model-b".to_string(); - selected_b.personality = Some(Personality::Pragmatic); let mut model_b = step_a.settings.model_info.as_ref().clone(); model_b.slug = "model-b".to_string(); model_b.context_window = Some(128_000); @@ -10190,7 +10182,7 @@ async fn build_initial_context_uses_retained_step_after_model_change() { .model_messages .as_mut() .unwrap() - .instructions_template = Some("B instructions: {{ personality }}".to_string()); + .instructions_template = Some("B instructions".to_string()); turn_context .current_settings .store(Arc::new(ResolvedStepSettings::new( @@ -10218,9 +10210,9 @@ async fn build_initial_context_uses_retained_step_after_model_change() { .collect::>(); let a_text = developer_input_texts(&initial_a).join("\n"); let b_text = developer_input_texts(&initial_b).join("\n"); - assert!(a_text.contains("A instructions: friendly")); + assert!(a_text.contains("A instructions")); assert!(!a_text.contains("")); - assert!(b_text.contains("B instructions: pragmatic")); + assert!(b_text.contains("B instructions")); assert!(!b_text.contains("A instructions:")); assert!(!a_text.contains("turn context extension enabled")); assert!(b_text.contains("turn context extension enabled")); @@ -11944,36 +11936,9 @@ async fn sample_rollout( let mut rollout_items = Vec::new(); let mut live_history = ContextManager::new(); - // Use the same turn_context source as record_initial_history so model_info (and thus - // personality_spec) matches reconstruction. + // Use the same turn_context source as record_initial_history so model_info matches reconstruction. let reconstruction_turn = session.new_default_turn().await; - let mut initial_context = build_initial_context(session, &reconstruction_turn).await; - // Ensure personality_spec is present when Personality is enabled, so expected matches - // what reconstruction produces (build_initial_context may omit it when baked into model). - if !initial_context.iter().any(|m| { - matches!(m, ResponseItem::Message { role, content, .. } - if role == "developer" - && content.iter().any(|c| { - matches!(c, ContentItem::InputText { text } if text.contains("")) - })) - }) && let Some(p) = reconstruction_turn.personality() - && session.features.enabled(Feature::Personality) - && let Some(personality_message) = reconstruction_turn - .model_info() - .model_messages - .as_ref() - .and_then(|m| m.get_personality_message(Some(p)).filter(|s| !s.is_empty())) - { - let msg = crate::context::ContextualUserFragment::into( - crate::context::PersonalitySpecInstructions::new(personality_message), - ); - let insert_at = initial_context - .iter() - .position(|m| matches!(m, ResponseItem::Message { role, .. } if role == "developer")) - .map(|i| i + 1) - .unwrap_or(0); - initial_context.insert(insert_at, msg); - } + let initial_context = build_initial_context(session, &reconstruction_turn).await; for item in &initial_context { rollout_items.push(RolloutItem::ResponseItem(item.clone().into())); } diff --git a/codex-rs/core/src/session/world_state.rs b/codex-rs/core/src/session/world_state.rs index 1c952c51ab..74b1defa86 100644 --- a/codex-rs/core/src/session/world_state.rs +++ b/codex-rs/core/src/session/world_state.rs @@ -18,7 +18,6 @@ use crate::context::world_state::MultiAgentModeState; use crate::context::world_state::MultiAgentUsageHintState; use crate::context::world_state::PermissionsState; use crate::context::world_state::PersistentModeState; -use crate::context::world_state::PersonalityState; use crate::context::world_state::PluginsInstructionsState; use crate::context::world_state::RealtimeState; use crate::context::world_state::ToolsState; @@ -57,29 +56,23 @@ impl Session { model_instructions }; let base_instructions = self.get_prompt_base_instructions().await.text; - let (previous_model, previous_context, base_instructions) = { + let previous_model = { let state = self.state.lock().await; - ( - state - .previous_turn_settings() - .map(|previous| previous.model) - .or_else(|| { - state - .base_instructions_provenance - .as_ref() - .and_then(|provenance| match provenance { - BaseInstructionsProvenance::Model { model } => Some(model), - BaseInstructionsProvenance::Custom => None, - }) - .filter(|_| base_instructions != model_instructions) - .cloned() - }), - state.reference_context_item(), - base_instructions, - ) + state + .previous_turn_settings() + .map(|previous| previous.model) + .or_else(|| { + state + .base_instructions_provenance + .as_ref() + .and_then(|provenance| match provenance { + BaseInstructionsProvenance::Model { model } => Some(model), + BaseInstructionsProvenance::Custom => None, + }) + .filter(|_| base_instructions != model_instructions) + .cloned() + }) }; - let personality_is_baked = - model_info.supports_personality() && base_instructions == model_instructions; let environment_subagents = if turn_context.config.include_environment_context { self.services .agent_control @@ -97,28 +90,6 @@ impl Session { previous_model.as_deref(), model_instructions, )); - if self.features.enabled(Feature::Personality) { - let personality_instructions = personality.and_then(|personality| { - model_info - .model_messages - .as_ref() - .and_then(|messages| messages.get_personality_message(Some(personality))) - .filter(|message| !message.is_empty()) - }); - world_state.add_section(PersonalityState::new( - &model_info.slug, - personality, - previous_context - .as_ref() - .map(|previous| previous.model.as_str()) - .or(previous_model.as_deref()), - previous_context - .as_ref() - .and_then(|previous| previous.personality), - personality_instructions, - personality_is_baked, - )); - } let token_budget_enabled = turn_context.config.features.enabled(Feature::TokenBudget) && step_context .settings diff --git a/codex-rs/core/tests/suite/model_switching.rs b/codex-rs/core/tests/suite/model_switching.rs index 6e0dc6df59..70e804ee1e 100644 --- a/codex-rs/core/tests/suite/model_switching.rs +++ b/codex-rs/core/tests/suite/model_switching.rs @@ -219,7 +219,7 @@ async fn first_turn_model_change_appends_model_instructions_developer_message( developer_texts .iter() .all(|text| !text.contains("")), - "model instructions already include the selected personality" + "model switch should not emit a personality update" ); Ok(()) @@ -389,7 +389,8 @@ async fn model_change_appends_model_instructions_developer_message() -> Result<( } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn model_and_personality_change_only_appends_model_instructions() -> Result<()> { +async fn model_change_with_legacy_personality_override_only_appends_model_instructions() +-> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; diff --git a/codex-rs/core/tests/suite/personality.rs b/codex-rs/core/tests/suite/personality.rs index cad0306d89..62bf008f73 100644 --- a/codex-rs/core/tests/suite/personality.rs +++ b/codex-rs/core/tests/suite/personality.rs @@ -1,23 +1,15 @@ use codex_config::types::Personality; use codex_core::TurnInputRequest; use codex_features::Feature; -use codex_models_manager::bundled_models_response; -use codex_models_manager::manager::RefreshStrategy; -use codex_models_manager::manager::SharedModelsManager; -use codex_models_manager::model_info::BASE_INSTRUCTIONS; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; use codex_protocol::models::BaseInstructionsProvenance; use codex_protocol::models::PermissionProfile; -use codex_protocol::openai_models::ModelInstructionsVariables; -use codex_protocol::openai_models::ModelVisibility; -use codex_protocol::openai_models::ModelsResponse; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ThreadSettingsOverrides; use codex_protocol::user_input::UserInput; -use core_test_support::responses::mount_models_once; use core_test_support::responses::mount_sse_once; use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse_completed; @@ -30,16 +22,11 @@ use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; use pretty_assertions::assert_eq; use test_case::test_case; -use tokio::time::Duration; -use tokio::time::Instant; -use tokio::time::sleep; -use wiremock::MockServer; const LOCAL_FRIENDLY_TEMPLATE: &str = "You optimize for team morale and being a supportive teammate as much as code quality."; const BUNDLED_FRIENDLY_TEMPLATE: &str = "You have a vivid inner life as Codex:"; const CUSTOM_INSTRUCTIONS: &str = "Custom instructions\n# Personality\nThis must remain\n## Writing Style\nThis must also remain\n# General\nGeneral instructions"; -const LOCAL_PRAGMATIC_TEMPLATE: &str = "You are a deeply pragmatic, effective software engineer."; fn read_only_text_turn( test: &TestCodex, @@ -102,10 +89,7 @@ async fn config_personality_none_sends_no_personality() -> anyhow::Result<()> { !instructions_text.contains(BUNDLED_FRIENDLY_TEMPLATE), "expected no friendly personality template, got: {instructions_text:?}" ); - assert!( - !instructions_text.contains(LOCAL_PRAGMATIC_TEMPLATE), - "expected no pragmatic personality template, got: {instructions_text:?}" - ); + assert!(!instructions_text.contains("# Personality")); assert!( !instructions_text.contains("{{ personality }}"), "expected personality placeholder to be removed, got: {instructions_text:?}" @@ -221,6 +205,7 @@ async fn default_instructions_are_friendly_without_config_toml() -> anyhow::Resu .expect("test config should allow feature update"); }); let test = builder.build(&server).await?; + assert_eq!(test.config.personality, None); test.codex .start_or_steer_turn(read_only_text_turn( @@ -244,278 +229,6 @@ async fn default_instructions_are_friendly_without_config_toml() -> anyhow::Resu Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn user_turn_personality_same_value_does_not_add_update_message() -> anyhow::Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let resp_mock = mount_sse_sequence( - &server, - vec![sse_completed("resp-1"), sse_completed("resp-2")], - ) - .await; - let mut builder = test_codex() - .with_model("exp-codex-personality") - .with_config(|config| { - config - .features - .enable(Feature::Personality) - .expect("test config should allow feature update"); - config.personality = Some(Personality::Pragmatic); - }); - let test = builder.build(&server).await?; - - test.codex - .start_or_steer_turn(read_only_text_turn( - &test, - "hello", - test.session_configured.model.clone(), - test.config.permissions.approval_policy.value(), - )) - .await?; - - wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - - core_test_support::submit_thread_settings( - &test.codex, - ThreadSettingsOverrides { - personality: Some(Personality::Pragmatic), - ..Default::default() - }, - ) - .await?; - - test.codex - .start_or_steer_turn(read_only_text_turn( - &test, - "hello", - test.session_configured.model.clone(), - test.config.permissions.approval_policy.value(), - )) - .await?; - - wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - - let requests = resp_mock.requests(); - assert_eq!(requests.len(), 2, "expected two requests"); - let request = requests - .last() - .expect("expected second request after personality override"); - - let developer_texts = request.message_input_texts("developer"); - let personality_text = developer_texts - .iter() - .find(|text| text.contains("")); - assert!( - personality_text.is_none(), - "expected no personality preamble for unchanged personality, got {personality_text:?}" - ); - - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn disabled_personality_sends_remote_default_instructions() -> anyhow::Result<()> { - skip_if_no_network!(Ok(())); - - let server = MockServer::start().await; - let remote_slug = "remote-disabled-personality"; - let default_personality = "default remote personality"; - let friendly_personality = "friendly remote personality"; - let mut remote_model = bundled_models_response()? - .models - .into_iter() - .find(|model| model.slug == "gpt-5.5") - .expect("bundled gpt-5.5 model"); - remote_model.slug = remote_slug.to_string(); - if let Some(model_messages) = remote_model.model_messages.as_mut() { - model_messages.instructions_template = Some("remote base\n{{ personality }}".to_string()); - model_messages.instructions_variables = Some(ModelInstructionsVariables { - personality_default: Some(default_personality.to_string()), - personality_friendly: Some(friendly_personality.to_string()), - personality_pragmatic: Some("pragmatic remote personality".to_string()), - }); - } - mount_models_once( - &server, - ModelsResponse { - models: vec![remote_model], - }, - ) - .await; - let response = mount_sse_once(&server, sse_completed("resp-1")).await; - let mut builder = test_codex() - .with_auth(codex_login::CodexAuth::create_dummy_chatgpt_auth_for_testing()) - .with_config(move |config| { - config - .features - .disable(Feature::Personality) - .expect("test config should allow feature update"); - config.model = Some(remote_slug.to_string()); - config.personality = Some(Personality::Friendly); - }); - let test = builder.build_with_auto_env(&server).await?; - - test.submit_turn("use the default personality").await?; - - assert_eq!( - response.single_request().instructions_text(), - format!("remote base\n{default_personality}") - ); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn user_turn_personality_skips_if_feature_disabled() -> anyhow::Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let resp_mock = mount_sse_sequence( - &server, - vec![sse_completed("resp-1"), sse_completed("resp-2")], - ) - .await; - let mut builder = test_codex() - .with_model("exp-codex-personality") - .with_config(|config| { - config.update_plan_enabled = true; - config - .features - .disable(Feature::Personality) - .expect("test config should allow feature update"); - }); - let test = builder.build(&server).await?; - - test.codex - .start_or_steer_turn(read_only_text_turn( - &test, - "hello", - test.session_configured.model.clone(), - test.config.permissions.approval_policy.value(), - )) - .await?; - - wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - - core_test_support::submit_thread_settings( - &test.codex, - ThreadSettingsOverrides { - personality: Some(Personality::Pragmatic), - ..Default::default() - }, - ) - .await?; - - test.codex - .start_or_steer_turn(read_only_text_turn( - &test, - "hello", - test.session_configured.model.clone(), - test.config.permissions.approval_policy.value(), - )) - .await?; - - wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - - let requests = resp_mock.requests(); - assert_eq!(requests.len(), 2, "expected two requests"); - assert!( - requests - .iter() - .all(|request| request.instructions_text() == BASE_INSTRUCTIONS) - ); - let request = requests - .last() - .expect("expected personality update request"); - - let developer_texts = request.message_input_texts("developer"); - let personality_text = developer_texts - .iter() - .find(|text| text.contains("")); - assert!( - personality_text.is_none(), - "expected no personality preamble, got {personality_text:?}" - ); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn legacy_remote_catalog_renders_selection_and_updates() -> anyhow::Result<()> { - skip_if_no_network!(Ok(())); - - let server = MockServer::start().await; - let remote_slug = "codex-remote-personality"; - let friendly = "Friendly from remote template"; - let pragmatic = "Pragmatic from remote template"; - let mut remote_model = bundled_models_response()? - .models - .into_iter() - .find(|model| model.slug == "gpt-5.5") - .expect("bundled gpt-5.5 model"); - remote_model.slug = remote_slug.to_string(); - remote_model.visibility = ModelVisibility::List; - let messages = remote_model - .model_messages - .as_mut() - .expect("model messages"); - messages.instructions_template = Some("Base instructions\n{{ personality }}\n".to_string()); - messages.instructions_variables = Some(ModelInstructionsVariables { - personality_default: Some("Default from remote template".to_string()), - personality_friendly: Some(friendly.to_string()), - personality_pragmatic: Some(pragmatic.to_string()), - }); - mount_models_once( - &server, - ModelsResponse { - models: vec![remote_model], - }, - ) - .await; - let responses = mount_sse_sequence( - &server, - vec![sse_completed("resp-1"), sse_completed("resp-2")], - ) - .await; - let mut builder = test_codex() - .with_auth(codex_login::CodexAuth::create_dummy_chatgpt_auth_for_testing()) - .with_model(remote_slug) - .with_config(|config| { - config - .features - .enable(Feature::Personality) - .expect("test config should allow feature update"); - config.personality = Some(Personality::Friendly); - }); - let test = builder.build_with_auto_env(&server).await?; - wait_for_model_available(&test.thread_manager.get_models_manager(), remote_slug).await; - test.submit_turn("first turn").await?; - - core_test_support::submit_thread_settings( - &test.codex, - ThreadSettingsOverrides { - personality: Some(Personality::Pragmatic), - ..Default::default() - }, - ) - .await?; - test.submit_turn("use the pragmatic selection").await?; - - let requests = responses.requests(); - assert_eq!(requests.len(), 2); - assert_eq!( - requests[0].instructions_text(), - format!("Base instructions\n{friendly}\n") - ); - let developer_texts = requests[1].message_input_texts("developer"); - let personality_text = developer_texts - .iter() - .find(|text| text.contains("")) - .expect("expected personality update message"); - assert!(personality_text.contains("The user has requested a new communication style.")); - assert!(personality_text.contains(pragmatic)); - Ok(()) -} - #[test_case("gpt-5.4", LOCAL_FRIENDLY_TEMPLATE; "gpt_5_4")] #[test_case("gpt-5.5", BUNDLED_FRIENDLY_TEMPLATE; "gpt_5_5")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -611,22 +324,3 @@ async fn legacy_personality_session_resumes_and_completes() -> anyhow::Result<() ); Ok(()) } - -async fn wait_for_model_available(manager: &SharedModelsManager, slug: &str) { - let deadline = Instant::now() + Duration::from_secs(2); - loop { - let models = manager - .list_models( - RefreshStrategy::OnlineIfUncached, - codex_core::test_support::default_http_client_factory(), - ) - .await; - if models.iter().any(|model| model.model == slug) { - return; - } - if Instant::now() >= deadline { - panic!("timed out waiting for the remote model {slug} to appear"); - } - sleep(Duration::from_millis(25)).await; - } -} diff --git a/codex-rs/core/tests/suite/step_settings.rs b/codex-rs/core/tests/suite/step_settings.rs index 33ec639d4e..055e1f1780 100644 --- a/codex-rs/core/tests/suite/step_settings.rs +++ b/codex-rs/core/tests/suite/step_settings.rs @@ -23,7 +23,6 @@ use codex_login::CodexAuth; use codex_models_manager::bundled_models_response; use codex_models_manager::manager::RefreshStrategy; use codex_protocol::config_types::ApprovalsReviewer; -use codex_protocol::config_types::Personality; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::ServiceTier; use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem; @@ -40,7 +39,6 @@ use codex_protocol::openai_models::ConfigShellToolType; use codex_protocol::openai_models::ConfirmationPolicies; use codex_protocol::openai_models::InputModality; use codex_protocol::openai_models::ModelInfo; -use codex_protocol::openai_models::ModelInstructionsVariables; use codex_protocol::openai_models::ModelTokenBudgetConfig; use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::MultiAgentMessages; @@ -1048,15 +1046,10 @@ async fn active_model_switch_updates_core_context_from_captured_settings( .features .enable(Feature::TokenBudget) .expect("enable token-budget feature"); - config - .features - .enable(Feature::Personality) - .expect("enable personality"); config .features .enable(Feature::MultiAgentV2) .expect("enable multi-agent V2"); - config.personality = Some(Personality::Pragmatic); if context_window_model.is_some() { config.model_context_window = None; } @@ -1073,14 +1066,8 @@ async fn active_model_switch_updates_core_context_from_captured_settings( model.max_context_window = None; } let messages = model.model_messages.as_mut().expect("model messages"); - messages.instructions_template = Some(format!( - "Instructions for {slug}. {{{{ personality }}}}" - )); - messages.instructions_variables = Some(ModelInstructionsVariables { - personality_default: Some(format!("Default {slug} personality.")), - personality_friendly: Some(format!("Friendly {slug} personality.")), - personality_pragmatic: Some(format!("Pragmatic {slug} personality.")), - }); + messages.instructions_template = Some(format!("Instructions for {slug}.")); + messages.instructions_variables = None; messages.collaboration_modes = Some(CollaborationModeMessages { default: Some(format!("Default collaboration for {slug}.")), plan: None, @@ -1164,8 +1151,7 @@ async fn active_model_switch_updates_core_context_from_captured_settings( .collect::>(), vec![json!(MODEL_A), json!(MODEL_B), json!(MODEL_B)] ); - let initial_instructions = - format!("Instructions for {MODEL_A}. Pragmatic {MODEL_A} personality."); + let initial_instructions = format!("Instructions for {MODEL_A}."); assert_eq!(requests[0].instructions_text(), initial_instructions); assert!(!requests[0].body_contains_text("")); for text in [ @@ -1193,12 +1179,10 @@ async fn active_model_switch_updates_core_context_from_captured_settings( .filter(|text| text.contains("")) .collect::>(); assert_eq!(switches.len(), 1); - assert!(switches[0].contains(&format!( - "Instructions for {MODEL_B}. Pragmatic {MODEL_B} personality." - ))); + assert!(switches[0].contains(&format!("Instructions for {MODEL_B}."))); assert!( !request.body_contains_text(""), - "personality is included in the model-switch instructions" + "model-switch instructions should not contain a personality update" ); for text in [ format!("Default collaboration for {MODEL_B}."), diff --git a/codex-rs/models-manager/src/model_info.rs b/codex-rs/models-manager/src/model_info.rs index fe5e055d9b..81538bc6b7 100644 --- a/codex-rs/models-manager/src/model_info.rs +++ b/codex-rs/models-manager/src/model_info.rs @@ -2,7 +2,6 @@ use codex_protocol::config_types::Personality; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::openai_models::ConfigShellToolType; use codex_protocol::openai_models::ModelInfo; -use codex_protocol::openai_models::ModelInstructionsVariables; use codex_protocol::openai_models::ModelMessages; use codex_protocol::openai_models::ModelVisibility; use codex_protocol::openai_models::TruncationMode; @@ -15,11 +14,6 @@ use codex_utils_output_truncation::approx_bytes_for_tokens; use tracing::warn; pub const BASE_INSTRUCTIONS: &str = include_str!("../prompt.md"); -const DEFAULT_PERSONALITY_HEADER: &str = "You are Codex, a coding agent based on GPT-5. You and the user share the same workspace and collaborate to achieve the user's goals."; -const LOCAL_FRIENDLY_TEMPLATE: &str = - "You optimize for team morale and being a supportive teammate as much as code quality."; -const LOCAL_PRAGMATIC_TEMPLATE: &str = "You are a deeply pragmatic, effective software engineer."; -const PERSONALITY_PLACEHOLDER: &str = "{{ personality }}"; const PERSONALITY_SECTION_HEADER: &str = "# Personality"; pub fn with_config_overrides(mut model: ModelInfo, config: &ModelsManagerConfig) -> ModelInfo { @@ -66,38 +60,14 @@ pub fn with_config_overrides(mut model: ModelInfo, config: &ModelsManagerConfig) }); model_messages.instructions_template = Some(base_instructions.clone()); model_messages.instructions_variables = None; - } else { - if config.personality_enabled - && config.personality == Some(Personality::None) - && let Some(instructions_template) = model - .model_messages - .as_mut() - .and_then(|messages| messages.instructions_template.as_mut()) - { - *instructions_template = - strip_personality_section(std::mem::take(instructions_template)); - } - let uses_local_personality_template = model.used_fallback_model_metadata - && matches!( - model.slug.as_str(), - "gpt-5.2-codex" | "exp-codex-personality" - ); - if !config.personality_enabled - && let Some(model_messages) = model.model_messages.as_mut() - { - if uses_local_personality_template { - model_messages.instructions_template = Some(BASE_INSTRUCTIONS.to_string()); - } else { - let personality_default = model_messages - .get_personality_message(/*personality*/ None) - .unwrap_or_default(); - if let Some(instructions_template) = model_messages.instructions_template.as_mut() { - *instructions_template = instructions_template - .replace(PERSONALITY_PLACEHOLDER, &personality_default); - } - } - model_messages.instructions_variables = None; - } + } else if config.personality_enabled + && config.personality == Some(Personality::None) + && let Some(instructions_template) = model + .model_messages + .as_mut() + .and_then(|messages| messages.instructions_template.as_mut()) + { + *instructions_template = strip_personality_section(std::mem::take(instructions_template)); } model @@ -158,7 +128,7 @@ pub fn model_info_from_slug(slug: &str) -> ModelInfo { available_access_programs: None, availability_nux: None, upgrade: None, - model_messages: Some(local_model_messages_for_slug(slug)), + model_messages: Some(local_model_messages()), include_skills_usage_instructions: false, include_plugin_usage_instructions: false, include_apps_usage_instructions: false, @@ -192,42 +162,20 @@ pub fn model_info_from_slug(slug: &str) -> ModelInfo { } } -fn local_model_messages_for_slug(slug: &str) -> ModelMessages { - match slug { - "gpt-5.2-codex" | "exp-codex-personality" => ModelMessages { - persistent_instructions: None, - tools: None, - instructions_template: Some(format!( - "{DEFAULT_PERSONALITY_HEADER}\n\n{PERSONALITY_PLACEHOLDER}\n\n{BASE_INSTRUCTIONS}" - )), - instructions_variables: Some(ModelInstructionsVariables { - personality_default: Some(String::new()), - personality_friendly: Some(LOCAL_FRIENDLY_TEMPLATE.to_string()), - personality_pragmatic: Some(LOCAL_PRAGMATIC_TEMPLATE.to_string()), - }), - approvals: None, - collaboration_modes: None, - auto_review: None, - permissions: None, - multi_agent: None, - token_budget: None, - confirmation_policies: None, - guardian_v2: None, - }, - _ => ModelMessages { - persistent_instructions: None, - tools: None, - instructions_template: Some(BASE_INSTRUCTIONS.to_string()), - instructions_variables: None, - approvals: None, - collaboration_modes: None, - auto_review: None, - permissions: None, - multi_agent: None, - token_budget: None, - confirmation_policies: None, - guardian_v2: None, - }, +fn local_model_messages() -> ModelMessages { + ModelMessages { + persistent_instructions: None, + tools: None, + instructions_template: Some(BASE_INSTRUCTIONS.to_string()), + instructions_variables: None, + approvals: None, + collaboration_modes: None, + auto_review: None, + permissions: None, + multi_agent: None, + token_budget: None, + confirmation_policies: None, + guardian_v2: None, } } diff --git a/codex-rs/models-manager/src/model_info_tests.rs b/codex-rs/models-manager/src/model_info_tests.rs index e57a5ab70d..012b0fbf68 100644 --- a/codex-rs/models-manager/src/model_info_tests.rs +++ b/codex-rs/models-manager/src/model_info_tests.rs @@ -6,6 +6,7 @@ use codex_protocol::openai_models::AutoReviewMessages; use codex_protocol::openai_models::CollaborationModeMessages; use codex_protocol::openai_models::ConfirmationPolicies; use codex_protocol::openai_models::GuardianV2ModelConfig; +use codex_protocol::openai_models::ModelInstructionsVariables; use codex_protocol::openai_models::ModelTokenBudgetConfig; use codex_protocol::openai_models::MultiAgentMessages; use codex_protocol::openai_models::MultiAgentModeMessages; @@ -135,99 +136,6 @@ fn base_instruction_override_is_literal_and_preserves_catalog_messages() { ); } -#[test] -fn disabled_personality_bakes_default_and_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, - never: None, - unless_trusted: None, - }; - model.model_messages = Some(ModelMessages { - persistent_instructions: Some(String::new()), - tools: Some(ToolMessages { - send_user_message_async: Some(ToolMessage { - description: Some(String::new()), - }), - }), - instructions_template: Some("before {{ personality }} after".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()), - collaboration_modes: None, - auto_review: None, - permissions: None, - multi_agent: None, - token_budget: None, - confirmation_policies: None, - guardian_v2: None, - }); - let config = ModelsManagerConfig { - personality_enabled: false, - ..Default::default() - }; - - let updated = with_config_overrides(model, &config); - - assert_eq!( - updated.model_messages, - Some(ModelMessages { - persistent_instructions: Some(String::new()), - tools: Some(ToolMessages { - send_user_message_async: Some(ToolMessage { - description: Some(String::new()), - }), - }), - instructions_template: Some("before default after".to_string()), - instructions_variables: None, - approvals: Some(approvals), - collaboration_modes: None, - auto_review: None, - permissions: None, - multi_agent: None, - token_budget: None, - confirmation_policies: None, - guardian_v2: None, - }) - ); -} - -#[test] -fn disabled_personality_uses_plain_base_instructions_for_local_personality_models() { - let config = ModelsManagerConfig { - personality_enabled: false, - personality: Some(Personality::Friendly), - ..Default::default() - }; - - for slug in ["gpt-5.2-codex", "exp-codex-personality"] { - let updated = with_config_overrides(model_info_from_slug(slug), &config); - - assert_eq!( - updated.model_messages, - Some(ModelMessages { - persistent_instructions: None, - tools: None, - instructions_template: Some(BASE_INSTRUCTIONS.to_string()), - instructions_variables: None, - approvals: None, - collaboration_modes: None, - auto_review: None, - permissions: None, - multi_agent: None, - token_budget: None, - confirmation_policies: None, - guardian_v2: None, - }), - "unexpected model messages for {slug}" - ); - } -} - #[test] fn personality_none_strips_catalog_instruction_sources_through_the_next_h1() { let cases = [ diff --git a/codex-rs/protocol/src/openai_models.rs b/codex-rs/protocol/src/openai_models.rs index e54d5df755..79f268ce3b 100644 --- a/codex-rs/protocol/src/openai_models.rs +++ b/codex-rs/protocol/src/openai_models.rs @@ -49,7 +49,6 @@ pub use access_programs::ModelAccessPrograms; pub use guardian_v2::GuardianV2ModelConfig; pub use guardian_v2::GuardianV2TranscriptModelConfig; -const PERSONALITY_PLACEHOLDER: &str = "{{ personality }}"; /// Backend model-catalog specialty identifying cybersecurity-focused models. pub const MODEL_SPECIALTY_CYBER: &str = "cyber"; pub const SPEED_TIER_FAST: &str = "fast"; @@ -248,7 +247,7 @@ pub struct ModelPreset { pub default_reasoning_effort: ReasoningEffort, /// Supported reasoning effort options. pub supported_reasoning_efforts: Vec, - /// Whether this model supports personality-specific instructions. + /// Deprecated catalog field, always false for new model presets. #[serde(default)] pub supports_personality: bool, /// Deprecated: use `service_tiers` instead. @@ -532,23 +531,13 @@ impl ModelInfo { config_limit } - pub fn supports_personality(&self) -> bool { - self.model_messages - .as_ref() - .is_some_and(ModelMessages::supports_personality) - } - - pub fn get_model_instructions(&self, personality: Option) -> String { + /// Returns the literal instruction template. The personality argument remains for older + /// callers; the `None` opt-out is applied separately by models-manager. + pub fn get_model_instructions(&self, _personality: Option) -> String { if let Some(model_messages) = &self.model_messages && let Some(template) = &model_messages.instructions_template { - if model_messages.instructions_variables.is_none() { - return template.clone(); - } - let personality_message = model_messages - .get_personality_message(personality) - .unwrap_or_default(); - template.replace(PERSONALITY_PLACEHOLDER, personality_message.as_str()) + template.clone() } else { warn!( model = %self.slug, @@ -561,8 +550,8 @@ impl ModelInfo { /// A strongly-typed template for assembling model instructions and developer messages. /// -/// When `instructions_variables` is absent, `instructions_template` is treated as literal text. -/// When variables are present but incomplete, missing values render as empty strings. +/// `instructions_template` is literal text. The deprecated `instructions_variables` field is +/// retained to decode catalogs produced before personality selection was removed. #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)] pub struct ModelMessages { /// Additional developer instructions for persistent mode. Missing or null uses the built-in @@ -681,29 +670,6 @@ pub struct MultiAgentModeMessages { pub hint_text: Option, } -impl ModelMessages { - fn has_personality_placeholder(&self) -> bool { - self.instructions_template - .as_ref() - .map(|spec| spec.contains(PERSONALITY_PLACEHOLDER)) - .unwrap_or(false) - } - - fn supports_personality(&self) -> bool { - self.has_personality_placeholder() - && self - .instructions_variables - .as_ref() - .is_some_and(ModelInstructionsVariables::is_complete) - } - - pub fn get_personality_message(&self, personality: Option) -> Option { - self.instructions_variables - .as_ref() - .and_then(|variables| variables.get_personality_message(personality)) - } -} - #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)] pub struct ModelInstructionsVariables { pub personality_default: Option, @@ -711,26 +677,6 @@ pub struct ModelInstructionsVariables { pub personality_pragmatic: Option, } -impl ModelInstructionsVariables { - pub fn is_complete(&self) -> bool { - self.personality_default.is_some() - && self.personality_friendly.is_some() - && self.personality_pragmatic.is_some() - } - - pub fn get_personality_message(&self, personality: Option) -> Option { - if let Some(personality) = personality { - match personality { - Personality::None => Some(String::new()), - Personality::Friendly => self.personality_friendly.clone(), - Personality::Pragmatic => self.personality_pragmatic.clone(), - } - } else { - self.personality_default.clone() - } - } -} - #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)] pub struct ModelInfoUpgrade { pub model: String, @@ -875,7 +821,6 @@ where // convert ModelInfo to ModelPreset impl From for ModelPreset { fn from(info: ModelInfo) -> Self { - let supports_personality = info.supports_personality(); ModelPreset { id: info.slug.clone(), model: info.slug.clone(), @@ -886,7 +831,7 @@ impl From for ModelPreset { .default_reasoning_level .unwrap_or(ReasoningEffort::None), supported_reasoning_efforts: info.supported_reasoning_levels.clone(), - supports_personality, + supports_personality: false, additional_speed_tiers: info.additional_speed_tiers, service_tiers: info.service_tiers, default_service_tier: info.default_service_tier, @@ -1035,14 +980,6 @@ mod tests { multi_agent_reasoning_effort: None, } } - fn personality_variables() -> ModelInstructionsVariables { - ModelInstructionsVariables { - personality_default: Some("default".to_string()), - personality_friendly: Some("friendly".to_string()), - personality_pragmatic: Some("pragmatic".to_string()), - } - } - #[test] fn model_messages_deserialize_without_optional_sections() { let messages: ModelMessages = from_str( @@ -1352,37 +1289,15 @@ mod tests { } #[test] - fn get_model_instructions_uses_template_when_placeholder_present() { + fn get_model_instructions_ignores_legacy_personality_variables() { let model = test_model(Some(ModelMessages { persistent_instructions: None, tools: None, instructions_template: Some("Hello {{ personality }}".to_string()), - instructions_variables: Some(personality_variables()), - approvals: None, - collaboration_modes: None, - auto_review: None, - permissions: None, - multi_agent: None, - token_budget: None, - confirmation_policies: None, - guardian_v2: None, - })); - - let instructions = model.get_model_instructions(Some(Personality::Friendly)); - - assert_eq!(instructions, "Hello friendly"); - } - - #[test] - fn get_model_instructions_strips_placeholder_with_incomplete_variables() { - let model = test_model(Some(ModelMessages { - persistent_instructions: None, - tools: None, - instructions_template: Some("Hello\n{{ personality }}".to_string()), instructions_variables: Some(ModelInstructionsVariables { - personality_default: None, + personality_default: Some("default".to_string()), personality_friendly: Some("friendly".to_string()), - personality_pragmatic: None, + personality_pragmatic: Some("pragmatic".to_string()), }), approvals: None, collaboration_modes: None, @@ -1393,49 +1308,16 @@ mod tests { confirmation_policies: None, guardian_v2: None, })); - assert_eq!( - model.get_model_instructions(Some(Personality::Pragmatic)), - "Hello\n" - ); - assert_eq!( - model.get_model_instructions(/*personality*/ None), - "Hello\n" - ); - let model_no_personality = test_model(Some(ModelMessages { - persistent_instructions: None, - tools: None, - instructions_template: Some("Hello\n{{ personality }}".to_string()), - instructions_variables: Some(ModelInstructionsVariables { - personality_default: None, - personality_friendly: None, - personality_pragmatic: None, - }), - approvals: None, - collaboration_modes: None, - auto_review: None, - permissions: None, - multi_agent: None, - token_budget: None, - confirmation_policies: None, - guardian_v2: None, - })); - assert_eq!( - model_no_personality.get_model_instructions(Some(Personality::Friendly)), - "Hello\n" - ); - assert_eq!( - model_no_personality.get_model_instructions(Some(Personality::Pragmatic)), - "Hello\n" - ); - assert_eq!( - model_no_personality.get_model_instructions(Some(Personality::None)), - "Hello\n" - ); - assert_eq!( - model_no_personality.get_model_instructions(/*personality*/ None), - "Hello\n" - ); + let instructions = "Hello {{ personality }}"; + for personality in [ + Some(Personality::Friendly), + Some(Personality::Pragmatic), + Some(Personality::None), + None, + ] { + assert_eq!(model.get_model_instructions(personality), instructions); + } } #[test] @@ -1537,7 +1419,7 @@ mod tests { } #[test] - fn models_response_serializes_rendered_legacy_base_instructions() { + fn models_response_serializes_literal_legacy_base_instructions() { let response = ModelsResponse { models: vec![test_model(Some(ModelMessages { persistent_instructions: None, @@ -1563,7 +1445,7 @@ mod tests { assert_eq!( serialized["models"][0]["base_instructions"], - "before default after" + "before {{ personality }} after" ); } @@ -1670,80 +1552,6 @@ mod tests { assert_eq!(response.models[0].model_messages, Some(canonical_messages)); } - #[test] - fn get_personality_message_returns_default_when_personality_is_none() { - let personality_template = personality_variables(); - assert_eq!( - personality_template.get_personality_message(/*personality*/ None), - Some("default".to_string()) - ); - } - - #[test] - fn get_personality_message() { - let personality_variables = personality_variables(); - assert_eq!( - personality_variables.get_personality_message(Some(Personality::Friendly)), - Some("friendly".to_string()) - ); - assert_eq!( - personality_variables.get_personality_message(Some(Personality::Pragmatic)), - Some("pragmatic".to_string()) - ); - assert_eq!( - personality_variables.get_personality_message(Some(Personality::None)), - Some(String::new()) - ); - assert_eq!( - personality_variables.get_personality_message(/*personality*/ None), - Some("default".to_string()) - ); - - let personality_variables = ModelInstructionsVariables { - personality_default: Some("default".to_string()), - personality_friendly: None, - personality_pragmatic: None, - }; - assert_eq!( - personality_variables.get_personality_message(Some(Personality::Friendly)), - None - ); - assert_eq!( - personality_variables.get_personality_message(Some(Personality::Pragmatic)), - None - ); - assert_eq!( - personality_variables.get_personality_message(Some(Personality::None)), - Some(String::new()) - ); - assert_eq!( - personality_variables.get_personality_message(/*personality*/ None), - Some("default".to_string()) - ); - - let personality_variables = ModelInstructionsVariables { - personality_default: None, - personality_friendly: Some("friendly".to_string()), - personality_pragmatic: Some("pragmatic".to_string()), - }; - assert_eq!( - personality_variables.get_personality_message(Some(Personality::Friendly)), - Some("friendly".to_string()) - ); - assert_eq!( - personality_variables.get_personality_message(Some(Personality::Pragmatic)), - Some("pragmatic".to_string()) - ); - assert_eq!( - personality_variables.get_personality_message(Some(Personality::None)), - Some(String::new()) - ); - assert_eq!( - personality_variables.get_personality_message(/*personality*/ None), - None - ); - } - #[test] fn model_info_defaults_availability_nux_to_none_when_omitted() { let model: ModelInfo = serde_json::from_value(serde_json::json!({ diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 59bdbc5493..d939bdc1bf 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -8708,6 +8708,7 @@ async fn override_turn_context_sends_thread_settings_update() { let thread_id = started.session.thread_id; let initial_model = started.session.model.clone(); let initial_effort = started.session.reasoning_effort.clone(); + let initial_personality = started.session.personality; app.enqueue_primary_thread_session(started.session, started.turns) .await .expect("primary thread should be registered"); @@ -8781,8 +8782,8 @@ async fn override_turn_context_sends_thread_settings_update() { collaboration_mode.settings.reasoning_effort ); assert_eq!( - notification.thread_settings.personality, - Some(Personality::Pragmatic) + notification.thread_settings.personality, initial_personality, + "the Pragmatic turn override should not change personality" ); app.handle_app_server_event( @@ -8811,7 +8812,7 @@ async fn override_turn_context_sends_thread_settings_update() { updated_mode.settings.reasoning_effort, collaboration_mode.settings.reasoning_effort ); - assert_eq!(updated_session.personality, Some(Personality::Pragmatic)); + assert_eq!(updated_session.personality, initial_personality); assert_eq!(updated_session.service_tier, Some(service_tier)); assert_eq!(updated_session.approval_policy, AskForApproval::OnRequest); assert_eq!( diff --git a/codex-rs/tui/src/app/thread_settings.rs b/codex-rs/tui/src/app/thread_settings.rs index 94c8955468..aac4a06395 100644 --- a/codex-rs/tui/src/app/thread_settings.rs +++ b/codex-rs/tui/src/app/thread_settings.rs @@ -5,6 +5,7 @@ use crate::app_command::AppCommand; use crate::app_event::AppEvent; use crate::app_event::PermissionProfileSelection; use crate::app_server_session::AppServerSession; +use crate::app_server_session::personality_opt_out_only; use crate::chatwidget::cyber_model_approval_reviewer; use crate::session_state::ThreadSessionState; use codex_app_server_protocol::ApprovalsReviewer as AppServerApprovalsReviewer; @@ -211,8 +212,9 @@ impl App { pub(super) async fn send_thread_settings_update( &mut self, app_server: &mut AppServerSession, - params: ThreadSettingsUpdateParams, + mut params: ThreadSettingsUpdateParams, ) -> bool { + params.personality = personality_opt_out_only(params.personality); if !thread_settings_update_has_changes(¶ms) { return false; } diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 0e86ee157a..ac427a4bbb 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -132,6 +132,7 @@ use codex_config::ConfigLayerSource; use codex_otel::TelemetryAuthMode; use codex_protocol::ThreadId; use codex_protocol::approvals::GuardianAssessmentEvent; +use codex_protocol::config_types::Personality; use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE; use codex_protocol::models::ActivePermissionProfile; use codex_protocol::models::BaseInstructionsProvenance; @@ -1356,7 +1357,7 @@ impl AppServerSession { service_tier_for_turn: None, effort, summary, - personality, + personality: personality_opt_out_only(personality), output_schema, collaboration_mode, multi_agent_mode: None, @@ -1824,6 +1825,11 @@ fn approvals_reviewer_override_from_config( Some(config.approvals_reviewer.into()) } +// Keep the explicit Bridge opt-out while retiring Friendly/Pragmatic selection. +pub(crate) fn personality_opt_out_only(personality: Option) -> Option { + personality.filter(|personality| *personality == Personality::None) +} + fn config_request_overrides_from_config( config: &Config, ) -> Option> { @@ -1881,9 +1887,7 @@ fn config_request_overrides_from_config( ); insert( "personality", - config - .personality - .map(|personality| personality.to_string()), + personality_opt_out_only(config.personality).map(|personality| personality.to_string()), ); insert( "web_search", @@ -3390,7 +3394,6 @@ mod tests { ("model_reasoning_effort".to_string(), string("high")), ("model_reasoning_summary".to_string(), string("detailed")), ("model_verbosity".to_string(), string("low")), - ("personality".to_string(), string("pragmatic")), ("web_search".to_string(), string("disabled")), ("bypass_hook_trust".to_string(), true.into()), ]); @@ -3412,6 +3415,7 @@ mod tests { config.model_provider_id = "configured-provider".to_string(); config.model_reasoning_effort = Some(ReasoningEffort::Ultra); config.model_reasoning_summary = Some(ReasoningSummary::Detailed); + config.personality = Some(Personality::Pragmatic); let params = thread_resume_params_from_config( config, @@ -3430,10 +3434,6 @@ mod tests { "model_reasoning_summary".to_string(), serde_json::Value::String("detailed".to_string()), ), - ( - "personality".to_string(), - serde_json::Value::String("pragmatic".to_string()), - ), ( "web_search".to_string(), serde_json::Value::String("cached".to_string()), @@ -3701,7 +3701,7 @@ mod tests { } #[tokio::test] - async fn config_request_overrides_preserve_implicit_personality_default() { + async fn config_request_overrides_only_forward_explicit_personality_opt_out() { let temp_dir = tempfile::tempdir().expect("tempdir"); let mut config = build_config(&temp_dir).await; config.personality = None; @@ -3711,6 +3711,13 @@ mod tests { assert!(!implicit_overrides.contains_key("personality")); + for personality in [Personality::Friendly, Personality::Pragmatic] { + config.personality = Some(personality); + let ordinary_overrides = + config_request_overrides_from_config(&config).expect("config overrides"); + assert!(!ordinary_overrides.contains_key("personality")); + } + config.personality = Some(Personality::None); let explicit_overrides = config_request_overrides_from_config(&config).expect("config overrides"); @@ -3719,6 +3726,34 @@ mod tests { explicit_overrides.get("personality"), Some(&serde_json::Value::String("none".to_string())) ); + + for mode in [ThreadParamsMode::Embedded, ThreadParamsMode::Remote] { + let start = thread_start_params_from_config( + &config, mode, /*remote_cwd_override*/ None, + /*session_start_source*/ None, + ); + let resume = thread_resume_params_from_config( + config.clone(), + ThreadId::new(), + mode, + /*remote_cwd_override*/ None, + ResumeModelSettings::OverrideFromCurrentConfig, + ); + let fork = thread_fork_params_from_config( + config.clone(), + ThreadId::new(), + mode, + /*remote_cwd_override*/ None, + ); + for overrides in [start.config, resume.config, fork.config] { + assert_eq!( + overrides + .as_ref() + .and_then(|overrides| overrides.get("personality")), + Some(&serde_json::Value::String("none".to_string())) + ); + } + } } #[tokio::test]