mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Print TUI session info on fatal exits (#27417)
## Summary TUI exits printed the resume/session summary only after checking the exit reason. On fatal exits, both CLI wrappers wrote the error and called `process::exit(1)` immediately, so an active session that ended on a fatal error could skip the session information entirely. This change prints the normal exit summary before returning the fatal nonzero exit code. If a fatal exit has a known thread id but no resumable rollout hint, it prints `Session ID: <id>` instead of staying silent. It also flushes stdout before `process::exit(1)` so the summary line is not lost during process teardown. ## Implementation - Apply the fatal-exit ordering fix in both `codex` and standalone `codex-tui`. - Keep normal user-requested exit behavior unchanged. - Preserve the existing resume hint when a rollout is resumable, and use the raw thread id only as a fatal-exit fallback.
This commit is contained in:
committed by
GitHub
Unverified
parent
df9dd22248
commit
9d88e299a7
+54
-12
@@ -36,10 +36,10 @@ use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_cli::CliConfigOverrides;
|
||||
use codex_utils_cli::ProfileV2Name;
|
||||
use codex_utils_cli::SharedCliOptions;
|
||||
use codex_utils_cli::resume_hint;
|
||||
use owo_colors::OwoColorize;
|
||||
use std::collections::HashSet;
|
||||
use std::io::IsTerminal;
|
||||
use std::io::Write;
|
||||
use std::path::PathBuf;
|
||||
use supports_color::Stream;
|
||||
|
||||
@@ -698,10 +698,11 @@ fn parse_socket_path(raw: &str) -> Result<AbsolutePathBuf, String> {
|
||||
}
|
||||
|
||||
fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec<String> {
|
||||
let is_fatal = matches!(&exit_info.exit_reason, ExitReason::Fatal(_));
|
||||
let AppExitInfo {
|
||||
token_usage,
|
||||
thread_id: conversation_id,
|
||||
thread_name,
|
||||
resume_hint,
|
||||
..
|
||||
} = exit_info;
|
||||
|
||||
@@ -710,13 +711,15 @@ fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec<Stri
|
||||
lines.push(token_usage.to_string());
|
||||
}
|
||||
|
||||
if let Some(resume_cmd) = resume_hint(thread_name.as_deref(), conversation_id) {
|
||||
if let Some(resume_cmd) = resume_hint {
|
||||
let command = if color_enabled {
|
||||
resume_cmd.cyan().to_string()
|
||||
} else {
|
||||
resume_cmd
|
||||
};
|
||||
lines.push(format!("To continue this session, run {command}"));
|
||||
} else if is_fatal && let Some(conversation_id) = conversation_id {
|
||||
lines.push(format!("Session ID: {conversation_id}"));
|
||||
}
|
||||
|
||||
lines
|
||||
@@ -724,19 +727,23 @@ fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec<Stri
|
||||
|
||||
/// Handle the app exit and print the results. Optionally run the update action.
|
||||
fn handle_app_exit(exit_info: AppExitInfo) -> anyhow::Result<()> {
|
||||
match exit_info.exit_reason {
|
||||
let is_fatal = match &exit_info.exit_reason {
|
||||
ExitReason::Fatal(message) => {
|
||||
eprintln!("ERROR: {message}");
|
||||
std::process::exit(1);
|
||||
true
|
||||
}
|
||||
ExitReason::UserRequested => { /* normal exit */ }
|
||||
}
|
||||
ExitReason::UserRequested => false,
|
||||
};
|
||||
|
||||
let update_action = exit_info.update_action;
|
||||
let color_enabled = supports_color::on(Stream::Stdout).is_some();
|
||||
for line in format_exit_messages(exit_info, color_enabled) {
|
||||
println!("{line}");
|
||||
}
|
||||
if is_fatal {
|
||||
std::io::stdout().flush()?;
|
||||
std::process::exit(1);
|
||||
}
|
||||
if let Some(action) = update_action {
|
||||
run_update_action(action)?;
|
||||
}
|
||||
@@ -3037,12 +3044,13 @@ mod tests {
|
||||
total_tokens: 2,
|
||||
..Default::default()
|
||||
};
|
||||
let thread_id = conversation_id
|
||||
.map(ThreadId::from_string)
|
||||
.map(Result::unwrap);
|
||||
AppExitInfo {
|
||||
token_usage,
|
||||
thread_id: conversation_id
|
||||
.map(ThreadId::from_string)
|
||||
.map(Result::unwrap),
|
||||
thread_name: thread_name.map(str::to_string),
|
||||
thread_id,
|
||||
resume_hint: codex_utils_cli::resume_hint(thread_name, thread_id),
|
||||
update_action: None,
|
||||
exit_reason: ExitReason::UserRequested,
|
||||
}
|
||||
@@ -3053,7 +3061,7 @@ mod tests {
|
||||
let exit_info = AppExitInfo {
|
||||
token_usage: TokenUsage::default(),
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
resume_hint: None,
|
||||
update_action: None,
|
||||
exit_reason: ExitReason::UserRequested,
|
||||
};
|
||||
@@ -3061,6 +3069,40 @@ mod tests {
|
||||
assert!(lines.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn format_exit_messages_includes_session_id_for_fatal_exit_without_resume_hint() {
|
||||
let exit_info = AppExitInfo {
|
||||
token_usage: TokenUsage::default(),
|
||||
thread_id: Some(ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap()),
|
||||
resume_hint: None,
|
||||
update_action: None,
|
||||
exit_reason: ExitReason::Fatal("boom".to_string()),
|
||||
};
|
||||
let lines = format_exit_messages(exit_info, /*color_enabled*/ false);
|
||||
assert_eq!(
|
||||
lines,
|
||||
vec!["Session ID: 123e4567-e89b-12d3-a456-426614174000".to_string()]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn format_exit_messages_includes_resume_hint_for_fatal_exit() {
|
||||
let mut exit_info = sample_exit_info(
|
||||
Some("123e4567-e89b-12d3-a456-426614174000"),
|
||||
/*thread_name*/ None,
|
||||
);
|
||||
exit_info.exit_reason = ExitReason::Fatal("boom".to_string());
|
||||
let lines = format_exit_messages(exit_info, /*color_enabled*/ false);
|
||||
assert_eq!(
|
||||
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(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn format_exit_messages_includes_resume_hint_without_color() {
|
||||
let exit_info = sample_exit_info(
|
||||
|
||||
+17
-10
@@ -397,7 +397,7 @@ const COMMIT_ANIMATION_TICK: Duration = tui::TARGET_FRAME_INTERVAL;
|
||||
pub struct AppExitInfo {
|
||||
pub token_usage: TokenUsage,
|
||||
pub thread_id: Option<ThreadId>,
|
||||
pub thread_name: Option<String>,
|
||||
pub resume_hint: Option<String>,
|
||||
pub update_action: Option<UpdateAction>,
|
||||
pub exit_reason: ExitReason,
|
||||
}
|
||||
@@ -407,7 +407,7 @@ impl AppExitInfo {
|
||||
Self {
|
||||
token_usage: TokenUsage::default(),
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
resume_hint: None,
|
||||
update_action: None,
|
||||
exit_reason: ExitReason::Fatal(message.into()),
|
||||
}
|
||||
@@ -433,10 +433,7 @@ fn session_summary(
|
||||
rollout_path: Option<&Path>,
|
||||
) -> Option<SessionSummary> {
|
||||
let usage_line = (!token_usage.is_zero()).then(|| token_usage.to_string());
|
||||
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))
|
||||
});
|
||||
let resume_hint = resume_hint_for_resumable_thread(thread_id, thread_name, rollout_path);
|
||||
|
||||
if usage_line.is_none() && resume_hint.is_none() {
|
||||
return None;
|
||||
@@ -467,6 +464,15 @@ fn resumable_thread(
|
||||
})
|
||||
}
|
||||
|
||||
fn resume_hint_for_resumable_thread(
|
||||
thread_id: Option<ThreadId>,
|
||||
thread_name: Option<String>,
|
||||
rollout_path: Option<&Path>,
|
||||
) -> Option<String> {
|
||||
let thread = resumable_thread(thread_id, thread_name, rollout_path)?;
|
||||
codex_utils_cli::resume_hint(thread.thread_name.as_deref(), Some(thread.thread_id))
|
||||
}
|
||||
|
||||
fn rollout_path_is_resumable(rollout_path: &Path) -> bool {
|
||||
std::fs::metadata(rollout_path).is_ok_and(|metadata| metadata.is_file() && metadata.len() > 0)
|
||||
}
|
||||
@@ -1216,15 +1222,16 @@ See the Codex keymap documentation for supported actions and examples."
|
||||
return Err(err);
|
||||
}
|
||||
};
|
||||
let resumable_thread = resumable_thread(
|
||||
app.chat_widget.thread_id(),
|
||||
let thread_id = app.chat_widget.thread_id().or(app.primary_thread_id);
|
||||
let resume_hint = resume_hint_for_resumable_thread(
|
||||
thread_id,
|
||||
app.chat_widget.thread_name(),
|
||||
app.chat_widget.rollout_path().as_deref(),
|
||||
);
|
||||
Ok(AppExitInfo {
|
||||
token_usage: app.token_usage(),
|
||||
thread_id: resumable_thread.as_ref().map(|thread| thread.thread_id),
|
||||
thread_name: resumable_thread.and_then(|thread| thread.thread_name),
|
||||
thread_id,
|
||||
resume_hint,
|
||||
update_action: app.pending_update_action,
|
||||
exit_reason,
|
||||
})
|
||||
|
||||
@@ -343,7 +343,7 @@ pub(super) async fn handle_model_migration_prompt_if_needed(
|
||||
return Some(AppExitInfo {
|
||||
token_usage: TokenUsage::default(),
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
resume_hint: None,
|
||||
update_action: None,
|
||||
exit_reason: ExitReason::UserRequested,
|
||||
});
|
||||
|
||||
@@ -1381,7 +1381,7 @@ async fn run_ratatui_app(
|
||||
return Ok(AppExitInfo {
|
||||
token_usage: crate::token_usage::TokenUsage::default(),
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
resume_hint: None,
|
||||
update_action: Some(action),
|
||||
exit_reason: ExitReason::UserRequested,
|
||||
});
|
||||
@@ -1474,7 +1474,7 @@ async fn run_ratatui_app(
|
||||
return Ok(AppExitInfo {
|
||||
token_usage: crate::token_usage::TokenUsage::default(),
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
resume_hint: None,
|
||||
update_action: None,
|
||||
exit_reason: ExitReason::UserRequested,
|
||||
});
|
||||
@@ -1524,7 +1524,7 @@ async fn run_ratatui_app(
|
||||
Ok(AppExitInfo {
|
||||
token_usage: crate::token_usage::TokenUsage::default(),
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
resume_hint: None,
|
||||
update_action: None,
|
||||
exit_reason: ExitReason::Fatal(format!(
|
||||
"No saved session found with ID {id_str}. Run `codex {action}` without an ID to choose from existing sessions."
|
||||
@@ -1581,7 +1581,7 @@ async fn run_ratatui_app(
|
||||
return Ok(AppExitInfo {
|
||||
token_usage: crate::token_usage::TokenUsage::default(),
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
resume_hint: None,
|
||||
update_action: None,
|
||||
exit_reason: ExitReason::UserRequested,
|
||||
});
|
||||
@@ -1642,7 +1642,7 @@ async fn run_ratatui_app(
|
||||
return Ok(AppExitInfo {
|
||||
token_usage: crate::token_usage::TokenUsage::default(),
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
resume_hint: None,
|
||||
update_action: None,
|
||||
exit_reason: ExitReason::UserRequested,
|
||||
});
|
||||
@@ -1687,7 +1687,7 @@ async fn run_ratatui_app(
|
||||
return Ok(AppExitInfo {
|
||||
token_usage: crate::token_usage::TokenUsage::default(),
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
resume_hint: None,
|
||||
update_action: None,
|
||||
exit_reason: ExitReason::UserRequested,
|
||||
});
|
||||
|
||||
@@ -7,14 +7,15 @@ use codex_tui::Cli;
|
||||
use codex_tui::ExitReason;
|
||||
use codex_tui::run_main;
|
||||
use codex_utils_cli::CliConfigOverrides;
|
||||
use codex_utils_cli::resume_hint;
|
||||
use std::io::Write;
|
||||
use supports_color::Stream;
|
||||
|
||||
fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec<String> {
|
||||
let is_fatal = matches!(&exit_info.exit_reason, ExitReason::Fatal(_));
|
||||
let AppExitInfo {
|
||||
token_usage,
|
||||
thread_id,
|
||||
thread_name,
|
||||
resume_hint,
|
||||
..
|
||||
} = exit_info;
|
||||
|
||||
@@ -23,13 +24,15 @@ fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec<Stri
|
||||
lines.push(token_usage.to_string());
|
||||
}
|
||||
|
||||
if let Some(resume_cmd) = resume_hint(thread_name.as_deref(), thread_id) {
|
||||
if let Some(resume_cmd) = resume_hint {
|
||||
let command = if color_enabled {
|
||||
format!("\u{1b}[36m{resume_cmd}\u{1b}[39m")
|
||||
} else {
|
||||
resume_cmd
|
||||
};
|
||||
lines.push(format!("To continue this session, run {command}"));
|
||||
} else if is_fatal && let Some(thread_id) = thread_id {
|
||||
lines.push(format!("Session ID: {thread_id}"));
|
||||
}
|
||||
|
||||
lines
|
||||
@@ -59,18 +62,22 @@ fn main() -> anyhow::Result<()> {
|
||||
/*explicit_remote_endpoint*/ None,
|
||||
)
|
||||
.await?;
|
||||
match exit_info.exit_reason {
|
||||
let is_fatal = match &exit_info.exit_reason {
|
||||
ExitReason::Fatal(message) => {
|
||||
eprintln!("ERROR: {message}");
|
||||
std::process::exit(1);
|
||||
true
|
||||
}
|
||||
ExitReason::UserRequested => {}
|
||||
}
|
||||
ExitReason::UserRequested => false,
|
||||
};
|
||||
|
||||
let color_enabled = supports_color::on(Stream::Stdout).is_some();
|
||||
for line in format_exit_messages(exit_info, color_enabled) {
|
||||
println!("{line}");
|
||||
}
|
||||
if is_fatal {
|
||||
std::io::stdout().flush()?;
|
||||
std::process::exit(1);
|
||||
}
|
||||
Ok(())
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user