From b132fec00044c255957659c5370f2c69813422f2 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 21 May 2026 09:33:17 -0700 Subject: [PATCH] Improve `/goal` error messages for ephemeral sessions (#23796) ## Why When a user runs `/goal` in a temporary session, the TUI can currently surface an internal app-server failure such as `thread/goal/get failed in TUI`. That message is technically true, but it does not explain the actual constraint: goals require a saved session because goal state is persisted with the thread. This is especially confusing when `codex doctor` reports the background app-server as running in ephemeral mode, since that wording is easy to conflate with ephemeral thread/session behavior. ## What changed - Added a TUI-side formatter for thread-goal RPC failures in `codex-rs/tui/src/app/thread_goal_actions.rs`. - Detects app-server/core errors that indicate goals are unsupported for an ephemeral thread/session. - Replaces the internal RPC failure with a user-facing explanation: ```text Goals need a saved session. This session is temporary. Run `codex` to start a saved session, or `codex resume` / `/resume` to reopen one. ``` - Preserves the existing generic failure wording for non-ephemeral goal errors. ## Verification - `cargo test -p codex-tui thread_goal_error_message --lib` I also tried `cargo test -p codex-tui`; it built successfully but the test runner aborted in an unrelated side-thread stack overflow (`app::tests::discard_side_thread_removes_agent_navigation_entry`), which reproduced when run by itself. --- ...emeral_error_message_renders_snapshot.snap | 9 ++ codex-rs/tui/src/app/thread_goal_actions.rs | 91 +++++++++++++++++-- 2 files changed, 93 insertions(+), 7 deletions(-) create mode 100644 codex-rs/tui/src/app/snapshots/codex_tui__app__thread_goal_actions__tests__thread_goal_ephemeral_error_message_renders_snapshot.snap diff --git a/codex-rs/tui/src/app/snapshots/codex_tui__app__thread_goal_actions__tests__thread_goal_ephemeral_error_message_renders_snapshot.snap b/codex-rs/tui/src/app/snapshots/codex_tui__app__thread_goal_actions__tests__thread_goal_ephemeral_error_message_renders_snapshot.snap new file mode 100644 index 000000000..d2faad28d --- /dev/null +++ b/codex-rs/tui/src/app/snapshots/codex_tui__app__thread_goal_actions__tests__thread_goal_ephemeral_error_message_renders_snapshot.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/app/thread_goal_actions.rs +expression: terminal.backend() +--- + + +■ Goals need a saved session. This session is temporary. +Run `codex` to start a saved session, or `codex resume` / `/resume` to +reopen one. diff --git a/codex-rs/tui/src/app/thread_goal_actions.rs b/codex-rs/tui/src/app/thread_goal_actions.rs index 89b5465a9..b9115ac28 100644 --- a/codex-rs/tui/src/app/thread_goal_actions.rs +++ b/codex-rs/tui/src/app/thread_goal_actions.rs @@ -11,6 +11,11 @@ use crate::goal_display::goal_usage_summary; use codex_app_server_protocol::ThreadGoalStatus; use codex_protocol::ThreadId; +const EPHEMERAL_THREAD_GOAL_ERROR_MESSAGE: &str = concat!( + "Goals need a saved session. This session is temporary.\n", + "Run `codex` to start a saved session, or `codex resume` / `/resume` to reopen one.", +); + impl App { pub(super) async fn open_thread_goal_menu( &mut self, @@ -26,7 +31,7 @@ impl App { Ok(response) => response, Err(err) => { self.chat_widget - .add_error_message(format!("Failed to read thread goal: {err}")); + .add_error_message(thread_goal_error_message("read", &err)); return; } }; @@ -91,7 +96,7 @@ impl App { Ok(response) => response, Err(err) => { self.chat_widget - .add_error_message(format!("Failed to read thread goal: {err}")); + .add_error_message(thread_goal_error_message("read", &err)); return; } }; @@ -125,7 +130,7 @@ impl App { Ok(_) => {} Err(err) => { self.chat_widget - .add_error_message(format!("Failed to read thread goal: {err}")); + .add_error_message(thread_goal_error_message("read", &err)); return; } } @@ -140,7 +145,7 @@ impl App { return; } self.chat_widget - .add_error_message(format!("Failed to replace thread goal: {err}")); + .add_error_message(thread_goal_error_message("replace", &err)); return; } } @@ -170,7 +175,7 @@ impl App { Err(err) => { let action = if replacing_goal { "replace" } else { "set" }; self.chat_widget - .add_error_message(format!("Failed to {action} thread goal: {err}")); + .add_error_message(thread_goal_error_message(action, &err)); } } } @@ -200,7 +205,7 @@ impl App { ), Err(err) => self .chat_widget - .add_error_message(format!("Failed to update thread goal: {err}")), + .add_error_message(thread_goal_error_message("update", &err)), } } @@ -228,7 +233,7 @@ impl App { } Err(err) => self .chat_widget - .add_error_message(format!("Failed to clear thread goal: {err}")), + .add_error_message(thread_goal_error_message("clear", &err)), } } @@ -274,3 +279,75 @@ impl App { ); } } + +fn thread_goal_error_message(action: &str, err: &color_eyre::Report) -> String { + if is_ephemeral_thread_goal_error(err) { + EPHEMERAL_THREAD_GOAL_ERROR_MESSAGE.to_string() + } else { + format!("Failed to {action} thread goal: {err}") + } +} + +fn is_ephemeral_thread_goal_error(err: &color_eyre::Report) -> bool { + err.chain().any(|cause| { + let message = cause.to_string(); + message.contains("ephemeral thread does not support goals") + || message.contains("thread goals require a persisted thread; this thread is ephemeral") + }) +} + +#[cfg(test)] +mod tests { + use crate::history_cell::HistoryCell; + use pretty_assertions::assert_eq; + use ratatui::layout::Rect; + + use super::*; + + #[test] + fn thread_goal_error_message_explains_temporary_session() { + let err = color_eyre::eyre::eyre!( + "thread/goal/get failed: ephemeral thread does not support goals: thread-1" + ) + .wrap_err("thread/goal/get failed in TUI"); + + assert_eq!( + thread_goal_error_message("read", &err), + EPHEMERAL_THREAD_GOAL_ERROR_MESSAGE + ); + } + + #[test] + fn thread_goal_ephemeral_error_message_renders_snapshot() { + let err = color_eyre::eyre::eyre!( + "thread/goal/get failed: ephemeral thread does not support goals: thread-1" + ) + .wrap_err("thread/goal/get failed in TUI"); + let cell = crate::history_cell::new_error_event(thread_goal_error_message("read", &err)); + let width = 72; + let height = 6; + let backend = crate::test_backend::VT100Backend::new(width, height); + let mut terminal = + crate::custom_terminal::Terminal::with_options(backend).expect("terminal"); + terminal.set_viewport_area(Rect::new(0, height - 1, width, 1)); + + crate::insert_history::insert_history_lines( + &mut terminal, + cell.display_lines(/*width*/ width), + ) + .expect("insert history lines"); + + insta::assert_snapshot!(terminal.backend()); + } + + #[test] + fn thread_goal_error_message_preserves_generic_failure_context() { + let err = + color_eyre::eyre::eyre!("server disappeared").wrap_err("thread/goal/get failed in TUI"); + + assert_eq!( + thread_goal_error_message("read", &err), + "Failed to read thread goal: thread/goal/get failed in TUI" + ); + } +}