Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions codex-rs/core/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1473,6 +1473,7 @@ impl Config {
tool_output_token_limit: self.tool_output_token_limit,
base_instructions: self.base_instructions.clone(),
personality_enabled: self.features.enabled(Feature::Personality),
personality: self.personality,
model_supports_reasoning_summaries: self.model_supports_reasoning_summaries,
model_catalog: self.model_catalog.clone(),
}
Expand Down
78 changes: 78 additions & 0 deletions codex-rs/core/tests/suite/personality.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,84 @@ async fn config_personality_none_sends_no_personality() -> anyhow::Result<()> {
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn config_personality_none_strips_baked_personality_section() -> 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_info_override("gpt-5.3-codex", |model_info| {
model_info.base_instructions = "Base instructions\n# Personality\nBaked personality\n## Writing Style\nNested writing style\n# General\nGeneral instructions".to_string();
model_info.model_messages = None;
})
.with_config(|config| {
config
.features
.enable(Feature::Personality)
.expect("test config should allow feature update");
config.personality = Some(Personality::None);
});
let test = builder.build(&server).await?;

test.codex
.submit(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;

assert_eq!(
resp_mock.single_request().instructions_text(),
"Base instructions\n# General\nGeneral instructions"
);

Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn config_personality_none_preserves_explicit_base_instructions() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));

const CUSTOM_INSTRUCTIONS: &str = "Custom instructions\n# Personality\nThis must remain\n## Writing Style\nThis must also remain\n# General\nGeneral instructions";

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.3-codex")
.with_config(|config| {
config
.features
.enable(Feature::Personality)
.expect("test config should allow feature update");
config.personality = Some(Personality::None);
config.base_instructions = Some(CUSTOM_INSTRUCTIONS.to_string());
});
let test = builder.build(&server).await?;

test.codex
.submit(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;

assert_eq!(
resp_mock.single_request().instructions_text(),
CUSTOM_INSTRUCTIONS
);

Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn default_personality_is_pragmatic_without_config_toml() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/models-manager/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use codex_protocol::config_types::Personality;
use codex_protocol::openai_models::ModelsResponse;

#[derive(Debug, Clone, Default)]
Expand All @@ -7,6 +8,7 @@ pub struct ModelsManagerConfig {
pub tool_output_token_limit: Option<usize>,
pub base_instructions: Option<String>,
pub personality_enabled: bool,
pub personality: Option<Personality>,
pub model_supports_reasoning_summaries: Option<bool>,
pub model_catalog: Option<ModelsResponse>,
}
55 changes: 53 additions & 2 deletions codex-rs/models-manager/src/model_info.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use codex_protocol::config_types::Personality;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::openai_models::ConfigShellToolType;
use codex_protocol::openai_models::ModelInfo;
Expand All @@ -19,6 +20,7 @@ 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 {
if let Some(supports_reasoning_summaries) = config.model_supports_reasoning_summaries
Expand Down Expand Up @@ -55,13 +57,62 @@ pub fn with_config_overrides(mut model: ModelInfo, config: &ModelsManagerConfig)
if let Some(base_instructions) = &config.base_instructions {
model.base_instructions = base_instructions.clone();
clear_instruction_messages(&mut model);
} else if !config.personality_enabled {
clear_instruction_messages(&mut model);
} else {
if config.personality_enabled && config.personality == Some(Personality::None) {
model.base_instructions = strip_personality_section(model.base_instructions);
if 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));
}
}
if !config.personality_enabled {
clear_instruction_messages(&mut model);
}
}

model
}

fn strip_personality_section(mut instructions: String) -> String {
let mut section_start = None;
let mut section_end = None;
let mut offset = 0;

for line_with_ending in instructions.split_inclusive('\n') {
let line = match line_with_ending.strip_suffix('\n') {
Some(line) => line.strip_suffix('\r').unwrap_or(line),
None => line_with_ending,
};
if section_start.is_some() {
if is_h1_heading(line) {
section_end = Some(offset);
break;
}
} else if line == PERSONALITY_SECTION_HEADER {
section_start = Some(offset);
}
offset += line_with_ending.len();
}

if let Some(section_start) = section_start {
let section_end = section_end.unwrap_or(instructions.len());
instructions.replace_range(section_start..section_end, "");
}

instructions
}

fn is_h1_heading(line: &str) -> bool {
let Some(rest) = line.strip_prefix('#') else {
return false;
};
rest.is_empty() || rest.starts_with(' ') || rest.starts_with('\t')
}

fn clear_instruction_messages(model: &mut ModelInfo) {
if let Some(model_messages) = &mut model.model_messages {
model_messages.instructions_template = None;
Expand Down
78 changes: 78 additions & 0 deletions codex-rs/models-manager/src/model_info_tests.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
use super::*;
use crate::ModelsManagerConfig;
use codex_protocol::config_types::Personality;
use codex_protocol::openai_models::ApprovalMessages;
use pretty_assertions::assert_eq;

fn config_with_personality(personality: Option<Personality>) -> ModelsManagerConfig {
ModelsManagerConfig {
personality_enabled: true,
personality,
..Default::default()
}
}

#[test]
fn reasoning_summaries_override_true_enables_support() {
let model = model_info_from_slug("unknown-model");
Expand Down Expand Up @@ -107,6 +116,75 @@ fn disabled_personality_preserves_catalog_approval_messages() {
);
}

#[test]
fn personality_none_strips_catalog_instruction_sources_through_the_next_h1() {
let cases = [
(
"Intro\n\n# Personality\n\nRemove me\n\n## Writing Style\n\nRemove me too\n\n# Safety\n\nKeep me",
"Intro\n\n# Safety\n\nKeep me",
),
("Intro\n\n# Personality\n\nRemove me", "Intro\n\n"),
(
"Intro\n\n## Personality\n\nKeep me",
"Intro\n\n## Personality\n\nKeep me",
),
(
"Intro\n\n# Personality \n\nKeep me",
"Intro\n\n# Personality \n\nKeep me",
),
(
"Intro\r\n\r\n# Personality\r\n\r\nRemove me\r\n\r\n## Writing Style\r\n\r\nRemove me too\r\n\r\n# General\r\n\r\nKeep me",
"Intro\r\n\r\n# General\r\n\r\nKeep me",
),
];
let config = config_with_personality(Some(Personality::None));

for (instructions, expected) in cases {
let mut model = model_info_from_slug("unknown-model");
model.base_instructions = instructions.to_string();
model.model_messages = Some(ModelMessages {
instructions_template: Some(instructions.to_string()),
instructions_variables: None,
approvals: None,
});

let updated = with_config_overrides(model, &config);
let instructions_template = updated
.model_messages
.as_ref()
.and_then(|messages| messages.instructions_template.as_deref());

assert_eq!(
(updated.base_instructions.as_str(), instructions_template),
(expected, Some(expected))
);
}
}

#[test]
fn baked_personality_section_is_preserved_without_enabled_explicit_none() {
let instructions = "Intro\n# Personality\nKeep me\n# General\nKeep me too";
let configs = [
config_with_personality(/*personality*/ None),
config_with_personality(Some(Personality::Friendly)),
config_with_personality(Some(Personality::Pragmatic)),
ModelsManagerConfig {
personality: Some(Personality::None),
..Default::default()
},
];

for config in configs {
let mut model = model_info_from_slug("unknown-model");
model.base_instructions = instructions.to_string();

assert_eq!(
with_config_overrides(model, &config).base_instructions,
instructions
);
}
}

#[test]
fn model_context_window_override_clamps_to_max_context_window() {
let mut model = model_info_from_slug("unknown-model");
Expand Down
17 changes: 10 additions & 7 deletions codex-rs/protocol/src/openai_models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,14 +466,17 @@ impl ModelInfo {
.get_personality_message(personality)
.unwrap_or_default();
template.replace(PERSONALITY_PLACEHOLDER, personality_message.as_str())
} else if let Some(personality) = personality {
warn!(
model = %self.slug,
%personality,
"Model personality requested but model_messages is missing, falling back to base instructions."
);
self.base_instructions.clone()
} else {
match personality {
Some(personality @ (Personality::Friendly | Personality::Pragmatic)) => {
warn!(
model = %self.slug,
%personality,
"Model personality requested but model_messages is missing, falling back to base instructions."
);
}
Some(Personality::None) | None => {}
}
self.base_instructions.clone()
}
}
Expand Down
Loading