mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Clarify resume hints for renamed threads (#23234)
Addresses #23181 ## Why Renamed threads can share names, so hints that suggest resuming directly by name are ambiguous. Issue #23181 asks for the picker hint to include the thread name and thread ID in parens so users can disambiguate safely. ## What - Adds a shared resume hint formatter for named threads: run `codex resume`, then select `<name> (<thread-id>)`. - Uses that hint for /rename confirmations, TUI session summaries, and CLI/TUI exit messages. - Keeps direct `codex resume <thread-id>` guidance for unnamed threads. ## Verification Manually verified that message after `/rename` and after `/exit` include session ID in parens. --------- Co-authored-by: Felipe Coury <felipe.coury@openai.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
0d344aca9b
commit
4ac3ea20a2
@@ -38,7 +38,7 @@ use codex_tui::UpdateAction;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_cli::CliConfigOverrides;
|
||||
use codex_utils_cli::ProfileV2Name;
|
||||
use codex_utils_cli::resume_command;
|
||||
use codex_utils_cli::resume_hint;
|
||||
use owo_colors::OwoColorize;
|
||||
use std::io::IsTerminal;
|
||||
use std::path::PathBuf;
|
||||
@@ -629,6 +629,7 @@ fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec<Stri
|
||||
let AppExitInfo {
|
||||
token_usage,
|
||||
thread_id: conversation_id,
|
||||
thread_name,
|
||||
..
|
||||
} = exit_info;
|
||||
|
||||
@@ -637,7 +638,7 @@ fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec<Stri
|
||||
lines.push(token_usage.to_string());
|
||||
}
|
||||
|
||||
if let Some(resume_cmd) = resume_command(/*thread_name*/ None, conversation_id) {
|
||||
if let Some(resume_cmd) = resume_hint(thread_name.as_deref(), conversation_id) {
|
||||
let command = if color_enabled {
|
||||
resume_cmd.cyan().to_string()
|
||||
} else {
|
||||
@@ -2689,7 +2690,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn format_exit_messages_uses_id_even_when_thread_has_name() {
|
||||
fn format_exit_messages_names_picker_item_when_thread_has_name() {
|
||||
let exit_info = sample_exit_info(
|
||||
Some("123e4567-e89b-12d3-a456-426614174000"),
|
||||
Some("my-thread"),
|
||||
@@ -2699,8 +2700,7 @@ mod tests {
|
||||
lines,
|
||||
vec![
|
||||
"Token usage: total=2 input=0 output=2".to_string(),
|
||||
"To continue this session, run codex resume 123e4567-e89b-12d3-a456-426614174000"
|
||||
.to_string(),
|
||||
"To continue this session, run codex resume, then select my-thread (123e4567-e89b-12d3-a456-426614174000)".to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
@@ -410,17 +410,18 @@ fn session_summary(
|
||||
rollout_path: Option<&Path>,
|
||||
) -> Option<SessionSummary> {
|
||||
let usage_line = (!token_usage.is_zero()).then(|| token_usage.to_string());
|
||||
let thread_id =
|
||||
resumable_thread(thread_id, thread_name, rollout_path).map(|thread| thread.thread_id);
|
||||
let resume_command = codex_utils_cli::resume_command(/*thread_name*/ None, thread_id);
|
||||
let resumable_thread = resumable_thread(thread_id, thread_name, rollout_path);
|
||||
let resume_hint = resumable_thread.as_ref().and_then(|thread| {
|
||||
codex_utils_cli::resume_hint(thread.thread_name.as_deref(), Some(thread.thread_id))
|
||||
});
|
||||
|
||||
if usage_line.is_none() && resume_command.is_none() {
|
||||
if usage_line.is_none() && resume_hint.is_none() {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(SessionSummary {
|
||||
usage_line,
|
||||
resume_command,
|
||||
resume_hint,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -459,7 +460,7 @@ fn errors_for_cwd(cwd: &Path, response: &SkillsListResponse) -> Vec<SkillErrorIn
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
struct SessionSummary {
|
||||
usage_line: Option<String>,
|
||||
resume_command: Option<String>,
|
||||
resume_hint: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
|
||||
@@ -152,7 +152,7 @@ impl App {
|
||||
if let Some(usage_line) = summary.usage_line {
|
||||
lines.push(usage_line.into());
|
||||
}
|
||||
if let Some(command) = summary.resume_command {
|
||||
if let Some(command) = summary.resume_hint {
|
||||
let spans = vec![
|
||||
"To continue this session, run ".into(),
|
||||
command.cyan(),
|
||||
|
||||
@@ -473,7 +473,7 @@ impl App {
|
||||
if let Some(usage_line) = summary.usage_line {
|
||||
lines.push(usage_line.into());
|
||||
}
|
||||
if let Some(command) = summary.resume_command {
|
||||
if let Some(command) = summary.resume_hint {
|
||||
let spans = vec!["To continue this session, run ".into(), command.cyan()];
|
||||
lines.push(spans.into());
|
||||
}
|
||||
@@ -701,7 +701,7 @@ impl App {
|
||||
if let Some(usage_line) = summary.usage_line {
|
||||
lines.push(usage_line.into());
|
||||
}
|
||||
if let Some(command) = summary.resume_command {
|
||||
if let Some(command) = summary.resume_hint {
|
||||
let spans =
|
||||
vec!["To continue this session, run ".into(), command.cyan()];
|
||||
lines.push(spans.into());
|
||||
|
||||
@@ -57,13 +57,13 @@ async fn session_summary_includes_resume_hint_for_persisted_rollout() {
|
||||
Some("Token usage: total=12 input=10 output=2".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
summary.resume_command,
|
||||
summary.resume_hint,
|
||||
Some("codex resume 123e4567-e89b-12d3-a456-426614174000".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn session_summary_uses_id_even_when_thread_has_name() {
|
||||
async fn session_summary_names_picker_item_when_thread_has_name() {
|
||||
let usage = TokenUsage {
|
||||
input_tokens: 10,
|
||||
output_tokens: 2,
|
||||
@@ -83,7 +83,10 @@ async fn session_summary_uses_id_even_when_thread_has_name() {
|
||||
)
|
||||
.expect("summary");
|
||||
assert_eq!(
|
||||
summary.resume_command,
|
||||
Some("codex resume 123e4567-e89b-12d3-a456-426614174000".to_string())
|
||||
summary.resume_hint,
|
||||
Some(
|
||||
"codex resume, then select my-session (123e4567-e89b-12d3-a456-426614174000)"
|
||||
.to_string()
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
@@ -166,7 +166,7 @@ use codex_terminal_detection::TerminalInfo;
|
||||
use codex_terminal_detection::TerminalName;
|
||||
use codex_terminal_detection::terminal_info;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_cli::resume_command;
|
||||
use codex_utils_cli::resume_hint;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyEventKind;
|
||||
@@ -1440,16 +1440,14 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
fn rename_confirmation_cell(name: &str, thread_id: Option<ThreadId>) -> PlainHistoryCell {
|
||||
let resume_cmd =
|
||||
resume_command(Some(name), thread_id).unwrap_or_else(|| format!("codex resume {name}"));
|
||||
let name = name.to_string();
|
||||
let line = vec![
|
||||
let mut line = vec![
|
||||
"• ".into(),
|
||||
"Thread renamed to ".into(),
|
||||
name.cyan(),
|
||||
", to resume this thread run ".into(),
|
||||
resume_cmd.cyan(),
|
||||
name.to_string().cyan(),
|
||||
];
|
||||
if let Some(hint) = resume_hint(Some(name), thread_id) {
|
||||
line.extend([". To resume this thread run ".into(), hint.cyan()]);
|
||||
}
|
||||
PlainHistoryCell::new(vec![line.into()])
|
||||
}
|
||||
|
||||
|
||||
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/app_server.rs
|
||||
expression: rendered
|
||||
---
|
||||
• Thread renamed to review-fix. To resume this thread run codex resume, then select review-fix (123e4567-e89b-12d3-a456-426614174000)
|
||||
@@ -861,7 +861,8 @@ async fn live_app_server_invalid_thread_name_update_is_ignored() {
|
||||
#[tokio::test]
|
||||
async fn live_app_server_thread_name_update_shows_resume_hint() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let thread_id = ThreadId::new();
|
||||
let thread_id =
|
||||
ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").expect("thread id");
|
||||
chat.thread_id = Some(thread_id);
|
||||
|
||||
chat.handle_server_notification(
|
||||
@@ -878,8 +879,7 @@ async fn live_app_server_thread_name_update_shows_resume_hint() {
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1);
|
||||
let rendered = lines_to_single_string(&cells[0]);
|
||||
assert!(rendered.contains("Thread renamed to review-fix"));
|
||||
assert!(rendered.contains("codex resume review-fix"));
|
||||
assert_chatwidget_snapshot!("thread_name_update_resume_hint", rendered);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -7,13 +7,14 @@ use codex_tui::Cli;
|
||||
use codex_tui::ExitReason;
|
||||
use codex_tui::run_main;
|
||||
use codex_utils_cli::CliConfigOverrides;
|
||||
use codex_utils_cli::resume_command;
|
||||
use codex_utils_cli::resume_hint;
|
||||
use supports_color::Stream;
|
||||
|
||||
fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec<String> {
|
||||
let AppExitInfo {
|
||||
token_usage,
|
||||
thread_id,
|
||||
thread_name,
|
||||
..
|
||||
} = exit_info;
|
||||
|
||||
@@ -22,7 +23,7 @@ fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec<Stri
|
||||
lines.push(token_usage.to_string());
|
||||
}
|
||||
|
||||
if let Some(resume_cmd) = resume_command(/*thread_name*/ None, thread_id) {
|
||||
if let Some(resume_cmd) = resume_hint(thread_name.as_deref(), thread_id) {
|
||||
let command = if color_enabled {
|
||||
format!("\u{1b}[36m{resume_cmd}\u{1b}[39m")
|
||||
} else {
|
||||
|
||||
@@ -2929,12 +2929,12 @@ fn render_expanded_session_details(
|
||||
width: u16,
|
||||
) -> Vec<Line<'static>> {
|
||||
let reference = state.relative_time_reference.unwrap_or_else(Utc::now);
|
||||
let session = row
|
||||
.thread_name
|
||||
.as_deref()
|
||||
.map(str::to_string)
|
||||
.or_else(|| row.thread_id.map(|thread_id| thread_id.to_string()))
|
||||
.unwrap_or_else(|| "-".to_string());
|
||||
let session = match (row.thread_name.as_deref(), row.thread_id) {
|
||||
(Some(thread_name), Some(thread_id)) => format!("{thread_name} ({thread_id})"),
|
||||
(Some(thread_name), None) => thread_name.to_string(),
|
||||
(None, Some(thread_id)) => thread_id.to_string(),
|
||||
(None, None) => "-".to_string(),
|
||||
};
|
||||
let directory = row
|
||||
.cwd
|
||||
.as_ref()
|
||||
@@ -3387,7 +3387,9 @@ mod tests {
|
||||
let expected_directory =
|
||||
format_directory_display(row.cwd.as_deref().expect("cwd"), /*max_width*/ None);
|
||||
|
||||
assert!(rendered.contains("Session: feat(tui): add raw scrollback mode"));
|
||||
assert!(rendered.contains(
|
||||
"Session: feat(tui): add raw scrollback mode (019dabc1-0ef5-7431-b81c-03037f51f62c)"
|
||||
));
|
||||
assert!(rendered.contains("Created: 17 minutes ago · 2026-05-02 14:31:08"));
|
||||
assert!(rendered.contains("Updated: now · 2026-05-02 14:48:19"));
|
||||
assert!(rendered.contains(&format!("Directory: {expected_directory}")));
|
||||
|
||||
@@ -10,5 +10,6 @@ pub use codex_protocol::config_types::ProfileV2Name;
|
||||
pub use config_override::CliConfigOverrides;
|
||||
pub use format_env_display::format_env_display;
|
||||
pub use resume_command::resume_command;
|
||||
pub use resume_command::resume_hint;
|
||||
pub use sandbox_mode_cli_arg::SandboxModeCliArg;
|
||||
pub use shared_options::SharedCliOptions;
|
||||
|
||||
@@ -19,6 +19,16 @@ pub fn resume_command(thread_name: Option<&str>, thread_id: Option<ThreadId>) ->
|
||||
})
|
||||
}
|
||||
|
||||
pub fn resume_hint(thread_name: Option<&str>, thread_id: Option<ThreadId>) -> Option<String> {
|
||||
let thread_id = thread_id?;
|
||||
match thread_name.filter(|name| !name.is_empty()) {
|
||||
Some(thread_name) => Some(format!(
|
||||
"codex resume, then select {thread_name} ({thread_id})"
|
||||
)),
|
||||
None => resume_command(/*thread_name*/ None, Some(thread_id)),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -61,4 +71,33 @@ mod tests {
|
||||
let command = resume_command(Some("quote'case"), /*thread_id*/ None);
|
||||
assert_eq!(command, Some("codex resume \"quote'case\"".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resume_hint_names_picker_item_with_id() {
|
||||
let thread_id = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap();
|
||||
let hint = resume_hint(Some("my-thread"), Some(thread_id));
|
||||
assert_eq!(
|
||||
hint,
|
||||
Some(
|
||||
"codex resume, then select my-thread (123e4567-e89b-12d3-a456-426614174000)"
|
||||
.to_string()
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resume_hint_uses_direct_id_command_without_name() {
|
||||
let thread_id = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap();
|
||||
let hint = resume_hint(/*thread_name*/ None, Some(thread_id));
|
||||
assert_eq!(
|
||||
hint,
|
||||
Some("codex resume 123e4567-e89b-12d3-a456-426614174000".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resume_hint_requires_thread_id() {
|
||||
let hint = resume_hint(Some("my-thread"), /*thread_id*/ None);
|
||||
assert_eq!(hint, None);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user