mirror of
https://github.com/openai/codex.git
synced 2026-10-28 18:58:46 +00:00
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
This commit is contained in:
committed by
copyberry
parent
c587d6e54c
commit
b7f7e7f7e8
@@ -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;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -96,18 +96,26 @@ impl App {
|
||||
}
|
||||
|
||||
pub(super) async fn rebuild_config_for_cwd(&self, cwd: PathBuf) -> Result<Config> {
|
||||
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<Config> {
|
||||
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());
|
||||
|
||||
@@ -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<Option<crate::local_settings::LocalSettings>, 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<Option<crate::local_settings::LocalSettings>, 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)
|
||||
}
|
||||
|
||||
@@ -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<PreparedResume, AppRunControl> {
|
||||
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,
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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<AppRunControl> {
|
||||
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))
|
||||
|
||||
@@ -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));
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
|
||||
@@ -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));
|
||||
|
||||
Reference in New Issue
Block a user