mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
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.
This commit is contained in:
committed by
GitHub
Unverified
parent
c07f66c9ec
commit
b132fec000
+9
@@ -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.
|
||||
@@ -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"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user