mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
Require remote personality metadata
This commit is contained in:
@@ -103,36 +103,12 @@ impl ModelsManager {
|
||||
Ok(self.build_available_models(remote_models))
|
||||
}
|
||||
|
||||
/// Determine whether a model supports personalities, with a local fallback when
|
||||
/// remote metadata omits the instructions template.
|
||||
/// Determine whether a model supports personalities based on remote metadata.
|
||||
pub fn supports_personality(&self, model: &str, config: &Config) -> bool {
|
||||
let remote_supports = self
|
||||
.try_get_remote_models(config)
|
||||
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());
|
||||
if remote_supports {
|
||||
return true;
|
||||
}
|
||||
|
||||
if self
|
||||
.local_models
|
||||
.iter()
|
||||
.find(|preset| preset.model == model)
|
||||
.is_some_and(|preset| preset.supports_personality)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
self.try_list_models(config)
|
||||
.ok()
|
||||
.and_then(|models| {
|
||||
models
|
||||
.into_iter()
|
||||
.find(|preset| preset.model == model)
|
||||
.map(|preset| preset.supports_personality)
|
||||
})
|
||||
.unwrap_or(false)
|
||||
.is_some_and(|info| info.supports_personality())
|
||||
}
|
||||
|
||||
// todo(aibrahim): should be visible to core only and sent on session_configured event
|
||||
@@ -174,14 +150,7 @@ impl ModelsManager {
|
||||
.await
|
||||
.into_iter()
|
||||
.find(|m| m.slug == model);
|
||||
let model = if let Some(mut remote) = remote {
|
||||
if !remote.supports_personality() && local.supports_personality() {
|
||||
remote.model_instructions_template = local.model_instructions_template.clone();
|
||||
}
|
||||
remote
|
||||
} else {
|
||||
local
|
||||
};
|
||||
let model = remote.unwrap_or(local);
|
||||
model_info::with_config_overrides(model, config)
|
||||
}
|
||||
|
||||
@@ -360,6 +329,12 @@ impl ModelsManager {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
/// Override remote model metadata for tests.
|
||||
pub async fn set_remote_models_for_testing(&self, models: Vec<ModelInfo>) {
|
||||
*self.remote_models.write().await = models;
|
||||
}
|
||||
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
/// Get model identifier without consulting remote state or cache.
|
||||
pub fn get_model_offline(model: Option<&str>) -> String {
|
||||
@@ -737,7 +712,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn supports_personality_falls_back_to_local_presets() {
|
||||
async fn supports_personality_requires_remote_metadata() {
|
||||
let codex_home = tempdir().expect("temp dir");
|
||||
let mut config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
@@ -751,7 +726,7 @@ mod tests {
|
||||
let manager =
|
||||
ModelsManager::with_provider(codex_home.path().to_path_buf(), auth_manager, provider);
|
||||
|
||||
// Remote metadata without personality messages should not disable local support.
|
||||
// Remote metadata must explicitly include personality support.
|
||||
let mut remote = remote_model("gpt-5.2-codex", "Remote gpt-5.2-codex", 0);
|
||||
remote.model_instructions_template = Some(ModelInstructionsTemplate {
|
||||
template: "{{ personality_message }}".to_string(),
|
||||
@@ -759,9 +734,9 @@ mod tests {
|
||||
});
|
||||
*manager.remote_models.write().await = vec![remote];
|
||||
|
||||
assert!(manager.supports_personality("gpt-5.2-codex", &config));
|
||||
assert!(!manager.supports_personality("gpt-5.2-codex", &config));
|
||||
let model = manager.get_model_info("gpt-5.2-codex", &config).await;
|
||||
assert!(model.supports_personality());
|
||||
assert!(!model.supports_personality());
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1,13 +1,8 @@
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
use codex_protocol::config_types::Personality;
|
||||
use codex_protocol::config_types::Verbosity;
|
||||
use codex_protocol::openai_models::ApplyPatchToolType;
|
||||
use codex_protocol::openai_models::ConfigShellToolType;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::ModelInstructionsTemplate;
|
||||
use codex_protocol::openai_models::ModelVisibility;
|
||||
use codex_protocol::openai_models::PersonalityMessages;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::TruncationMode;
|
||||
@@ -27,23 +22,9 @@ const GPT_5_2_INSTRUCTIONS: &str = include_str!("../../gpt_5_2_prompt.md");
|
||||
const GPT_5_1_CODEX_MAX_INSTRUCTIONS: &str = include_str!("../../gpt-5.1-codex-max_prompt.md");
|
||||
|
||||
const GPT_5_2_CODEX_INSTRUCTIONS: &str = include_str!("../../gpt-5.2-codex_prompt.md");
|
||||
const GPT_5_2_CODEX_INSTRUCTIONS_TEMPLATE: &str =
|
||||
include_str!("../../templates/model_instructions/gpt-5.2-codex_instructions_template.md");
|
||||
const PERSONALITY_FRIENDLY: &str = include_str!("../../templates/personalities/friendly.md");
|
||||
const PERSONALITY_PRAGMATIC: &str = include_str!("../../templates/personalities/pragmatic.md");
|
||||
|
||||
pub(crate) const CONTEXT_WINDOW_272K: i64 = 272_000;
|
||||
|
||||
fn gpt_5_2_codex_personality_template() -> ModelInstructionsTemplate {
|
||||
ModelInstructionsTemplate {
|
||||
template: GPT_5_2_CODEX_INSTRUCTIONS_TEMPLATE.to_string(),
|
||||
personality_messages: Some(PersonalityMessages(BTreeMap::from([
|
||||
(Personality::Friendly, PERSONALITY_FRIENDLY.to_string()),
|
||||
(Personality::Pragmatic, PERSONALITY_PRAGMATIC.to_string()),
|
||||
]))),
|
||||
}
|
||||
}
|
||||
|
||||
macro_rules! model_info {
|
||||
(
|
||||
$slug:expr $(, $key:ident : $value:expr )* $(,)?
|
||||
@@ -179,7 +160,6 @@ pub(crate) fn find_model_info_for_slug(slug: &str) -> ModelInfo {
|
||||
model_info!(
|
||||
slug,
|
||||
base_instructions: GPT_5_2_CODEX_INSTRUCTIONS.to_string(),
|
||||
model_instructions_template: Some(gpt_5_2_codex_personality_template()),
|
||||
apply_patch_tool_type: Some(ApplyPatchToolType::Freeform),
|
||||
shell_type: ConfigShellToolType::ShellCommand,
|
||||
supports_parallel_tool_calls: true,
|
||||
@@ -206,7 +186,6 @@ pub(crate) fn find_model_info_for_slug(slug: &str) -> ModelInfo {
|
||||
model_info!(
|
||||
slug,
|
||||
base_instructions: GPT_5_2_CODEX_INSTRUCTIONS.to_string(),
|
||||
model_instructions_template: Some(gpt_5_2_codex_personality_template()),
|
||||
apply_patch_tool_type: Some(ApplyPatchToolType::Freeform),
|
||||
shell_type: ConfigShellToolType::ShellCommand,
|
||||
supports_parallel_tool_calls: true,
|
||||
|
||||
@@ -38,9 +38,6 @@ 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)])
|
||||
}
|
||||
@@ -116,7 +113,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_sets_instructions_template() -> anyhow::Result<()> {
|
||||
async fn config_personality_some_ignores_without_remote_template() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
@@ -151,10 +148,12 @@ async fn config_personality_some_sets_instructions_template() -> anyhow::Result<
|
||||
|
||||
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.contains(LOCAL_FRIENDLY_TEMPLATE),
|
||||
"expected personality update to include the local friendly template, got: {instructions_text:?}"
|
||||
instructions_text == model_info.base_instructions,
|
||||
"expected base instructions without a remote personality template, got: {instructions_text:?}"
|
||||
);
|
||||
|
||||
let developer_texts = request.message_input_texts("developer");
|
||||
@@ -169,7 +168,7 @@ async fn config_personality_some_sets_instructions_template() -> anyhow::Result<
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn user_turn_personality_some_adds_update_message() -> anyhow::Result<()> {
|
||||
async fn user_turn_personality_some_ignores_without_remote_template() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
@@ -246,18 +245,11 @@ async fn user_turn_personality_some_adds_update_message() -> anyhow::Result<()>
|
||||
.expect("expected personality update request");
|
||||
|
||||
let developer_texts = request.message_input_texts("developer");
|
||||
let personality_text = developer_texts
|
||||
.iter()
|
||||
.find(|text| text.contains("<personality_spec>"))
|
||||
.expect("expected personality update message in developer input");
|
||||
|
||||
assert!(
|
||||
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:?}"
|
||||
!developer_texts
|
||||
.iter()
|
||||
.any(|text| text.contains("<personality_spec>")),
|
||||
"did not expect a personality update message without a remote template"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
@@ -292,10 +284,16 @@ async fn user_turn_personality_remote_model_template_includes_update_message() -
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_instructions_template: Some(ModelInstructionsTemplate {
|
||||
template: "Base instructions\n{{ personality_message }}\n".to_string(),
|
||||
personality_messages: Some(PersonalityMessages(BTreeMap::from([(
|
||||
Personality::Friendly,
|
||||
remote_personality_message.to_string(),
|
||||
)]))),
|
||||
personality_messages: Some(PersonalityMessages(BTreeMap::from([
|
||||
(
|
||||
Personality::Friendly,
|
||||
remote_personality_message.to_string(),
|
||||
),
|
||||
(
|
||||
Personality::Pragmatic,
|
||||
"Pragmatic from remote template".to_string(),
|
||||
),
|
||||
]))),
|
||||
}),
|
||||
supports_reasoning_summaries: false,
|
||||
support_verbosity: false,
|
||||
|
||||
@@ -67,7 +67,10 @@ use codex_protocol::config_types::CollaborationMode;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::Personality;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::ModelInstructionsTemplate;
|
||||
use codex_protocol::openai_models::ModelPreset;
|
||||
use codex_protocol::openai_models::PersonalityMessages;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::parse_command::ParsedCommand;
|
||||
use codex_protocol::plan_tool::PlanItemArg;
|
||||
@@ -82,8 +85,10 @@ use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyModifiers;
|
||||
use insta::assert_snapshot;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
#[cfg(target_os = "windows")]
|
||||
use serial_test::serial;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashSet;
|
||||
use std::path::PathBuf;
|
||||
use tempfile::NamedTempFile;
|
||||
@@ -124,6 +129,58 @@ fn snapshot(percent: f64) -> RateLimitSnapshot {
|
||||
}
|
||||
}
|
||||
|
||||
fn personality_template() -> ModelInstructionsTemplate {
|
||||
ModelInstructionsTemplate {
|
||||
template: "Base instructions\n{{ personality_message }}\n".to_string(),
|
||||
personality_messages: Some(PersonalityMessages(BTreeMap::from([
|
||||
(Personality::Friendly, "Friendly message".to_string()),
|
||||
(Personality::Pragmatic, "Pragmatic message".to_string()),
|
||||
]))),
|
||||
}
|
||||
}
|
||||
|
||||
fn remote_model_with_personality(slug: &str) -> ModelInfo {
|
||||
let mut model: ModelInfo = serde_json::from_value(json!({
|
||||
"slug": slug,
|
||||
"display_name": slug,
|
||||
"description": format!("{slug} description"),
|
||||
"default_reasoning_level": "medium",
|
||||
"supported_reasoning_levels": [
|
||||
{"effort": "low", "description": "low"},
|
||||
{"effort": "medium", "description": "medium"}
|
||||
],
|
||||
"shell_type": "shell_command",
|
||||
"visibility": "list",
|
||||
"supported_in_api": true,
|
||||
"priority": 0,
|
||||
"upgrade": null,
|
||||
"base_instructions": "base instructions",
|
||||
"supports_reasoning_summaries": false,
|
||||
"support_verbosity": false,
|
||||
"default_verbosity": null,
|
||||
"apply_patch_tool_type": null,
|
||||
"truncation_policy": {"mode": "bytes", "limit": 10_000},
|
||||
"supports_parallel_tool_calls": false,
|
||||
"context_window": 128_000,
|
||||
"experimental_supported_tools": [],
|
||||
}))
|
||||
.expect("valid model");
|
||||
model.model_instructions_template = Some(personality_template());
|
||||
model
|
||||
}
|
||||
|
||||
async fn enable_personality_support(chat: &mut ChatWidget, models: &[&str]) {
|
||||
chat.set_feature_enabled(Feature::RemoteModels, true);
|
||||
let remote_models = models
|
||||
.iter()
|
||||
.copied()
|
||||
.map(remote_model_with_personality)
|
||||
.collect();
|
||||
chat.models_manager
|
||||
.set_remote_models_for_testing(remote_models)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resumed_initial_messages_render_history() {
|
||||
let (mut chat, mut rx, _ops) = make_chatwidget_manual(None).await;
|
||||
@@ -1151,6 +1208,7 @@ fn session_configured_event_for(model: &str) -> Event {
|
||||
#[tokio::test]
|
||||
async fn personality_nudge_respects_hidden_notice() {
|
||||
let (mut chat, _rx, _) = make_chatwidget_manual(Some("bengalfox")).await;
|
||||
enable_personality_support(&mut chat, &["bengalfox"]).await;
|
||||
chat.config.notices.hide_personality_nudge = Some(true);
|
||||
|
||||
chat.handle_codex_event(session_configured_event_for("bengalfox"));
|
||||
@@ -1165,6 +1223,7 @@ async fn personality_nudge_respects_hidden_notice() {
|
||||
#[tokio::test]
|
||||
async fn personality_nudge_shows_once_and_hides_after_seen() {
|
||||
let (mut chat, mut rx, _) = make_chatwidget_manual(Some("bengalfox")).await;
|
||||
enable_personality_support(&mut chat, &["bengalfox"]).await;
|
||||
|
||||
chat.handle_codex_event(session_configured_event_for("bengalfox"));
|
||||
assert!(matches!(
|
||||
@@ -1200,6 +1259,7 @@ async fn personality_nudge_shows_once_and_hides_after_seen() {
|
||||
#[tokio::test]
|
||||
async fn personality_nudge_escape_does_not_persist_hidden() {
|
||||
let (mut chat, mut rx, _) = make_chatwidget_manual(Some("bengalfox")).await;
|
||||
enable_personality_support(&mut chat, &["bengalfox"]).await;
|
||||
|
||||
chat.handle_codex_event(session_configured_event_for("bengalfox"));
|
||||
assert!(matches!(
|
||||
@@ -1233,13 +1293,15 @@ async fn personality_nudge_escape_does_not_persist_hidden() {
|
||||
|
||||
#[tokio::test]
|
||||
async fn gpt_52_codex_supports_personality_command() {
|
||||
let (chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await;
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await;
|
||||
enable_personality_support(&mut chat, &["gpt-5.2-codex"]).await;
|
||||
assert!(chat.current_model_supports_personality());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn personality_nudge_shows_on_model_switch_after_escape() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("bengalfox")).await;
|
||||
enable_personality_support(&mut chat, &["bengalfox"]).await;
|
||||
|
||||
chat.handle_codex_event(session_configured_event_for("bengalfox"));
|
||||
assert!(matches!(
|
||||
@@ -2551,6 +2613,7 @@ async fn collab_mode_enabling_keeps_custom_until_selected() {
|
||||
#[tokio::test]
|
||||
async fn user_turn_includes_personality_from_config() {
|
||||
let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(Some("bengalfox")).await;
|
||||
enable_personality_support(&mut chat, &["bengalfox"]).await;
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
chat.set_personality_nudge_hidden(true);
|
||||
chat.set_model("bengalfox");
|
||||
@@ -3115,6 +3178,7 @@ async fn model_selection_popup_snapshot() {
|
||||
#[tokio::test]
|
||||
async fn personality_selection_popup_snapshot() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("bengalfox")).await;
|
||||
enable_personality_support(&mut chat, &["bengalfox"]).await;
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
chat.open_personality_popup();
|
||||
|
||||
@@ -3125,6 +3189,7 @@ async fn personality_selection_popup_snapshot() {
|
||||
#[tokio::test]
|
||||
async fn personality_nudge_popup_snapshot() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("bengalfox")).await;
|
||||
enable_personality_support(&mut chat, &["bengalfox"]).await;
|
||||
chat.handle_codex_event(session_configured_event_for("bengalfox"));
|
||||
|
||||
let popup = render_bottom_popup(&chat, 80);
|
||||
|
||||
Reference in New Issue
Block a user