From b7f7e7f7e85a1c63e520f27cac42b836f242d9f7 Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Tue, 27 Oct 2026 04:01:42 +0000 Subject: [PATCH] Separate TUI session resume preparation from activation (#57342) ## What changed Split saved-thread resume into preparation and activation. Preparation resolves configuration and attaches to the destination thread before activation replaces the displayed conversation. Keep stored-backend configuration and local settings separate from active settings during preparation, while retaining the existing eager settings refresh for active-backend resumes. Use the destination backend for working-directory, permission, and directory-trust checks. Carry the backend identity in the prepared result and reject activation through a different backend before changing the active conversation or composer. ## Testing Extend coverage for preserving active settings and composer drafts during preparation, rejecting activation through another backend, applying destination remote-permission checks, and retaining active-backend settings refresh behavior. GitOrigin-RevId: 7171f1abcf4bdd62793c21232a8b895bb809cf70 --- codex-rs/tui/src/app.rs | 1 + codex-rs/tui/src/app/agents_overview.rs | 13 +- codex-rs/tui/src/app/config_persistence.rs | 90 ++++++++-- codex-rs/tui/src/app/resume_config.rs | 156 +++++++++++++----- codex-rs/tui/src/app/resume_preparation.rs | 122 ++++++++++++++ codex-rs/tui/src/app/session_lifecycle.rs | 79 ++++----- codex-rs/tui/src/app/tests.rs | 68 ++++++-- .../src/app/tests/backend_ownership_tests.rs | 49 ++++++ .../tests/terminal_size_telemetry_tests.rs | 8 +- 9 files changed, 463 insertions(+), 123 deletions(-) create mode 100644 codex-rs/tui/src/app/resume_preparation.rs diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index d2bb38e890..ca996b0d27 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -260,6 +260,7 @@ mod reconnect; mod replay_filter; mod resize_reflow; mod resume_config; +mod resume_preparation; mod right_click_paste; pub(crate) use right_click_paste::PasteEnvironment; mod safety_buffering; diff --git a/codex-rs/tui/src/app/agents_overview.rs b/codex-rs/tui/src/app/agents_overview.rs index 82160d24d9..3cb5a425a9 100644 --- a/codex-rs/tui/src/app/agents_overview.rs +++ b/codex-rs/tui/src/app/agents_overview.rs @@ -558,7 +558,12 @@ impl App { history_mode: Some(target_thread.history_mode), }; match self - .resume_config_for_target(tui, app_server, &target_session) + .resume_config_for_target( + tui, + app_server, + &target_session, + super::resume_preparation::ResumeSource::Active, + ) .await { Ok(config) => config, @@ -572,7 +577,11 @@ impl App { }; let current_cwd = self.config.cwd.to_path_buf(); match self - .rebuild_config_for_resume_or_fallback(¤t_cwd, config_cwd) + .rebuild_config_for_resume_or_fallback( + ¤t_cwd, + config_cwd, + (&self.config, &self.local_settings), + ) .await { Ok(config) => config, diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index ef5b912785..8a04d72452 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -96,18 +96,26 @@ impl App { } pub(super) async fn rebuild_config_for_cwd(&self, cwd: PathBuf) -> Result { + self.rebuild_config_for_cwd_from(&self.config, cwd).await + } + + pub(super) async fn rebuild_config_for_cwd_from( + &self, + base: &Config, + cwd: PathBuf, + ) -> Result { let mut overrides = self.harness_overrides.clone(); overrides.cwd = Some(cwd.clone()); let cwd_display = cwd.display().to_string(); let builder = ConfigBuilder::default() - .codex_home(self.config.codex_home.to_path_buf()) + .codex_home(base.codex_home.to_path_buf()) .cli_overrides(self.cli_kv_overrides.clone()) .harness_overrides(overrides) .loader_overrides(self.loader_overrides.clone()) .cloud_config_bundle(self.cloud_config_bundle.clone()); build_config_on_runtime_worker( builder, - self.config.application_network_policy.clone(), + base.application_network_policy.clone(), format!("Failed to rebuild config for cwd {cwd_display}"), ) .await @@ -525,13 +533,17 @@ impl App { } pub(super) async fn rebuild_config_for_resume_or_fallback( - &mut self, + &self, current_cwd: &Path, resume_cwd: PathBuf, + fallback: (&Config, &crate::local_settings::LocalSettings), ) -> Result<(Config, crate::local_settings::LocalSettings)> { - match self.rebuild_config_for_cwd(resume_cwd.clone()).await { + match self + .rebuild_config_for_cwd_from(fallback.0, resume_cwd.clone()) + .await + { Ok(config) => { - let local_settings = self.local_settings.reloaded(&config); + let local_settings = fallback.1.reloaded(&config); Ok((config, local_settings)) } Err(err) => { @@ -544,7 +556,7 @@ impl App { cwd = %resume_cwd_display, "failed to rebuild config for same-cwd resume; using current in-memory config" ); - Ok((self.config.clone(), self.local_settings.clone())) + Ok((fallback.0.clone(), fallback.1.clone())) } } } @@ -554,6 +566,19 @@ impl App { &mut self, config: &mut Config, scope: RuntimePolicyOverrideScope, + ) -> Result<()> { + self.apply_runtime_policy_overrides_with_reviewer( + config, + scope, + self.config.approvals_reviewer, + ) + } + + pub(super) fn apply_runtime_policy_overrides_with_reviewer( + &mut self, + config: &mut Config, + scope: RuntimePolicyOverrideScope, + restored_reviewer: ApprovalsReviewer, ) -> Result<()> { if let Some(policy) = self.runtime_approval_policy_override && (scope == RuntimePolicyOverrideScope::All @@ -584,7 +609,7 @@ impl App { .filter(|profile| { profile.turn_override == RuntimePermissionProfileTurnOverride::Preserve }) - .map(|_| self.config.approvals_reviewer) + .map(|_| restored_reviewer) }) { config .config_layer_stack @@ -1137,10 +1162,12 @@ impl App { resume_model_settings_for_overrides(&self.config, &self.harness_overrides) } - pub(super) fn reject_remote_resume_permission_override(&mut self, config: &Config) -> bool { - if self.app_server_target.thread_params_mode() - != crate::app_server_session::ThreadParamsMode::Remote - { + pub(super) fn reject_remote_resume_permission_override( + &mut self, + config: &Config, + target: &AppServerTarget, + ) -> bool { + if target.thread_params_mode() != crate::app_server_session::ThreadParamsMode::Remote { return false; } let explicitly_selected = @@ -1925,6 +1952,35 @@ enabled = false assert_eq!(app.chat_widget.config_ref().cwd.to_path_buf(), next_cwd); assert_eq!(app.config.cwd, original_cwd); + std::fs::write(app.config.codex_home.join("config.toml"), "")?; + crate::legacy_core::config::set_project_trust_level( + &app.config.codex_home, + &next_cwd, + codex_protocol::config_types::TrustLevel::Trusted, + ) + .map_err(std::io::Error::other)?; + let mut server = crate::start_embedded_app_server_for_picker(&app.config).await?; + let target = SessionTarget { + backend_id: server.backend_id(), + thread_id: ThreadId::new(), + path: None, + cwd: None, + history_mode: None, + }; + let mut tui = crate::tui::test_support::make_test_tui()?; + let candidate = app + .resume_config_for_target( + &mut tui, + &mut server, + &target, + super::resume_preparation::ResumeSource::Active, + ) + .await + .map_err(|_| color_eyre::eyre::eyre!("resume configuration should succeed"))?; + assert_eq!(candidate.0.cwd.to_path_buf(), next_cwd); + assert_eq!(app.config.cwd.to_path_buf(), next_cwd); + server.shutdown().await?; + app.refresh_in_memory_config_from_disk().await?; assert_eq!(app.config.cwd, app.chat_widget.config_ref().cwd); @@ -2038,7 +2094,11 @@ theme = "dracula" let current_cwd = current_config.cwd.clone(); let resume_config = app - .rebuild_config_for_resume_or_fallback(¤t_cwd, current_cwd.to_path_buf()) + .rebuild_config_for_resume_or_fallback( + ¤t_cwd, + current_cwd.to_path_buf(), + (&app.config, &app.local_settings), + ) .await?; assert_eq!(resume_config, (current_config, app.local_settings.clone())); @@ -2056,7 +2116,11 @@ theme = "dracula" let next_cwd = next_cwd_tmp.path().to_path_buf(); let result = app - .rebuild_config_for_resume_or_fallback(¤t_cwd, next_cwd) + .rebuild_config_for_resume_or_fallback( + ¤t_cwd, + next_cwd, + (&app.config, &app.local_settings), + ) .await; assert!(result.is_err()); diff --git a/codex-rs/tui/src/app/resume_config.rs b/codex-rs/tui/src/app/resume_config.rs index 5d6dc7f6e4..b28eb82f91 100644 --- a/codex-rs/tui/src/app/resume_config.rs +++ b/codex-rs/tui/src/app/resume_config.rs @@ -1,6 +1,6 @@ //! Folder-entry consent and configuration for command-center dispatch and resume. //! Keeps CLI/runtime cwd precedence, remote-workspace checks, and interactive prompts aligned. -//! Carries local preferences alongside the resolved configuration for session replacement. +//! Stored-backend settings stay with the candidate; active-backend resume refreshes preferences. use super::*; use crate::onboarding::onboarding_screen::check_directory_trust; @@ -31,13 +31,53 @@ impl App { tui: &mut tui::Tui, app_server: &mut AppServerSession, target_session: &SessionTarget, + source: super::resume_preparation::ResumeSource<'_>, ) -> std::result::Result<(Config, crate::local_settings::LocalSettings), AppRunControl> { if let Err(error) = target_session.ensure_backend(app_server.backend_id()) { self.add_session_picker_error(error.to_string()); return Err(AppRunControl::Continue); } - self.refresh_in_memory_config_from_disk_best_effort("resuming a thread") - .await; + // Active-backend resume keeps its existing eager settings refresh. A stored backend must + // not replace the displayed conversation's settings before attachment succeeds. + let (target, current_config, local_settings) = match source { + super::resume_preparation::ResumeSource::Active => { + self.refresh_in_memory_config_from_disk_best_effort("resuming a thread") + .await; + ( + self.app_server_target.clone(), + self.config.clone(), + self.local_settings.clone(), + ) + } + super::resume_preparation::ResumeSource::Backend(settings) => { + let refreshed = async { + let mut config = self + .rebuild_config_for_cwd_from( + &settings.config, + settings.config.cwd.to_path_buf(), + ) + .await?; + self.apply_runtime_policy_overrides_with_reviewer( + &mut config, + RuntimePolicyOverrideScope::All, + settings.config.approvals_reviewer, + )?; + Ok::<_, color_eyre::Report>(config) + } + .await; + let (config, local_settings) = match refreshed { + Ok(config) => { + let local_settings = settings.local_settings.reloaded(&config); + (config, local_settings) + } + Err(error) => { + tracing::warn!(%error, "failed to refresh resume settings; using stored backend configuration"); + (settings.config.clone(), settings.local_settings.clone()) + } + }; + (settings.target.clone(), config, local_settings) + } + }; let cwd_override = self .runtime_working_directory_override .as_deref() @@ -46,19 +86,17 @@ impl App { .map(Path::to_path_buf); let cwd_override = cwd_override.as_deref(); let resume_cwd_mode = crate::session_resume::effective_resume_cwd_mode( - self.local_settings.tui.resume_cwd, + local_settings.tui.resume_cwd, cwd_override, ); let remembered_current_cwd = cwd_override.unwrap_or(self.launch_cwd.as_path()); let current_cwd = if matches!(resume_cwd_mode, Some(ResumeCwdMode::Current)) { remembered_current_cwd.to_path_buf() } else { - self.config.cwd.to_path_buf() + current_config.cwd.to_path_buf() }; - let uses_remote_workspace_or_environment = crate::uses_remote_workspace_or_environment( - &self.app_server_target, - &self.environment_manager, - ); + let uses_remote_workspace_or_environment = + crate::uses_remote_workspace_or_environment(&target, &self.environment_manager); if uses_remote_workspace_or_environment && self.harness_overrides.cwd.is_none() && app_server.remote_cwd_override().is_none() @@ -70,7 +108,7 @@ impl App { ); return Err(AppRunControl::Continue); } - let resume_cwd = if self.app_server_target.uses_remote_workspace() { + let resume_cwd = if target.uses_remote_workspace() { current_cwd.clone() } else { let history_cwd = if matches!(resume_cwd_mode, Some(ResumeCwdMode::Current)) { @@ -82,7 +120,7 @@ impl App { }; let outcome = crate::session_resume::resolve_cwd_for_resume_or_fork( tui, - &self.config, + ¤t_config, history_cwd, CwdPromptAction::Resume, crate::session_resume::ResumeCwdContext { @@ -110,15 +148,18 @@ impl App { } }; - let (config_current_cwd, config_resume_cwd) = - if self.app_server_target.uses_remote_workspace() { - let local_config_cwd = self.config.cwd.to_path_buf(); - (local_config_cwd.clone(), local_config_cwd) - } else { - (current_cwd, resume_cwd) - }; + let (config_current_cwd, config_resume_cwd) = if target.uses_remote_workspace() { + let local_config_cwd = current_config.cwd.to_path_buf(); + (local_config_cwd.clone(), local_config_cwd) + } else { + (current_cwd, resume_cwd) + }; let mut resume_config = match self - .rebuild_config_for_resume_or_fallback(&config_current_cwd, config_resume_cwd) + .rebuild_config_for_resume_or_fallback( + &config_current_cwd, + config_resume_cwd, + (¤t_config, &local_settings), + ) .await { Ok(cfg) => cfg, @@ -129,30 +170,31 @@ impl App { return Err(AppRunControl::Continue); } }; - if self.reject_remote_resume_permission_override(&resume_config.0) { + if self.reject_remote_resume_permission_override(&resume_config.0, &target) { return Err(AppRunControl::Continue); } - let resumed_thread = - if matches!(self.app_server_target, AppServerTarget::LocalDaemon { .. }) { - Some( - app_server - .thread_read(target_session.thread_id, /*include_turns*/ false) - .await - .map_err(|error| { - self.add_session_picker_error(format!( - "Unable to check resumed folder: {error}" - )); - AppRunControl::Continue - })?, - ) - } else { - None - }; + let resumed_thread = if matches!(target, AppServerTarget::LocalDaemon { .. }) { + Some( + app_server + .thread_read(target_session.thread_id, /*include_turns*/ false) + .await + .map_err(|error| { + self.add_session_picker_error(format!( + "Unable to check resumed folder: {error}" + )); + AppRunControl::Continue + })?, + ) + } else { + None + }; let trust_cwd = resume_config.0.cwd.to_path_buf(); if let Some(local_settings) = self - .confirm_directory_trust( + .confirm_directory_trust_for_target( tui, app_server, + &target, + &local_settings, &mut resume_config.0, &trust_cwd, crate::onboarding::DirectoryTrustOptions { @@ -175,11 +217,41 @@ impl App { config: &mut Config, cwd: &Path, options: crate::onboarding::DirectoryTrustOptions<'_>, + startup_draft: Option<&mut StartupDraftPump>, + ) -> std::result::Result, AppRunControl> { + let target = self.app_server_target.clone(); + let local_settings = self.local_settings.clone(); + self.confirm_directory_trust_for_target( + tui, + app_server, + &target, + &local_settings, + config, + cwd, + options, + startup_draft, + ) + .await + } + + #[expect( + clippy::too_many_arguments, + reason = "The destination target is explicit alongside the existing trust interaction inputs." + )] + async fn confirm_directory_trust_for_target( + &mut self, + tui: &mut tui::Tui, + app_server: &mut AppServerSession, + target: &AppServerTarget, + local_settings: &crate::local_settings::LocalSettings, + config: &mut Config, + cwd: &Path, + options: crate::onboarding::DirectoryTrustOptions<'_>, mut startup_draft: Option<&mut StartupDraftPump>, ) -> std::result::Result, AppRunControl> { // Keep the existing explicit remote --cd gate, including retries after cancellation. // Other remote destinations await authoritative trust-root metadata. - let cwd = if self.app_server_target.uses_remote_workspace() { + let cwd = if target.uses_remote_workspace() { let Some(cwd) = app_server.remote_cwd_override() else { return Ok(None); }; @@ -191,7 +263,7 @@ impl App { tui, app_server, config, - &self.app_server_target, + target, cwd, options, startup_draft.as_deref_mut(), @@ -205,7 +277,7 @@ impl App { if options.cancel == Some(crate::onboarding::TrustCancelAction::CurrentTask) { return Err(AppRunControl::Continue); } - if !self.app_server_target.supports_command_center() { + if !target.supports_command_center() { return Err(AppRunControl::Exit(ExitReason::UserRequested)); } if self @@ -224,7 +296,7 @@ impl App { *config = StartupDraftPump::run_with_optional_draft( startup_draft.as_deref_mut(), tui, - self.rebuild_config_for_cwd(config.cwd.to_path_buf()), + self.rebuild_config_for_cwd_from(config, config.cwd.to_path_buf()), ) .await .map_err(|error| { @@ -287,7 +359,7 @@ impl App { } } } - return Ok(Some(self.local_settings.reloaded(config))); + return Ok(Some(local_settings.reloaded(config))); } Ok(None) } diff --git a/codex-rs/tui/src/app/resume_preparation.rs b/codex-rs/tui/src/app/resume_preparation.rs new file mode 100644 index 0000000000..086329b5c7 --- /dev/null +++ b/codex-rs/tui/src/app/resume_preparation.rs @@ -0,0 +1,122 @@ +//! Prepare saved-thread attachment before replacing the active conversation UI. +//! +//! Stored-backend preparation leaves active settings unchanged; active-backend resume retains its +//! eager preference refresh. Neither detaches the active thread or replaces its composer. The +//! prepared result retains the connection identity. + +use super::*; + +/// Settings retained for a backend that is not currently displayed. +#[derive(Clone)] +pub(super) struct BackendSettings { + pub(super) config: Config, + pub(super) local_settings: crate::local_settings::LocalSettings, + pub(super) target: AppServerTarget, +} + +/// The settings source for a resume attempt. +/// +/// The active source may refresh the displayed conversation's settings eagerly. A stored backend +/// is refreshed without adopting its settings until attachment succeeds. +#[derive(Clone, Copy)] +pub(super) enum ResumeSource<'a> { + Active, + #[cfg_attr(not(test), allow(dead_code, reason = "Used by backend handoff."))] + Backend(&'a BackendSettings), +} + +pub(super) struct PreparedResume { + pub(super) backend_id: crate::backend_id::BackendId, + pub(super) config: Config, + pub(super) local_settings: crate::local_settings::LocalSettings, + pub(super) thread: AppServerStartedThread, + pub(super) read_only: bool, + pub(super) history_notice: Option<&'static str>, +} + +impl App { + #[cfg_attr(not(test), allow(dead_code, reason = "Used by backend handoff."))] + pub(super) fn backend_settings(&self) -> BackendSettings { + BackendSettings { + config: self.config.clone(), + local_settings: self.local_settings.clone(), + target: self.app_server_target.clone(), + } + } + + pub(super) async fn prepare_target_session( + &mut self, + tui: &mut tui::Tui, + app_server: &mut AppServerSession, + target_session: &SessionTarget, + source: ResumeSource<'_>, + ) -> std::result::Result { + let (mut resume_config, local_settings) = self + .resume_config_for_target(tui, app_server, target_session, source) + .await?; + if let Err(error) = self.apply_runtime_policy_overrides( + &mut resume_config, + RuntimePolicyOverrideScope::ExplicitOnly, + ) { + self.add_session_picker_error(format!("{error:#}")); + return Err(AppRunControl::Continue); + } + let permission_overrides = self.resume_permission_overrides(&resume_config); + let model_settings = match source { + ResumeSource::Active => self.resume_model_settings(), + ResumeSource::Backend(_) => { + super::config_persistence::resume_model_settings_for_overrides( + &resume_config, + &self.harness_overrides, + ) + } + }; + + if let Some(history_mode) = target_session.history_mode { + app_server.remember_thread_history_mode(target_session.thread_id, history_mode); + } + let resumed = app_server + .resume_thread_with_permission_overrides( + &local_settings, + resume_config.clone(), + target_session.thread_id, + model_settings, + permission_overrides, + ) + .await; + let mut history_notice = None; + let (resumed, read_only) = match resumed { + Ok(resumed) => (resumed, false), + Err(err) if crate::app_server_session::is_active_writer_error(&err) => match app_server + .read_thread_for_viewing(&resume_config, &local_settings, target_session.thread_id) + .await + { + Ok((thread, notice)) => { + history_notice = notice; + (thread, true) + } + Err(read_err) => { + self.add_session_picker_error(format!( + "Failed to view thread open elsewhere: {read_err}" + )); + return Err(AppRunControl::Continue); + } + }, + Err(err) => { + let path_display = target_session.display_label(); + self.add_session_picker_error(format!( + "Failed to resume session from {path_display}: {err}" + )); + return Err(AppRunControl::Continue); + } + }; + Ok(PreparedResume { + backend_id: app_server.backend_id(), + config: resume_config, + local_settings, + thread: resumed, + read_only, + history_notice, + }) + } +} diff --git a/codex-rs/tui/src/app/session_lifecycle.rs b/codex-rs/tui/src/app/session_lifecycle.rs index 0e4ced1cde..c7dc18d544 100644 --- a/codex-rs/tui/src/app/session_lifecycle.rs +++ b/codex-rs/tui/src/app/session_lifecycle.rs @@ -1362,60 +1362,43 @@ impl App { if self.reject_pending_permission_root_switch() { return Ok(AppRunControl::Continue); } - let (mut resume_config, local_settings) = match self - .resume_config_for_target(tui, app_server, &target_session) + let prepared = match self + .prepare_target_session( + tui, + app_server, + &target_session, + super::resume_preparation::ResumeSource::Active, + ) .await { - Ok(config) => config, + Ok(prepared) => prepared, Err(control) => return Ok(control), }; - if let Err(error) = self.apply_runtime_policy_overrides( - &mut resume_config, - RuntimePolicyOverrideScope::ExplicitOnly, - ) { - self.add_session_picker_error(format!("{error:#}")); + self.activate_prepared_session(tui, app_server, prepared) + .await + } + + pub(super) async fn activate_prepared_session( + &mut self, + tui: &mut tui::Tui, + app_server: &mut AppServerSession, + prepared: super::resume_preparation::PreparedResume, + ) -> Result { + let super::resume_preparation::PreparedResume { + backend_id, + config: resume_config, + local_settings, + thread: resumed, + read_only, + history_notice, + } = prepared; + if backend_id != app_server.backend_id() { + self.add_session_picker_error( + "This session belongs to a different backend. Open the session picker again." + .to_string(), + ); return Ok(AppRunControl::Continue); } - let permission_overrides = self.resume_permission_overrides(&resume_config); - - if let Some(history_mode) = target_session.history_mode { - app_server.remember_thread_history_mode(target_session.thread_id, history_mode); - } - let resumed = app_server - .resume_thread_with_permission_overrides( - &local_settings, - resume_config.clone(), - target_session.thread_id, - self.resume_model_settings(), - permission_overrides, - ) - .await; - let mut history_notice = None; - let (resumed, read_only) = match resumed { - Ok(resumed) => (resumed, false), - Err(err) if crate::app_server_session::is_active_writer_error(&err) => match app_server - .read_thread_for_viewing(&resume_config, &local_settings, target_session.thread_id) - .await - { - Ok((thread, notice)) => { - history_notice = notice; - (thread, true) - } - Err(read_err) => { - self.add_session_picker_error(format!( - "Failed to view thread open elsewhere: {read_err}" - )); - return Ok(AppRunControl::Continue); - } - }, - Err(err) => { - let path_display = target_session.display_label(); - self.add_session_picker_error(format!( - "Failed to resume session from {path_display}: {err}" - )); - return Ok(AppRunControl::Continue); - } - }; let resumed_thread_id = resumed.session.thread_id; let retained_input = (self.chat_widget.is_external_writer_view() && self.chat_widget.thread_id() == Some(resumed_thread_id)) diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 87f5bceb68..cbad3bf375 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -3610,7 +3610,7 @@ async fn server_only_profile_selection_keeps_turns_on_the_selected_profile() -> async fn remote_resume_rejects_explicit_permission_override() -> Result<()> { let (mut app, mut events, _ops) = make_test_app_with_channels().await; let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:8765")?; - app.app_server_target = crate::AppServerTarget::Remote { endpoint }; + let home = tempdir()?; std::fs::write( home.path().join("config.toml"), @@ -3635,18 +3635,23 @@ async fn remote_resume_rejects_explicit_permission_override() -> Result<()> { &app.harness_overrides )); let mut tui = crate::tui::test_support::make_test_tui()?; - app.resume_target_session( - &mut tui, - &mut server, - SessionTarget { - backend_id, - thread_id: ThreadId::new(), - path: None, - cwd: None, - history_mode: None, - }, - ) - .await?; + let mut settings = app.backend_settings(); + settings.target = crate::AppServerTarget::Remote { endpoint }; + let result = app + .prepare_target_session( + &mut tui, + &mut server, + &SessionTarget { + backend_id, + thread_id: ThreadId::new(), + path: None, + cwd: None, + history_mode: None, + }, + super::resume_preparation::ResumeSource::Backend(&settings), + ) + .await; + assert!(matches!(result, Err(AppRunControl::Continue))); assert_eq!(app.active_thread_id, None); let cell = std::iter::from_fn(|| events.try_recv().ok()) .find_map(|event| match event { @@ -8235,6 +8240,7 @@ async fn in_app_resume_session_cwd_without_metadata_is_non_fatal() -> Result<()> )?; app.state_db = None; let active_thread_id = app.chat_widget.thread_id(); + app.chat_widget.insert_str("keep this draft"); let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; let backend_id = app_server.backend_id(); let mut tui = crate::tui::test_support::make_test_tui()?; @@ -8255,6 +8261,10 @@ async fn in_app_resume_session_cwd_without_metadata_is_non_fatal() -> Result<()> assert!(matches!(control, AppRunControl::Continue)); assert_eq!(app.chat_widget.thread_id(), active_thread_id); + assert_eq!( + app.chat_widget.composer_text_with_pending(), + "keep this draft" + ); let cell = match app_event_rx.try_recv() { Ok(AppEvent::InsertHistoryCell(cell)) => cell, other => panic!("expected InsertHistoryCell event, got {other:?}"), @@ -8481,18 +8491,44 @@ async fn in_app_resume_uses_configured_or_explicit_cwd() -> Result<()> { .handle_thread_session_quiet(test_thread_session(ThreadId::new(), active_cwd.clone())); let mut tui = crate::tui::test_support::make_test_tui()?; - let control = app - .resume_target_session( + let settings = app.backend_settings(); + // Prepare a destination while another backend owns the displayed configuration. + let other_home = tempdir()?; + app.config.codex_home = other_home.path().to_path_buf().abs(); + let active_settings = (app.config.clone(), app.local_settings.clone()); + let active_thread_id = app.chat_widget.thread_id(); + app.chat_widget + .insert_str("keep this draft until attachment"); + let prepared = match app + .prepare_target_session( &mut tui, app_server, - crate::resume_picker::SessionTarget { + &crate::resume_picker::SessionTarget { backend_id, path: Some(rollout_path), thread_id, cwd: None, history_mode: None, }, + super::resume_preparation::ResumeSource::Backend(&settings), ) + .await + { + Ok(prepared) => prepared, + Err(_) => panic!("saved thread should be ready for attachment"), + }; + assert_eq!( + (&app.config, &app.local_settings), + (&active_settings.0, &active_settings.1) + ); + assert_eq!(app.chat_widget.thread_id(), active_thread_id); + assert_eq!( + app.chat_widget.composer_text_with_pending(), + "keep this draft until attachment" + ); + assert_eq!(app.chat_widget.config_ref().cwd.as_path(), active_cwd); + let control = app + .activate_prepared_session(&mut tui, app_server, prepared) .await?; assert!(matches!(control, AppRunControl::Continue)); diff --git a/codex-rs/tui/src/app/tests/backend_ownership_tests.rs b/codex-rs/tui/src/app/tests/backend_ownership_tests.rs index b9ccf3fced..587a63f7d6 100644 --- a/codex-rs/tui/src/app/tests/backend_ownership_tests.rs +++ b/codex-rs/tui/src/app/tests/backend_ownership_tests.rs @@ -273,3 +273,52 @@ async fn startup_rejects_foreign_resume_and_fork_before_requests() -> Result<()> } Ok(()) } + +#[tokio::test] +async fn prepared_resume_cannot_activate_through_another_backend() -> Result<()> { + let (mut app, mut events, _) = make_test_app_with_channels().await; + let mut tui = crate::tui::test_support::make_test_tui()?; + let (mut server, requests, proxy) = start_recording_app_server( + &app.config, + /*blocked_thread_list*/ None, + /*failed_thread_name*/ None, + ) + .await?; + let active_thread = ThreadId::new(); + app.enqueue_primary_thread_session( + test_thread_session(active_thread, app.config.cwd.to_path_buf()), + Vec::new(), + ) + .await?; + app.chat_widget.insert_str("keep this draft"); + let prepared = super::super::resume_preparation::PreparedResume { + backend_id: BackendId::new(), + config: app.config.clone(), + local_settings: app.local_settings.clone(), + thread: AppServerStartedThread { + session: test_thread_session(ThreadId::new(), app.config.cwd.to_path_buf()), + turns: Vec::new(), + blocks_direct_input: false, + task_tools_available: true, + }, + read_only: false, + history_notice: None, + }; + requests.lock().unwrap().clear(); + while events.try_recv().is_ok() {} + app.activate_prepared_session(&mut tui, &mut server, prepared) + .await?; + assert_eq!(app.current_displayed_thread_id(), Some(active_thread)); + assert_eq!( + app.chat_widget.composer_text_with_pending(), + "keep this draft" + ); + assert!(requests.lock().unwrap().is_empty()); + let Some(AppEvent::InsertHistoryCell(cell)) = events.recv().await else { + panic!("expected backend mismatch error"); + }; + insta::assert_snapshot!(lines_to_single_string(&cell.display_lines(/*width*/ 100)), @"■ This session belongs to a different backend. Open the session picker again."); + server.shutdown().await?; + proxy.abort(); + Ok(()) +} diff --git a/codex-rs/tui/src/app/tests/terminal_size_telemetry_tests.rs b/codex-rs/tui/src/app/tests/terminal_size_telemetry_tests.rs index 9542f599f2..5075ba1149 100644 --- a/codex-rs/tui/src/app/tests/terminal_size_telemetry_tests.rs +++ b/codex-rs/tui/src/app/tests/terminal_size_telemetry_tests.rs @@ -15,8 +15,12 @@ async fn terminal_size_telemetry_respects_reloaded_exporter_and_consent() -> Res "analytics.enabled = false\n[otel.metrics_exporter.otlp-http]\nendpoint = \"http://127.0.0.1:4318/v1/metrics\"\nprotocol = \"json\"\n", ] { std::fs::write(app.config.codex_home.join("config.toml"), config)?; - let (config, local_settings) = - Box::pin(app.rebuild_config_for_resume_or_fallback(&cwd, cwd.clone())).await?; + let (config, local_settings) = Box::pin(app.rebuild_config_for_resume_or_fallback( + &cwd, + cwd.clone(), + (&app.config, &app.local_settings), + )) + .await?; app.config = config; app.local_settings = local_settings; enabled.push(crate::terminal_size_telemetry::enabled(&app.config));