mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Remove initial messages from session configuration events (#50360)
## What changed Remove `initial_messages` from `SessionConfiguredEvent` and the corresponding schema. Stop attaching replay history to startup events for resumed or rejoined threads, and remove `InitialHistory::get_event_msgs`. ## Testing Update the MCP resume test to verify persisted tool events through `load_history`, and retain coverage that resumed requests include prior conversation items. GitOrigin-RevId: 54b72bc44423c9b1ef2051891ef392c0ede32fe5
This commit is contained in:
BIN
Binary file not shown.
@@ -66,7 +66,6 @@ async fn interrupt_if_no_pending_input_handles_cancelled_submission() {
|
||||
active_permission_profile: None,
|
||||
cwd: turn_context.config.cwd.clone(),
|
||||
reasoning_effort: None,
|
||||
initial_messages: None,
|
||||
network_proxy: None,
|
||||
rollout_path: None,
|
||||
}),
|
||||
|
||||
@@ -1857,8 +1857,6 @@ impl Session {
|
||||
*guard = Arc::downgrade(&sess);
|
||||
}
|
||||
// Dispatch the SessionConfiguredEvent first and then report any errors.
|
||||
// If resuming, include converted initial messages in the payload so UIs can render them immediately.
|
||||
let initial_messages = initial_history.get_event_msgs();
|
||||
let thread_config =
|
||||
session_configuration.thread_config_snapshot(turn_environments.selections());
|
||||
let events = std::iter::once(Event {
|
||||
@@ -1884,7 +1882,6 @@ impl Session {
|
||||
permission_profile: thread_config.permission_profile,
|
||||
active_permission_profile: thread_config.active_permission_profile,
|
||||
reasoning_effort: thread_config.reasoning_effort,
|
||||
initial_messages,
|
||||
rollout_path,
|
||||
}),
|
||||
})
|
||||
|
||||
@@ -2156,9 +2156,8 @@ impl ThreadManagerState {
|
||||
resumed.conversation_id
|
||||
)));
|
||||
}
|
||||
let session_configured = thread
|
||||
.startup_metadata()
|
||||
.to_session_configured_event(initial_history.get_event_msgs());
|
||||
let session_configured =
|
||||
thread.startup_metadata().to_session_configured_event();
|
||||
startup_state.release_membership();
|
||||
if let Some(startup_guard) = startup_guard {
|
||||
startup_guard.disarm();
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
//! Retained startup metadata, separate from the replay sent in startup responses.
|
||||
//! Retained startup metadata for new and resumed threads.
|
||||
|
||||
use codex_protocol::SessionId;
|
||||
use codex_protocol::ThreadId;
|
||||
@@ -7,14 +7,13 @@ use codex_protocol::models::ActivePermissionProfile;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::SessionConfiguredEvent;
|
||||
use codex_protocol::protocol::SessionNetworkProxyRuntime;
|
||||
use codex_protocol::protocol::ThreadSource;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::path::PathBuf;
|
||||
|
||||
/// The configuration reported when a thread started, without its replay history.
|
||||
/// The configuration reported when a thread started.
|
||||
/// Current settings are available through [`crate::CodexThread::config_snapshot`].
|
||||
#[derive(Debug)]
|
||||
pub struct ThreadStartupMetadata {
|
||||
@@ -55,7 +54,6 @@ impl From<&SessionConfiguredEvent> for ThreadStartupMetadata {
|
||||
active_permission_profile,
|
||||
cwd,
|
||||
reasoning_effort,
|
||||
initial_messages: _,
|
||||
network_proxy,
|
||||
rollout_path,
|
||||
} = event;
|
||||
@@ -82,10 +80,7 @@ impl From<&SessionConfiguredEvent> for ThreadStartupMetadata {
|
||||
}
|
||||
|
||||
impl ThreadStartupMetadata {
|
||||
pub(crate) fn to_session_configured_event(
|
||||
&self,
|
||||
initial_messages: Option<Vec<EventMsg>>,
|
||||
) -> SessionConfiguredEvent {
|
||||
pub(crate) fn to_session_configured_event(&self) -> SessionConfiguredEvent {
|
||||
SessionConfiguredEvent {
|
||||
session_id: self.session_id,
|
||||
thread_id: self.thread_id,
|
||||
@@ -102,7 +97,6 @@ impl ThreadStartupMetadata {
|
||||
active_permission_profile: self.active_permission_profile.clone(),
|
||||
cwd: self.cwd.clone(),
|
||||
reasoning_effort: self.reasoning_effort.clone(),
|
||||
initial_messages,
|
||||
network_proxy: self.network_proxy.clone(),
|
||||
rollout_path: self.rollout_path.clone(),
|
||||
}
|
||||
|
||||
@@ -901,7 +901,7 @@ fn non_zero_u64(value: u64) -> NonZeroU64 {
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn resume_includes_initial_messages_and_sends_prior_items() {
|
||||
async fn resume_sends_prior_items() {
|
||||
skip_if_no_network!();
|
||||
|
||||
// Create a fake rollout session file with prior user + system + assistant messages.
|
||||
@@ -1016,18 +1016,8 @@ async fn resume_includes_initial_messages_and_sends_prior_items() {
|
||||
.await
|
||||
.expect("resume conversation");
|
||||
let codex = test.codex.clone();
|
||||
let session_configured = test.session_configured;
|
||||
|
||||
// 1) Assert initial_messages only includes existing EventMsg entries; response items are not converted
|
||||
let initial_msgs = session_configured
|
||||
.initial_messages
|
||||
.clone()
|
||||
.expect("expected initial messages option for resumed session");
|
||||
let initial_json = serde_json::to_value(&initial_msgs).unwrap();
|
||||
let expected_initial_json = json!([]);
|
||||
assert_eq!(initial_json, expected_initial_json);
|
||||
|
||||
// 2) Submit new input; the request body must include the prior items, then initial context, then new user input.
|
||||
// Submit new input; the request body must include the prior items, then initial context, then new user input.
|
||||
codex
|
||||
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
|
||||
@@ -200,11 +200,17 @@ async fn mcp_app_ui_survives_tool_events_and_resume(
|
||||
|
||||
let resumed = builder.restart(&server, &test).await?;
|
||||
let completed_history = resumed
|
||||
.session_configured
|
||||
.initial_messages
|
||||
.expect("resumed history")
|
||||
.codex
|
||||
.load_history(/*include_archived*/ false)
|
||||
.await?
|
||||
.items
|
||||
.into_iter()
|
||||
.filter(|event| matches!(event, EventMsg::McpToolCallEnd(end) if end.call_id == call_id))
|
||||
.filter_map(|item| match item {
|
||||
RolloutItem::EventMsg(EventMsg::McpToolCallEnd(end)) if end.call_id == call_id => {
|
||||
Some(EventMsg::McpToolCallEnd(end))
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(
|
||||
serde_json::to_value(completed_history)?,
|
||||
|
||||
@@ -7,13 +7,10 @@ use codex_protocol::mcp::ClientMcpExtensions;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::ThreadHistoryMode;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::user_input::ByteRange;
|
||||
use codex_protocol::user_input::TextElement;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_thread_store::LoadThreadHistoryParams;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_reasoning_item;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::mount_sse_once;
|
||||
use core_test_support::responses::mount_sse_sequence;
|
||||
@@ -126,148 +123,6 @@ async fn resume_restores_windows_sandbox_override() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn resume_includes_initial_messages_from_rollout_events() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex();
|
||||
let initial = builder.build(&server).await?;
|
||||
let codex = Arc::clone(&initial.codex);
|
||||
|
||||
let initial_sse = sse(vec![
|
||||
ev_response_created("resp-initial"),
|
||||
ev_assistant_message("msg-1", "Completed first turn"),
|
||||
ev_completed("resp-initial"),
|
||||
]);
|
||||
mount_sse_once(&server, initial_sse).await;
|
||||
|
||||
let text_elements = vec![TextElement::new(
|
||||
ByteRange { start: 0, end: 6 },
|
||||
Some("<note>".into()),
|
||||
)];
|
||||
|
||||
codex
|
||||
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "Record some messages".into(),
|
||||
text_elements: text_elements.clone(),
|
||||
}]))
|
||||
.await?;
|
||||
|
||||
wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await;
|
||||
let mut resumed = builder.restart(&server, &initial).await?;
|
||||
let initial_messages = resumed
|
||||
.session_configured
|
||||
.initial_messages
|
||||
.take()
|
||||
.expect("expected initial messages to be present for resumed session");
|
||||
match initial_messages.as_slice() {
|
||||
[
|
||||
EventMsg::TurnStarted(started),
|
||||
EventMsg::UserMessage(first_user),
|
||||
EventMsg::AgentMessage(assistant_message),
|
||||
EventMsg::TokenCount(_),
|
||||
EventMsg::TurnComplete(completed),
|
||||
] => {
|
||||
assert_eq!(first_user.message, "Record some messages");
|
||||
assert_eq!(first_user.text_elements, text_elements);
|
||||
assert_eq!(assistant_message.message, "Completed first turn");
|
||||
assert_eq!(completed.turn_id, started.turn_id);
|
||||
assert_eq!(
|
||||
completed.last_agent_message.as_deref(),
|
||||
Some("Completed first turn")
|
||||
);
|
||||
}
|
||||
other => panic!("unexpected initial messages after resume: {other:#?}"),
|
||||
}
|
||||
|
||||
resumed.codex.flush_rollout().await?;
|
||||
let mut rejoined = resumed
|
||||
.thread_manager
|
||||
.resume_legacy_thread_from_rollout(
|
||||
resumed.config.clone(),
|
||||
resumed.codex.rollout_path().expect("resumed rollout path"),
|
||||
resumed.thread_manager.auth_manager(),
|
||||
/*parent_trace*/ None,
|
||||
ClientMcpExtensions::default(),
|
||||
)
|
||||
.await?;
|
||||
assert!(Arc::ptr_eq(&rejoined.thread, &resumed.codex));
|
||||
let rejoined_messages = rejoined
|
||||
.session_configured
|
||||
.initial_messages
|
||||
.take()
|
||||
.expect("rejoining a loaded thread must still provide replay messages");
|
||||
assert_eq!(
|
||||
serde_json::to_value(&rejoined.session_configured)?,
|
||||
serde_json::to_value(&resumed.session_configured)?,
|
||||
);
|
||||
assert_eq!(
|
||||
serde_json::to_value(&rejoined_messages[..initial_messages.len()])?,
|
||||
serde_json::to_value(&initial_messages)?,
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn resume_includes_initial_messages_from_reasoning_events() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
config.show_raw_agent_reasoning = true;
|
||||
});
|
||||
let initial = builder.build(&server).await?;
|
||||
let codex = Arc::clone(&initial.codex);
|
||||
|
||||
let initial_sse = sse(vec![
|
||||
ev_response_created("resp-initial"),
|
||||
ev_reasoning_item("reason-1", &["Summarized step"], &["raw detail"]),
|
||||
ev_assistant_message("msg-1", "Completed reasoning turn"),
|
||||
ev_completed("resp-initial"),
|
||||
]);
|
||||
mount_sse_once(&server, initial_sse).await;
|
||||
|
||||
codex
|
||||
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "Record reasoning messages".into(),
|
||||
text_elements: Vec::new(),
|
||||
}]))
|
||||
.await?;
|
||||
|
||||
wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await;
|
||||
let resumed = builder.restart(&server, &initial).await?;
|
||||
let initial_messages = resumed
|
||||
.session_configured
|
||||
.initial_messages
|
||||
.expect("expected initial messages to be present for resumed session");
|
||||
match initial_messages.as_slice() {
|
||||
[
|
||||
EventMsg::TurnStarted(started),
|
||||
EventMsg::UserMessage(first_user),
|
||||
EventMsg::AgentReasoning(reasoning),
|
||||
EventMsg::AgentReasoningRawContent(raw),
|
||||
EventMsg::AgentMessage(assistant_message),
|
||||
EventMsg::TokenCount(_),
|
||||
EventMsg::TurnComplete(completed),
|
||||
] => {
|
||||
assert_eq!(first_user.message, "Record reasoning messages");
|
||||
assert_eq!(reasoning.text, "Summarized step");
|
||||
assert_eq!(raw.text, "raw detail");
|
||||
assert_eq!(assistant_message.message, "Completed reasoning turn");
|
||||
assert_eq!(completed.turn_id, started.turn_id);
|
||||
assert_eq!(
|
||||
completed.last_agent_message.as_deref(),
|
||||
Some("Completed reasoning turn")
|
||||
);
|
||||
}
|
||||
other => panic!("unexpected initial messages after resume: {other:#?}"),
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn resume_switches_models_preserves_base_instructions() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -230,7 +230,6 @@ async fn config_summary_entries_include_runtime_workspace_roots() {
|
||||
active_permission_profile: None,
|
||||
cwd,
|
||||
reasoning_effort: None,
|
||||
initial_messages: None,
|
||||
network_proxy: None,
|
||||
rollout_path: None,
|
||||
};
|
||||
|
||||
@@ -1651,7 +1651,6 @@ fn session_configured_from_thread_response(
|
||||
active_permission_profile,
|
||||
cwd,
|
||||
reasoning_effort,
|
||||
initial_messages: None,
|
||||
network_proxy: None,
|
||||
rollout_path,
|
||||
})
|
||||
|
||||
@@ -124,7 +124,6 @@ fn session_configured_produces_thread_started_event() {
|
||||
active_permission_profile: None,
|
||||
cwd: test_path_buf("/tmp/project").abs(),
|
||||
reasoning_effort: None,
|
||||
initial_messages: None,
|
||||
network_proxy: None,
|
||||
rollout_path: None,
|
||||
};
|
||||
|
||||
@@ -423,31 +423,6 @@ impl InitialHistory {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn get_event_msgs(&self) -> Option<Vec<EventMsg>> {
|
||||
match self {
|
||||
Self::New | Self::Cleared => None,
|
||||
Self::Resumed(resumed) => Some(
|
||||
resumed
|
||||
.history
|
||||
.iter()
|
||||
.filter_map(|item| match item {
|
||||
RolloutItem::EventMsg(event) => Some(event.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.collect(),
|
||||
),
|
||||
Self::Forked(items) => Some(
|
||||
items
|
||||
.iter()
|
||||
.filter_map(|item| match item {
|
||||
RolloutItem::EventMsg(event) => Some(event.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.collect(),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn get_base_instructions(&self) -> Option<BaseInstructions> {
|
||||
match self {
|
||||
Self::New | Self::Cleared => None,
|
||||
|
||||
@@ -4007,11 +4007,6 @@ pub struct SessionConfiguredEvent {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub reasoning_effort: Option<ReasoningEffortConfig>,
|
||||
|
||||
/// Optional initial messages (as events) for resumed sessions.
|
||||
/// When present, UIs can use these to seed the history.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub initial_messages: Option<Vec<EventMsg>>,
|
||||
|
||||
/// Runtime proxy bind addresses, when the managed proxy was started for this session.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
@@ -4053,7 +4048,6 @@ impl<'de> Deserialize<'de> for SessionConfiguredEvent {
|
||||
active_permission_profile: Option<ActivePermissionProfile>,
|
||||
cwd: AbsolutePathBuf,
|
||||
reasoning_effort: Option<ReasoningEffortConfig>,
|
||||
initial_messages: Option<Vec<EventMsg>>,
|
||||
network_proxy: Option<SessionNetworkProxyRuntime>,
|
||||
rollout_path: Option<PathBuf>,
|
||||
}
|
||||
@@ -4086,7 +4080,6 @@ impl<'de> Deserialize<'de> for SessionConfiguredEvent {
|
||||
active_permission_profile: wire.active_permission_profile,
|
||||
cwd: wire.cwd,
|
||||
reasoning_effort: wire.reasoning_effort,
|
||||
initial_messages: wire.initial_messages,
|
||||
network_proxy: wire.network_proxy,
|
||||
rollout_path: wire.rollout_path,
|
||||
})
|
||||
@@ -6296,7 +6289,6 @@ mod tests {
|
||||
active_permission_profile: None,
|
||||
cwd: test_path_buf("/home/user/project").abs(),
|
||||
reasoning_effort: Some(ReasoningEffortConfig::default()),
|
||||
initial_messages: None,
|
||||
network_proxy: None,
|
||||
rollout_path: Some(rollout_file.path().to_path_buf()),
|
||||
}),
|
||||
|
||||
Reference in New Issue
Block a user