From 7e91d7e3652be4813d08ea93442ec2841e03d76f Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Tue, 27 Jan 2026 14:11:13 -0800 Subject: [PATCH] Restore local personality fallback --- codex-rs/core/src/models_manager/manager.rs | 22 ++++++++-------- codex-rs/core/tests/suite/personality.rs | 28 +++++++++++++-------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index aeb4488bb8..09fa4043d3 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -105,10 +105,15 @@ impl ModelsManager { /// Determine whether a model supports personalities based on remote metadata. pub fn supports_personality(&self, model: &str, config: &Config) -> bool { - self.try_get_remote_models(config) + let remote = self + .try_get_remote_models(config) .ok() - .and_then(|remote_models| remote_models.into_iter().find(|info| info.slug == model)) - .is_some_and(|info| info.supports_personality()) + .and_then(|remote_models| remote_models.into_iter().find(|info| info.slug == model)); + if let Some(remote) = remote { + return remote.supports_personality(); + } + model_info::with_config_overrides(model_info::find_model_info_for_slug(model), config) + .supports_personality() } // todo(aibrahim): should be visible to core only and sent on session_configured event @@ -150,11 +155,7 @@ impl ModelsManager { .await .into_iter() .find(|m| m.slug == model); - let remote_supports = remote.as_ref().is_some_and(ModelInfo::supports_personality); - let mut model = remote.unwrap_or(local); - if !remote_supports { - model.model_instructions_template = None; - } + let model = remote.unwrap_or(local); model_info::with_config_overrides(model, config) } @@ -357,10 +358,7 @@ impl ModelsManager { #[cfg(any(test, feature = "test-support"))] /// Build `ModelInfo` without consulting remote state or cache. pub fn construct_model_info_offline(model: &str, config: &Config) -> ModelInfo { - let mut model_info = - model_info::with_config_overrides(model_info::find_model_info_for_slug(model), config); - model_info.model_instructions_template = None; - model_info + model_info::with_config_overrides(model_info::find_model_info_for_slug(model), config) } } diff --git a/codex-rs/core/tests/suite/personality.rs b/codex-rs/core/tests/suite/personality.rs index 918cb5da96..f0064a51cd 100644 --- a/codex-rs/core/tests/suite/personality.rs +++ b/codex-rs/core/tests/suite/personality.rs @@ -38,6 +38,9 @@ use tokio::time::sleep; use wiremock::BodyPrintLimit; use wiremock::MockServer; +const LOCAL_FRIENDLY_TEMPLATE: &str = + "You optimize for team morale and being a supportive teammate as much as code quality."; + fn sse_completed(id: &str) -> String { sse(vec![ev_response_created(id), ev_completed(id)]) } @@ -113,7 +116,7 @@ async fn user_turn_personality_none_does_not_add_update_message() -> anyhow::Res } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn config_personality_some_ignores_without_remote_template() -> anyhow::Result<()> { +async fn config_personality_some_sets_instructions_template() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -148,12 +151,10 @@ async fn config_personality_some_ignores_without_remote_template() -> anyhow::Re let request = resp_mock.single_request(); let instructions_text = request.instructions_text(); - let model_info = - ModelsManager::construct_model_info_offline("exp-codex-personality", &test.config); assert!( - instructions_text == model_info.base_instructions, - "expected base instructions without a remote personality template, got: {instructions_text:?}" + instructions_text.contains(LOCAL_FRIENDLY_TEMPLATE), + "expected personality update to include the local friendly template, got: {instructions_text:?}" ); let developer_texts = request.message_input_texts("developer"); @@ -168,7 +169,7 @@ async fn config_personality_some_ignores_without_remote_template() -> anyhow::Re } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn user_turn_personality_some_ignores_without_remote_template() -> anyhow::Result<()> { +async fn user_turn_personality_some_adds_update_message() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -245,11 +246,18 @@ async fn user_turn_personality_some_ignores_without_remote_template() -> anyhow: .expect("expected personality update request"); let developer_texts = request.message_input_texts("developer"); + let personality_text = developer_texts + .iter() + .find(|text| text.contains("")) + .expect("expected personality update message in developer input"); + assert!( - !developer_texts - .iter() - .any(|text| text.contains("")), - "did not expect a personality update message without a remote template" + personality_text.contains("The user has requested a new communication style."), + "expected personality update preamble, got {personality_text:?}" + ); + assert!( + personality_text.contains(LOCAL_FRIENDLY_TEMPLATE), + "expected personality update to include the local friendly template, got: {personality_text:?}" ); Ok(())