github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 17:39:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Allow guarded legacy resume with background migration enabled (#43178)

Browse Source 
## Why

Enabling background rollout migration disabled the TUI's cached legacy resume shortcut, even when the rollout maintenance lock could prevent migration during resume.

## What changed

Allow embedded sessions to use the shortcut regardless of migration feature settings when they acquire the maintenance lock and revalidate that the thread still uses legacy history. Hold the lock through `thread/resume` so the response can safely include turns. Remove the session's cached startup migration setting.

## Testing

Update the regression test to cover all four combinations of startup and workspace migration settings, checking lock acquisition before metadata revalidation, a two-request resume, and nonempty history.

GitOrigin-RevId: cf525a6edb813050469dff39b83e5e61214485a1
This commit is contained in:
Eric Traut
2026-09-06 07:42:32 +00:00
committed by copyberry
parent 9587c9ef36
commit ac192cd793
6 changed files with 75 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/app/reconnect.rs
+1 -1
View File
@@ -55,7 +55,7 @@ pub(super) async fn reconnect(
tokio::time::sleep(Duration::from_secs(delay)).await;
let client = crate::app_server_connection::connect(&target).await?;
let mut session = AppServerSession::new(client, mode)
.with_startup_config(&config)
.with_local_codex_home(&config.codex_home)
.with_remote_cwd_override(remote_cwd.clone())
.with_thread_tool_transport(task_tools.clone());
let bootstrap = session.bootstrap(&config).await?;
codex-rs/tui/src/app/tests/session_lifecycle_requests.rs
+2 -1
View File
@@ -380,7 +380,8 @@ async fn start_recording_app_server_with_history(
.await?;
Ok((
AppServerSession::new(app_server, thread_params_mode).with_startup_config(config),
AppServerSession::new(app_server, thread_params_mode)
.with_local_codex_home(&config.codex_home),
requests,
proxy,
))
codex-rs/tui/src/app_server_session.rs
-2
View File
@@ -306,7 +306,6 @@ pub(crate) struct AppServerSession {
task_search_generation: Arc<AtomicU64>,
remote_cwd_override: Option<PathBuf>,
thread_params_mode: ThreadParamsMode,
background_rollout_migration_enabled: bool,
history_support: ThreadHistorySupport,
thread_settings_update_supported: bool,
default_model: Option<String>,
@@ -403,7 +402,6 @@ impl AppServerSession {
task_search_generation: Arc::new(AtomicU64::new(0)),
remote_cwd_override: None,
thread_params_mode,
background_rollout_migration_enabled: true,
history_support: ThreadHistorySupport::Paginated,
thread_settings_update_supported: true,
default_model: None,
codex-rs/tui/src/app_server_session/rollout_history.rs
+17 -26
View File
@@ -14,20 +14,14 @@ use codex_app_server_client::TypedRequestError;
use codex_app_server_protocol::ClientRequest;
use codex_app_server_protocol::ThreadHistoryMode;
use codex_app_server_protocol::ThreadResumeResponse;
use codex_features::Feature;
use codex_protocol::ThreadId;
use codex_utils_absolute_path::AbsolutePathBuf;
use color_eyre::eyre::Result;
impl AppServerSession {
/// Captures the server's startup migration policy before workspace config can change.
///
/// Sessions without a recorded startup config conservatively assume migration is enabled.
pub(crate) fn with_startup_config(mut self, config: &Config) -> Self {
self.background_rollout_migration_enabled = config
.features
.enabled(Feature::BackgroundPaginatedRolloutMigration);
pub(crate) fn with_local_codex_home(mut self, codex_home: &AbsolutePathBuf) -> Self {
self.task_tool_capabilities_dir = (!self.uses_embedded_app_server())
.then(|| config.codex_home.join("tui-thread-reference-capabilities"));
.then(|| codex_home.join("tui-thread-reference-capabilities"));
self
}
@@ -79,23 +73,20 @@ impl AppServerSession {
.get(&thread_id)
.is_some_and(|state| state.history_mode == ThreadHistoryMode::Legacy)
&& (!self.uses_embedded_app_server()
|| (!self.background_rollout_migration_enabled
&& !config
.features
.enabled(Feature::BackgroundPaginatedRolloutMigration)
&& {
rollout_maintenance_guard =
codex_rollout::try_acquire_rollout_maintenance_lock(
config.codex_home.as_path(),
)
.ok()
.flatten();
rollout_maintenance_guard.is_some()
}
&& self
.thread_read(thread_id, /*include_turns*/ false)
.await
.is_ok_and(|thread| thread.history_mode == ThreadHistoryMode::Legacy)));
|| ({
// The guard prevents migration through the full resume,
// regardless of the server's migration feature settings.
rollout_maintenance_guard =
codex_rollout::try_acquire_rollout_maintenance_lock(
config.codex_home.as_path(),
)
.ok()
.flatten();
rollout_maintenance_guard.is_some()
} && self
.thread_read(thread_id, /*include_turns*/ false)
.await
.is_ok_and(|thread| thread.history_mode == ThreadHistoryMode::Legacy)));
!known_legacy_history
} else {
false
codex-rs/tui/src/app_server_session/rollout_history_tests.rs
+49 -64
View File
@@ -7,6 +7,7 @@ use codex_app_server_protocol::ThreadHistoryMode;
use codex_features::Feature;
use codex_protocol::ThreadId;
use color_eyre::eyre::Result;
use futures::FutureExt;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
@@ -59,80 +60,64 @@ async fn legacy_resume_preserves_history_mode_after_picker_server_replacement()
}
#[tokio::test]
async fn background_migration_disables_cached_legacy_resume_shortcut() -> Result<()> {
let codex_home = tempfile::tempdir().expect("tempdir");
let config = build_config(&codex_home).await;
let legacy_thread_id = ThreadId::from_string(
&create_fake_rollout(
codex_home.path(),
"2025-01-05T12-00-00",
"2025-01-05T12:00:00Z",
"Saved legacy user message",
Some(config.model_provider_id.as_str()),
/*git_info*/ None,
)
.expect("create legacy rollout"),
)?;
let thread_id = ThreadId::from_string(
&create_fake_paginated_rollout(
codex_home.path(),
"2025-01-05T12-00-01",
"2025-01-05T12:00:01Z",
"Saved paginated user message",
Some(config.model_provider_id.as_str()),
/*git_info*/ None,
)
.expect("create paginated rollout"),
)?;
let mut migration_config = config.clone();
migration_config
.features
.enable(Feature::BackgroundPaginatedRolloutMigration)?;
for (startup_config, resume_config) in
[(&config, &migration_config), (&migration_config, &config)]
async fn cached_legacy_resume_revalidates_history_across_migration_settings() -> Result<()> {
for (startup_enabled, workspace_enabled) in
[(false, false), (false, true), (true, false), (true, true)]
{
// Keep the real worker disabled while exercising both startup/request mismatches.
let mut app_server = crate::start_embedded_app_server_for_picker(&config)
.await?
.with_startup_config(startup_config);
let codex_home = tempfile::tempdir().expect("tempdir");
let config = build_config(&codex_home).await;
let legacy_thread_id = ThreadId::from_string(
&create_fake_rollout(
codex_home.path(),
"2025-01-05T12-00-00",
"2025-01-05T12:00:00Z",
"Saved legacy user message",
Some(config.model_provider_id.as_str()),
/*git_info*/ None,
)
.expect("create legacy rollout"),
)?;
let mut startup_config = config.clone();
if startup_enabled {
startup_config
.features
.enable(Feature::BackgroundPaginatedRolloutMigration)?;
}
let mut resume_config = config;
if workspace_enabled {
resume_config
.features
.enable(Feature::BackgroundPaginatedRolloutMigration)?;
}
// Keep the real startup worker from migrating the legacy fixture before selection.
let maintenance_guard =
codex_rollout::try_acquire_rollout_maintenance_lock(codex_home.path())?
.expect("acquire rollout maintenance lock");
let mut app_server = crate::start_embedded_app_server_for_picker(&startup_config).await?;
app_server.remember_thread_history_mode(legacy_thread_id, ThreadHistoryMode::Legacy);
let local_settings = crate::local_settings::LocalSettings::from(&resume_config);
let next_request_id = app_server.next_request_id;
let legacy = app_server
.resume_thread(
&crate::local_settings::LocalSettings::from(resume_config),
let legacy = {
let resume = app_server.resume_thread(
&local_settings,
resume_config.clone(),
legacy_thread_id,
ResumeModelSettings::RestoreFromThread,
)
.await?;
);
tokio::pin!(resume);
drop(maintenance_guard);
// This current-thread test polls resume before yielding to the startup worker.
// Resume must acquire its guard before waiting for metadata revalidation.
assert!(resume.as_mut().now_or_never().is_none());
assert!(
codex_rollout::try_acquire_rollout_maintenance_lock(codex_home.path())?.is_none()
);
resume.await?
};
assert_eq!(app_server.next_request_id, next_request_id + 2);
assert!(!legacy.turns.is_empty());
app_server.remember_thread_history_mode(thread_id, ThreadHistoryMode::Legacy);
let next_request_id = app_server.next_request_id;
let resumed = app_server
.resume_thread(
&crate::local_settings::LocalSettings::from(resume_config),
resume_config.clone(),
thread_id,
ResumeModelSettings::RestoreFromThread,
)
.await?;
assert!(app_server.next_request_id > next_request_id + 1);
assert_eq!(resumed.session.thread_id, thread_id);
assert_eq!(
app_server
.history_pagination
.get(&thread_id)
.map(|state| state.history_mode),
Some(ThreadHistoryMode::Paginated)
);
app_server.shutdown().await?;
}
Ok(())
}
codex-rs/tui/src/lib.rs
+6 -3
View File
@@ -557,7 +557,10 @@ pub(crate) async fn start_app_server_for_picker(
environment_manager,
)
.await?;
Ok(AppServerSession::new(app_server, target.thread_params_mode()).with_startup_config(config))
Ok(
AppServerSession::new(app_server, target.thread_params_mode())
.with_local_codex_home(&config.codex_home),
)
}
#[cfg(test)]
@@ -1081,7 +1084,7 @@ async fn run_ratatui_app(
let app_server_session = match startup_app_server {
Ok(Ok(app_server)) => {
AppServerSession::new(app_server, app_server_target.thread_params_mode())
.with_startup_config(&initial_config)
.with_local_codex_home(&initial_config.codex_home)
}
Ok(Err(err)) => {
terminal_restore_guard.restore_silently();
@@ -1693,7 +1696,7 @@ async fn run_ratatui_app(
// A picker can replace the server; account reads belong to their original session.
startup_account = None;
AppServerSession::new(app_server, app_server_target.thread_params_mode())
.with_startup_config(&config)
.with_local_codex_home(&config.codex_home)
.with_remote_cwd_override(remote_cwd_override.clone())
}
Ok(Err(err)) => {