mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
chore(core) personality migration tests (#10650)
## Summary Adds additional tests for personality edge cases ## Testing - [x] These are tests
This commit is contained in:
@@ -2,6 +2,7 @@ use anyhow::Result;
|
||||
use app_test_support::McpProcess;
|
||||
use app_test_support::create_apply_patch_sse_response;
|
||||
use app_test_support::create_exec_command_sse_response;
|
||||
use app_test_support::create_fake_rollout;
|
||||
use app_test_support::create_final_assistant_message_sse_response;
|
||||
use app_test_support::create_mock_responses_server_sequence;
|
||||
use app_test_support::create_mock_responses_server_sequence_unchecked;
|
||||
@@ -34,8 +35,10 @@ use codex_app_server_protocol::TurnStartResponse;
|
||||
use codex_app_server_protocol::TurnStartedNotification;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_app_server_protocol::UserInput as V2UserInput;
|
||||
use codex_core::config::ConfigToml;
|
||||
use codex_core::features::FEATURES;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::personality_migration::PERSONALITY_MIGRATION_FILENAME;
|
||||
use codex_core::protocol_config_types::ReasoningSummary;
|
||||
use codex_protocol::config_types::CollaborationMode;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
@@ -52,6 +55,7 @@ use tokio::time::timeout;
|
||||
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||
const TEST_ORIGINATOR: &str = "codex_vscode";
|
||||
const LOCAL_PRAGMATIC_TEMPLATE: &str = "You are a deeply pragmatic, effective software engineer.";
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_start_sends_originator_header() -> Result<()> {
|
||||
@@ -595,6 +599,96 @@ async fn turn_start_change_personality_mid_thread_v2() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_start_uses_migrated_pragmatic_personality_without_override_v2() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let body = responses::sse(vec![
|
||||
responses::ev_response_created("resp-1"),
|
||||
responses::ev_assistant_message("msg-1", "Done"),
|
||||
responses::ev_completed("resp-1"),
|
||||
]);
|
||||
let response_mock = responses::mount_sse_once(&server, body).await;
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(
|
||||
codex_home.path(),
|
||||
&server.uri(),
|
||||
"never",
|
||||
&BTreeMap::from([(Feature::Personality, true)]),
|
||||
)?;
|
||||
create_fake_rollout(
|
||||
codex_home.path(),
|
||||
"2025-01-01T00-00-00",
|
||||
"2025-01-01T00:00:00Z",
|
||||
"history user message",
|
||||
Some("mock_provider"),
|
||||
None,
|
||||
)?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let persisted_toml: ConfigToml = toml::from_str(&std::fs::read_to_string(
|
||||
codex_home.path().join("config.toml"),
|
||||
)?)?;
|
||||
assert_eq!(persisted_toml.personality, Some(Personality::Pragmatic));
|
||||
assert!(
|
||||
codex_home
|
||||
.path()
|
||||
.join(PERSONALITY_MIGRATION_FILENAME)
|
||||
.exists(),
|
||||
"expected personality migration marker to be written on startup"
|
||||
);
|
||||
|
||||
let thread_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
model: Some("gpt-5.2-codex".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let thread_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(thread_req)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
|
||||
|
||||
let turn_req = mcp
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
thread_id: thread.id,
|
||||
input: vec![V2UserInput::Text {
|
||||
text: "Hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
personality: None,
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let turn_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(turn_req)),
|
||||
)
|
||||
.await??;
|
||||
let _turn: TurnStartResponse = to_response::<TurnStartResponse>(turn_resp)?;
|
||||
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let request = response_mock.single_request();
|
||||
let instructions_text = request.instructions_text();
|
||||
assert!(
|
||||
instructions_text.contains(LOCAL_PRAGMATIC_TEMPLATE),
|
||||
"expected startup-migrated pragmatic personality in model instructions, got: {instructions_text:?}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_start_accepts_local_image_input() -> Result<()> {
|
||||
// Two Codex turns hit the mock model (session start + turn/start).
|
||||
|
||||
@@ -42,6 +42,16 @@ async fn write_archived_session_with_user_event(codex_home: &Path) -> io::Result
|
||||
write_rollout_with_user_event(&dir, thread_id).await
|
||||
}
|
||||
|
||||
async fn write_session_with_meta_only(codex_home: &Path) -> io::Result<()> {
|
||||
let thread_id = ThreadId::new();
|
||||
let dir = codex_home
|
||||
.join(SESSIONS_SUBDIR)
|
||||
.join("2025")
|
||||
.join("01")
|
||||
.join("01");
|
||||
write_rollout_with_meta_only(&dir, thread_id).await
|
||||
}
|
||||
|
||||
async fn write_rollout_with_user_event(dir: &Path, thread_id: ThreadId) -> io::Result<()> {
|
||||
tokio::fs::create_dir_all(&dir).await?;
|
||||
let file_path = dir.join(format!("rollout-{TEST_TIMESTAMP}-{thread_id}.jsonl"));
|
||||
@@ -83,6 +93,40 @@ async fn write_rollout_with_user_event(dir: &Path, thread_id: ThreadId) -> io::R
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn write_rollout_with_meta_only(dir: &Path, thread_id: ThreadId) -> io::Result<()> {
|
||||
tokio::fs::create_dir_all(&dir).await?;
|
||||
let file_path = dir.join(format!("rollout-{TEST_TIMESTAMP}-{thread_id}.jsonl"));
|
||||
let mut file = tokio::fs::File::create(&file_path).await?;
|
||||
|
||||
let session_meta = SessionMetaLine {
|
||||
meta: SessionMeta {
|
||||
id: thread_id,
|
||||
forked_from_id: None,
|
||||
timestamp: TEST_TIMESTAMP.to_string(),
|
||||
cwd: std::path::PathBuf::from("."),
|
||||
originator: "test_originator".to_string(),
|
||||
cli_version: "test_version".to_string(),
|
||||
source: SessionSource::Cli,
|
||||
model_provider: None,
|
||||
base_instructions: None,
|
||||
dynamic_tools: None,
|
||||
},
|
||||
git: None,
|
||||
};
|
||||
let meta_line = RolloutLine {
|
||||
timestamp: TEST_TIMESTAMP.to_string(),
|
||||
item: RolloutItem::SessionMeta(session_meta),
|
||||
};
|
||||
|
||||
let meta_json = serde_json::to_string(&meta_line)?;
|
||||
file.write_all(format!("{meta_json}\n").as_bytes()).await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn parse_config_toml(contents: &str) -> io::Result<ConfigToml> {
|
||||
toml::from_str(contents).map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn migration_marker_exists_no_sessions_no_change() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
@@ -135,6 +179,138 @@ async fn no_marker_sessions_sets_personality() -> io::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn no_marker_sessions_preserves_existing_config_fields() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
write_session_with_user_event(temp.path()).await?;
|
||||
tokio::fs::write(temp.path().join("config.toml"), "model = \"gpt-5-codex\"\n").await?;
|
||||
let config_toml = read_config_toml(temp.path()).await?;
|
||||
|
||||
let status = maybe_migrate_personality(temp.path(), &config_toml).await?;
|
||||
|
||||
assert_eq!(status, PersonalityMigrationStatus::Applied);
|
||||
let persisted = read_config_toml(temp.path()).await?;
|
||||
assert_eq!(persisted.model, Some("gpt-5-codex".to_string()));
|
||||
assert_eq!(persisted.personality, Some(Personality::Pragmatic));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn no_marker_meta_only_rollout_is_treated_as_no_sessions() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
write_session_with_meta_only(temp.path()).await?;
|
||||
|
||||
let status = maybe_migrate_personality(temp.path(), &ConfigToml::default()).await?;
|
||||
|
||||
assert_eq!(status, PersonalityMigrationStatus::SkippedNoSessions);
|
||||
assert_eq!(
|
||||
tokio::fs::try_exists(temp.path().join(PERSONALITY_MIGRATION_FILENAME)).await?,
|
||||
true
|
||||
);
|
||||
assert_eq!(
|
||||
tokio::fs::try_exists(temp.path().join("config.toml")).await?,
|
||||
false
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn no_marker_explicit_global_personality_skips_migration() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
write_session_with_user_event(temp.path()).await?;
|
||||
let config_toml = parse_config_toml("personality = \"friendly\"\n")?;
|
||||
|
||||
let status = maybe_migrate_personality(temp.path(), &config_toml).await?;
|
||||
|
||||
assert_eq!(
|
||||
status,
|
||||
PersonalityMigrationStatus::SkippedExplicitPersonality
|
||||
);
|
||||
assert_eq!(
|
||||
tokio::fs::try_exists(temp.path().join(PERSONALITY_MIGRATION_FILENAME)).await?,
|
||||
true
|
||||
);
|
||||
assert_eq!(
|
||||
tokio::fs::try_exists(temp.path().join("config.toml")).await?,
|
||||
false
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn no_marker_profile_personality_skips_migration() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
write_session_with_user_event(temp.path()).await?;
|
||||
let config_toml = parse_config_toml(
|
||||
r#"
|
||||
profile = "work"
|
||||
|
||||
[profiles.work]
|
||||
personality = "friendly"
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let status = maybe_migrate_personality(temp.path(), &config_toml).await?;
|
||||
|
||||
assert_eq!(
|
||||
status,
|
||||
PersonalityMigrationStatus::SkippedExplicitPersonality
|
||||
);
|
||||
assert_eq!(
|
||||
tokio::fs::try_exists(temp.path().join(PERSONALITY_MIGRATION_FILENAME)).await?,
|
||||
true
|
||||
);
|
||||
assert_eq!(
|
||||
tokio::fs::try_exists(temp.path().join("config.toml")).await?,
|
||||
false
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn marker_short_circuits_invalid_profile_resolution() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
tokio::fs::write(temp.path().join(PERSONALITY_MIGRATION_FILENAME), "v1\n").await?;
|
||||
let config_toml = parse_config_toml("profile = \"missing\"\n")?;
|
||||
|
||||
let status = maybe_migrate_personality(temp.path(), &config_toml).await?;
|
||||
|
||||
assert_eq!(status, PersonalityMigrationStatus::SkippedMarker);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn invalid_selected_profile_returns_error_and_does_not_write_marker() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
let config_toml = parse_config_toml("profile = \"missing\"\n")?;
|
||||
|
||||
let err = maybe_migrate_personality(temp.path(), &config_toml)
|
||||
.await
|
||||
.expect_err("missing profile should fail");
|
||||
|
||||
assert_eq!(err.kind(), io::ErrorKind::InvalidData);
|
||||
assert_eq!(
|
||||
tokio::fs::try_exists(temp.path().join(PERSONALITY_MIGRATION_FILENAME)).await?,
|
||||
false
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn applied_migration_is_idempotent_on_second_run() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
write_session_with_user_event(temp.path()).await?;
|
||||
|
||||
let first_status = maybe_migrate_personality(temp.path(), &ConfigToml::default()).await?;
|
||||
let second_status = maybe_migrate_personality(temp.path(), &ConfigToml::default()).await?;
|
||||
|
||||
assert_eq!(first_status, PersonalityMigrationStatus::Applied);
|
||||
assert_eq!(second_status, PersonalityMigrationStatus::SkippedMarker);
|
||||
let persisted = read_config_toml(temp.path()).await?;
|
||||
assert_eq!(persisted.personality, Some(Personality::Pragmatic));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn no_marker_archived_sessions_sets_personality() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
|
||||
Reference in New Issue
Block a user