From ab82568536d63528e6222eba32661ca329a674ca Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 16 Apr 2026 09:03:55 -0700 Subject: [PATCH] Fix invalid TUI resume hints (#18059) Addresses #18011 Problem: #16987 allowed zero-token TUI exits to print resume hints, which exposed precomputed thread ids before their rollout files were persisted; #17222 made the same invalid hint visible when switching sessions via `/resume`. Solution: Only include resume commands for TUI sessions backed by a materialized non-empty rollout, and cover both missing-rollout and persisted-rollout summary behavior. Testing: Manually verified by pressing Ctrl+D before the first prompt and confirming that no "to continue this session" message was generated. --- codex-rs/tui/src/app.rs | 86 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 78 insertions(+), 8 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 32f90a99b..3fbcd6490 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -325,8 +325,12 @@ fn session_summary( token_usage: TokenUsage, thread_id: Option, thread_name: Option, + rollout_path: Option<&Path>, ) -> Option { let usage_line = (!token_usage.is_zero()).then(|| FinalOutput::from(token_usage).to_string()); + let (thread_id, thread_name) = resumable_thread(thread_id, thread_name, rollout_path) + .map(|thread| (Some(thread.thread_id), thread.thread_name)) + .unwrap_or((None, None)); let resume_command = crate::legacy_core::util::resume_command(thread_name.as_deref(), thread_id); @@ -340,6 +344,29 @@ fn session_summary( }) } +#[derive(Debug, Clone, PartialEq, Eq)] +struct ResumableThread { + thread_id: ThreadId, + thread_name: Option, +} + +fn resumable_thread( + thread_id: Option, + thread_name: Option, + rollout_path: Option<&Path>, +) -> Option { + let thread_id = thread_id?; + let rollout_path = rollout_path?; + rollout_path_is_resumable(rollout_path).then_some(ResumableThread { + thread_id, + thread_name, + }) +} + +fn rollout_path_is_resumable(rollout_path: &Path) -> bool { + std::fs::metadata(rollout_path).is_ok_and(|metadata| metadata.is_file() && metadata.len() > 0) +} + fn errors_for_cwd(cwd: &Path, response: &ListSkillsResponseEvent) -> Vec { response .skills @@ -3448,6 +3475,7 @@ impl App { self.chat_widget.token_usage(), self.chat_widget.thread_id(), self.chat_widget.thread_name(), + self.chat_widget.rollout_path().as_deref(), ); self.shutdown_current_thread(app_server).await; let tracked_thread_ids: Vec = @@ -4126,10 +4154,15 @@ impl App { return Err(err); } }; + let resumable_thread = resumable_thread( + app.chat_widget.thread_id(), + app.chat_widget.thread_name(), + app.chat_widget.rollout_path().as_deref(), + ); Ok(AppExitInfo { token_usage: app.token_usage(), - thread_id: app.chat_widget.thread_id(), - thread_name: app.chat_widget.thread_name(), + thread_id: resumable_thread.as_ref().map(|thread| thread.thread_id), + thread_name: resumable_thread.and_then(|thread| thread.thread_name), update_action: app.pending_update_action, exit_reason, }) @@ -4249,6 +4282,7 @@ impl App { self.chat_widget.token_usage(), self.chat_widget.thread_id(), self.chat_widget.thread_name(), + self.chat_widget.rollout_path().as_deref(), ); match app_server .resume_thread(resume_config.clone(), target_session.thread_id) @@ -4403,6 +4437,7 @@ impl App { self.chat_widget.token_usage(), self.chat_widget.thread_id(), self.chat_widget.thread_name(), + self.chat_widget.rollout_path().as_deref(), ); self.chat_widget .add_plain_history_lines(vec!["/fork".magenta().into()]); @@ -11496,14 +11531,33 @@ guardian_approval = true session_summary( TokenUsage::default(), /*thread_id*/ None, - /*thread_name*/ None + /*thread_name*/ None, + /*rollout_path*/ None, ) .is_none() ); } #[tokio::test] - async fn session_summary_includes_resume_hint() { + async fn session_summary_skips_resume_hint_until_rollout_exists() { + let usage = TokenUsage::default(); + let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let temp_dir = tempdir().expect("temp dir"); + let rollout_path = temp_dir.path().join("rollout.jsonl"); + + assert!( + session_summary( + usage, + Some(conversation), + /*thread_name*/ None, + Some(&rollout_path), + ) + .is_none() + ); + } + + #[tokio::test] + async fn session_summary_includes_resume_hint_for_persisted_rollout() { let usage = TokenUsage { input_tokens: 10, output_tokens: 2, @@ -11511,9 +11565,17 @@ guardian_approval = true ..Default::default() }; let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let temp_dir = tempdir().expect("temp dir"); + let rollout_path = temp_dir.path().join("rollout.jsonl"); + std::fs::write(&rollout_path, "{}\n").expect("write rollout"); - let summary = - session_summary(usage, Some(conversation), /*thread_name*/ None).expect("summary"); + let summary = session_summary( + usage, + Some(conversation), + /*thread_name*/ None, + Some(&rollout_path), + ) + .expect("summary"); assert_eq!( summary.usage_line, Some("Token usage: total=12 input=10 output=2".to_string()) @@ -11533,9 +11595,17 @@ guardian_approval = true ..Default::default() }; let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let temp_dir = tempdir().expect("temp dir"); + let rollout_path = temp_dir.path().join("rollout.jsonl"); + std::fs::write(&rollout_path, "{}\n").expect("write rollout"); - let summary = session_summary(usage, Some(conversation), Some("my-session".to_string())) - .expect("summary"); + let summary = session_summary( + usage, + Some(conversation), + Some("my-session".to_string()), + Some(&rollout_path), + ) + .expect("summary"); assert_eq!( summary.resume_command, Some("codex resume my-session".to_string())