Defer managed worktree transitions to fresh TUI loop iterations (#43298)

## Why

Managed worktree transitions run through the event handler into the large synchronous `ChatWidget` constructor. Split this work so each stage runs on a fresh event-loop stack.

## What changed

- Queue worktree setup, checkout completion and configuration loading, session transition, and widget attachment as separate stages with boxed futures.
- Recheck the source session before transitioning and report retained checkout cleanup instructions when the transition cannot proceed.
- Keep the old configuration paired with the old widget until attachment, and protect input during the handoff.

## Testing

Update worktree lifecycle and submission tests to drain the deferred stages explicitly and assert that checkout creation starts before completion.

GitOrigin-RevId: 0435f56d08bf3a6392d35b5c1cfdcc0f1fe02753
This commit is contained in:
Benjamin Carlsson
2026-09-06 22:34:30 +00:00
committed by copyberry
parent 3cd6004dc4
commit 52e12e0cb5
10 changed files with 235 additions and 36 deletions
+8
View File
@@ -635,7 +635,15 @@ pub(crate) struct App {
tokio::sync::broadcast::Sender<codex_app_server_protocol::ThreadStatusChangedNotification>,
dynamic_tool_tasks: HashMap<codex_app_server_protocol::RequestId, (String, JoinHandle<()>)>,
pending_startup_thread_start: bool,
/// Starts worktree setup after the event handler returns, with a fresh stack.
pending_start_managed_worktree: Option<(crate::app_event::ManagedWorktreeMode, Option<String>)>,
pending_managed_worktree_creation: bool,
/// Defers checkout completion and config loading until the event handler returns.
pending_managed_worktree_created: Option<Box<crate::app_event::ManagedWorktreeCreated>>,
/// Defers the saved-history fork until the event handler has returned.
pending_managed_worktree_transition: Option<Box<crate::app_event::ManagedWorktreeTransition>>,
/// Holds notifications until the new widget is attached on a fresh loop iteration.
pending_managed_worktree_attach: Option<Box<working_directory::ManagedWorktreeAttach>>,
/// Keeps protected screens quarantined until initialized chat receives genuine user input.
startup_protected_input_boundary: bool,
/// Keeps that boundary armed while a startup approval waits for the typing-idle timer.
+7 -2
View File
@@ -92,10 +92,15 @@ impl App {
}
}
AppEvent::StartManagedWorktree { mode, name } => {
self.start_managed_worktree(app_server, mode, name).await;
if self.pending_start_managed_worktree.is_some() {
self.chat_widget
.add_error_message("A worktree is already being created.".to_string());
} else {
self.pending_start_managed_worktree = Some((mode, name));
}
}
AppEvent::ManagedWorktreeCreated(created) => {
self.finish_managed_worktree(tui, app_server, *created).await;
self.pending_managed_worktree_created = Some(created);
}
AppEvent::BrowseManagedWorktrees => {
if let Some(request) = self.chat_widget.request_managed_worktrees() {
@@ -5,6 +5,7 @@
use super::*;
use crate::app_event::ManagedWorktreeCreated;
use crate::app_event::ManagedWorktreeTransition;
use crate::history_cell::McpInventoryLoadingCell as LoadingCell;
use codex_app_server_protocol::ThreadBackgroundTerminalsListParams;
use codex_app_server_protocol::ThreadBackgroundTerminalsListResponse as ListResponse;
@@ -186,12 +187,7 @@ impl App {
}
}
pub(super) async fn finish_managed_worktree(
&mut self,
tui: &mut tui::Tui,
app_server: &mut AppServerSession,
created: ManagedWorktreeCreated,
) {
pub(super) async fn finish_managed_worktree(&mut self, created: ManagedWorktreeCreated) {
self.pending_managed_worktree_creation = false;
let (manager, checkout) = match created.result {
Ok(created) => created,
@@ -230,19 +226,30 @@ impl App {
);
}
}
if let Err(error) = self
.switch_to_managed_worktree(
tui,
app_server,
manager,
checkout.clone(),
created.mode,
created.name,
)
.await
{
self.retained_worktree_error(&checkout, error.to_string());
}
let config = match self.rebuild_config_for_cwd(checkout.cwd.clone()).await {
Ok(config) if config.active_project.trust_level.is_some() => config,
Ok(_) => {
return self.retained_worktree_error(
&checkout,
"The new worktree is not trusted; run Codex there.",
);
}
Err(error) => {
return self.retained_worktree_error(
&checkout,
format!("Cannot load the new worktree configuration: {error}"),
);
}
};
self.pending_managed_worktree_transition = Some(Box::new(ManagedWorktreeTransition {
source_thread_id: created.source_thread_id,
source_cwd: created.source_cwd,
manager,
checkout,
config: Box::new(config),
mode: created.mode,
name: created.name,
}));
}
fn retained_worktree_error(
+26
View File
@@ -690,7 +690,11 @@ See the Codex keymap documentation for supported actions and examples."
dynamic_tool_status_updates,
dynamic_tool_tasks: HashMap::new(),
pending_startup_thread_start,
pending_start_managed_worktree: None,
pending_managed_worktree_creation: false,
pending_managed_worktree_created: None,
pending_managed_worktree_transition: None,
pending_managed_worktree_attach: None,
startup_protected_input_boundary: true,
startup_pending_protected_request: false,
rate_limit_hard_stop_generation: 0,
@@ -884,6 +888,28 @@ See the Codex keymap documentation for supported actions and examples."
Ok(exit_reason)
} else {
loop {
if let Some((mode, name)) = app.pending_start_managed_worktree.take() {
Box::pin(app.start_managed_worktree(&mut app_server, mode, name)).await;
continue;
}
// Complete the fork and widget attachment on separate fresh loop iterations.
if let Some(attach) = app.pending_managed_worktree_attach.take() {
Box::pin(app.attach_working_directory(tui, &mut app_server, *attach)).await;
continue;
}
if let Some(transition) = app.pending_managed_worktree_transition.take() {
if let Err(error) =
Box::pin(app.switch_to_managed_worktree(tui, &mut app_server, *transition))
.await
{
app.chat_widget.add_error_message(error.to_string());
}
continue;
}
if let Some(created) = app.pending_managed_worktree_created.take() {
Box::pin(app.finish_managed_worktree(*created)).await;
continue;
}
if app.reconnect.offline && !app.reconnect.failed && reconnect.is_none() {
reconnect = Some(Box::pin(reconnect::reconnect(
app.app_server_target.clone(),
+4
View File
@@ -80,7 +80,11 @@ pub(super) async fn make_test_app() -> App {
dynamic_tool_status_updates: tokio::sync::broadcast::channel(/*capacity*/ 64).0,
dynamic_tool_tasks: HashMap::new(),
pending_startup_thread_start: false,
pending_start_managed_worktree: None,
pending_managed_worktree_creation: false,
pending_managed_worktree_created: None,
pending_managed_worktree_transition: None,
pending_managed_worktree_attach: None,
startup_protected_input_boundary: false,
startup_pending_protected_request: false,
rate_limit_hard_stop_generation: 0,
+14
View File
@@ -63,6 +63,12 @@ use crate::app_backtrack::BacktrackSelection;
use crate::app_backtrack::BacktrackState;
use crate::app_backtrack::user_count;
use crate::app_event::HistoryBatchEntryResponse;
async fn drain_managed_worktree_start(app: &mut App, server: &mut AppServerSession) {
if let Some((mode, name)) = app.pending_start_managed_worktree.take() {
app.start_managed_worktree(server, mode, name).await;
}
}
use codex_utils_absolute_path::test_support::PathExt;
use crate::chatwidget::ChatWidgetInit;
@@ -5540,6 +5546,10 @@ async fn make_test_app() -> App {
dynamic_tool_tasks: HashMap::new(),
pending_startup_thread_start: false,
pending_managed_worktree_creation: false,
pending_start_managed_worktree: None,
pending_managed_worktree_created: None,
pending_managed_worktree_transition: None,
pending_managed_worktree_attach: None,
startup_protected_input_boundary: false,
startup_pending_protected_request: false,
rate_limit_hard_stop_generation: 0,
@@ -5627,6 +5637,10 @@ pub(super) async fn make_test_app_with_channels() -> (
dynamic_tool_tasks: HashMap::new(),
pending_startup_thread_start: false,
pending_managed_worktree_creation: false,
pending_start_managed_worktree: None,
pending_managed_worktree_created: None,
pending_managed_worktree_transition: None,
pending_managed_worktree_attach: None,
startup_protected_input_boundary: false,
startup_pending_protected_request: false,
rate_limit_hard_stop_generation: 0,
@@ -47,12 +47,32 @@ async fn complete_managed_worktree_creation(
server: &mut AppServerSession,
events: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
) -> Result<()> {
drain_managed_worktree_start(app, server).await;
assert!(
app.pending_managed_worktree_creation,
"checkout task started"
);
loop {
let event = tokio::time::timeout(Duration::from_secs(15), events.recv())
.await?
.ok_or_else(|| color_eyre::eyre::eyre!("worktree creation event channel closed"))?;
if matches!(event, AppEvent::ManagedWorktreeCreated(_)) {
app.handle_event(tui, server, event).await?;
if let Some(created) = app.pending_managed_worktree_created.take() {
app.finish_managed_worktree(*created).await;
}
if let Some(transition) = app.pending_managed_worktree_transition.take() {
assert!(app.pending_managed_worktree_attach.is_none());
if let Err(error) = app
.switch_to_managed_worktree(tui, server, *transition)
.await
{
app.chat_widget.add_error_message(error.to_string());
}
if let Some(attach) = app.pending_managed_worktree_attach.take() {
app.attach_working_directory(tui, server, *attach).await;
}
}
return Ok(());
}
}
@@ -3061,6 +3081,7 @@ terminal_visualization_instructions = true
)
.await?;
assert_eq!(app.chat_widget.thread_id(), Some(original));
drain_managed_worktree_start(&mut app, &mut server).await;
assert!(app.pending_managed_worktree_creation);
let created = loop {
let event = tokio::time::timeout(Duration::from_secs(15), events.recv())
@@ -3091,6 +3112,11 @@ terminal_visualization_instructions = true
AppEvent::ManagedWorktreeCreated(created),
)
.await?;
let created = app
.pending_managed_worktree_created
.take()
.expect("deferred checkout completion");
app.finish_managed_worktree(*created).await;
app.primary_thread_id = Some(original);
assert_eq!(app.chat_widget.thread_id(), Some(original));
assert!(!app.pending_managed_worktree_creation);
@@ -3261,6 +3287,7 @@ terminal_visualization_instructions = true
AppEvent::StartManagedWorktree { mode, name: None },
)
.await?;
drain_managed_worktree_start(&mut app, &mut server).await;
let is_new = mode == ManagedWorktreeMode::New;
if is_new {
complete_managed_worktree_creation(&mut app, &mut tui, &mut server, &mut events)
@@ -26,6 +26,7 @@ async fn worktree_creation_event_requires_feature() -> Result<()> {
},
)
.await?;
drain_managed_worktree_start(&mut app, &mut app_server).await;
let message = std::iter::from_fn(|| events.try_recv().ok())
.find_map(|event| match event {
AppEvent::InsertHistoryCell(cell) => {
@@ -72,6 +73,7 @@ async fn worktree_creation_rejects_untrusted_source_before_allocation() -> Resul
AppEvent::StartManagedWorktree { mode, name: None },
)
.await?;
drain_managed_worktree_start(&mut app, &mut app_server).await;
let message = std::iter::from_fn(|| events.try_recv().ok())
.find_map(|event| match event {
AppEvent::InsertHistoryCell(cell) => {
@@ -118,6 +120,7 @@ async fn worktree_creation_rejects_running_agent_before_allocation() -> Result<(
},
)
.await?;
drain_managed_worktree_start(&mut app, &mut server).await;
assert!(!app.pending_managed_worktree_creation);
assert!(!home.path().join("worktrees").exists());
@@ -152,6 +155,7 @@ async fn turn_start_failure_is_shown_without_exiting() -> Result<()> {
let op = next_user_turn_op(&mut op_rx);
while let Ok(event) = app_event_rx.try_recv() {
app.handle_event(&mut tui, &mut app_server, event).await?;
drain_managed_worktree_start(&mut app, &mut app_server).await;
}
let control = app
+107 -15
View File
@@ -1,13 +1,30 @@
//! Local directory transitions with fresh or preserved conversation history.
//! Local directory transitions and managed worktrees with fresh or preserved conversation history.
//! Managed transitions and widget attachment run separately at the top of the event loop.
use super::session_lifecycle::ThreadAttachPresentation;
use super::*;
use crate::app_event::ManagedWorktreeTransition;
use crate::app_server_session::ForkGoalContinuation::DeferUntilNextTurn;
use crate::history_cell::McpInventoryLoadingCell as LoadingCell;
use crate::terminal_visualization_instructions::with_terminal_visualization_instructions;
use codex_app_server_protocol::ThreadBackgroundTerminalsListParams;
use codex_app_server_protocol::ThreadBackgroundTerminalsListResponse as ListResponse;
enum DestinationConfig {
Load,
Prepared(Box<Config>),
}
/// Session and configuration prepared before the event loop attaches a managed checkout.
pub(super) struct ManagedWorktreeAttach {
started: AppServerStartedThread,
config: Box<Config>,
local_settings: crate::local_settings::LocalSettings,
keymap: RuntimeKeymap,
cwd: AbsolutePathBuf,
name_error: Option<String>,
}
impl App {
pub(super) fn working_directory_error(&mut self, message: impl Into<String>) {
self.chat_widget.add_error_message(message.into());
@@ -17,12 +34,43 @@ impl App {
&mut self,
tui: &mut tui::Tui,
app_server: &mut AppServerSession,
manager: codex_worktree::WorktreeManager,
checkout: codex_worktree::ManagedWorktree,
mode: crate::app_event::ManagedWorktreeMode,
name: Option<String>,
transition: ManagedWorktreeTransition,
) -> Result<()> {
let previous_thread_id = self.chat_widget.thread_id();
let source_thread_id = transition.source_thread_id;
let source_cwd = &transition.source_cwd;
let checkout = &transition.checkout;
let mode = transition.mode;
if self.reconnect.offline
|| (mode == crate::app_event::ManagedWorktreeMode::Fork
&& self.chat_widget.has_misalignment_policy_violation())
{
self.chat_widget.add_error_message(format!(
"Cannot continue into the new worktree while the session is offline or blocked by a policy warning. An unused checkout was created at {}; remove it with `git worktree remove <checkout-path>` from the source repository.",
checkout.root.display()
));
return Ok(());
}
if self.primary_thread_id != Some(source_thread_id)
|| self.config.cwd.as_path() != source_cwd.as_path()
|| !self
.chat_widget
.can_change_working_directory(source_thread_id)
{
self.chat_widget.add_error_message(format!(
"The source conversation changed while creating the worktree. An unused checkout was created at {}; remove it with `git worktree remove <checkout-path>` from the source repository.",
checkout.root.display()
));
return Ok(());
}
let ManagedWorktreeTransition {
manager,
checkout,
config,
mode,
name,
..
} = transition;
let checkout_root = checkout.root.clone();
let cwd = AbsolutePathBuf::from_absolute_path(checkout.cwd.clone())
.map_err(|error| color_eyre::eyre::eyre!(error.to_string()))?;
self.change_working_directory_with_managed(
@@ -30,11 +78,13 @@ impl App {
app_server,
cwd,
Some((manager, checkout, mode, name)),
DestinationConfig::Prepared(config),
)
.await;
if self.chat_widget.thread_id() == previous_thread_id {
if self.pending_managed_worktree_attach.is_none() {
return Err(color_eyre::eyre::eyre!(
"Could not start a session in the managed worktree"
"Could not start a session in the managed worktree. A checkout was retained at {}; remove it with `git worktree remove <checkout-path>` from the source repository if it is no longer needed.",
checkout_root.display()
));
}
Ok(())
@@ -47,7 +97,11 @@ impl App {
cwd: AbsolutePathBuf,
) {
self.change_working_directory_with_managed(
tui, app_server, cwd, /*managed_worktree*/ None,
tui,
app_server,
cwd,
/*managed_worktree*/ None,
DestinationConfig::Load,
)
.await;
}
@@ -63,6 +117,7 @@ impl App {
crate::app_event::ManagedWorktreeMode,
Option<String>,
)>,
destination_config: DestinationConfig,
) {
if self.config.ephemeral || !cwd.as_path().is_dir() {
return self.working_directory_error("This task cannot be safely replaced.");
@@ -94,9 +149,14 @@ impl App {
.iter()
.filter_map(|(id, agent)| (!agent.is_closed).then_some(*id))
.collect();
let mut config = match self.rebuild_config_for_cwd(cwd.to_path_buf()).await {
Ok(config) => config,
Err(err) => return self.working_directory_error(format!("Cannot load {cwd:?}: {err}")),
let mut config = match destination_config {
DestinationConfig::Prepared(config) => *config,
DestinationConfig::Load => match self.rebuild_config_for_cwd(cwd.to_path_buf()).await {
Ok(config) => config,
Err(err) => {
return self.working_directory_error(format!("Cannot load {cwd:?}: {err}"));
}
},
};
if config.active_project.trust_level.is_none() {
return self.working_directory_error("This directory is not trusted; run Codex there.");
@@ -301,8 +361,40 @@ impl App {
tracing::warn!("failed to unsubscribe tracked thread {tracked_id}: {error}");
}
}
let attach = ManagedWorktreeAttach {
started: transitioned,
config: Box::new(config),
local_settings,
keymap,
cwd,
name_error,
};
if managed_worktree.is_some() {
// Let the large synchronous ChatWidget constructor run on a fresh event-loop stack.
// Keep the old config paired with the old widget until the continuation runs.
self.startup_protected_input_boundary = true;
self.pending_managed_worktree_attach = Some(Box::new(attach));
} else {
self.attach_working_directory(tui, app_server, attach).await;
}
}
pub(super) async fn attach_working_directory(
&mut self,
tui: &mut tui::Tui,
app_server: &mut AppServerSession,
attach: ManagedWorktreeAttach,
) {
let ManagedWorktreeAttach {
started,
config,
local_settings,
keymap,
cwd,
name_error,
} = attach;
self.local_settings = local_settings;
self.config = config;
self.config = *config;
self.file_search
.update_search_dir(self.config.cwd.to_path_buf());
let notify = &self.local_settings.tui.notification_settings;
@@ -310,9 +402,9 @@ impl App {
if let Err(error) = tui.clear_ambient_pet_image() {
tracing::warn!(%error, "failed to clear ambient pet image");
}
let attach = App::replace_chat_widget_with_app_server_thread;
let attach_widget = App::replace_chat_widget_with_app_server_thread;
let (lineage, message) = (ThreadAttachPresentation::SessionLineage, None);
if let Err(error) = attach(self, tui, transitioned, lineage, message).await {
if let Err(error) = attach_widget(self, tui, started, lineage, message).await {
return self.working_directory_error(format!("Could not restore session: {error}"));
}
if let Some(error) = name_error {
+12
View File
@@ -74,6 +74,18 @@ pub(crate) enum ManagedWorktreeMode {
}
/// Checkout creation result returned from the blocking Git task to the TUI event loop.
/// Prepared checkout and destination configuration consumed on a fresh event-loop stack.
#[derive(Debug)]
pub(crate) struct ManagedWorktreeTransition {
pub(crate) source_thread_id: ThreadId,
pub(crate) source_cwd: AbsolutePathBuf,
pub(crate) manager: codex_worktree::WorktreeManager,
pub(crate) checkout: codex_worktree::ManagedWorktree,
pub(crate) config: Box<crate::legacy_core::config::Config>,
pub(crate) mode: ManagedWorktreeMode,
pub(crate) name: Option<String>,
}
#[derive(Debug)]
pub(crate) struct ManagedWorktreeCreated {
pub(crate) source_thread_id: ThreadId,