mirror of
https://github.com/openai/codex.git
synced 2026-11-03 19:53:25 +00:00
Avoid redundant settings checkpoints when resuming threads (#56250)
## Why Resuming a thread with unchanged settings appended another `ThreadSettingsApplied` event to its rollout. ## What changed Compare the effective settings with the latest persisted settings for the same thread before writing a checkpoint. Persist settings when they differ or no matching checkpoint exists, even if no turn follows the resume. ## Testing Add a regression test that restarts the app server, resumes a materialized thread without overrides, and verifies that its persisted settings events remain unchanged. GitOrigin-RevId: c545791ed3bb4020fa3da6d233607a9ba73e5c8d
This commit is contained in:
@@ -6058,6 +6058,51 @@ async fn thread_resume_with_overrides_preserves_recency_and_checkpoints_model()
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_resume_does_not_checkpoint_unchanged_settings() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
mock_responses_config(&server.uri()).write(codex_home.path())?;
|
||||
|
||||
let RestartedThreadFixture {
|
||||
mut mcp,
|
||||
thread_id,
|
||||
rollout_file_path,
|
||||
..
|
||||
} = start_materialized_thread_and_restart(codex_home.path(), "materialize").await?;
|
||||
|
||||
let (items, _, _) = RolloutRecorder::load_rollout_items(&rollout_file_path).await?;
|
||||
let settings_before_resume = items
|
||||
.into_iter()
|
||||
.filter_map(|item| match item {
|
||||
RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => Some(event),
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert!(!settings_before_resume.is_empty());
|
||||
|
||||
let resume_id = mcp
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id,
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let _: ThreadResumeResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??;
|
||||
|
||||
let (items, _, _) = RolloutRecorder::load_rollout_items(&rollout_file_path).await?;
|
||||
let settings_after_resume = items
|
||||
.into_iter()
|
||||
.filter_map(|item| match item {
|
||||
RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => Some(event),
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(settings_after_resume, settings_before_resume);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_resume_fails_when_required_mcp_server_fails_to_initialize() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
|
||||
@@ -155,6 +155,7 @@ use codex_protocol::protocol::RawResponseItemEvent;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_protocol::protocol::ThreadHistoryMode;
|
||||
use codex_protocol::protocol::ThreadSettingsAppliedEvent;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::protocol::ThreadSettingsSnapshot;
|
||||
use codex_protocol::protocol::ThreadSource;
|
||||
@@ -1728,11 +1729,26 @@ impl Session {
|
||||
self.state.lock().await.latest_token_usage_record =
|
||||
Self::last_token_usage_record_from_rollout(&rollout_items);
|
||||
|
||||
// Checkpoint effective settings even when no turn follows the resume.
|
||||
self.persist_rollout_items(&[RolloutItem::EventMsg(
|
||||
thread_settings::applied_event(self).await,
|
||||
)])
|
||||
.await;
|
||||
let thread_id = self.thread_id();
|
||||
let current_settings = self.thread_settings_snapshot().await;
|
||||
let persisted_settings = rollout_items.iter().rev().find_map(|item| match item {
|
||||
RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event))
|
||||
if event.thread_id == Some(thread_id) =>
|
||||
{
|
||||
Some(&event.thread_settings)
|
||||
}
|
||||
_ => None,
|
||||
});
|
||||
// Persist changed effective settings even when no turn follows the resume.
|
||||
if persisted_settings != Some(¤t_settings) {
|
||||
self.persist_rollout_items(&[RolloutItem::EventMsg(
|
||||
EventMsg::ThreadSettingsApplied(ThreadSettingsAppliedEvent {
|
||||
thread_id: Some(thread_id),
|
||||
thread_settings: current_settings,
|
||||
}),
|
||||
)])
|
||||
.await;
|
||||
}
|
||||
|
||||
// Defer seeding the session's initial context until the first turn starts so
|
||||
// turn/start overrides can be merged before we write model-visible context.
|
||||
|
||||
Reference in New Issue
Block a user