mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Clarify session exit messages (#40670)
## What changed - Report `Session archived: <thread-id>` after successfully archiving the current session. - Show the session ID when no resume hint is available, including for non-fatal exits. - Keep deleted-thread exits distinct from successful archive exits. ## Testing - Add coverage for archive lifecycle handling and exit-message formatting. GitOrigin-RevId: 1812374e4c74fa1b3f11feb323de18a9dbff745b
This commit is contained in:
1 parent
afb797cae6
commit
731d969d03
6 files changed
+86
-16
No files matched your search
@@ -790,9 +790,10 @@ fn handle_app_exit(exit_info: AppExitInfo) -> anyhow::Result<()> {
|
|||||||
eprintln!("ERROR: {message}");
|
eprintln!("ERROR: {message}");
|
||||||
true
|
true
|
||||||
}
|
}
|
||||||
ExitReason::UserRequested | ExitReason::TurnInterrupted | ExitReason::ThreadRemoved => {
|
ExitReason::UserRequested
|
||||||
false
|
| ExitReason::Archived(_)
|
||||||
}
|
| ExitReason::TurnInterrupted
|
||||||
|
| ExitReason::ThreadRemoved => false,
|
||||||
};
|
};
|
||||||
|
|
||||||
let update_action = exit_info.update_action;
|
let update_action = exit_info.update_action;
|
||||||
@@ -3788,6 +3789,32 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn format_exit_messages_includes_session_id_without_resume_hint() {
|
||||||
|
let mut exit_info = sample_exit_info(
|
||||||
|
Some("123e4567-e89b-12d3-a456-426614174000"),
|
||||||
|
/*thread_name*/ None,
|
||||||
|
);
|
||||||
|
exit_info.token_usage = TokenUsage::default();
|
||||||
|
exit_info.resume_hint = None;
|
||||||
|
let lines = exit_info.format_exit_messages(/*color_enabled*/ false);
|
||||||
|
insta::assert_snapshot!(lines.join("\n"), @"Session ID: 123e4567-e89b-12d3-a456-426614174000");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn format_exit_messages_confirms_archive() {
|
||||||
|
let mut exit_info = sample_exit_info(
|
||||||
|
Some("123e4567-e89b-12d3-a456-426614174000"),
|
||||||
|
/*thread_name*/ None,
|
||||||
|
);
|
||||||
|
exit_info.exit_reason = ExitReason::Archived(exit_info.thread_id.unwrap());
|
||||||
|
let lines = exit_info.format_exit_messages(/*color_enabled*/ false);
|
||||||
|
insta::assert_snapshot!(lines.join("\n"), @"
|
||||||
|
Token usage: total=2 input=0 output=2
|
||||||
|
Session archived: 123e4567-e89b-12d3-a456-426614174000
|
||||||
|
");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn format_exit_messages_includes_session_id_for_fatal_exit_without_resume_hint() {
|
fn format_exit_messages_includes_session_id_for_fatal_exit_without_resume_hint() {
|
||||||
let exit_info = AppExitInfo {
|
let exit_info = AppExitInfo {
|
||||||
|
|||||||
@@ -449,8 +449,9 @@ pub(crate) enum AppRunControl {
|
|||||||
#[derive(Debug, Clone)]
|
#[derive(Debug, Clone)]
|
||||||
pub enum ExitReason {
|
pub enum ExitReason {
|
||||||
UserRequested,
|
UserRequested,
|
||||||
|
Archived(ThreadId),
|
||||||
TurnInterrupted,
|
TurnInterrupted,
|
||||||
/// The current thread was archived or deleted, rather than disconnected.
|
/// The current thread was deleted, rather than disconnected.
|
||||||
ThreadRemoved,
|
ThreadRemoved,
|
||||||
Fatal(String),
|
Fatal(String),
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3106,7 +3106,7 @@ impl App {
|
|||||||
}
|
}
|
||||||
|
|
||||||
match app_server.thread_archive(thread_id).await {
|
match app_server.thread_archive(thread_id).await {
|
||||||
Ok(()) => AppRunControl::Exit(ExitReason::ThreadRemoved),
|
Ok(()) => AppRunControl::Exit(ExitReason::Archived(thread_id)),
|
||||||
Err(err) => {
|
Err(err) => {
|
||||||
self.chat_widget
|
self.chat_widget
|
||||||
.add_error_message(format!("Failed to archive current thread: {err}"));
|
.add_error_message(format!("Failed to archive current thread: {err}"));
|
||||||
|
|||||||
@@ -16,7 +16,7 @@ pub struct DisconnectInfo {
|
|||||||
impl App {
|
impl App {
|
||||||
pub(super) fn exit_info(&self, exit_reason: ExitReason) -> AppExitInfo {
|
pub(super) fn exit_info(&self, exit_reason: ExitReason) -> AppExitInfo {
|
||||||
let thread_id = match exit_reason {
|
let thread_id = match exit_reason {
|
||||||
ExitReason::ThreadRemoved => None,
|
ExitReason::Archived(_) | ExitReason::ThreadRemoved => None,
|
||||||
ExitReason::UserRequested | ExitReason::TurnInterrupted | ExitReason::Fatal(_) => {
|
ExitReason::UserRequested | ExitReason::TurnInterrupted | ExitReason::Fatal(_) => {
|
||||||
self.chat_widget.thread_id().or(self.primary_thread_id)
|
self.chat_widget.thread_id().or(self.primary_thread_id)
|
||||||
}
|
}
|
||||||
@@ -85,7 +85,7 @@ impl AppExitInfo {
|
|||||||
{
|
{
|
||||||
let turn_interrupted = matches!(self.exit_reason, ExitReason::TurnInterrupted);
|
let turn_interrupted = matches!(self.exit_reason, ExitReason::TurnInterrupted);
|
||||||
let message = match self.exit_reason {
|
let message = match self.exit_reason {
|
||||||
ExitReason::UserRequested | ExitReason::ThreadRemoved => {
|
ExitReason::UserRequested | ExitReason::Archived(_) | ExitReason::ThreadRemoved => {
|
||||||
"Disconnected from this task. Any running work continues."
|
"Disconnected from this task. Any running work continues."
|
||||||
}
|
}
|
||||||
ExitReason::Fatal(_) => "Disconnected from this task. Work may still be running.",
|
ExitReason::Fatal(_) => "Disconnected from this task. Work may still be running.",
|
||||||
@@ -123,14 +123,14 @@ impl AppExitInfo {
|
|||||||
if !self.token_usage.is_zero() {
|
if !self.token_usage.is_zero() {
|
||||||
lines.push(self.token_usage.to_string());
|
lines.push(self.token_usage.to_string());
|
||||||
}
|
}
|
||||||
if let Some(resume_command) = self.resume_hint {
|
if let ExitReason::Archived(thread_id) = self.exit_reason {
|
||||||
|
lines.push(format!("Session archived: {thread_id}"));
|
||||||
|
} else if let Some(resume_command) = self.resume_hint {
|
||||||
lines.push(format!(
|
lines.push(format!(
|
||||||
"To continue this session, run {}",
|
"To continue this session, run {}",
|
||||||
color_command(resume_command),
|
color_command(resume_command),
|
||||||
));
|
));
|
||||||
} else if matches!(self.exit_reason, ExitReason::Fatal(_))
|
} else if let Some(thread_id) = self.thread_id {
|
||||||
&& let Some(thread_id) = self.thread_id
|
|
||||||
{
|
|
||||||
lines.push(format!("Session ID: {thread_id}"));
|
lines.push(format!("Session ID: {thread_id}"));
|
||||||
}
|
}
|
||||||
lines
|
lines
|
||||||
|
|||||||
@@ -385,20 +385,29 @@ async fn removing_remote_thread_omits_disconnect_guidance() -> Result<()> {
|
|||||||
app.active_thread_id = Some(thread_id);
|
app.active_thread_id = Some(thread_id);
|
||||||
app.chat_widget.handle_thread_session(resumed.session);
|
app.chat_widget.handle_thread_session(resumed.session);
|
||||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||||
|
let archived = matches!(&event, AppEvent::ArchiveCurrentThread);
|
||||||
let AppRunControl::Exit(reason) = app.handle_event(&mut tui, &mut server, event).await?
|
let AppRunControl::Exit(reason) = app.handle_event(&mut tui, &mut server, event).await?
|
||||||
else {
|
else {
|
||||||
panic!("removing the current thread must exit");
|
panic!("removing the current thread must exit");
|
||||||
};
|
};
|
||||||
assert_matches!(reason, ExitReason::ThreadRemoved);
|
if archived {
|
||||||
|
assert_matches!(reason, ExitReason::Archived(id) if id == thread_id);
|
||||||
|
} else {
|
||||||
|
assert_matches!(reason, ExitReason::ThreadRemoved);
|
||||||
|
}
|
||||||
let mut exit_info = app.exit_info(reason);
|
let mut exit_info = app.exit_info(reason);
|
||||||
exit_info.token_usage = TokenUsage {
|
exit_info.token_usage = TokenUsage {
|
||||||
output_tokens: 2,
|
output_tokens: 2,
|
||||||
total_tokens: 2,
|
total_tokens: 2,
|
||||||
..Default::default()
|
..Default::default()
|
||||||
};
|
};
|
||||||
|
let mut expected = vec!["Token usage: total=2 input=0 output=2".to_string()];
|
||||||
|
if archived {
|
||||||
|
expected.push(format!("Session archived: {thread_id}"));
|
||||||
|
}
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
exit_info.format_exit_messages(/*color_enabled*/ false),
|
exit_info.format_exit_messages(/*color_enabled*/ false),
|
||||||
vec!["Token usage: total=2 input=0 output=2",]
|
expected
|
||||||
);
|
);
|
||||||
server.shutdown().await?;
|
server.shutdown().await?;
|
||||||
proxy.await??;
|
proxy.await??;
|
||||||
@@ -547,6 +556,38 @@ async fn external_transport_registers_dynamic_tools_and_finds_task_mentions() ->
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn archive_current_thread_reports_success_only_after_archiving() -> Result<()> {
|
||||||
|
let (mut app, _codex_home) = make_history_test_app().await?;
|
||||||
|
let thread_id = ThreadId::from_string(
|
||||||
|
&create_fake_rollout(
|
||||||
|
&app.config.codex_home,
|
||||||
|
"2026-08-25T01-00-00",
|
||||||
|
"2026-08-25T01:00:00Z",
|
||||||
|
"archive me",
|
||||||
|
Some(&app.config.model_provider_id),
|
||||||
|
/*git_info*/ None,
|
||||||
|
)
|
||||||
|
.expect("create rollout"),
|
||||||
|
)?;
|
||||||
|
let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?;
|
||||||
|
|
||||||
|
app.active_thread_id = Some(ThreadId::new());
|
||||||
|
assert_matches!(
|
||||||
|
app.archive_current_thread(&mut app_server).await,
|
||||||
|
AppRunControl::Continue
|
||||||
|
);
|
||||||
|
|
||||||
|
app.active_thread_id = Some(thread_id);
|
||||||
|
assert_matches!(
|
||||||
|
app.archive_current_thread(&mut app_server).await,
|
||||||
|
AppRunControl::Exit(ExitReason::Archived(archived_id)) if archived_id == thread_id
|
||||||
|
);
|
||||||
|
|
||||||
|
app_server.shutdown().await?;
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths() -> Result<()> {
|
async fn local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths() -> Result<()> {
|
||||||
let (mut app, mut events, _ops) = make_test_app_with_channels().await;
|
let (mut app, mut events, _ops) = make_test_app_with_channels().await;
|
||||||
|
|||||||
@@ -38,9 +38,10 @@ fn main() -> anyhow::Result<()> {
|
|||||||
eprintln!("ERROR: {message}");
|
eprintln!("ERROR: {message}");
|
||||||
true
|
true
|
||||||
}
|
}
|
||||||
ExitReason::UserRequested | ExitReason::TurnInterrupted | ExitReason::ThreadRemoved => {
|
ExitReason::UserRequested
|
||||||
false
|
| ExitReason::Archived(_)
|
||||||
}
|
| ExitReason::TurnInterrupted
|
||||||
|
| ExitReason::ThreadRemoved => false,
|
||||||
};
|
};
|
||||||
|
|
||||||
let color_enabled = supports_color::on(Stream::Stdout).is_some();
|
let color_enabled = supports_color::on(Stream::Stdout).is_some();
|
||||||
|
|||||||
Reference in new issue
Block a user