diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 64e1b7c7c9..7ca0458d16 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -6,6 +6,7 @@ use super::resize_reflow::trailing_run_start; use super::session_lifecycle::ThreadAttachPresentation; use super::*; +use crate::app_event::RecapTrigger; use crate::app_event::ThreadTitleDestination; use crate::app_server_session::ForkGoalContinuation; use crate::app_server_session::UnsupportedLegacyPermissionProfile; @@ -2897,11 +2898,71 @@ impl App { AppEvent::KeymapCleared { context, action } => { self.apply_keymap_clear(context, action).await; } + AppEvent::GenerateRecap { thread_id } => { + if self.current_displayed_thread_id() == Some(thread_id) { + if self.chat_widget.is_user_turn_pending_or_running() { + self.chat_widget.add_error_message( + "Wait for the current task to finish before running /recap.".to_string(), + ); + } else { + self.request_recap(app_server, thread_id, RecapTrigger::Manual); + } + } + } AppEvent::CheckRecap { thread_id } => { - // The inference layer stacked on this change consumes the scheduled check. - let _ = thread_id; + if self.current_displayed_thread_id() == Some(thread_id) + && !self.chat_widget.is_user_turn_pending_or_running() + && self.recap.should_generate(std::time::Instant::now()) + { + self.request_recap(app_server, thread_id, RecapTrigger::Automatic); + } + } + AppEvent::RecapStarted { + thread_id, + request_id, + trigger, + completed_turn_count, + turn_revision, + history, + result, + } => { + self.handle_recap_started( + app_server, + recap::RecapRequest { + thread_id, + request_id, + trigger, + completed_turn_count, + turn_revision, + }, + history, + result, + ); + } + AppEvent::RecapGenerated { + thread_id, + request_id, + trigger, + temporary_thread_id, + completed_turn_count, + turn_revision, + result, + } => { + self.temporary_structured_requests.remove(&temporary_thread_id); + if let Some(cell) = self.handle_generated_recap( + recap::RecapRequest { + thread_id, + request_id, + trigger, + completed_turn_count, + turn_revision, + }, + temporary_thread_id, + result, + ) { + self.insert_history_cell(tui, Box::new(cell)); + } } - AppEvent::RecapStarted { .. } | AppEvent::RecapGenerated { .. } => {} } Ok(AppRunControl::Continue) } diff --git a/codex-rs/tui/src/app/recap.rs b/codex-rs/tui/src/app/recap.rs index 04783c9067..6cfc75f25e 100644 --- a/codex-rs/tui/src/app/recap.rs +++ b/codex-rs/tui/src/app/recap.rs @@ -1,21 +1,21 @@ //! Determines when an unfocused conversation is ready for an automatic recap. -// The stacked event-dispatch change activates the generation methods. -#![allow(dead_code)] - use std::sync::Arc; use std::time::Duration; use std::time::Instant; use super::App; use crate::app_event::AppEvent; +use crate::app_event::RecapTrigger; use crate::app_event_sender::AppEventSender; use crate::app_server_session::AppServerSession; use crate::history_cell::AgentMarkdownCell; use crate::history_cell::AgentMessageCell; use crate::history_cell::HistoryCell; use crate::history_cell::ThreadRecapHistoryCell; +use crate::history_cell::ThreadRecapLoadingCell; use crate::history_cell::UserHistoryCell; +use crate::pager_overlay::Overlay; use crate::temporary_structured_request::TemporaryStructuredThreadOptions; use crate::temporary_structured_request::run_temporary_structured_turn; use crate::temporary_structured_request::start_temporary_thread; @@ -36,6 +36,9 @@ pub(super) const RECAP_DELAY: Duration = Duration::from_secs(/*secs*/ 3 * 60); const RECAP_HISTORY_MAX_TURNS: usize = 8; const RECAP_MAX_CHARS: usize = 320; const RECAP_RETRY_DELAY: Duration = Duration::from_secs(/*secs*/ 30); +const MANUAL_RECAP_FAILURE_MESSAGE: &str = "Could not generate a recap. Please try again."; +const MANUAL_RECAP_IN_PROGRESS_MESSAGE: &str = "A recap is already being generated."; +const MANUAL_RECAP_EMPTY_HISTORY_MESSAGE: &str = "There is no conversation history to recap."; const RECAP_PROMPT_PREFIX: &str = concat!( "Write a brief catch-up for a user returning to this Codex task. ", "In at most 40 words and one or two plain-text sentences, explain the ", @@ -46,7 +49,7 @@ const RECAP_PROMPT_PREFIX: &str = concat!( "of inventing more work. Use the user's language; omit greetings, ", "markdown, lists, and tool chatter.\n\nRecent conversation:\n", ); -const RECAP_PROMPT_MAX_BYTES: usize = 900; +pub(super) const RECAP_PROMPT_MAX_BYTES: usize = 900; fn render_recap_message(role: &str, content: &str, max_bytes: usize) -> Option { let prefix = format!("{role}: "); @@ -167,24 +170,79 @@ fn parse_recap(response: &str) -> Option { pub(super) struct RecapRequest { pub(super) thread_id: ThreadId, pub(super) request_id: Uuid, + pub(super) trigger: RecapTrigger, pub(super) completed_turn_count: usize, pub(super) turn_revision: usize, } impl App { - pub(super) fn request_recap(&mut self, app_server: &AppServerSession, thread_id: ThreadId) { + fn clear_recap_request(&mut self, trigger: RecapTrigger) { + self.recap.clear_in_flight_request(); + + if !matches!(trigger, RecapTrigger::Manual) { + return; + } + + self.chat_widget.clear_recap_loading(); + + let Some(index) = self + .transcript_cells + .iter() + .rposition(|cell| cell.as_any().is::()) + else { + return; + }; + + self.transcript_cells.remove(index); + if let Some(Overlay::Transcript(overlay)) = &mut self.overlay { + overlay.replace_cells(self.transcript_cells.clone()); + } + } + + fn retry_or_report_recap_failure(&mut self, request: RecapRequest) { + match request.trigger { + RecapTrigger::Automatic => self.recap.schedule_retry( + request.thread_id, + self.app_event_tx.clone(), + request.turn_revision, + ), + RecapTrigger::Manual => self + .chat_widget + .add_error_message(MANUAL_RECAP_FAILURE_MESSAGE.to_string()), + } + } + + pub(super) fn request_recap( + &mut self, + app_server: &AppServerSession, + thread_id: ThreadId, + trigger: RecapTrigger, + ) { if self.recap.in_flight_request.is_some() { + if matches!(trigger, RecapTrigger::Manual) { + self.chat_widget + .add_error_message(MANUAL_RECAP_IN_PROGRESS_MESSAGE.to_string()); + } return; } let history = recap_history(&self.transcript_cells); if history.is_empty() { + if matches!(trigger, RecapTrigger::Manual) { + self.chat_widget + .add_error_message(MANUAL_RECAP_EMPTY_HISTORY_MESSAGE.to_string()); + } return; } + if matches!(trigger, RecapTrigger::Manual) { + self.chat_widget.show_recap_loading(); + } + let request = RecapRequest { thread_id, request_id: Uuid::new_v4(), + trigger, completed_turn_count: self.recap.completed_turns, turn_revision: self.recap.turn_revision, }; @@ -219,6 +277,7 @@ impl App { event_sender.send(AppEvent::RecapStarted { thread_id: request.thread_id, request_id: request.request_id, + trigger: request.trigger, completed_turn_count: request.completed_turn_count, turn_revision: request.turn_revision, history, @@ -227,6 +286,7 @@ impl App { }); self.recap.in_flight_request_id = Some(request.request_id); + self.recap.in_flight_trigger = Some(request.trigger); self.recap.in_flight_request = Some(task); } @@ -240,10 +300,12 @@ impl App { let RecapRequest { thread_id, request_id, + trigger, completed_turn_count, turn_revision, } = request; - let is_current_request = self.recap.in_flight_request_id == Some(request_id); + let is_current_request = self.recap.in_flight_request_id == Some(request_id) + && self.recap.in_flight_trigger == Some(trigger); if is_current_request { self.recap.in_flight_request.take(); } @@ -252,34 +314,37 @@ impl App { Ok(temporary_thread_id) => temporary_thread_id, Err(error) => { if is_current_request { - self.recap.in_flight_request_id = None; + self.clear_recap_request(trigger); tracing::warn!(%thread_id, %error, "failed to start thread recap request"); - self.recap - .schedule_retry(thread_id, self.app_event_tx.clone(), turn_revision); + self.retry_or_report_recap_failure(request); } return; } }; + let trigger_is_eligible = match trigger { + RecapTrigger::Automatic => self.recap.should_generate(Instant::now()), + RecapTrigger::Manual => true, + }; + let request_is_fresh = is_current_request && self.current_displayed_thread_id() == Some(thread_id) && !self.chat_widget.is_user_turn_pending_or_running() && self.recap.completed_turns == completed_turn_count && self.recap.turn_revision == turn_revision - && self.recap.should_generate(Instant::now()); + && trigger_is_eligible; let Ok(temporary_thread_id) = ThreadId::from_string(&temporary_thread_id_text) else { if is_current_request { - self.recap.in_flight_request_id = None; + self.clear_recap_request(trigger); tracing::warn!(%thread_id, "thread recap request returned an invalid thread ID"); - self.recap - .schedule_retry(thread_id, self.app_event_tx.clone(), turn_revision); + self.retry_or_report_recap_failure(request); } return; }; if !request_is_fresh { if is_current_request { - self.recap.in_flight_request_id = None; + self.clear_recap_request(trigger); } let request_handle = app_server.request_handle(); tokio::spawn(async move { @@ -309,6 +374,7 @@ impl App { event_sender.send(AppEvent::RecapGenerated { thread_id, request_id, + trigger, temporary_thread_id, completed_turn_count, turn_revision, @@ -329,18 +395,18 @@ impl App { let RecapRequest { thread_id, request_id, + trigger, completed_turn_count, turn_revision, } = request; if self.recap.in_flight_request_id != Some(request_id) + || self.recap.in_flight_trigger != Some(trigger) || self.recap.in_flight_thread_id != Some(temporary_thread_id) { return None; } - self.recap.in_flight_request_id = None; - self.recap.in_flight_thread_id = None; - self.recap.in_flight_request.take(); + self.clear_recap_request(trigger); if self.current_displayed_thread_id() != Some(thread_id) || self.chat_widget.is_user_turn_pending_or_running() @@ -354,8 +420,7 @@ impl App { Ok(response) => { let Some(recap) = parse_recap(&response) else { tracing::warn!(%thread_id, "generated thread recap was invalid"); - self.recap - .schedule_retry(thread_id, self.app_event_tx.clone(), turn_revision); + self.retry_or_report_recap_failure(request); return None; }; @@ -364,8 +429,7 @@ impl App { } Err(error) => { tracing::warn!(%thread_id, %error, "failed to generate thread recap"); - self.recap - .schedule_retry(thread_id, self.app_event_tx.clone(), turn_revision); + self.retry_or_report_recap_failure(request); None } } @@ -407,11 +471,19 @@ pub(super) struct RecapState { scheduled_check: Option>, retry_revision: Option, in_flight_request_id: Option, + in_flight_trigger: Option, in_flight_thread_id: Option, in_flight_request: Option>, } impl RecapState { + fn clear_in_flight_request(&mut self) { + self.in_flight_request_id = None; + self.in_flight_trigger = None; + self.in_flight_thread_id = None; + self.in_flight_request.take(); + } + pub(super) fn seed_from_turns(&mut self, turns: &[Turn], now: Instant) { self.seed_from_progress(RecapProgress::from_turns(turns), now); } @@ -456,9 +528,9 @@ impl RecapState { } self.retry_revision = None; - self.in_flight_request_id = None; - self.in_flight_thread_id = None; - self.in_flight_request.take(); + if self.in_flight_trigger == Some(RecapTrigger::Automatic) { + self.clear_in_flight_request(); + } } pub(super) fn note_turn_finished(&mut self, status: &TurnStatus, now: Instant) { diff --git a/codex-rs/tui/src/app/recap_tests.rs b/codex-rs/tui/src/app/recap_tests.rs index b9618f28ed..0e738385e0 100644 --- a/codex-rs/tui/src/app/recap_tests.rs +++ b/codex-rs/tui/src/app/recap_tests.rs @@ -12,12 +12,14 @@ use super::recap_history; use super::recap_prompt; use crate::app::test_support::make_test_app; use crate::app_event::AppEvent; +use crate::app_event::RecapTrigger; use crate::app_event_sender::AppEventSender; use crate::history_cell::AgentMarkdownCell; use crate::history_cell::AgentMessageCell; use crate::history_cell::HistoryCell; use crate::history_cell::PlainHistoryCell; use crate::history_cell::ThreadRecapHistoryCell; +use crate::history_cell::ThreadRecapLoadingCell; use crate::history_cell::UserHistoryCell; use codex_app_server_protocol::Turn; use codex_app_server_protocol::TurnStatus; @@ -275,6 +277,43 @@ async fn regaining_focus_cancels_scheduled_check() { )); } +#[test] +fn regaining_focus_preserves_only_manual_in_flight_request() { + let request_id = Uuid::new_v4(); + let thread_id = ThreadId::new(); + let mut state = RecapState::default(); + state.in_flight_request_id = Some(request_id); + state.in_flight_trigger = Some(RecapTrigger::Manual); + state.in_flight_thread_id = Some(thread_id); + + state.note_focus_gained(); + + assert_eq!( + ( + state.in_flight_request_id, + state.in_flight_trigger, + state.in_flight_thread_id, + ), + ( + Some(request_id), + Some(RecapTrigger::Manual), + Some(thread_id), + ) + ); + + state.in_flight_trigger = Some(RecapTrigger::Automatic); + state.note_focus_gained(); + + assert_eq!( + ( + state.in_flight_request_id, + state.in_flight_trigger, + state.in_flight_thread_id, + ), + (None, None, None) + ); +} + #[tokio::test(flavor = "current_thread", start_paused = true)] async fn scheduled_check_fires_at_recap_deadline() { let thread_id = ThreadId::new(); @@ -537,6 +576,7 @@ fn track_in_flight_recap(app: &mut App, thread_id: ThreadId) -> (RecapRequest, T let request = RecapRequest { thread_id, request_id: Uuid::new_v4(), + trigger: RecapTrigger::Automatic, completed_turn_count: 3, turn_revision: 3, }; @@ -544,6 +584,7 @@ fn track_in_flight_recap(app: &mut App, thread_id: ThreadId) -> (RecapRequest, T app.recap.completed_turns = request.completed_turn_count; app.recap.turn_revision = request.turn_revision; app.recap.in_flight_request_id = Some(request.request_id); + app.recap.in_flight_trigger = Some(request.trigger); app.recap.in_flight_thread_id = Some(temporary_thread_id); app.recap.in_flight_request = Some(tokio::spawn(async {})); (request, temporary_thread_id) @@ -618,6 +659,39 @@ async fn newer_terminal_turn_invalidates_generated_recap() { assert!(app.recap.in_flight_request.is_none()); } +#[tokio::test(flavor = "current_thread", start_paused = true)] +async fn manual_recap_failure_does_not_schedule_retry() { + let thread_id = ThreadId::new(); + let mut app = app_with_visible_thread(thread_id).await; + let (mut request, temporary_thread_id) = track_in_flight_recap(&mut app, thread_id); + request.trigger = RecapTrigger::Manual; + app.recap.in_flight_trigger = Some(RecapTrigger::Manual); + app.transcript_cells + .push(Arc::new(ThreadRecapLoadingCell::new( + /*animations_enabled*/ false, + ))); + + assert!( + app.handle_generated_recap( + request, + temporary_thread_id, + Err("temporary failure".to_string()), + ) + .is_none() + ); + + tokio::time::advance(RECAP_RETRY_DELAY).await; + tokio::task::yield_now().await; + + assert_eq!(app.recap.retry_revision, None); + assert!(app.recap.scheduled_check.is_none()); + assert!( + !app.transcript_cells + .iter() + .any(|cell| cell.as_any().is::()) + ); +} + #[tokio::test(flavor = "current_thread", start_paused = true)] async fn recap_failure_retries_once_for_the_same_turn_revision() { let thread_id = ThreadId::new(); diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index f5c76a0e13..066df4e05e 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -17,6 +17,8 @@ mod patch_approval_tests; mod permission_shortcuts_tests; mod plugin_catalog; mod rate_limits; +#[path = "tests/recap_generation_tests.rs"] +mod recap_generation; mod safety_buffering; #[path = "tests/session_lifecycle_requests.rs"] mod session_lifecycle_requests; diff --git a/codex-rs/tui/src/app/tests/recap_generation_tests.rs b/codex-rs/tui/src/app/tests/recap_generation_tests.rs new file mode 100644 index 0000000000..98d10c6dfa --- /dev/null +++ b/codex-rs/tui/src/app/tests/recap_generation_tests.rs @@ -0,0 +1,417 @@ +use super::*; +use crate::app::session_lifecycle::ThreadAttachPresentation; +use crate::app::tests::session_lifecycle_requests::recorded_params; +use crate::app::tests::session_lifecycle_requests::start_recording_remote_app_server; +use crate::app_event::RecapTrigger; +use core_test_support::responses; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_response_created; +use core_test_support::streaming_sse::StreamingSseChunk; +use core_test_support::streaming_sse::start_streaming_sse_server; +use pretty_assertions::assert_eq; +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use serde_json::Value; + +const MODEL: &str = "gpt-5.2"; +const MODEL_PROVIDER_ID: &str = "recap-generation-test"; + +fn render_chat_widget(app: &App) -> String { + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 24, + ); + let mut buffer = Buffer::empty(area); + app.chat_widget.render(area, &mut buffer); + (area.y..area.bottom()) + .map(|y| { + (area.x..area.right()) + .map(|x| buffer[(x, y)].symbol()) + .collect::() + }) + .collect::>() + .join("\n") +} + +fn prepare_eligible_recap(app: &mut App, thread_id: ThreadId) { + app.active_thread_id = Some(thread_id); + app.transcript_cells + .push(Arc::new(crate::history_cell::UserHistoryCell { + message: "Finish the recap implementation".to_string(), + text_elements: Vec::new(), + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + })); + let ready_at = Instant::now() - recap::RECAP_DELAY; + app.recap.note_focus_lost(ready_at); + for _ in 0..3 { + app.recap + .note_turn_finished(&TurnStatus::Completed, ready_at); + } +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn recap_generation_uses_bounded_structured_request_and_inserts_result() -> Result<()> { + let chunks = [ + ev_response_created("recap-response"), + ev_assistant_message( + "recap-message", + r#"{"recap":"Finished parsing. Next: run focused tests."}"#, + ), + ev_completed("recap-response"), + ] + .into_iter() + .map(|event| StreamingSseChunk { + gate: None, + body: responses::sse(vec![event]), + }) + .collect(); + let (model_server, _completions) = start_streaming_sse_server(vec![chunks]).await; + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let codex_home = tempdir()?; + std::fs::write( + codex_home.path().join("config.toml"), + format!( + r#" +model = "{MODEL}" +model_provider = "{MODEL_PROVIDER_ID}" + +[model_providers.{MODEL_PROVIDER_ID}] +name = "Recap generation test" +base_url = "{}/v1" +wire_api = "responses" +request_max_retries = 0 +stream_max_retries = 0 +"#, + model_server.uri() + ), + )?; + app.config.codex_home = codex_home.path().to_path_buf().abs(); + app.config.sqlite = codex_state::SqliteConfig::new_for_testing(codex_home.path().abs()); + app.config.model = Some(MODEL.to_string()); + app.config.model_provider_id = MODEL_PROVIDER_ID.to_string(); + app.config.model_provider = ModelProviderInfo { + name: "Recap generation test".to_string(), + base_url: Some(format!("{}/v1", model_server.uri())), + request_max_retries: Some(0), + stream_max_retries: Some(0), + ..ModelProviderInfo::default() + }; + + let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; + let started = app_server.start_thread(&app.config).await?; + let thread_id = started.session.thread_id; + let mut tui = crate::tui::test_support::make_test_tui()?; + app.replace_chat_widget_with_app_server_thread( + &mut tui, + started, + ThreadAttachPresentation::SessionLineage, + /*initial_user_message*/ None, + ) + .await?; + while app_event_rx.try_recv().is_ok() {} + + prepare_eligible_recap(&mut app, thread_id); + + app.handle_event( + &mut tui, + &mut app_server, + AppEvent::CheckRecap { thread_id }, + ) + .await?; + let started_event = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), app_event_rx.recv()) + .await? + .expect("recap start event"); + assert!(matches!(started_event, AppEvent::RecapStarted { .. })); + app.handle_event(&mut tui, &mut app_server, started_event) + .await?; + + let generated_event = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), async { + loop { + tokio::select! { + event = app_event_rx.recv() => { + let event = event.expect("app event stream"); + if matches!(event, AppEvent::RecapGenerated { .. }) { + break Ok::<_, color_eyre::Report>(event); + } + app.handle_event(&mut tui, &mut app_server, event).await?; + } + event = app_server.next_event() => { + let event = event.expect("app-server event stream"); + app.handle_app_server_event(&app_server, event).await; + } + } + } + }) + .await??; + app.handle_event(&mut tui, &mut app_server, generated_event) + .await?; + + let recap = app + .transcript_cells + .last() + .and_then(|cell| { + cell.as_any() + .downcast_ref::() + }) + .expect("recap history cell"); + assert_eq!( + recap + .raw_lines() + .iter() + .map(ToString::to_string) + .collect::>(), + vec![ + "Conversation recap", + "Finished parsing. Next: run focused tests.", + ] + ); + + let model_requests = model_server.requests().await; + assert_eq!(model_requests.len(), 1); + let request: Value = serde_json::from_slice(&model_requests[0])?; + let prompt = request["input"] + .as_array() + .and_then(|items| items.iter().rfind(|item| item["role"] == "user")) + .and_then(|item| item["content"].as_array()) + .and_then(|content| content.iter().find(|item| item["type"] == "input_text")) + .and_then(|item| item["text"].as_str()) + .expect("recap prompt"); + assert!( + prompt.contains("Finish the recap implementation"), + "prompt: {prompt}\nrequest: {request}" + ); + assert!(prompt.len() <= recap::RECAP_PROMPT_MAX_BYTES); + assert_eq!(request["text"]["format"]["type"], "json_schema"); + assert_eq!(request["tools"], serde_json::json!([])); + + app_server.shutdown().await?; + model_server.shutdown().await; + Ok(()) +} + +#[tokio::test] +async fn manual_recap_bypasses_automatic_eligibility_and_reports_failure() -> Result<()> { + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let thread_id = ThreadId::new(); + app.active_thread_id = Some(thread_id); + app.transcript_cells + .push(Arc::new(crate::history_cell::UserHistoryCell { + message: "Summarize this conversation".to_string(), + text_elements: Vec::new(), + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + })); + let (mut app_server, _requests, proxy) = start_recording_remote_app_server(&app.config).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + + app.handle_event( + &mut tui, + &mut app_server, + AppEvent::GenerateRecap { thread_id }, + ) + .await?; + assert!( + render_chat_widget(&app).contains("Generating conversation recap…"), + "manual recap should show progress while generation is in flight" + ); + let started_event = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), app_event_rx.recv()) + .await? + .expect("manual recap start event"); + let (request, temporary_thread_id) = match &started_event { + AppEvent::RecapStarted { + thread_id: requested_thread_id, + request_id, + trigger, + completed_turn_count, + turn_revision, + result: Ok(temporary_thread_id), + .. + } if *requested_thread_id == thread_id && *trigger == RecapTrigger::Manual => ( + recap::RecapRequest { + thread_id: *requested_thread_id, + request_id: *request_id, + trigger: *trigger, + completed_turn_count: *completed_turn_count, + turn_revision: *turn_revision, + }, + ThreadId::from_string(temporary_thread_id).expect("temporary recap thread ID"), + ), + other => panic!("expected a started manual recap, got {other:?}"), + }; + + app.handle_event(&mut tui, &mut app_server, started_event) + .await?; + + assert_eq!(app.temporary_structured_requests.len(), 1); + assert!( + app.handle_generated_recap( + request, + temporary_thread_id, + Err("temporary failure".to_string()), + ) + .is_none() + ); + assert!( + !render_chat_widget(&app).contains("Generating conversation recap…"), + "manual recap progress should clear when generation finishes" + ); + + let cell = match app_event_rx.try_recv() { + Ok(AppEvent::InsertHistoryCell(cell)) => cell, + other => panic!("expected visible manual recap error, got {other:?}"), + }; + let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 80)); + assert!( + rendered.contains("Could not generate a recap. Please try again."), + "rendered error: {rendered:?}" + ); + + app_server.shutdown().await?; + proxy.await??; + Ok(()) +} + +#[tokio::test] +async fn recap_generation_uses_remote_workspace_cwd() -> Result<()> { + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let remote_cwd = if cfg!(windows) { + PathBuf::from(r"C:\remote\project") + } else { + PathBuf::from("/srv/remote/project") + }; + let (app_server, requests, proxy) = start_recording_remote_app_server(&app.config).await?; + let mut app_server = app_server.with_remote_cwd_override(Some(remote_cwd.clone())); + let mut tui = crate::tui::test_support::make_test_tui()?; + app.transcript_cells + .push(Arc::new(crate::history_cell::UserHistoryCell { + message: "Finish the recap implementation".to_string(), + text_elements: Vec::new(), + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + })); + + app.request_recap(&app_server, ThreadId::new(), RecapTrigger::Automatic); + let started_event = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), app_event_rx.recv()) + .await? + .expect("recap start event"); + app.handle_event(&mut tui, &mut app_server, started_event) + .await?; + + let starts = recorded_params(&requests, "thread/start"); + assert_eq!(starts.len(), 1); + assert_eq!(starts[0]["cwd"], remote_cwd.to_string_lossy().into_owned()); + assert_eq!(starts[0]["ephemeral"], true); + assert_eq!(starts[0]["environments"], serde_json::json!([])); + + app_server.shutdown().await?; + proxy.await??; + Ok(()) +} + +#[tokio::test] +async fn temporary_recap_threads_disable_memories_and_remote_mcp_servers() -> Result<()> { + let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await; + let codex_home = tempdir()?; + std::fs::write( + codex_home.path().join("config.toml"), + "[mcp_servers.filesystem]\ncommand = 'true'\n", + )?; + app.config.codex_home = codex_home.path().to_path_buf().abs(); + app.config.sqlite = codex_state::SqliteConfig::new_for_testing(codex_home.path().abs()); + let (app_server, requests, proxy) = start_recording_remote_app_server(&app.config).await?; + + let config = app.chat_widget.config_ref(); + let options = crate::temporary_structured_request::TemporaryStructuredThreadOptions { + model: app.chat_widget.current_model().to_string(), + model_provider: config.model_provider_id.clone(), + cwd: config.cwd.display().to_string(), + active_permission_profile: config + .permissions + .active_permission_profile() + .map(|profile| profile.id), + mcp_server_names: Vec::new(), + }; + crate::temporary_structured_request::start_temporary_thread( + &app_server.request_handle(), + options, + ) + .await?; + + let starts = recorded_params(&requests, "thread/start"); + assert_eq!(starts.len(), 1); + assert_eq!(starts[0]["config"]["features.memories"], false); + assert!(starts[0]["config"].get("features.memory_tool").is_none()); + assert_eq!( + starts[0]["config"]["mcp_servers"]["filesystem"]["enabled"], + false + ); + + app_server.shutdown().await?; + proxy.await??; + Ok(()) +} + +#[tokio::test] +async fn recap_check_rejects_a_non_displayed_thread() -> Result<()> { + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let displayed_thread_id = ThreadId::new(); + prepare_eligible_recap(&mut app, displayed_thread_id); + while app_event_rx.try_recv().is_ok() {} + let (mut app_server, requests, proxy) = start_recording_remote_app_server(&app.config).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + + app.handle_event( + &mut tui, + &mut app_server, + AppEvent::CheckRecap { + thread_id: ThreadId::new(), + }, + ) + .await?; + tokio::task::yield_now().await; + + assert!(app_event_rx.try_recv().is_err()); + assert_eq!( + recorded_params(&requests, "thread/start"), + Vec::::new() + ); + + app_server.shutdown().await?; + proxy.await??; + Ok(()) +} + +#[tokio::test] +async fn recap_check_rejects_a_running_turn() -> Result<()> { + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let thread_id = ThreadId::new(); + prepare_eligible_recap(&mut app, thread_id); + app.chat_widget + .handle_thread_session(test_thread_session(thread_id, app.config.cwd.to_path_buf())); + app.chat_widget.handle_server_notification( + turn_started_notification(thread_id, "active-turn"), + /*replay_kind*/ None, + ); + while app_event_rx.try_recv().is_ok() {} + let (mut app_server, requests, proxy) = start_recording_remote_app_server(&app.config).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + + app.handle_event( + &mut tui, + &mut app_server, + AppEvent::CheckRecap { thread_id }, + ) + .await?; + tokio::task::yield_now().await; + + assert!(app_event_rx.try_recv().is_err()); + assert_eq!( + recorded_params(&requests, "thread/start"), + Vec::::new() + ); + + app_server.shutdown().await?; + proxy.await??; + Ok(()) +} diff --git a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs index 5e88addf3c..8732ae3dfa 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -37,8 +37,8 @@ use tokio::task::JoinHandle; use tokio_tungstenite::accept_async; use tokio_tungstenite::tungstenite::Message; -type RecordedRequests = Arc>>; -type RecordingAppServer = (AppServerSession, RecordedRequests, JoinHandle>); +pub(super) type RecordedRequests = Arc>>; +pub(super) type RecordingAppServer = (AppServerSession, RecordedRequests, JoinHandle>); #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum HistoryCapabilities { @@ -75,6 +75,20 @@ pub(super) async fn start_recording_app_server( HistoryCapabilities::Current, blocked_thread_list, failed_thread_name, + crate::app_server_session::ThreadParamsMode::Embedded, + ) + .await +} + +pub(super) async fn start_recording_remote_app_server( + config: &Config, +) -> Result { + start_recording_app_server_with_history( + config, + HistoryCapabilities::Current, + /*blocked_thread_list*/ None, + /*failed_thread_name*/ None, + crate::app_server_session::ThreadParamsMode::Remote, ) .await } @@ -85,6 +99,7 @@ async fn start_recording_app_server_with_history( history_capabilities: HistoryCapabilities, mut blocked_thread_list: Option<(ThreadId, oneshot::Sender<()>, oneshot::Receiver<()>)>, failed_thread_name: Option<&'static str>, + thread_params_mode: crate::app_server_session::ThreadParamsMode, ) -> Result { let state_db = crate::init_state_db_for_app_server_target(config, &crate::AppServerTarget::Embedded) @@ -299,11 +314,7 @@ async fn start_recording_app_server_with_history( .await?; Ok(( - AppServerSession::new( - app_server, - crate::app_server_session::ThreadParamsMode::Embedded, - ) - .with_startup_config(config), + AppServerSession::new(app_server, thread_params_mode).with_startup_config(config), requests, proxy, )) @@ -330,7 +341,7 @@ fn create_history_rollout( Ok(ThreadId::from_string(&thread_id)?) } -fn recorded_params(requests: &RecordedRequests, method: &str) -> Vec { +pub(super) fn recorded_params(requests: &RecordedRequests, method: &str) -> Vec { requests .lock() .expect("request recorder lock") @@ -930,6 +941,7 @@ async fn older_external_server_starts_without_unsupported_dynamic_tools_or_histo HistoryCapabilities::LegacyDynamicToolsAndHistory, /*blocked_thread_list*/ None, /*failed_thread_name*/ None, + crate::app_server_session::ThreadParamsMode::Embedded, ) .await?; @@ -1762,6 +1774,7 @@ async fn remote_legacy_history_start_negotiates_once_for_resume_and_fork() -> Re HistoryCapabilities::LegacyOnly, /*blocked_thread_list*/ None, /*failed_thread_name*/ None, + crate::app_server_session::ThreadParamsMode::Embedded, ) .await?; @@ -1852,6 +1865,7 @@ async fn remote_legacy_history_start_retries_unsupported_paginated_variant() -> HistoryCapabilities::LegacyOnlyUnsupportedVariant, /*blocked_thread_list*/ None, /*failed_thread_name*/ None, + crate::app_server_session::ThreadParamsMode::Embedded, ) .await?; @@ -1882,6 +1896,7 @@ async fn assert_remote_legacy_history_retry(request: LegacyHistoryRequest) -> Re HistoryCapabilities::LegacyOnly, /*blocked_thread_list*/ None, /*failed_thread_name*/ None, + crate::app_server_session::ThreadParamsMode::Embedded, ) .await?; @@ -1947,6 +1962,7 @@ async fn paginated_fork_survives_post_response_hydration_failure() -> Result<()> HistoryCapabilities::ForkHydrationFails, /*blocked_thread_list*/ None, /*failed_thread_name*/ None, + crate::app_server_session::ThreadParamsMode::Embedded, ) .await?; diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 8740e3da62..a8126066d9 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -204,6 +204,13 @@ pub(crate) enum ThreadTitleDestination { RenameSuggestion { request_id: Uuid }, } +/// Identifies the policy that initiated a recap request. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum RecapTrigger { + Automatic, + Manual, +} + #[allow(clippy::large_enum_variant)] #[derive(Debug, IntoStaticStr)] pub(crate) enum AppEvent { @@ -1326,16 +1333,21 @@ pub(crate) enum AppEvent { action: String, }, + /// Generate a recap for the displayed idle thread at the user's request. + GenerateRecap { + thread_id: ThreadId, + }, + /// Recheck whether an unfocused thread is ready for an automatic recap. CheckRecap { thread_id: ThreadId, }, - /// Deliver the result of starting an automatic recap's temporary thread. - #[allow(dead_code)] + /// Deliver the result of starting a recap's temporary thread. RecapStarted { thread_id: ThreadId, request_id: Uuid, + trigger: RecapTrigger, completed_turn_count: usize, turn_revision: usize, history: String, @@ -1343,10 +1355,10 @@ pub(crate) enum AppEvent { }, /// Deliver the generated recap from a temporary structured turn. - #[allow(dead_code)] RecapGenerated { thread_id: ThreadId, request_id: Uuid, + trigger: RecapTrigger, temporary_thread_id: ThreadId, completed_turn_count: usize, turn_revision: usize, diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__command_popup__tests__command_popup_default_items.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__command_popup__tests__command_popup_default_items.snap index 7dcde80358..8bbb6892df 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__command_popup__tests__command_popup_default_items.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__command_popup__tests__command_popup_default_items.snap @@ -23,6 +23,7 @@ expression: commands /app - continue this session in the Desktop app /init - create an AGENTS.md file with instructions for Codex /compact - summarize conversation to prevent hitting the context limit +/recap - summarize the current conversation now /agents - view and switch between all active agent sessions /side - start a side conversation in an ephemeral fork /copy - copy the last response, code block, or quote diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 6f0367cd46..4fee2ad703 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -401,6 +401,7 @@ use self::rate_limits::RateLimitWarningState; use self::rate_limits::app_server_rate_limit_error_kind; pub(crate) use self::rate_limits::fallback_limit_label; use self::rate_limits::is_app_server_cyber_policy_error; +mod recap; mod reset_credits; pub(crate) use self::rate_limits::limit_label_for_window; mod reasoning_shortcuts; diff --git a/codex-rs/tui/src/chatwidget/input_submission.rs b/codex-rs/tui/src/chatwidget/input_submission.rs index c0b78cac92..1fc8b6ba04 100644 --- a/codex-rs/tui/src/chatwidget/input_submission.rs +++ b/codex-rs/tui/src/chatwidget/input_submission.rs @@ -377,6 +377,11 @@ impl ChatWidget { // App-event submissions are handled serially, and turn/start can wait on remote work. // Queue the optimistic prompt first so the user's input is visible while that happens. // Direct submissions do not share that queue, so keep their existing failure behavior. + if render_in_history { + // Do not let a transient manual-recap progress cell become permanent terminal + // scrollback when the new user prompt flushes the active history cell. + self.clear_recap_loading(); + } let render_before_submit = render_in_history && matches!(&self.codex_op_target, CodexOpTarget::AppEvent); if render_before_submit { diff --git a/codex-rs/tui/src/chatwidget/recap.rs b/codex-rs/tui/src/chatwidget/recap.rs new file mode 100644 index 0000000000..ac003930a8 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/recap.rs @@ -0,0 +1,28 @@ +//! Manual recap progress presentation for `ChatWidget`. + +use super::*; + +impl ChatWidget { + pub(crate) fn show_recap_loading(&mut self) { + self.flush_answer_stream_with_separator(); + self.flush_active_cell(); + self.transcript.active_cell = Some(Box::new(history_cell::ThreadRecapLoadingCell::new( + self.config.animations, + ))); + self.bump_active_cell_revision(); + self.request_redraw(); + } + + pub(crate) fn clear_recap_loading(&mut self) { + let Some(active) = self.transcript.active_cell.as_ref() else { + return; + }; + if !active.as_any().is::() { + return; + } + + self.transcript.active_cell = None; + self.bump_active_cell_revision(); + self.request_redraw(); + } +} diff --git a/codex-rs/tui/src/chatwidget/slash_dispatch.rs b/codex-rs/tui/src/chatwidget/slash_dispatch.rs index fb1b75fd8f..69229ba4dd 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/codex-rs/tui/src/chatwidget/slash_dispatch.rs @@ -273,6 +273,16 @@ impl ChatWidget { self.input_queue.user_turn_pending_start = true; self.app_event_tx.compact(); } + SlashCommand::Recap => { + let Some(thread_id) = self.thread_id else { + self.add_error_message( + "Session is still starting; try /recap again in a moment.".to_string(), + ); + return; + }; + self.app_event_tx + .send(AppEvent::GenerateRecap { thread_id }); + } SlashCommand::Review => { self.open_review_popup(); if self.mcp_startup_status.is_some() { @@ -1139,6 +1149,7 @@ impl ChatWidget { | SlashCommand::Diff | SlashCommand::App | SlashCommand::Rename + | SlashCommand::Recap | SlashCommand::TestApproval => QueueDrain::Continue, SlashCommand::Cd => match self.thread_id { Some(thread_id) if self.can_change_working_directory(thread_id) => QueueDrain::Stop, diff --git a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs index 1acc1e5a27..394fe45c33 100644 --- a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs +++ b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs @@ -1,5 +1,6 @@ use super::*; use crate::app_event::ConnectorsSnapshot; +use crate::history_cell::ThreadRecapLoadingCell; use codex_protocol::models::ManagedFileSystemPermissions; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; @@ -32,6 +33,25 @@ fn assert_hidden_shell_payload_is_literal(op: Result, payload: } } +#[tokio::test] +async fn user_submission_does_not_commit_recap_loading_to_history() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.show_recap_loading(); + while rx.try_recv().is_ok() {} + + chat.submit_user_message(UserMessage::from("Continue with the task")); + + let mut saw_user_message = false; + while let Ok(event) = rx.try_recv() { + if let AppEvent::InsertHistoryCell(cell) = event { + assert!(!cell.as_any().is::()); + saw_user_message |= cell.as_any().is::(); + } + } + assert!(saw_user_message); +} + #[tokio::test] async fn hidden_shell_paste_submits_literal_prompt() { let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index 2433ee6e05..2b681d40e2 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -177,6 +177,22 @@ async fn slash_compact_eagerly_queues_follow_up_before_turn_start() { assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty)); } +#[tokio::test] +async fn slash_recap_requests_generation_for_current_thread() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + + chat.dispatch_command(SlashCommand::Recap); + + assert_matches!( + rx.try_recv(), + Ok(AppEvent::GenerateRecap { + thread_id: requested_thread_id, + }) if requested_thread_id == thread_id + ); +} + #[tokio::test] async fn queued_slash_compact_dispatches_after_active_turn() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/history_cell/notices.rs b/codex-rs/tui/src/history_cell/notices.rs index ca360c9b5f..3862797ab2 100644 --- a/codex-rs/tui/src/history_cell/notices.rs +++ b/codex-rs/tui/src/history_cell/notices.rs @@ -230,6 +230,52 @@ pub(crate) fn new_error_event(message: String) -> PlainHistoryCell { PlainHistoryCell { lines } } +#[derive(Debug)] +pub(crate) struct ThreadRecapLoadingCell { + start_time: Instant, + animations_enabled: bool, +} + +impl ThreadRecapLoadingCell { + pub(crate) fn new(animations_enabled: bool) -> Self { + Self { + start_time: Instant::now(), + animations_enabled, + } + } +} + +impl HistoryCell for ThreadRecapLoadingCell { + fn display_lines(&self, _width: u16) -> Vec> { + vec![ + vec![ + activity_indicator( + Some(self.start_time), + MotionMode::from_animations_enabled(self.animations_enabled), + ReducedMotionIndicator::StaticBullet, + ) + .unwrap_or_else(|| "•".dim()), + " ".into(), + "Generating conversation recap".bold(), + "…".dim(), + ] + .into(), + ] + } + + fn raw_lines(&self) -> Vec> { + vec![Line::from("Generating conversation recap...")] + } + + fn transcript_animation_tick(&self) -> Option { + if !self.animations_enabled { + return None; + } + + Some((self.start_time.elapsed().as_millis() / 50) as u64) + } +} + #[cfg_attr(not(test), allow(dead_code))] #[derive(Debug)] pub(crate) struct ThreadRecapHistoryCell { diff --git a/codex-rs/tui/src/history_cell/tests.rs b/codex-rs/tui/src/history_cell/tests.rs index 5333aadc31..59e05fbd65 100644 --- a/codex-rs/tui/src/history_cell/tests.rs +++ b/codex-rs/tui/src/history_cell/tests.rs @@ -1420,6 +1420,14 @@ fn mcp_inventory_loading_without_animations_is_stable() { assert_eq!(first, vec!["• Loading MCP inventory…".to_string()]); } +#[test] +fn thread_recap_loading_without_animations_snapshot() { + let cell = ThreadRecapLoadingCell::new(/*animations_enabled*/ false); + let rendered = render_lines(&cell.display_lines(/*width*/ 80)).join("\n"); + + insta::assert_snapshot!(rendered, @"• Generating conversation recap…"); +} + #[test] fn completed_mcp_tool_call_success_snapshot() { let invocation = McpInvocation { diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 3c12b3e284..aab133ffb7 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -38,6 +38,7 @@ pub enum SlashCommand { App, Init, Compact, + Recap, Plan, Goal, Agents, @@ -90,6 +91,7 @@ impl SlashCommand { SlashCommand::New => "start a new chat during a conversation", SlashCommand::Init => "create an AGENTS.md file with instructions for Codex", SlashCommand::Compact => "summarize conversation to prevent hitting the context limit", + SlashCommand::Recap => "summarize the current conversation now", SlashCommand::Review => "review my current changes and find issues", SlashCommand::Rename => "rename the current thread", SlashCommand::Resume => "resume a saved chat", @@ -210,6 +212,7 @@ impl SlashCommand { | SlashCommand::Fork | SlashCommand::Init | SlashCommand::Compact + | SlashCommand::Recap | SlashCommand::Export | SlashCommand::Keymap | SlashCommand::Vim