mirror of
https://github.com/openai/codex.git
synced 2026-04-30 01:16:54 +00:00
feat(core) update Personality on turn (#9644)
## Summary Support updating Personality mid-Thread via UserTurn/OverwriteTurn. This is explicitly unused by the clients so far, to simplify PRs - app-server and tui implementations will be follow-ups. ## Testing - [x] added integration tests
This commit is contained in:
Dylan Hurd
@@ -1,8 +1,49 @@
|
||||
use codex_core::config::types::Personality;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::models_manager::manager::ModelsManager;
|
||||
use codex_core::models_manager::manager::RefreshStrategy;
|
||||
use codex_core::protocol::AskForApproval;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::Op;
|
||||
use codex_core::protocol::SandboxPolicy;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
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::ModelsResponse;
|
||||
use codex_protocol::openai_models::PersonalityMessages;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::TruncationPolicyConfig;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::load_default_config_for_test;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
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;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::BTreeMap;
|
||||
use std::sync::Arc;
|
||||
use tempfile::TempDir;
|
||||
use tokio::time::Duration;
|
||||
use tokio::time::Instant;
|
||||
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)])
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn model_personality_does_not_mutate_base_instructions_without_template() {
|
||||
@@ -32,3 +73,362 @@ async fn base_instructions_override_disables_personality_template() {
|
||||
"override instructions"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn user_turn_personality_none_does_not_add_update_message() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let resp_mock = mount_sse_once(&server, sse_completed("resp-1")).await;
|
||||
let mut builder = test_codex().with_model("gpt-5.2-codex");
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd_path().to_path_buf(),
|
||||
approval_policy: test.config.approval_policy.value(),
|
||||
sandbox_policy: SandboxPolicy::ReadOnly,
|
||||
model: test.session_configured.model.clone(),
|
||||
effort: test.config.model_reasoning_effort,
|
||||
summary: ReasoningSummary::Auto,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
let request = resp_mock.single_request();
|
||||
let developer_texts = request.message_input_texts("developer");
|
||||
assert!(
|
||||
!developer_texts
|
||||
.iter()
|
||||
.any(|text| text.contains("<personality_spec>")),
|
||||
"did not expect a personality update message when personality is None"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_personality_some_sets_instructions_template() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let resp_mock = mount_sse_once(&server, sse_completed("resp-1")).await;
|
||||
let mut builder = test_codex()
|
||||
.with_model("gpt-5.2-codex")
|
||||
.with_config(|config| {
|
||||
config.model_personality = Some(Personality::Friendly);
|
||||
config.features.disable(Feature::RemoteModels);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd_path().to_path_buf(),
|
||||
approval_policy: test.config.approval_policy.value(),
|
||||
sandbox_policy: SandboxPolicy::ReadOnly,
|
||||
model: test.session_configured.model.clone(),
|
||||
effort: test.config.model_reasoning_effort,
|
||||
summary: ReasoningSummary::Auto,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
let request = resp_mock.single_request();
|
||||
let instructions_text = request.instructions_text();
|
||||
|
||||
assert!(
|
||||
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");
|
||||
for text in developer_texts {
|
||||
assert!(
|
||||
!text.contains("<personality_spec>"),
|
||||
"expected no personality update message in developer input"
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn user_turn_personality_some_adds_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("gpt-5.2-codex")
|
||||
.with_config(|config| {
|
||||
config.features.disable(Feature::RemoteModels);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd_path().to_path_buf(),
|
||||
approval_policy: test.config.approval_policy.value(),
|
||||
sandbox_policy: SandboxPolicy::ReadOnly,
|
||||
model: test.session_configured.model.clone(),
|
||||
effort: test.config.model_reasoning_effort,
|
||||
summary: ReasoningSummary::Auto,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
test.codex
|
||||
.submit(Op::OverrideTurnContext {
|
||||
cwd: None,
|
||||
approval_policy: None,
|
||||
sandbox_policy: None,
|
||||
model: None,
|
||||
effort: None,
|
||||
summary: None,
|
||||
collaboration_mode: None,
|
||||
personality: Some(Personality::Friendly),
|
||||
})
|
||||
.await?;
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd_path().to_path_buf(),
|
||||
approval_policy: test.config.approval_policy.value(),
|
||||
sandbox_policy: SandboxPolicy::ReadOnly,
|
||||
model: test.session_configured.model.clone(),
|
||||
effort: test.config.model_reasoning_effort,
|
||||
summary: ReasoningSummary::Auto,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.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 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:?}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn user_turn_personality_remote_model_template_includes_update_message() -> anyhow::Result<()>
|
||||
{
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = MockServer::builder()
|
||||
.body_print_limit(BodyPrintLimit::Limited(80_000))
|
||||
.start()
|
||||
.await;
|
||||
|
||||
let remote_slug = "codex-remote-personality";
|
||||
let remote_personality_message = "Friendly from remote template";
|
||||
let remote_model = ModelInfo {
|
||||
slug: remote_slug.to_string(),
|
||||
display_name: "Remote personality test".to_string(),
|
||||
description: Some("Remote model with personality template".to_string()),
|
||||
default_reasoning_level: Some(ReasoningEffort::Medium),
|
||||
supported_reasoning_levels: vec![ReasoningEffortPreset {
|
||||
effort: ReasoningEffort::Medium,
|
||||
description: ReasoningEffort::Medium.to_string(),
|
||||
}],
|
||||
shell_type: ConfigShellToolType::UnifiedExec,
|
||||
visibility: ModelVisibility::List,
|
||||
supported_in_api: true,
|
||||
priority: 1,
|
||||
upgrade: None,
|
||||
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(),
|
||||
)]))),
|
||||
}),
|
||||
supports_reasoning_summaries: false,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
apply_patch_tool_type: None,
|
||||
truncation_policy: TruncationPolicyConfig::bytes(10_000),
|
||||
supports_parallel_tool_calls: false,
|
||||
context_window: Some(128_000),
|
||||
auto_compact_token_limit: None,
|
||||
effective_context_window_percent: 95,
|
||||
experimental_supported_tools: Vec::new(),
|
||||
};
|
||||
|
||||
let _models_mock = mount_models_once(
|
||||
&server,
|
||||
ModelsResponse {
|
||||
models: vec![remote_model],
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let resp_mock = mount_sse_sequence(
|
||||
&server,
|
||||
vec![sse_completed("resp-1"), sse_completed("resp-2")],
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = test_codex()
|
||||
.with_auth(codex_core::CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::RemoteModels);
|
||||
config.model = Some("gpt-5.2-codex".to_string());
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
wait_for_model_available(
|
||||
&test.thread_manager.get_models_manager(),
|
||||
remote_slug,
|
||||
&test.config,
|
||||
)
|
||||
.await;
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd_path().to_path_buf(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::ReadOnly,
|
||||
model: test.session_configured.model.clone(),
|
||||
effort: test.config.model_reasoning_effort,
|
||||
summary: ReasoningSummary::Auto,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
test.codex
|
||||
.submit(Op::OverrideTurnContext {
|
||||
cwd: None,
|
||||
approval_policy: None,
|
||||
sandbox_policy: None,
|
||||
model: Some(remote_slug.to_string()),
|
||||
effort: None,
|
||||
summary: None,
|
||||
collaboration_mode: None,
|
||||
personality: Some(Personality::Friendly),
|
||||
})
|
||||
.await?;
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd_path().to_path_buf(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::ReadOnly,
|
||||
model: remote_slug.to_string(),
|
||||
effort: test.config.model_reasoning_effort,
|
||||
summary: ReasoningSummary::Auto,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.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 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(remote_personality_message),
|
||||
"expected personality update to include remote template, got: {personality_text:?}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn wait_for_model_available(
|
||||
manager: &Arc<ModelsManager>,
|
||||
slug: &str,
|
||||
config: &codex_core::config::Config,
|
||||
) {
|
||||
let deadline = Instant::now() + Duration::from_secs(2);
|
||||
loop {
|
||||
let models = manager
|
||||
.list_models(config, RefreshStrategy::OnlineIfUncached)
|
||||
.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;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user