mirror of
https://github.com/openai/codex.git
synced 2026-11-05 16:29:31 +00:00
Use the originating model when recording conversation history (#44243)
## Why History recording used the model captured at turn start, so switching models within a turn could apply stale image preparation and tool-output truncation rules. ## What changed Pass explicit model metadata through conversation recording. Record model responses and tool results using their originating step's settings, and capture the current model for pending input recorded during startup interruption or task completion. ## Testing Add regression coverage for tool results across a model switch, explicit model selection for plain and annotated history, and image input retained during interruption and completion. Extend model-switching tests to verify that prepared media and item identity survive switching back to a multimodal model in live, resumed, and forked threads. GitOrigin-RevId: 5bc750c30c334acb9b802cb80fdea01daffa84a1
This commit is contained in:
1 parent
205f3671e1
commit
b64de2f3ad
28 files changed
+721
-123
No files matched your search
@@ -21,6 +21,7 @@ use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::mcp::ClientMcpExtensions;
|
||||
use codex_protocol::models::BaseInstructionsProvenance;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ConfigShellToolType;
|
||||
@@ -942,14 +943,26 @@ async fn null_service_tier_override_is_omitted_from_http_turn_with_catalog_defau
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum MediaHistorySource {
|
||||
Live,
|
||||
Resume,
|
||||
Fork,
|
||||
}
|
||||
|
||||
#[test_case(MediaHistorySource::Live; "live history")]
|
||||
#[test_case(MediaHistorySource::Resume; "resumed history")]
|
||||
#[test_case(MediaHistorySource::Fork; "forked history")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn model_change_from_multimodal_to_text_strips_prior_media_content() -> Result<()> {
|
||||
async fn model_change_projects_media_without_changing_live_or_replayed_history(
|
||||
source: MediaHistorySource,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = MockServer::start().await;
|
||||
let multimodal_model_slug = "test-multimodal-model";
|
||||
let text_model_slug = "test-text-only-model";
|
||||
let multimodal_model = test_model_info(
|
||||
let mut multimodal_model = test_model_info(
|
||||
multimodal_model_slug,
|
||||
"Test Multimodal Model",
|
||||
"supports image and audio input",
|
||||
@@ -959,6 +972,7 @@ async fn model_change_from_multimodal_to_text_strips_prior_media_content() -> Re
|
||||
InputModality::Audio,
|
||||
],
|
||||
);
|
||||
multimodal_model.supports_image_detail_original = true;
|
||||
let text_model = test_model_info(
|
||||
text_model_slug,
|
||||
"Test Text Model",
|
||||
@@ -982,9 +996,13 @@ async fn model_change_from_multimodal_to_text_strips_prior_media_content() -> Re
|
||||
let mut builder = test_codex()
|
||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(move |config| {
|
||||
config.model = Some(multimodal_model_slug.to_string());
|
||||
config.model = Some(text_model_slug.to_string());
|
||||
config
|
||||
.features
|
||||
.enable(Feature::UnifiedImageBudget)
|
||||
.expect("enable unified image budget");
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
let test = builder.build_with_auto_env(&server).await?;
|
||||
let models_manager = test.thread_manager.get_models_manager();
|
||||
let _ = models_manager
|
||||
.list_models(
|
||||
@@ -992,8 +1010,10 @@ async fn model_change_from_multimodal_to_text_strips_prior_media_content() -> Re
|
||||
codex_core::test_support::default_http_client_factory(),
|
||||
)
|
||||
.await;
|
||||
let image_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg=="
|
||||
.to_string();
|
||||
let mut png = std::io::Cursor::new(Vec::new());
|
||||
image::DynamicImage::new_rgba8(/*w*/ 2048, /*h*/ 2048)
|
||||
.write_to(&mut png, image::ImageFormat::Png)?;
|
||||
let image_url = codex_utils_image::data_url_from_bytes("image/png", &png.into_inner());
|
||||
|
||||
test.codex
|
||||
.start_or_steer_turn(read_only_user_turn(
|
||||
@@ -1069,6 +1089,60 @@ async fn model_change_from_multimodal_to_text_strips_prior_media_content() -> Re
|
||||
.any(|text| text == "audio content omitted because you do not support audio input"),
|
||||
"second request should include the audio-omitted placeholder text"
|
||||
);
|
||||
|
||||
let original = first_request
|
||||
.inputs_of_type("message")
|
||||
.into_iter()
|
||||
.find(|item| {
|
||||
item["content"]
|
||||
.as_array()
|
||||
.is_some_and(|content| content.iter().any(|part| part["type"] == "input_image"))
|
||||
})
|
||||
.expect("original media message");
|
||||
if !matches!(source, MediaHistorySource::Live) {
|
||||
test.codex.shutdown_and_wait().await?;
|
||||
}
|
||||
let rollout_path = test.codex.rollout_path().expect("rollout path");
|
||||
let thread = match source {
|
||||
MediaHistorySource::Live => Arc::clone(&test.codex),
|
||||
MediaHistorySource::Resume => {
|
||||
test.thread_manager
|
||||
.resume_thread_from_rollout(
|
||||
test.config.clone(),
|
||||
rollout_path,
|
||||
test.thread_manager.auth_manager(),
|
||||
/*parent_trace*/ None,
|
||||
ClientMcpExtensions::default(),
|
||||
)
|
||||
.await?
|
||||
.thread
|
||||
}
|
||||
MediaHistorySource::Fork => {
|
||||
test.thread_manager
|
||||
.fork_thread(
|
||||
ForkSnapshot::Interrupted,
|
||||
codex_core::StartThreadOptions::new(test.config.clone()),
|
||||
rollout_path,
|
||||
)
|
||||
.await?
|
||||
.thread
|
||||
}
|
||||
};
|
||||
let response = mount_sse_once(&server, sse_completed("resp-restored")).await;
|
||||
submit_model_turn(
|
||||
&thread,
|
||||
multimodal_model_slug,
|
||||
ThreadSettingsOverrides::default(),
|
||||
)
|
||||
.await?;
|
||||
let restored = response
|
||||
.single_request()
|
||||
.inputs_of_type("message")
|
||||
.into_iter()
|
||||
.find(|item| item["id"] == original["id"]);
|
||||
// Preserve the prepared media, item ID, turn ID, and creation timestamp together.
|
||||
assert_eq!(restored.as_ref(), Some(&original));
|
||||
thread.shutdown_and_wait().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -51,6 +51,7 @@ use codex_protocol::mcp_policy::McpServerIdentity;
|
||||
use codex_protocol::mcp_policy::McpServerRequirement;
|
||||
use codex_protocol::mcp_policy::PluginMcpRequirements;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ImageDetail;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::PermissionProfileSnapshot;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
@@ -79,6 +80,9 @@ use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelections;
|
||||
use codex_protocol::protocol::TurnSettingsUpdate;
|
||||
use codex_protocol::protocol::TurnSettingsUpdateOutcome;
|
||||
use codex_protocol::turn_input::TurnInput;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_cargo_bin::cargo_bin;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
@@ -1427,7 +1431,28 @@ async fn interrupt_during_mcp_startup_preserves_user_input_in_history(
|
||||
let pending_mcp_url = format!("http://{}/mcp", pending_mcp_listener.local_addr()?);
|
||||
|
||||
let fixture = test_codex()
|
||||
.with_model_info_override("gpt-5.4", |model| {
|
||||
model.use_responses_lite = false;
|
||||
model.supports_image_detail_original = false;
|
||||
})
|
||||
.with_config(move |config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::StepModelSwitching)
|
||||
.expect("enable step model switching");
|
||||
config
|
||||
.features
|
||||
.enable(Feature::UnifiedImageBudget)
|
||||
.expect("enable unified image budget");
|
||||
let models = &mut config.model_catalog.as_mut().expect("model catalog").models;
|
||||
let mut current_model = models
|
||||
.iter()
|
||||
.find(|model| model.slug == "gpt-5.4")
|
||||
.expect("initial model")
|
||||
.clone();
|
||||
current_model.slug = "startup-image-model".to_string();
|
||||
current_model.supports_image_detail_original = true;
|
||||
models.push(current_model);
|
||||
config.model_provider.supports_websockets =
|
||||
matches!(startup_phase, InterruptedMcpStartupPhase::StartupPrewarm);
|
||||
if matches!(
|
||||
@@ -1457,14 +1482,36 @@ async fn interrupt_during_mcp_startup_preserves_user_input_in_history(
|
||||
.await
|
||||
.context("MCP startup should connect before the turn is interrupted")??;
|
||||
let prompt = "keep this interrupted prompt in conversation history";
|
||||
fixture
|
||||
.codex
|
||||
.start_or_steer_turn(read_only_user_turn(&fixture, prompt))
|
||||
.await?;
|
||||
wait_for_event(&fixture.codex, |event| {
|
||||
matches!(event, EventMsg::TurnStarted(_))
|
||||
let mut input = read_only_user_turn(&fixture, prompt);
|
||||
let TurnInput::UserInput { content, .. } = &mut input.input else {
|
||||
unreachable!("read_only_user_turn creates user input");
|
||||
};
|
||||
content.push(UserInput::Image {
|
||||
image_url: OPENAI_PNG.to_string(),
|
||||
detail: Some(ImageDetail::High),
|
||||
});
|
||||
fixture.codex.start_or_steer_turn(input).await?;
|
||||
let turn_id = core_test_support::wait_for_event_match(&fixture.codex, |event| match event {
|
||||
EventMsg::TurnStarted(started) => Some(started.turn_id.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
let (reply, outcome) = tokio::sync::oneshot::channel();
|
||||
fixture
|
||||
.codex
|
||||
.submit(Op::TurnSettings {
|
||||
turn_id,
|
||||
update: TurnSettingsUpdate {
|
||||
model: Some("startup-image-model".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
reply,
|
||||
})
|
||||
.await?;
|
||||
assert_eq!(
|
||||
tokio::time::timeout(Duration::from_secs(/*secs*/ 10), outcome).await??,
|
||||
TurnSettingsUpdateOutcome::Applied
|
||||
);
|
||||
|
||||
fixture.codex.submit(Op::Interrupt).await?;
|
||||
wait_for_event(&fixture.codex, |event| {
|
||||
@@ -1494,6 +1541,19 @@ async fn interrupt_during_mcp_startup_preserves_user_input_in_history(
|
||||
)
|
||||
})
|
||||
.expect("an interrupted turn should retain its submitted user prompt");
|
||||
let RolloutItem::ResponseItem(envelope) = &history.items[user_prompt_index] else {
|
||||
unreachable!("found the user prompt");
|
||||
};
|
||||
let ResponseItem::Message { content, .. } = &envelope.item else {
|
||||
unreachable!("found the user message");
|
||||
};
|
||||
assert!(
|
||||
content.contains(&ContentItem::InputImage {
|
||||
image_url: OPENAI_PNG.to_string(),
|
||||
detail: Some(ImageDetail::Original),
|
||||
}),
|
||||
"interrupted input must use the current model's unified image budget"
|
||||
);
|
||||
let interruption_marker_index = history
|
||||
.items
|
||||
.iter()
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
use anyhow::Result;
|
||||
use base64::Engine;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
|
||||
use codex_config::McpServerConfig;
|
||||
use codex_core::CodexThread;
|
||||
use codex_core::StartThreadOptions;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::Constrained;
|
||||
@@ -14,6 +17,7 @@ use codex_extension_api::PromptFragment;
|
||||
use codex_extension_api::ToolContributor;
|
||||
use codex_extension_api::TurnContextContributionInput;
|
||||
use codex_features::Feature;
|
||||
use codex_history::RolloutItem;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_models_manager::bundled_models_response;
|
||||
use codex_models_manager::manager::RefreshStrategy;
|
||||
@@ -21,7 +25,12 @@ use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::config_types::Personality;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem;
|
||||
use codex_protocol::dynamic_tools::DynamicToolFunctionSpec;
|
||||
use codex_protocol::dynamic_tools::DynamicToolResponse;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::ApplyPatchToolType;
|
||||
use codex_protocol::openai_models::ApprovalMessages;
|
||||
use codex_protocol::openai_models::CollaborationModeMessages;
|
||||
@@ -60,6 +69,9 @@ use codex_tools::ToolExecutorFuture;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolOutput;
|
||||
use codex_tools::ToolSpec;
|
||||
use codex_utils_image::data_url_from_bytes;
|
||||
use codex_utils_output_truncation::TruncationPolicy;
|
||||
use codex_utils_output_truncation::truncate_text;
|
||||
use core_test_support::apps_test_server::AppsTestServer;
|
||||
use core_test_support::apps_test_server::recorded_apps_tool_calls;
|
||||
use core_test_support::responses::ResponsesRequest;
|
||||
@@ -84,6 +96,7 @@ use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use core_test_support::wait_for_event_match;
|
||||
use core_test_support::wait_for_mcp_server;
|
||||
use image::GenericImageView;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
@@ -271,6 +284,180 @@ fn request_turn_id(request: &ResponsesRequest) -> String {
|
||||
.to_string()
|
||||
}
|
||||
|
||||
// Dynamic tools return the original payload, so handler truncation cannot hide a recorder bug.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn tool_result_history_keeps_originating_model_across_switch() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let server = start_mock_server().await;
|
||||
let mut test = step_settings_test()
|
||||
.with_config(|config| {
|
||||
for model in &mut config.model_catalog.as_mut().expect("models").models {
|
||||
model.truncation_policy =
|
||||
TruncationPolicyConfig::tokens(if model.slug == MODEL_B { 400 } else { 100 });
|
||||
model.supports_image_detail_original = model.slug == MODEL_B;
|
||||
model.use_responses_lite = false;
|
||||
}
|
||||
config
|
||||
.features
|
||||
.enable(Feature::UnifiedImageBudget)
|
||||
.expect("enable unified image budget");
|
||||
})
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
let started = test
|
||||
.thread_manager
|
||||
.start_thread(StartThreadOptions {
|
||||
dynamic_tools: vec![DynamicToolSpec::Function(DynamicToolFunctionSpec {
|
||||
name: "diagnostics".to_string(),
|
||||
description: "Returns diagnostic text and a screenshot.".to_string(),
|
||||
input_schema: json!({"type": "object", "properties": {}}),
|
||||
defer_loading: false,
|
||||
})],
|
||||
..StartThreadOptions::new(test.config.clone())
|
||||
})
|
||||
.await?;
|
||||
test.codex = started.thread;
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-a"),
|
||||
ev_function_call("call-a", "diagnostics", "{}"),
|
||||
ev_completed("resp-a"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-b"),
|
||||
ev_function_call("call-b", "diagnostics", "{}"),
|
||||
ev_completed("resp-b"),
|
||||
]),
|
||||
sse_completed("resp-done"),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
let mut png = std::io::Cursor::new(Vec::new());
|
||||
image::DynamicImage::new_rgba8(/*w*/ 2048, /*h*/ 2048)
|
||||
.write_to(&mut png, image::ImageFormat::Png)?;
|
||||
let text = "diagnostic line\n".repeat(500);
|
||||
let response = DynamicToolResponse {
|
||||
content_items: vec![
|
||||
DynamicToolCallOutputContentItem::InputText { text: text.clone() },
|
||||
DynamicToolCallOutputContentItem::InputImage {
|
||||
image_url: data_url_from_bytes("image/png", &png.into_inner()),
|
||||
},
|
||||
],
|
||||
success: true,
|
||||
};
|
||||
test.codex
|
||||
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "collect diagnostics".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}]))
|
||||
.await?;
|
||||
let mut raw_outputs = Vec::new();
|
||||
for call_id in ["call-a", "call-b"] {
|
||||
let call = wait_for_event_match(&test.codex, |event| match event {
|
||||
EventMsg::DynamicToolCallRequest(request) => Some(request.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
assert_eq!(call.call_id, call_id);
|
||||
if call_id == "call-a" {
|
||||
// Apply B while A's result is pending in the same turn.
|
||||
assert_eq!(
|
||||
submit_turn_settings(
|
||||
&test.codex,
|
||||
&call.turn_id,
|
||||
TurnSettingsUpdate {
|
||||
model: Some(MODEL_B.to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?,
|
||||
TurnSettingsUpdateOutcome::Applied
|
||||
);
|
||||
}
|
||||
test.codex
|
||||
.submit(Op::DynamicToolResponse {
|
||||
id: call.call_id,
|
||||
response: response.clone(),
|
||||
})
|
||||
.await?;
|
||||
raw_outputs.push(
|
||||
wait_for_event_match(&test.codex, |event| match event {
|
||||
EventMsg::RawResponseItem(event)
|
||||
if matches!(&event.item, ResponseItem::FunctionCallOutput { .. }) =>
|
||||
{
|
||||
Some(serde_json::to_value(&event.item).expect("raw output"))
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.await,
|
||||
);
|
||||
}
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
|
||||
let requests = responses.requests();
|
||||
assert_eq!(
|
||||
requests
|
||||
.iter()
|
||||
.map(|request| request.body_json()["model"].clone())
|
||||
.collect::<Vec<_>>(),
|
||||
vec![json!(MODEL_A), json!(MODEL_B), json!(MODEL_B)]
|
||||
);
|
||||
let turn_id = request_turn_id(&requests[0]);
|
||||
for request in &requests[1..] {
|
||||
assert_eq!(request_turn_id(request), turn_id);
|
||||
}
|
||||
for (index, (call_id, limit, dimensions)) in
|
||||
[("call-a", 100, (1600, 1600)), ("call-b", 400, (2048, 2048))]
|
||||
.into_iter()
|
||||
.enumerate()
|
||||
{
|
||||
let raw = &raw_outputs[index];
|
||||
assert_eq!(raw["output"][0]["text"], text);
|
||||
assert!(raw["id"].is_string());
|
||||
assert!(raw["internal_chat_message_metadata_passthrough"]["create_time"].is_number());
|
||||
let url = raw["output"][1]["image_url"].as_str().expect("tool image");
|
||||
let (_, data) = url.split_once(',').expect("image data URL");
|
||||
assert_eq!(
|
||||
image::load_from_memory(&BASE64_STANDARD.decode(data)?)?.dimensions(),
|
||||
dimensions
|
||||
);
|
||||
let mut expected = raw.clone();
|
||||
expected["output"][0]["text"] =
|
||||
json!(truncate_text(&text, TruncationPolicy::Tokens(limit) * 1.2));
|
||||
assert_eq!(requests[2].function_call_output(call_id), expected);
|
||||
}
|
||||
assert_eq!(
|
||||
requests[1].function_call_output("call-a"),
|
||||
requests[2].function_call_output("call-a")
|
||||
);
|
||||
|
||||
// Persistence and raw notifications retain the prepared, untruncated payload in append order.
|
||||
test.codex.shutdown_and_wait().await?;
|
||||
let history = codex_rollout::RolloutRecorder::get_rollout_history(
|
||||
&test.codex.rollout_path().expect("rollout path"),
|
||||
)
|
||||
.await?;
|
||||
let saved_outputs = history
|
||||
.get_rollout_items()
|
||||
.iter()
|
||||
.filter_map(|item| match item {
|
||||
RolloutItem::ResponseItem(envelope)
|
||||
if matches!(&envelope.item, ResponseItem::FunctionCallOutput { .. }) =>
|
||||
{
|
||||
Some(serde_json::to_value(&envelope.item).expect("saved output"))
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(saved_outputs, raw_outputs);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum SettingsTarget {
|
||||
Thread,
|
||||
|
||||
Reference in new issue
Block a user