agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Use app server thread names in TUI picker (#18633)

Browse Source 
## Problem

The TUI resume/fork picker was backfilling thread names from local
rollout indexes. This was left over from before the TUI was moved to the
app server. It should be using app-server APIs because the TUI might be
connected to a remote connection.

This bug wasn't (yet) reported by a user. I found it by asking Codex to
review places in the TUI code where it was still directly accessing the
CODEX_HOME directory rather than going through app-server APIs.

## Solution

The resume picker and session lookups should use app-server thread APIs
only. Remove legacy rollout name/list backfills, and avoid local name
reads in fork history.

## Testing

I manually tested `codex resume` and `codex resume --all` to look for
functional or performance regressions in the resume picker.
This commit is contained in:
Eric Traut
2026-04-20 08:16:24 -07:00
committed by GitHub
Unverified
parent 5a8700abcc
commit 43a69c50eb
6 changed files with 62 additions and 905 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/app.rs
+1 -7
View File
@@ -4620,13 +4620,7 @@ impl App {
tui.frame_requester().schedule_frame();
}
AppEvent::ResumeSessionByIdOrName(id_or_name) => {
match crate::lookup_session_target_with_app_server(
app_server,
self.config.codex_home.as_path(),
&id_or_name,
)
.await?
{
match crate::lookup_session_target_with_app_server(app_server, &id_or_name).await? {
Some(target_session) => {
return self
.resume_target_session(tui, app_server, target_session)
codex-rs/tui/src/chatwidget.rs
+10 -40
View File
@@ -226,7 +226,6 @@ use codex_protocol::request_user_input::RequestUserInputQuestionOption;
use codex_protocol::user_input::ByteRange;
use codex_protocol::user_input::TextElement;
use codex_protocol::user_input::UserInput;
use codex_rollout::find_thread_name_by_id;
use codex_terminal_detection::Multiplexer;
use codex_terminal_detection::TerminalInfo;
use codex_terminal_detection::TerminalName;
@@ -2186,45 +2185,16 @@ impl ChatWidget {
}
fn emit_forked_thread_event(&mut self, forked_from_id: ThreadId) {
let app_event_tx = self.app_event_tx.clone();
let codex_home = self.config.codex_home.clone();
tokio::spawn(async move {
let forked_from_id_text = forked_from_id.to_string();
let send_name_and_id = |name: String| {
let line: Line<'static> = vec![
"• ".dim(),
"Thread forked from ".into(),
name.cyan(),
" (".into(),
forked_from_id_text.clone().cyan(),
")".into(),
]
.into();
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
PlainHistoryCell::new(vec![line]),
)));
};
let send_id_only = || {
let line: Line<'static> = vec![
"• ".dim(),
"Thread forked from ".into(),
forked_from_id_text.clone().cyan(),
]
.into();
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
PlainHistoryCell::new(vec![line]),
)));
};
match find_thread_name_by_id(&codex_home, &forked_from_id).await {
Ok(Some(name)) if !name.trim().is_empty() => send_name_and_id(name),
Ok(_) => send_id_only(),
Err(err) => {
tracing::warn!("Failed to read forked thread name: {err}");
send_id_only();
}
}
});
let forked_from_id_text = forked_from_id.to_string();
let line: Line<'static> = vec![
"• ".dim(),
"Thread forked from ".into(),
forked_from_id_text.cyan(),
]
.into();
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
PlainHistoryCell::new(vec![line]),
)));
}
fn on_thread_name_updated(&mut self, event: codex_protocol::protocol::ThreadNameUpdatedEvent) {
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__forked_thread_history_line.snap
-5
View File
@@ -1,5 +0,0 @@
---
source: tui/src/chatwidget/tests.rs
expression: combined
---
• Thread forked from named-thread (e9f18a88-8081-4e51-9d4e-8af5cde2d8dd)
codex-rs/tui/src/chatwidget/tests/history_replay.rs
-39
View File
@@ -387,45 +387,6 @@ async fn replayed_user_message_with_only_local_images_does_not_render_history_ce
assert!(!found_user_history_cell);
}
#[tokio::test]
async fn forked_thread_history_line_includes_name_and_id_snapshot() {
let (chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
let mut chat = chat;
let temp = tempdir().expect("tempdir");
chat.config.codex_home =
codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(temp.path())
.expect("temp dir is absolute");
let forked_from_id =
ThreadId::from_string("e9f18a88-8081-4e51-9d4e-8af5cde2d8dd").expect("forked id");
let session_index_entry = format!(
"{{\"id\":\"{forked_from_id}\",\"thread_name\":\"named-thread\",\"updated_at\":\"2024-01-02T00:00:00Z\"}}\n"
);
std::fs::write(temp.path().join("session_index.jsonl"), session_index_entry)
.expect("write session index");
chat.emit_forked_thread_event(forked_from_id);
let history_cell = tokio::time::timeout(std::time::Duration::from_secs(2), async {
loop {
match rx.recv().await {
Some(AppEvent::InsertHistoryCell(cell)) => break cell,
Some(_) => continue,
None => panic!("app event channel closed before forked thread history was emitted"),
}
}
})
.await
.expect("timed out waiting for forked thread history");
let combined = lines_to_single_string(&history_cell.display_lines(/*width*/ 80));
assert!(
combined.contains("Thread forked from"),
"expected forked thread message in history"
);
assert_chatwidget_snapshot!("forked_thread_history_line", combined);
}
#[tokio::test]
async fn forked_thread_history_line_without_name_shows_id_once_snapshot() {
let (chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
codex-rs/tui/src/lib.rs
+6 -85
View File
@@ -49,7 +49,6 @@ use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::RolloutLine;
use codex_protocol::protocol::TurnContextItem;
use codex_rollout::find_thread_meta_by_name_str;
use codex_rollout::read_session_meta_line;
use codex_rollout::state_db::get_state_db;
use codex_state::log_db;
@@ -509,7 +508,6 @@ fn session_target_from_app_server_thread(
async fn lookup_session_target_by_name_with_app_server(
app_server: &mut AppServerSession,
codex_home: &Path,
name: &str,
) -> color_eyre::Result<Option<resume_picker::SessionTarget>> {
let mut cursor = None;
@@ -535,15 +533,7 @@ async fn lookup_session_target_by_name_with_app_server(
return Ok(session_target_from_app_server_thread(thread));
}
if response.next_cursor.is_none() {
if app_server.is_remote() {
return Ok(None);
}
return Ok(find_thread_meta_by_name_str(codex_home, name).await?.map(
|(path, session_meta)| resume_picker::SessionTarget {
path: Some(path),
thread_id: session_meta.meta.id,
},
));
return Ok(None);
}
cursor = response.next_cursor;
}
@@ -551,7 +541,6 @@ async fn lookup_session_target_by_name_with_app_server(
async fn lookup_session_target_with_app_server(
app_server: &mut AppServerSession,
codex_home: &Path,
id_or_name: &str,
) -> color_eyre::Result<Option<resume_picker::SessionTarget>> {
if Uuid::parse_str(id_or_name).is_ok() {
@@ -582,7 +571,7 @@ async fn lookup_session_target_with_app_server(
};
}
lookup_session_target_by_name_with_app_server(app_server, codex_home, id_or_name).await
lookup_session_target_by_name_with_app_server(app_server, id_or_name).await
}
async fn lookup_latest_session_target_with_app_server(
@@ -1202,13 +1191,7 @@ async fn run_ratatui_app(
let Some(startup_app_server) = app_server.as_mut() else {
unreachable!("app server should be initialized for --fork <id>");
};
match lookup_session_target_with_app_server(
startup_app_server,
config.codex_home.as_path(),
id_str,
)
.await?
{
match lookup_session_target_with_app_server(startup_app_server, id_str).await? {
Some(target_session) => resume_picker::SessionSelection::Fork(target_session),
None => {
shutdown_app_server_if_present(app_server.take()).await;
@@ -1269,13 +1252,7 @@ async fn run_ratatui_app(
let Some(startup_app_server) = app_server.as_mut() else {
unreachable!("app server should be initialized for --resume <id>");
};
match lookup_session_target_with_app_server(
startup_app_server,
config.codex_home.as_path(),
id_str,
)
.await?
{
match lookup_session_target_with_app_server(startup_app_server, id_str).await? {
Some(target_session) => resume_picker::SessionSelection::Resume(target_session),
None => {
shutdown_app_server_if_present(app_server.take()).await;
@@ -2107,12 +2084,8 @@ mod tests {
AppServerSession::new(codex_app_server_client::AppServerClient::InProcess(
start_test_embedded_app_server(config).await?,
));
let target = lookup_session_target_by_name_with_app_server(
&mut app_server,
temp_dir.path(),
"saved-session",
)
.await?;
let target =
lookup_session_target_by_name_with_app_server(&mut app_server, "saved-session").await?;
let target = target.expect("name lookup should find the saved thread");
assert_eq!(target.path, Some(rollout_path));
assert_eq!(target.thread_id, thread_id);
@@ -2121,58 +2094,6 @@ mod tests {
Ok(())
}
#[tokio::test]
async fn lookup_session_target_by_name_falls_back_to_legacy_index() -> color_eyre::Result<()> {
let temp_dir = TempDir::new()?;
let config = build_config(&temp_dir).await?;
let thread_id = ThreadId::new();
let rollout_path = temp_dir
.path()
.join("sessions/2025/02/01")
.join(format!("rollout-2025-02-01T10-00-00-{thread_id}.jsonl"));
std::fs::create_dir_all(rollout_path.parent().expect("rollout parent"))?;
let session_meta = SessionMeta {
id: thread_id,
timestamp: "2025-02-01T10:00:00Z".to_string(),
model_provider: Some(config.model_provider_id.clone()),
..SessionMeta::default()
};
let line = RolloutLine {
timestamp: session_meta.timestamp.clone(),
item: RolloutItem::SessionMeta(SessionMetaLine {
meta: session_meta,
git: None,
}),
};
std::fs::write(
&rollout_path,
format!("{}\n", serde_json::to_string(&line)?),
)?;
std::fs::write(
temp_dir.path().join("session_index.jsonl"),
format!(
"{{\"id\":\"{thread_id}\",\"thread_name\":\"hello\",\"updated_at\":\"2025-02-02T10:00:00Z\"}}\n"
),
)?;
let mut app_server =
AppServerSession::new(codex_app_server_client::AppServerClient::InProcess(
start_test_embedded_app_server(config).await?,
));
let target = lookup_session_target_by_name_with_app_server(
&mut app_server,
temp_dir.path(),
"hello",
)
.await?;
let target = target.expect("legacy name lookup should find the saved thread");
assert_eq!(target.path, Some(rollout_path));
assert_eq!(target.thread_id, thread_id);
app_server.shutdown().await?;
Ok(())
}
#[tokio::test]
async fn embedded_app_server_start_failure_is_returned() -> color_eyre::Result<()> {
let temp_dir = TempDir::new()?;
codex-rs/tui/src/resume_picker.rs
+45 -729
View File
File diff suppressed because it is too large Load Diff