clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-24 14:45:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix tui_app_server agent picker closed-state regression (#16014)

Browse Source 
Addresses #15992

The app-server TUI was treating tracked agent threads as closed based on
listener-task bookkeeping that does not reflect live thread state during
normal thread switching. That caused the `/agent` picker to gray out
live agents and could show a false "Agent thread ... is closed" replay
message after switching branches.

This PR fixes the picker refresh path to query the app server for each
tracked thread and derive closed vs loaded state from `thread/read`
status, while preserving cached agent metadata for replay-only threads.
This commit is contained in:
Eric Traut
2026-03-27 19:05:43 -06:00
committed by GitHub
parent 8e24d5aaea
commit ed977b42ac
2 changed files with 173 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

187
codex-rs/tui_app_server/src/app.rs
View File

@@ -2626,18 +2626,52 @@ impl App {
/// refreshes from the backend. Refresh failures are treated as "thread is only inspectable by
/// historical id now" and converted into closed picker entries instead of deleting them, so
/// the stable traversal order remains intact for review and keyboard navigation.
async fn open_agent_picker(&mut self) {
async fn open_agent_picker(&mut self, app_server: &mut AppServerSession) {
let thread_ids: Vec<ThreadId> = self.thread_event_channels.keys().cloned().collect();
for thread_id in thread_ids {
if self.thread_event_listener_tasks.contains_key(&thread_id) {
if self.agent_navigation.get(&thread_id).is_none() {
let existing_entry = self.agent_navigation.get(&thread_id).cloned();
match app_server
.thread_read(thread_id, /*include_turns*/ false)
.await
{
Ok(thread) => {
self.upsert_agent_picker_thread(
thread_id, /*agent_nickname*/ None, /*agent_role*/ None,
/*is_closed*/ false,
thread_id,
thread.agent_nickname.or_else(|| {
existing_entry
.as_ref()
.and_then(|entry| entry.agent_nickname.clone())
}),
thread.agent_role.or_else(|| {
existing_entry
.as_ref()
.and_then(|entry| entry.agent_role.clone())
}),
matches!(
thread.status,
codex_app_server_protocol::ThreadStatus::NotLoaded
),
);
}
} else {
self.mark_agent_picker_thread_closed(thread_id);
Err(err) => {
let is_closed = Self::closed_state_for_thread_read_error(
&err,
existing_entry.as_ref().map(|entry| entry.is_closed),
);
if let Some(entry) = existing_entry {
self.upsert_agent_picker_thread(
thread_id,
entry.agent_nickname,
entry.agent_role,
is_closed,
);
} else {
self.upsert_agent_picker_thread(
thread_id, /*agent_nickname*/ None, /*agent_role*/ None,
is_closed,
);
}
}
}
}
@@ -2698,6 +2732,18 @@ impl App {
});
}
fn is_terminal_thread_read_error(err: &color_eyre::Report) -> bool {
err.chain()
.any(|cause| cause.to_string().contains("thread not loaded:"))
}
fn closed_state_for_thread_read_error(
err: &color_eyre::Report,
existing_is_closed: Option<bool>,
) -> bool {
Self::is_terminal_thread_read_error(err) || existing_is_closed.unwrap_or(false)
}
/// Updates cached picker metadata and then mirrors any visible-label change into the footer.
///
/// These two writes stay paired so the picker rows and contextual footer continue to describe
@@ -4816,7 +4862,7 @@ impl App {
self.chat_widget.open_approvals_popup();
}
AppEvent::OpenAgentPicker => {
self.open_agent_picker().await;
self.open_agent_picker(app_server).await;
}
AppEvent::SelectAgentThread(thread_id) => {
self.select_agent_thread(tui, app_server, thread_id).await?;
@@ -6855,11 +6901,15 @@ mod tests {
#[tokio::test]
async fn open_agent_picker_keeps_missing_threads_for_replay() -> Result<()> {
let mut app = make_test_app().await;
let mut app_server =
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
.await
.expect("embedded app server");
let thread_id = ThreadId::new();
app.thread_event_channels
.insert(thread_id, ThreadEventChannel::new(1));
app.open_agent_picker().await;
app.open_agent_picker(&mut app_server).await;
assert_eq!(app.thread_event_channels.contains_key(&thread_id), true);
assert_eq!(
@@ -6875,8 +6925,12 @@ mod tests {
}
#[tokio::test]
async fn open_agent_picker_keeps_cached_closed_threads() -> Result<()> {
async fn open_agent_picker_preserves_cached_metadata_for_replay_threads() -> Result<()> {
let mut app = make_test_app().await;
let mut app_server =
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
.await
.expect("embedded app server");
let thread_id = ThreadId::new();
app.thread_event_channels
.insert(thread_id, ThreadEventChannel::new(1));
@@ -6884,10 +6938,10 @@ mod tests {
thread_id,
Some("Robie".to_string()),
Some("explorer".to_string()),
false,
true,
);
app.open_agent_picker().await;
app.open_agent_picker(&mut app_server).await;
assert_eq!(app.thread_event_channels.contains_key(&thread_id), true);
assert_eq!(
@@ -6901,12 +6955,113 @@ mod tests {
Ok(())
}
#[tokio::test]
async fn open_agent_picker_marks_terminal_read_errors_closed() -> Result<()> {
let mut app = make_test_app().await;
let mut app_server =
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
.await
.expect("embedded app server");
let thread_id = ThreadId::new();
app.thread_event_channels
.insert(thread_id, ThreadEventChannel::new(1));
app.agent_navigation.upsert(
thread_id,
Some("Robie".to_string()),
Some("explorer".to_string()),
false,
);
app.open_agent_picker(&mut app_server).await;
assert_eq!(
app.agent_navigation.get(&thread_id),
Some(&AgentPickerThreadEntry {
agent_nickname: Some("Robie".to_string()),
agent_role: Some("explorer".to_string()),
is_closed: true,
})
);
Ok(())
}
#[test]
fn terminal_thread_read_error_detection_matches_not_loaded_errors() {
let err = color_eyre::eyre::eyre!(
"thread/read failed during TUI session lookup: thread/read failed: thread not loaded: thr_123"
);
assert!(App::is_terminal_thread_read_error(&err));
}
#[test]
fn terminal_thread_read_error_detection_ignores_transient_failures() {
let err = color_eyre::eyre::eyre!(
"thread/read failed during TUI session lookup: thread/read transport error: broken pipe"
);
assert!(!App::is_terminal_thread_read_error(&err));
}
#[test]
fn closed_state_for_thread_read_error_preserves_live_state_without_cache_on_transient_error() {
let err = color_eyre::eyre::eyre!(
"thread/read failed during TUI session lookup: thread/read transport error: broken pipe"
);
assert!(!App::closed_state_for_thread_read_error(
&err, /*existing_is_closed*/ None
));
}
#[test]
fn closed_state_for_thread_read_error_marks_terminal_uncached_threads_closed() {
let err = color_eyre::eyre::eyre!(
"thread/read failed during TUI session lookup: thread/read failed: thread not loaded: thr_123"
);
assert!(App::closed_state_for_thread_read_error(
&err, /*existing_is_closed*/ None
));
}
#[tokio::test]
async fn open_agent_picker_marks_loaded_threads_open() -> Result<()> {
let mut app = make_test_app().await;
let mut app_server =
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
.await
.expect("embedded app server");
let started = app_server
.start_thread(app.chat_widget.config_ref())
.await?;
let thread_id = started.session.thread_id;
app.thread_event_channels
.insert(thread_id, ThreadEventChannel::new(1));
app.open_agent_picker(&mut app_server).await;
assert_eq!(
app.agent_navigation.get(&thread_id),
Some(&AgentPickerThreadEntry {
agent_nickname: None,
agent_role: None,
is_closed: false,
})
);
Ok(())
}
#[tokio::test]
async fn open_agent_picker_prompts_to_enable_multi_agent_when_disabled() -> Result<()> {
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
let mut app_server =
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
.await
.expect("embedded app server");
let _ = app.config.features.disable(Feature::Collab);
app.open_agent_picker().await;
app.open_agent_picker(&mut app_server).await;
app.chat_widget
.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
@@ -7461,11 +7616,15 @@ guardian_approval = true
async fn open_agent_picker_allows_existing_agent_threads_when_feature_is_disabled() -> Result<()>
{
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
let mut app_server =
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
.await
.expect("embedded app server");
let thread_id = ThreadId::new();
app.thread_event_channels
.insert(thread_id, ThreadEventChannel::new(1));
app.open_agent_picker().await;
app.open_agent_picker(&mut app_server).await;
app.chat_widget
.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));

6
codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__permissions_selection_history_after_mode_switch.snap.new
View File

@@ -1,6 +0,0 @@
---
source: tui_app_server/src/chatwidget/tests.rs
assertion_line: 10252
expression: "lines_to_single_string(&cells[0])"
---
• Permissions updated to Guardian Approvals