mirror of
https://github.com/openai/codex.git
synced 2026-09-28 08:43:01 +08:00
Preserve blank TUI sessions when switching tasks (#48628)
## Why Untouched threads have no rollout for `thread/resume` yet. Switching away from a blank startup or fresh session needs to preserve its live subscription, draft, and settings so it can be reopened. ## What changed - Retain blank startup and fresh sessions for non-ephemeral connections to an external app server, and save their input state before navigating away. - Restore the current thread name and apply background settings updates after restoring drafts, so saved model choices do not overwrite newer server settings. ## Testing Extend the task-switching regression test to cover blank startup and fresh sessions, preserved drafts and model choices, updated thread names, and restoration without `thread/resume` or `thread/unsubscribe`. Verify that background model, permission, and approval-policy updates are used for the first submitted turn. GitOrigin-RevId: 958b0cb48a5a873d8d8f5858ac334fecbfad60c7
This commit is contained in:
@@ -381,6 +381,7 @@ impl App {
|
||||
if startup_draft.is_none() {
|
||||
loading::draw(tui)?;
|
||||
}
|
||||
let mut restored_blank_session = false;
|
||||
if self.primary_thread_id != Some(root_thread_id) {
|
||||
let previous_displayed_thread_id = self.current_displayed_thread_id();
|
||||
if let Some(id) = previous_displayed_thread_id
|
||||
@@ -560,7 +561,10 @@ impl App {
|
||||
{
|
||||
// An untouched task has no rollout for thread/resume yet. Its live
|
||||
// subscription and saved settings are sufficient to restore the editor.
|
||||
(blank.clone(), false)
|
||||
restored_blank_session = true;
|
||||
let mut blank = blank.clone();
|
||||
blank.session.thread_name.clone_from(&target_thread.name);
|
||||
(blank, false)
|
||||
} else {
|
||||
match app_server
|
||||
.resume_thread(
|
||||
@@ -778,8 +782,12 @@ impl App {
|
||||
.await;
|
||||
}
|
||||
if self.current_displayed_thread_id() == Some(root_thread_id)
|
||||
&& let Some(input_state) = self.agents_overview.input_states.remove(&root_thread_id)
|
||||
&& let Some(mut input_state) = self.agents_overview.input_states.remove(&root_thread_id)
|
||||
{
|
||||
// A saved draft includes model settings, so apply newer server settings after it.
|
||||
let pending_settings = restored_blank_session
|
||||
.then(|| input_state.pending_thread_settings.take())
|
||||
.flatten();
|
||||
let preserve_in_flight_turn = !read_only
|
||||
&& self
|
||||
.active_turn_id_for_thread(root_thread_id)
|
||||
@@ -791,6 +799,9 @@ impl App {
|
||||
preserve_in_flight_turn,
|
||||
},
|
||||
);
|
||||
if let Some(settings) = pending_settings {
|
||||
self.chat_widget.on_thread_settings_updated(settings);
|
||||
}
|
||||
if !preserve_in_flight_turn {
|
||||
self.chat_widget.maybe_send_next_queued_input();
|
||||
}
|
||||
|
||||
@@ -220,6 +220,18 @@ impl App {
|
||||
.or_default();
|
||||
}
|
||||
self.track_agents_overview_notification(¬ification);
|
||||
// Retained blank sessions stay subscribed after their event channels are cleared.
|
||||
if let ServerNotification::ThreadSettingsUpdated(settings) = ¬ification
|
||||
&& let Ok(thread_id) = ThreadId::from_string(&settings.thread_id)
|
||||
&& self.agents_overview.blank_sessions.contains_key(&thread_id)
|
||||
&& !self.thread_event_channels.contains_key(&thread_id)
|
||||
{
|
||||
self.apply_thread_settings_to_cached_session(thread_id, &settings.thread_settings)
|
||||
.await;
|
||||
if let Some(input) = self.agents_overview.input_states.get_mut(&thread_id) {
|
||||
input.pending_thread_settings = Some(settings.clone());
|
||||
}
|
||||
}
|
||||
if matches!(
|
||||
¬ification,
|
||||
ServerNotification::ThreadStarted(_)
|
||||
|
||||
@@ -895,6 +895,22 @@ impl App {
|
||||
if started.blocks_direct_input {
|
||||
self.mark_primary_thread_parent_owned(thread_id);
|
||||
}
|
||||
// Lifecycle notifications may arrive before the thread/start response.
|
||||
if !self.config.ephemeral
|
||||
&& !matches!(self.app_server_target, AppServerTarget::Embedded)
|
||||
&& !self.pending_primary_events.iter().any(|event| {
|
||||
matches!(event, ThreadBufferedEvent::Notification(notification)
|
||||
if matches!(notification.as_ref(),
|
||||
ServerNotification::TurnStarted(_)
|
||||
| ServerNotification::ThreadClosed(_)
|
||||
| ServerNotification::ThreadArchived(_)
|
||||
| ServerNotification::ThreadDeleted(_)))
|
||||
})
|
||||
{
|
||||
self.agents_overview
|
||||
.blank_sessions
|
||||
.insert(thread_id, started.clone());
|
||||
}
|
||||
// A full usage read can finish before thread/start. Apply its cached fallback
|
||||
// after attachment but before the initial prompt or queued draft is submitted.
|
||||
let recovery_was_pending = self.chat_widget.hold_rate_limit_recovery();
|
||||
@@ -975,6 +991,18 @@ impl App {
|
||||
.await
|
||||
{
|
||||
Ok(mut started) => {
|
||||
if let Some(thread_id) = self.current_displayed_thread_id()
|
||||
&& let Some(blank) = self.agents_overview.blank_sessions.get_mut(&thread_id)
|
||||
{
|
||||
if let Some(channel) = self.thread_event_channels.get(&thread_id)
|
||||
&& let Some(session) = channel.store.lock().await.session.as_ref()
|
||||
{
|
||||
blank.session = session.clone();
|
||||
}
|
||||
if let Some(input) = self.chat_widget.capture_thread_input_state() {
|
||||
self.agents_overview.input_states.insert(thread_id, input);
|
||||
}
|
||||
}
|
||||
self.detach_current_thread_for_navigation(
|
||||
app_server,
|
||||
Some(started.session.thread_id),
|
||||
@@ -998,6 +1026,14 @@ impl App {
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let thread_id = started.session.thread_id;
|
||||
if !self.config.ephemeral
|
||||
&& !matches!(self.app_server_target, AppServerTarget::Embedded)
|
||||
{
|
||||
self.agents_overview
|
||||
.blank_sessions
|
||||
.insert(thread_id, started.clone());
|
||||
}
|
||||
if let Err(err) = self
|
||||
.replace_chat_widget_with_app_server_thread(
|
||||
tui,
|
||||
@@ -1007,6 +1043,7 @@ impl App {
|
||||
)
|
||||
.await
|
||||
{
|
||||
self.agents_overview.blank_sessions.remove(&thread_id);
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Failed to attach to fresh app-server thread: {err}"
|
||||
));
|
||||
|
||||
@@ -809,16 +809,51 @@ async fn command_center_new_restores_blank_drafts_and_builtin_permissions() -> R
|
||||
.await?;
|
||||
let mut tui = make_test_tui()?;
|
||||
tui.pause_events();
|
||||
let started = server.start_thread(&app.config).await?;
|
||||
let startup = started.session.thread_id;
|
||||
app.pending_startup_thread_start = true;
|
||||
app.handle_startup_thread_started(&mut server, Ok(started))
|
||||
.await?;
|
||||
app.new_agents_overview_session(&mut tui, &mut server, /*cwd*/ None)
|
||||
.await?;
|
||||
app.select_agents_overview_thread(&mut tui, &mut server, startup)
|
||||
.await?;
|
||||
assert_eq!(app.chat_widget.thread_id(), Some(startup));
|
||||
app.chat_widget.set_model("gpt-local-choice");
|
||||
app.start_fresh_session_with_summary_hint(
|
||||
&mut tui,
|
||||
&mut server,
|
||||
/*session_start_source*/ None,
|
||||
/*initial_user_message*/ None,
|
||||
/*new_thread_name*/ None,
|
||||
)
|
||||
.await;
|
||||
let first = app.chat_widget.thread_id().unwrap();
|
||||
server
|
||||
.thread_set_name(first, "Blank session".into())
|
||||
.await?;
|
||||
app.chat_widget.insert_str("Keep this unsent draft");
|
||||
app.new_agents_overview_session(&mut tui, &mut server, /*cwd*/ None)
|
||||
.await?;
|
||||
let other = app.chat_widget.thread_id().unwrap();
|
||||
app.select_agents_overview_thread(&mut tui, &mut server, startup)
|
||||
.await?;
|
||||
assert_eq!(app.chat_widget.current_model(), "gpt-local-choice");
|
||||
app.select_agents_overview_thread(&mut tui, &mut server, first)
|
||||
.await?;
|
||||
assert_eq!(app.chat_widget.thread_id(), Some(first));
|
||||
assert_eq!(
|
||||
app.chat_widget.thread_name().as_deref(),
|
||||
Some("Blank session")
|
||||
);
|
||||
assert!(recorded_params(&requests, "thread/resume").is_empty());
|
||||
assert!(
|
||||
recorded_params(&requests, "thread/unsubscribe")
|
||||
.iter()
|
||||
.all(|params| {
|
||||
params["threadId"] != startup.to_string() && params["threadId"] != first.to_string()
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
app.chat_widget.composer_text_with_pending(),
|
||||
"Keep this unsent draft"
|
||||
@@ -897,6 +932,57 @@ async fn command_center_new_restores_blank_drafts_and_builtin_permissions() -> R
|
||||
"Keep this unsent draft"
|
||||
);
|
||||
assert!(recorded_params(&requests, "turn/start").is_empty());
|
||||
app.select_agents_overview_thread(&mut tui, &mut server, startup)
|
||||
.await?;
|
||||
app.chat_widget.insert_str("Background draft");
|
||||
app.select_agents_overview_thread(&mut tui, &mut server, first)
|
||||
.await?;
|
||||
assert!(
|
||||
server
|
||||
.thread_settings_update(codex_app_server_protocol::ThreadSettingsUpdateParams {
|
||||
thread_id: startup.to_string(),
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
approvals_reviewer: Some(codex_app_server_protocol::ApprovalsReviewer::User),
|
||||
permissions: Some(":read-only".into()),
|
||||
model: Some("gpt-5.5".into()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?
|
||||
);
|
||||
let settings = next_thread_settings_updated(&mut server, startup).await;
|
||||
app.handle_app_server_event(
|
||||
&server,
|
||||
codex_app_server_client::AppServerEvent::ServerNotification(Box::new(
|
||||
ServerNotification::ThreadSettingsUpdated(settings),
|
||||
)),
|
||||
)
|
||||
.await;
|
||||
app.select_agents_overview_thread(&mut tui, &mut server, startup)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
app.chat_widget.composer_text_with_pending(),
|
||||
"Background draft"
|
||||
);
|
||||
assert_eq!(app.chat_widget.current_model(), "gpt-5.5");
|
||||
let op = app
|
||||
.chat_widget
|
||||
.submit_user_message_as_plain_user_turn(crate::chatwidget::UserMessage::from("Hello"))
|
||||
.expect("submit the first turn");
|
||||
app.submit_thread_op(&mut server, startup, op).await?;
|
||||
let turns = recorded_params(&requests, "turn/start");
|
||||
let params = turns.last().expect("turn/start was sent");
|
||||
assert_eq!(
|
||||
(
|
||||
¶ms["permissions"],
|
||||
¶ms["approvalPolicy"],
|
||||
¶ms["model"]
|
||||
),
|
||||
(
|
||||
&serde_json::json!(":read-only"),
|
||||
&serde_json::json!("on-request"),
|
||||
&serde_json::json!("gpt-5.5")
|
||||
)
|
||||
);
|
||||
server.shutdown().await?;
|
||||
proxy.await??;
|
||||
Ok(())
|
||||
|
||||
@@ -562,6 +562,7 @@ impl ChatWidget {
|
||||
.questions
|
||||
.as_deref_mut()
|
||||
.map(crate::bottom_pane::AsyncQuestions::capture),
|
||||
pending_thread_settings: None,
|
||||
composer: composer.has_content().then_some(composer),
|
||||
safety_buffering_prompt: self.safety_buffering_prompt.clone(),
|
||||
safety_buffering_source: self.safety_buffering_source,
|
||||
|
||||
@@ -1809,6 +1809,7 @@ async fn restore_thread_input_state_applies_running_state_policy() {
|
||||
});
|
||||
let input_state = ThreadInputState {
|
||||
questions: None,
|
||||
pending_thread_settings: None,
|
||||
composer: Some(ThreadComposerState {
|
||||
text: "composer draft".to_string(),
|
||||
..Default::default()
|
||||
|
||||
@@ -378,6 +378,7 @@ async fn restore_thread_input_state_restores_pending_steers_without_downgrading_
|
||||
|
||||
chat.restore_thread_input_state(
|
||||
Some(ThreadInputState {
|
||||
pending_thread_settings: None,
|
||||
questions: None,
|
||||
composer: None,
|
||||
safety_buffering_prompt: None,
|
||||
|
||||
@@ -133,6 +133,8 @@ impl ThreadComposerState {
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub(crate) struct ThreadInputState {
|
||||
pub(crate) questions: Option<crate::bottom_pane::QuestionState>,
|
||||
pub(crate) pending_thread_settings:
|
||||
Option<codex_app_server_protocol::ThreadSettingsUpdatedNotification>,
|
||||
pub(super) composer: Option<ThreadComposerState>,
|
||||
pub(super) safety_buffering_prompt: Option<UserMessage>,
|
||||
pub(super) safety_buffering_source: UserMessageSource,
|
||||
|
||||
Reference in New Issue
Block a user