diff --git a/codex-rs/exec/tests/fixtures/apply_patch_freeform_final.txt b/codex-rs/exec/tests/fixtures/apply_patch_freeform_final.txt new file mode 100644 index 0000000000..b01923e0e5 --- /dev/null +++ b/codex-rs/exec/tests/fixtures/apply_patch_freeform_final.txt @@ -0,0 +1,4 @@ +class BaseClass: + def method(): + + return True diff --git a/codex-rs/exec/tests/fixtures/sse_apply_patch_add.json b/codex-rs/exec/tests/fixtures/sse_apply_patch_add.json new file mode 100644 index 0000000000..8d2bf261af --- /dev/null +++ b/codex-rs/exec/tests/fixtures/sse_apply_patch_add.json @@ -0,0 +1,25 @@ +[ + { + "type": "response.output_item.done", + "item": { + "type": "custom_tool_call", + "name": "apply_patch", + "input": "*** Begin Patch\n*** Add File: test.md\n+Hello world\n*** End Patch", + "call_id": "__ID__" + } + }, + { + "type": "response.completed", + "response": { + "id": "__ID__", + "usage": { + "input_tokens": 0, + "input_tokens_details": null, + "output_tokens": 0, + "output_tokens_details": null, + "total_tokens": 0 + }, + "output": [] + } + } +] diff --git a/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_add.json b/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_add.json new file mode 100644 index 0000000000..ce05e7d482 --- /dev/null +++ b/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_add.json @@ -0,0 +1,25 @@ +[ + { + "type": "response.output_item.done", + "item": { + "type": "custom_tool_call", + "name": "apply_patch", + "input": "*** Begin Patch\n*** Add File: app.py\n+class BaseClass:\n+ def method():\n+ return False\n*** End Patch", + "call_id": "__ID__" + } + }, + { + "type": "response.completed", + "response": { + "id": "__ID__", + "usage": { + "input_tokens": 0, + "input_tokens_details": null, + "output_tokens": 0, + "output_tokens_details": null, + "total_tokens": 0 + }, + "output": [] + } + } +] diff --git a/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_update.json b/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_update.json new file mode 100644 index 0000000000..8329d9628c --- /dev/null +++ b/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_update.json @@ -0,0 +1,25 @@ +[ + { + "type": "response.output_item.done", + "item": { + "type": "custom_tool_call", + "name": "apply_patch", + "input": "*** Begin Patch\n*** Update File: app.py\n@@ def method():\n- return False\n+\n+ return True\n*** End Patch", + "call_id": "__ID__" + } + }, + { + "type": "response.completed", + "response": { + "id": "__ID__", + "usage": { + "input_tokens": 0, + "input_tokens_details": null, + "output_tokens": 0, + "output_tokens_details": null, + "total_tokens": 0 + }, + "output": [] + } + } +] diff --git a/codex-rs/exec/tests/fixtures/sse_apply_patch_update.json b/codex-rs/exec/tests/fixtures/sse_apply_patch_update.json new file mode 100644 index 0000000000..79689bece3 --- /dev/null +++ b/codex-rs/exec/tests/fixtures/sse_apply_patch_update.json @@ -0,0 +1,25 @@ +[ + { + "type": "response.output_item.done", + "item": { + "type": "function_call", + "name": "apply_patch", + "arguments": "{\n \"input\": \"*** Begin Patch\\n*** Update File: test.md\\n@@\\n-Hello world\\n+Final text\\n*** End Patch\"\n}", + "call_id": "__ID__" + } + }, + { + "type": "response.completed", + "response": { + "id": "__ID__", + "usage": { + "input_tokens": 0, + "input_tokens_details": null, + "output_tokens": 0, + "output_tokens_details": null, + "total_tokens": 0 + }, + "output": [] + } + } +] diff --git a/codex-rs/exec/tests/fixtures/sse_response_completed.json b/codex-rs/exec/tests/fixtures/sse_response_completed.json new file mode 100644 index 0000000000..1774dc5e84 --- /dev/null +++ b/codex-rs/exec/tests/fixtures/sse_response_completed.json @@ -0,0 +1,16 @@ +[ + { + "type": "response.completed", + "response": { + "id": "__ID__", + "usage": { + "input_tokens": 0, + "input_tokens_details": null, + "output_tokens": 0, + "output_tokens_details": null, + "total_tokens": 0 + }, + "output": [] + } + } +] diff --git a/codex-rs/exec/tests/suite/apply_patch.rs b/codex-rs/exec/tests/suite/apply_patch.rs index f05bb73298..5537853b02 100644 --- a/codex-rs/exec/tests/suite/apply_patch.rs +++ b/codex-rs/exec/tests/suite/apply_patch.rs @@ -41,148 +41,31 @@ fn test_standalone_exec_cli_can_use_apply_patch() -> anyhow::Result<()> { } #[cfg(not(target_os = "windows"))] -#[tokio::test] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_apply_patch_tool() -> anyhow::Result<()> { - use core_test_support::load_sse_fixture_with_id_from_str; - use tempfile::TempDir; - use wiremock::Mock; - use wiremock::MockServer; - use wiremock::ResponseTemplate; - use wiremock::matchers::method; - use wiremock::matchers::path; + use crate::suite::common::run_e2e_exec_test; + use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; - const SSE_TOOL_CALL_ADD: &str = r#"[ - { - "type": "response.output_item.done", - "item": { - "type": "function_call", - "name": "apply_patch", - "arguments": "{\n \"input\": \"*** Begin Patch\\n*** Add File: test.md\\n+Hello world\\n*** End Patch\"\n}", - "call_id": "__ID__" - } - }, - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -]"#; - - const SSE_TOOL_CALL_UPDATE: &str = r#"[ - { - "type": "response.output_item.done", - "item": { - "type": "function_call", - "name": "apply_patch", - "arguments": "{\n \"input\": \"*** Begin Patch\\n*** Update File: test.md\\n@@\\n-Hello world\\n+Final text\\n*** End Patch\"\n}", - "call_id": "__ID__" - } - }, - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -]"#; - - const SSE_TOOL_CALL_COMPLETED: &str = r#"[ - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -]"#; - - // Start a mock model server - let server = MockServer::start().await; - - // First response: model calls apply_patch to create test.md - let first = ResponseTemplate::new(200) - .insert_header("content-type", "text/event-stream") - .set_body_raw( - load_sse_fixture_with_id_from_str(SSE_TOOL_CALL_ADD, "call1"), - "text/event-stream", + if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() { + println!( + "Skipping test because it cannot execute when network is disabled in a Codex sandbox." ); + return Ok(()); + } - Mock::given(method("POST")) - // .and(path("/v1/responses")) - .respond_with(first) - .up_to_n_times(1) - .mount(&server) - .await; + let tmp_cwd = tempdir().expect("failed to create temp dir"); + let tmp_path = tmp_cwd.path().to_path_buf(); + run_e2e_exec_test( + tmp_cwd.path(), + vec![ + include_str!("../fixtures/sse_apply_patch_add.json").to_string(), + include_str!("../fixtures/sse_apply_patch_update.json").to_string(), + include_str!("../fixtures/sse_response_completed.json").to_string(), + ], + ) + .await; - // Second response: model calls apply_patch to update test.md - let second = ResponseTemplate::new(200) - .insert_header("content-type", "text/event-stream") - .set_body_raw( - load_sse_fixture_with_id_from_str(SSE_TOOL_CALL_UPDATE, "call2"), - "text/event-stream", - ); - - Mock::given(method("POST")) - .and(path("/v1/responses")) - .respond_with(second) - .up_to_n_times(1) - .mount(&server) - .await; - - let final_completed = ResponseTemplate::new(200) - .insert_header("content-type", "text/event-stream") - .set_body_raw( - load_sse_fixture_with_id_from_str(SSE_TOOL_CALL_COMPLETED, "resp3"), - "text/event-stream", - ); - - Mock::given(method("POST")) - .and(path("/v1/responses")) - .respond_with(final_completed) - .expect(1) - .mount(&server) - .await; - - let tmp_cwd = TempDir::new().unwrap(); - Command::cargo_bin("codex-exec") - .context("should find binary for codex-exec")? - .current_dir(tmp_cwd.path()) - .env("CODEX_HOME", tmp_cwd.path()) - .env("OPENAI_API_KEY", "dummy") - .env("OPENAI_BASE_URL", format!("{}/v1", server.uri())) - .arg("--skip-git-repo-check") - .arg("-s") - .arg("workspace-write") - .arg("foo") - .assert() - .success(); - - // Verify final file contents - let final_path = tmp_cwd.path().join("test.md"); + let final_path = tmp_path.join("test.md"); let contents = std::fs::read_to_string(&final_path) .unwrap_or_else(|e| panic!("failed reading {}: {e}", final_path.display())); assert_eq!(contents, "Final text\n"); @@ -190,150 +73,36 @@ async fn test_apply_patch_tool() -> anyhow::Result<()> { } #[cfg(not(target_os = "windows"))] -#[tokio::test] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_apply_patch_freeform_tool() -> anyhow::Result<()> { - use core_test_support::load_sse_fixture_with_id_from_str; - use tempfile::TempDir; - use wiremock::Mock; - use wiremock::MockServer; - use wiremock::ResponseTemplate; - use wiremock::matchers::method; - use wiremock::matchers::path; + use crate::suite::common::run_e2e_exec_test; + use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; - const SSE_TOOL_CALL_ADD: &str = r#"[ - { - "type": "response.output_item.done", - "item": { - "type": "custom_tool_call", - "name": "apply_patch", - "input": "*** Begin Patch\n*** Add File: test.md\n+Hello world\n*** End Patch", - "call_id": "__ID__" - } - }, - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -]"#; - - const SSE_TOOL_CALL_UPDATE: &str = r#"[ - { - "type": "response.output_item.done", - "item": { - "type": "custom_tool_call", - "name": "apply_patch", - "input": "*** Begin Patch\n*** Update File: test.md\n@@\n-Hello world\n+Final text\n*** End Patch", - "call_id": "__ID__" - } - }, - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -]"#; - - const SSE_TOOL_CALL_COMPLETED: &str = r#"[ - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -]"#; - - // Start a mock model server - let server = MockServer::start().await; - - // First response: model calls apply_patch to create test.md - let first = ResponseTemplate::new(200) - .insert_header("content-type", "text/event-stream") - .set_body_raw( - load_sse_fixture_with_id_from_str(SSE_TOOL_CALL_ADD, "call1"), - "text/event-stream", + if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() { + println!( + "Skipping test because it cannot execute when network is disabled in a Codex sandbox." ); + return Ok(()); + } - Mock::given(method("POST")) - // .and(path("/v1/responses")) - .respond_with(first) - .up_to_n_times(1) - .mount(&server) - .await; - - // Second response: model calls apply_patch to update test.md - let second = ResponseTemplate::new(200) - .insert_header("content-type", "text/event-stream") - .set_body_raw( - load_sse_fixture_with_id_from_str(SSE_TOOL_CALL_UPDATE, "call2"), - "text/event-stream", - ); - - Mock::given(method("POST")) - .and(path("/v1/responses")) - .respond_with(second) - .up_to_n_times(1) - .mount(&server) - .await; - - let final_completed = ResponseTemplate::new(200) - .insert_header("content-type", "text/event-stream") - .set_body_raw( - load_sse_fixture_with_id_from_str(SSE_TOOL_CALL_COMPLETED, "resp3"), - "text/event-stream", - ); - - Mock::given(method("POST")) - // .and(path("/v1/responses")) - .respond_with(final_completed) - .expect(1) - .mount(&server) - .await; - - let tmp_cwd = TempDir::new().unwrap(); - Command::cargo_bin("codex-exec") - .context("should find binary for codex-exec")? - .current_dir(tmp_cwd.path()) - .env("CODEX_HOME", tmp_cwd.path()) - .env("OPENAI_API_KEY", "dummy") - .env("OPENAI_BASE_URL", format!("{}/v1", server.uri())) - .arg("--skip-git-repo-check") - .arg("-s") - .arg("workspace-write") - .arg("foo") - .assert() - .success(); + let tmp_cwd = tempdir().expect("failed to create temp dir"); + run_e2e_exec_test( + tmp_cwd.path(), + vec![ + include_str!("../fixtures/sse_apply_patch_freeform_add.json").to_string(), + include_str!("../fixtures/sse_apply_patch_freeform_update.json").to_string(), + include_str!("../fixtures/sse_response_completed.json").to_string(), + ], + ) + .await; // Verify final file contents - let final_path = tmp_cwd.path().join("test.md"); + let final_path = tmp_cwd.path().join("app.py"); let contents = std::fs::read_to_string(&final_path) .unwrap_or_else(|e| panic!("failed reading {}: {e}", final_path.display())); - assert_eq!(contents, "Final text\n"); + assert_eq!( + contents, + include_str!("../fixtures/apply_patch_freeform_final.txt") + ); Ok(()) } diff --git a/codex-rs/exec/tests/suite/common.rs b/codex-rs/exec/tests/suite/common.rs new file mode 100644 index 0000000000..49747dca05 --- /dev/null +++ b/codex-rs/exec/tests/suite/common.rs @@ -0,0 +1,73 @@ +// this file is only used for e2e tests which are currently disabled on windows +#![cfg(not(target_os = "windows"))] +#![allow(clippy::expect_used)] + +use anyhow::Context; +use assert_cmd::prelude::*; +use core_test_support::load_sse_fixture_with_id_from_str; +use std::path::Path; +use std::process::Command; +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::Ordering; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::matchers::method; +use wiremock::matchers::path; + +use wiremock::Respond; + +struct SeqResponder { + num_calls: AtomicUsize, + responses: Vec, +} + +impl Respond for SeqResponder { + fn respond(&self, _: &wiremock::Request) -> wiremock::ResponseTemplate { + let call_num = self.num_calls.fetch_add(1, Ordering::SeqCst); + match self.responses.get(call_num) { + Some(body) => wiremock::ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw( + load_sse_fixture_with_id_from_str(body, &format!("request_{}", call_num)), + "text/event-stream", + ), + None => panic!("no response for {call_num}"), + } + } +} + +/// Helper function to run an E2E test of a codex-exec call. Starts a wiremock +/// server, and returns the response_streams in order for each api call. Runs +/// the codex-exec command with the wiremock server as the model server. +pub(crate) async fn run_e2e_exec_test(cwd: &Path, response_streams: Vec) { + let server = MockServer::start().await; + + let num_calls = response_streams.len(); + let seq_responder = SeqResponder { + num_calls: AtomicUsize::new(0), + responses: response_streams, + }; + + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with(seq_responder) + .expect(num_calls as u64) + .mount(&server) + .await; + + let cwd = cwd.to_path_buf(); + let uri = server.uri(); + Command::cargo_bin("codex-exec") + .context("should find binary for codex-exec") + .expect("should find binary for codex-exec") + .current_dir(cwd.clone()) + .env("CODEX_HOME", cwd.clone()) + .env("OPENAI_API_KEY", "dummy") + .env("OPENAI_BASE_URL", format!("{}/v1", uri)) + .arg("--skip-git-repo-check") + .arg("-s") + .arg("danger-full-access") + .arg("foo") + .assert() + .success(); +} diff --git a/codex-rs/exec/tests/suite/mod.rs b/codex-rs/exec/tests/suite/mod.rs index 8a83474ef9..75b19ee1b2 100644 --- a/codex-rs/exec/tests/suite/mod.rs +++ b/codex-rs/exec/tests/suite/mod.rs @@ -1,3 +1,4 @@ // Aggregates all former standalone integration tests as modules. mod apply_patch; +mod common; mod sandbox; diff --git a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs index 6a6d2aa89e..8f21a242e4 100644 --- a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs +++ b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs @@ -28,16 +28,6 @@ pub(crate) trait BottomPaneView { /// Render the view: this will be displayed in place of the composer. fn render(&self, area: Rect, buf: &mut Buffer); - /// Update the status indicator animated header. Default no-op. - fn update_status_header(&mut self, _header: String) { - // no-op - } - - /// Called when task completes to check if the view should be hidden. - fn should_hide_when_task_is_done(&mut self) -> bool { - false - } - /// Try to handle approval request; return the original value if not /// consumed. fn try_consume_approval_request( @@ -46,8 +36,4 @@ pub(crate) trait BottomPaneView { ) -> Option { Some(request) } - - /// Optional hook for views that expose a live status line. Views that do not - /// support this can ignore the call. - fn update_status_text(&mut self, _text: String) {} } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 0d8a34a809..75272dcf77 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -155,7 +155,7 @@ impl ChatComposer { ActivePopup::None => 1, }; let [textarea_rect, _] = - Layout::vertical([Constraint::Min(0), Constraint::Max(popup_height)]).areas(area); + Layout::vertical([Constraint::Min(1), Constraint::Max(popup_height)]).areas(area); let mut textarea_rect = textarea_rect; textarea_rect.width = textarea_rect.width.saturating_sub(1); textarea_rect.x += 1; @@ -232,6 +232,20 @@ impl ChatComposer { true } + /// Replace the entire composer content with `text` and reset cursor. + pub(crate) fn set_text_content(&mut self, text: String) { + self.textarea.set_text(&text); + self.textarea.set_cursor(0); + self.sync_command_popup(); + self.sync_file_search_popup(); + } + + /// Get the current composer text. + #[cfg(test)] + pub(crate) fn current_text(&self) -> String { + self.textarea.text().to_string() + } + pub fn attach_image(&mut self, path: PathBuf, width: u32, height: u32, format_label: &str) { let placeholder = format!("[image {width}x{height} {format_label}]"); // Insert as an element to match large paste placeholder behavior: @@ -1099,7 +1113,7 @@ impl ChatComposer { } } -impl WidgetRef for &ChatComposer { +impl WidgetRef for ChatComposer { fn render_ref(&self, area: Rect, buf: &mut Buffer) { let popup_height = match &self.active_popup { ActivePopup::Command(popup) => popup.calculate_required_height(), @@ -1107,7 +1121,7 @@ impl WidgetRef for &ChatComposer { ActivePopup::None => 1, }; let [textarea_rect, popup_rect] = - Layout::vertical([Constraint::Min(0), Constraint::Max(popup_height)]).areas(area); + Layout::vertical([Constraint::Min(1), Constraint::Max(popup_height)]).areas(area); match &self.active_popup { ActivePopup::Command(popup) => { popup.render_ref(popup_rect, buf); @@ -1496,7 +1510,7 @@ mod tests { } terminal - .draw(|f| f.render_widget_ref(&composer, f.area())) + .draw(|f| f.render_widget_ref(composer, f.area())) .unwrap_or_else(|e| panic!("Failed to draw {name} composer: {e}")); assert_snapshot!(name, terminal.backend()); diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 8c4802eaa5..9120a78083 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -24,7 +24,6 @@ mod list_selection_view; mod popup_consts; mod scroll_state; mod selection_popup_common; -mod status_indicator_view; mod textarea; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -36,10 +35,10 @@ pub(crate) enum CancellationEvent { pub(crate) use chat_composer::ChatComposer; pub(crate) use chat_composer::InputResult; +use crate::status_indicator_widget::StatusIndicatorWidget; use approval_modal_view::ApprovalModalView; pub(crate) use list_selection_view::SelectionAction; pub(crate) use list_selection_view::SelectionItem; -use status_indicator_view::StatusIndicatorView; /// Pane displayed in the lower half of the chat UI. pub(crate) struct BottomPane { @@ -47,7 +46,7 @@ pub(crate) struct BottomPane { /// input state is retained when the view is closed. composer: ChatComposer, - /// If present, this is displayed instead of the `composer`. + /// If present, this is displayed instead of the `composer` (e.g. modals). active_view: Option>, app_event_tx: AppEventSender, @@ -58,9 +57,10 @@ pub(crate) struct BottomPane { ctrl_c_quit_hint: bool, esc_backtrack_hint: bool, - /// True if the active view is the StatusIndicatorView that replaces the - /// composer during a running task. - status_view_active: bool, + /// Inline status indicator shown above the composer while a task is running. + status: Option, + /// Queued user messages to show under the status indicator. + queued_user_messages: Vec, } pub(crate) struct BottomPaneParams { @@ -88,42 +88,60 @@ impl BottomPane { has_input_focus: params.has_input_focus, is_task_running: false, ctrl_c_quit_hint: false, + status: None, + queued_user_messages: Vec::new(), esc_backtrack_hint: false, - status_view_active: false, } } pub fn desired_height(&self, width: u16) -> u16 { - let view_height = if let Some(view) = self.active_view.as_ref() { + let top_margin = if self.active_view.is_some() { 0 } else { 1 }; + + // Base height depends on whether a modal/overlay is active. + let mut base = if let Some(view) = self.active_view.as_ref() { view.desired_height(width) } else { self.composer.desired_height(width) }; - let top_pad = if self.active_view.is_none() || self.status_view_active { - 1 - } else { - 0 - }; - view_height - .saturating_add(Self::BOTTOM_PAD_LINES) - .saturating_add(top_pad) + // If a status indicator is active and no modal is covering the composer, + // include its height above the composer. + if self.active_view.is_none() + && let Some(status) = self.status.as_ref() + { + base = base.saturating_add(status.desired_height(width)); + } + // Account for bottom padding rows. Top spacing is handled in layout(). + base.saturating_add(Self::BOTTOM_PAD_LINES) + .saturating_add(top_margin) } - fn layout(&self, area: Rect) -> Rect { - let top = if self.active_view.is_none() || self.status_view_active { - 1 + fn layout(&self, area: Rect) -> [Rect; 2] { + // Prefer showing the status header when space is extremely tight. + // Drop the top spacer if there is only one row available. + let mut top_margin = if self.active_view.is_some() { 0 } else { 1 }; + if area.height <= 1 { + top_margin = 0; + } + + let status_height = if self.active_view.is_none() { + if let Some(status) = self.status.as_ref() { + status.desired_height(area.width) + } else { + 0 + } } else { 0 }; - let [_, content, _] = Layout::vertical([ - Constraint::Max(top), + let [_, status, content, _] = Layout::vertical([ + Constraint::Max(top_margin), + Constraint::Max(status_height), Constraint::Min(1), Constraint::Max(BottomPane::BOTTOM_PAD_LINES), ]) .areas(area); - content + [status, content] } pub fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { @@ -131,10 +149,10 @@ impl BottomPane { // status indicator shown while a task is running, or approval modal). // In these states the textarea is not interactable, so we should not // show its caret. - if self.active_view.is_some() || self.status_view_active { + if self.active_view.is_some() { None } else { - let content = self.layout(area); + let [_, content] = self.layout(area); self.composer.cursor_pos(content) } } @@ -145,18 +163,21 @@ impl BottomPane { view.handle_key_event(self, key_event); if !view.is_complete() { self.active_view = Some(view); - } else if self.is_task_running { - let mut v = StatusIndicatorView::new( - self.app_event_tx.clone(), - self.frame_requester.clone(), - ); - v.update_text("waiting for model".to_string()); - self.active_view = Some(Box::new(v)); - self.status_view_active = true; } self.request_redraw(); InputResult::None } else { + // If a task is running and a status line is visible, allow Esc to + // send an interrupt even while the composer has focus. + if matches!(key_event.code, crossterm::event::KeyCode::Esc) + && self.is_task_running + && let Some(status) = &self.status + { + // Send Op::Interrupt + status.interrupt(); + self.request_redraw(); + return InputResult::None; + } let (input_result, needs_redraw) = self.composer.handle_key_event(key_event); if needs_redraw { self.request_redraw(); @@ -178,15 +199,6 @@ impl BottomPane { CancellationEvent::Handled => { if !view.is_complete() { self.active_view = Some(view); - } else if self.is_task_running { - // Modal aborted but task still running – restore status indicator. - let mut v = StatusIndicatorView::new( - self.app_event_tx.clone(), - self.frame_requester.clone(), - ); - v.update_text("waiting for model".to_string()); - self.active_view = Some(Box::new(v)); - self.status_view_active = true; } self.show_ctrl_c_quit_hint(); } @@ -211,13 +223,24 @@ impl BottomPane { self.request_redraw(); } + /// Replace the composer text with `text`. + pub(crate) fn set_composer_text(&mut self, text: String) { + self.composer.set_text_content(text); + self.request_redraw(); + } + + /// Get the current composer text (for tests and programmatic checks). + #[cfg(test)] + pub(crate) fn composer_text(&self) -> String { + self.composer.current_text() + } + /// Update the animated header shown to the left of the brackets in the - /// status indicator (defaults to "Working"). This will update the active - /// StatusIndicatorView if present; otherwise, if a live overlay is active, - /// it will update that. If neither is present, this call is a no-op. + /// status indicator (defaults to "Working"). No-ops if the status + /// indicator is not active. pub(crate) fn update_status_header(&mut self, header: String) { - if let Some(view) = self.active_view.as_mut() { - view.update_status_header(header.clone()); + if let Some(status) = self.status.as_mut() { + status.update_header(header); self.request_redraw(); } } @@ -262,23 +285,19 @@ impl BottomPane { self.is_task_running = running; if running { - if self.active_view.is_none() { - self.active_view = Some(Box::new(StatusIndicatorView::new( + if self.status.is_none() { + self.status = Some(StatusIndicatorWidget::new( self.app_event_tx.clone(), self.frame_requester.clone(), - ))); - self.status_view_active = true; + )); + } + if let Some(status) = self.status.as_mut() { + status.set_queued_messages(self.queued_user_messages.clone()); } self.request_redraw(); } else { - // Drop the status view when a task completes, but keep other - // modal views (e.g. approval dialogs). - if let Some(mut view) = self.active_view.take() { - if !view.should_hide_when_task_is_done() { - self.active_view = Some(view); - } - self.status_view_active = false; - } + // Hide the status indicator when a task completes, but keep other modal views. + self.status = None; } } @@ -298,21 +317,16 @@ impl BottomPane { self.app_event_tx.clone(), ); self.active_view = Some(Box::new(view)); - self.status_view_active = false; self.request_redraw(); } - /// Update the live status text shown while a task is running. - /// If a modal view is active (i.e., not the status indicator), this is a no‑op. - pub(crate) fn update_status_text(&mut self, text: String) { - if !self.is_task_running || !self.status_view_active { - return; - } - if let Some(mut view) = self.active_view.take() { - view.update_status_text(text); - self.active_view = Some(view); - self.request_redraw(); + /// Update the queued messages shown under the status header. + pub(crate) fn set_queued_user_messages(&mut self, queued: Vec) { + self.queued_user_messages = queued.clone(); + if let Some(status) = self.status.as_mut() { + status.set_queued_messages(queued); } + self.request_redraw(); } pub(crate) fn composer_is_empty(&self) -> bool { @@ -353,7 +367,6 @@ impl BottomPane { // Otherwise create a new approval modal overlay. let modal = ApprovalModalView::new(request, self.app_event_tx.clone()); self.active_view = Some(Box::new(modal)); - self.status_view_active = false; self.request_redraw() } @@ -409,12 +422,20 @@ impl BottomPane { impl WidgetRef for &BottomPane { fn render_ref(&self, area: Rect, buf: &mut Buffer) { - let content = self.layout(area); + let [status_area, content] = self.layout(area); + // When a modal view is active, it owns the whole content area. if let Some(view) = &self.active_view { view.render(content, buf); } else { - (&self.composer).render_ref(content, buf); + // No active modal: + // If a status indicator is active, render it above the composer. + if let Some(status) = &self.status { + status.render_ref(status_area, buf); + } + + // Render the composer in the remaining area. + self.composer.render_ref(content, buf); } } } @@ -485,7 +506,7 @@ mod tests { } #[test] - fn composer_not_shown_after_denied_if_task_running() { + fn composer_shown_after_denied_while_task_running() { let (tx_raw, rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); let mut pane = BottomPane::new(BottomPaneParams { @@ -496,7 +517,7 @@ mod tests { placeholder_text: "Ask Codex to do anything".to_string(), }); - // Start a running task so the status indicator replaces the composer. + // Start a running task so the status indicator is active above the composer. pane.set_task_running(true); // Push an approval modal (e.g., command approval) which should hide the status view. @@ -508,16 +529,17 @@ mod tests { use crossterm::event::KeyModifiers; pane.handle_key_event(KeyEvent::new(KeyCode::Char('n'), KeyModifiers::NONE)); - // After denial, since the task is still running, the status indicator - // should be restored as the active view; the composer should NOT be visible. + // After denial, since the task is still running, the status indicator should be + // visible above the composer. The modal should be gone. assert!( - pane.status_view_active, - "status view should be active after denial" + pane.active_view.is_none(), + "no active modal view after denial" ); - assert!(pane.active_view.is_some(), "active view should be present"); - // Render and ensure the top row includes the Working header instead of the composer. - let area = Rect::new(0, 0, 40, 3); + // Render and ensure the top row includes the Working header and a composer line below. + // Give the animation thread a moment to tick. + std::thread::sleep(std::time::Duration::from_millis(120)); + let area = Rect::new(0, 0, 40, 6); let mut buf = Buffer::empty(area); (&pane).render_ref(area, &mut buf); let mut row1 = String::new(); @@ -529,6 +551,23 @@ mod tests { "expected Working header after denial on row 1: {row1:?}" ); + // Composer placeholder should be visible somewhere below. + let mut found_composer = false; + for y in 1..area.height.saturating_sub(2) { + let mut row = String::new(); + for x in 0..area.width { + row.push(buf[(x, y)].symbol().chars().next().unwrap_or(' ')); + } + if row.contains("Ask Codex") { + found_composer = true; + break; + } + } + assert!( + found_composer, + "expected composer visible under status line" + ); + // Drain the channel to avoid unused warnings. drop(rx); } @@ -548,7 +587,8 @@ mod tests { // Begin a task: show initial status. pane.set_task_running(true); - let area = Rect::new(0, 0, 40, 3); + // Use a height that allows the status line to be visible above the composer. + let area = Rect::new(0, 0, 40, 6); let mut buf = Buffer::empty(area); (&pane).render_ref(area, &mut buf); @@ -563,7 +603,7 @@ mod tests { } #[test] - fn bottom_padding_present_for_status_view() { + fn bottom_padding_present_with_status_above_composer() { let (tx_raw, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); let mut pane = BottomPane::new(BottomPaneParams { @@ -592,19 +632,29 @@ mod tests { for x in 0..area.width { top.push(buf[(x, 1)].symbol().chars().next().unwrap_or(' ')); } - assert_eq!(buf[(0, 1)].symbol().chars().next().unwrap_or(' '), '▌'); + assert!( + top.trim_start().starts_with("Working"), + "expected top row to start with 'Working': {top:?}" + ); assert!( top.contains("Working"), "expected Working header on top row: {top:?}" ); - // Bottom two rows are blank padding + // Next row (spacer) is blank, and bottom two rows are blank padding + let mut spacer = String::new(); let mut r_last = String::new(); let mut r_last2 = String::new(); for x in 0..area.width { + // Spacer row immediately below the status header lives at y=2. + spacer.push(buf[(x, 2)].symbol().chars().next().unwrap_or(' ')); r_last.push(buf[(x, height - 1)].symbol().chars().next().unwrap_or(' ')); r_last2.push(buf[(x, height - 2)].symbol().chars().next().unwrap_or(' ')); } + assert!( + spacer.trim().is_empty(), + "expected spacer line blank: {spacer:?}" + ); assert!( r_last.trim().is_empty(), "expected last row blank: {r_last:?}" @@ -629,7 +679,7 @@ mod tests { pane.set_task_running(true); - // Height=2 → with spacer, spinner on row 1; no bottom padding. + // Height=2 → composer visible; status is hidden to preserve composer. Spacer may collapse. let area2 = Rect::new(0, 0, 20, 2); let mut buf2 = Buffer::empty(area2); (&pane).render_ref(area2, &mut buf2); @@ -639,13 +689,17 @@ mod tests { row0.push(buf2[(x, 0)].symbol().chars().next().unwrap_or(' ')); row1.push(buf2[(x, 1)].symbol().chars().next().unwrap_or(' ')); } - assert!(row0.trim().is_empty(), "expected spacer on row 0: {row0:?}"); + let has_composer = row0.contains("Ask Codex") || row1.contains("Ask Codex"); assert!( - row1.contains("Working"), - "expected Working on row 1: {row1:?}" + has_composer, + "expected composer to be visible on one of the rows: row0={row0:?}, row1={row1:?}" + ); + assert!( + !row0.contains("Working") && !row1.contains("Working"), + "status header should be hidden when height=2" ); - // Height=1 → no padding; single row is the spinner. + // Height=1 → no padding; single row is the composer (status hidden). let area1 = Rect::new(0, 0, 20, 1); let mut buf1 = Buffer::empty(area1); (&pane).render_ref(area1, &mut buf1); @@ -654,8 +708,8 @@ mod tests { only.push(buf1[(x, 0)].symbol().chars().next().unwrap_or(' ')); } assert!( - only.contains("Working"), - "expected Working header with no padding: {only:?}" + only.contains("Ask Codex"), + "expected composer with no padding: {only:?}" ); } } diff --git a/codex-rs/tui/src/bottom_pane/status_indicator_view.rs b/codex-rs/tui/src/bottom_pane/status_indicator_view.rs deleted file mode 100644 index 7aeade1bc8..0000000000 --- a/codex-rs/tui/src/bottom_pane/status_indicator_view.rs +++ /dev/null @@ -1,59 +0,0 @@ -use crossterm::event::KeyCode; -use crossterm::event::KeyEvent; -use ratatui::buffer::Buffer; -use ratatui::widgets::WidgetRef; - -use crate::app_event_sender::AppEventSender; -use crate::bottom_pane::BottomPane; -use crate::status_indicator_widget::StatusIndicatorWidget; -use crate::tui::FrameRequester; - -use super::BottomPaneView; - -pub(crate) struct StatusIndicatorView { - view: StatusIndicatorWidget, -} - -impl StatusIndicatorView { - pub fn new(app_event_tx: AppEventSender, frame_requester: FrameRequester) -> Self { - Self { - view: StatusIndicatorWidget::new(app_event_tx, frame_requester), - } - } - - pub fn update_text(&mut self, text: String) { - self.view.update_text(text); - } - - pub fn update_header(&mut self, header: String) { - self.view.update_header(header); - } -} - -impl BottomPaneView for StatusIndicatorView { - fn update_status_header(&mut self, header: String) { - self.update_header(header); - } - - fn should_hide_when_task_is_done(&mut self) -> bool { - true - } - - fn desired_height(&self, width: u16) -> u16 { - self.view.desired_height(width) - } - - fn render(&self, area: ratatui::layout::Rect, buf: &mut Buffer) { - self.view.render_ref(area, buf); - } - - fn handle_key_event(&mut self, _pane: &mut BottomPane, key_event: KeyEvent) { - if key_event.code == KeyCode::Esc { - self.view.interrupt(); - } - } - - fn update_status_text(&mut self, text: String) { - self.update_text(text); - } -} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 5e047bbd41..cfe9dfe14d 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::collections::VecDeque; use std::path::PathBuf; use std::sync::Arc; @@ -30,8 +31,10 @@ use codex_core::protocol::TurnAbortReason; use codex_core::protocol::TurnDiffEvent; use codex_core::protocol::WebSearchBeginEvent; use codex_protocol::parse_command::ParsedCommand; +use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; +use crossterm::event::KeyModifiers; use rand::Rng; use ratatui::buffer::Buffer; use ratatui::layout::Constraint; @@ -105,8 +108,6 @@ pub(crate) struct ChatWidget { task_complete_pending: bool, // Queue of interruptive UI events deferred during an active write cycle interrupts: InterruptManager, - // Whether a redraw is needed after handling the current event - needs_redraw: bool, // Accumulates the current reasoning block text to extract a header reasoning_buffer: String, // Accumulates full reasoning content for transcript-only recording @@ -116,6 +117,8 @@ pub(crate) struct ChatWidget { // Whether to include the initial welcome banner on session configured show_welcome_banner: bool, last_history_was_exec: bool, + // User messages queued while a turn is in progress + queued_user_messages: VecDeque, } struct UserMessage { @@ -141,10 +144,6 @@ fn create_initial_user_message(text: String, image_paths: Vec) -> Optio } impl ChatWidget { - #[inline] - fn mark_needs_redraw(&mut self) { - self.needs_redraw = true; - } fn flush_answer_stream_with_separator(&mut self) { let sink = AppEventHistorySink(self.app_event_tx.clone()); let _ = self.stream.finalize(true, &sink); @@ -162,14 +161,14 @@ impl ChatWidget { if let Some(user_message) = self.initial_user_message.take() { self.submit_user_message(user_message); } - self.mark_needs_redraw(); + self.request_redraw(); } fn on_agent_message(&mut self, message: String) { let sink = AppEventHistorySink(self.app_event_tx.clone()); let finished = self.stream.apply_final_answer(&message, &sink); self.handle_if_stream_finished(finished); - self.mark_needs_redraw(); + self.request_redraw(); } fn on_agent_message_delta(&mut self, delta: String) { @@ -188,7 +187,7 @@ impl ChatWidget { } else { // Fallback while we don't yet have a bold header: leave existing header as-is. } - self.mark_needs_redraw(); + self.request_redraw(); } fn on_agent_reasoning_final(&mut self) { @@ -202,7 +201,7 @@ impl ChatWidget { } self.reasoning_buffer.clear(); self.full_reasoning_buffer.clear(); - self.mark_needs_redraw(); + self.request_redraw(); } fn on_reasoning_section_break(&mut self) { @@ -220,7 +219,7 @@ impl ChatWidget { self.stream.reset_headers_for_new_turn(); self.full_reasoning_buffer.clear(); self.reasoning_buffer.clear(); - self.mark_needs_redraw(); + self.request_redraw(); } fn on_task_complete(&mut self) { @@ -233,7 +232,10 @@ impl ChatWidget { // Mark task stopped and request redraw now that all content is in history. self.bottom_pane.set_task_running(false); self.running_commands.clear(); - self.mark_needs_redraw(); + self.request_redraw(); + + // If there is a queued user message, send exactly one now to begin the next turn. + self.maybe_send_next_queued_input(); } fn on_token_count(&mut self, token_usage: TokenUsage) { @@ -251,7 +253,10 @@ impl ChatWidget { self.bottom_pane.set_task_running(false); self.running_commands.clear(); self.stream.clear_all(); - self.mark_needs_redraw(); + self.request_redraw(); + + // After an error ends the turn, try sending the next queued input. + self.maybe_send_next_queued_input(); } fn on_plan_update(&mut self, update: codex_core::plan_tool::UpdatePlanArgs) { @@ -354,7 +359,7 @@ impl ChatWidget { fn on_stream_error(&mut self, message: String) { // Show stream errors in the transcript so users see retry/backoff info. self.add_to_history(history_cell::new_stream_error_event(message)); - self.mark_needs_redraw(); + self.request_redraw(); } /// Periodic tick to commit at most one queued line to history with a small delay, /// animating the output. @@ -408,7 +413,7 @@ impl ChatWidget { let sink = AppEventHistorySink(self.app_event_tx.clone()); self.stream.begin(&sink); self.stream.push_and_maybe_commit(&delta, &sink); - self.mark_needs_redraw(); + self.request_redraw(); } pub(crate) fn handle_exec_end_now(&mut self, ev: ExecCommandEndEvent) { @@ -473,7 +478,7 @@ impl ChatWidget { reason: ev.reason, }; self.bottom_pane.push_approval_request(request); - self.mark_needs_redraw(); + self.request_redraw(); } pub(crate) fn handle_apply_patch_approval_now( @@ -613,11 +618,11 @@ impl ChatWidget { pending_exec_completions: Vec::new(), task_complete_pending: false, interrupts: InterruptManager::new(), - needs_redraw: false, reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), session_id: None, last_history_was_exec: false, + queued_user_messages: VecDeque::new(), show_welcome_banner: true, } } @@ -659,11 +664,11 @@ impl ChatWidget { pending_exec_completions: Vec::new(), task_complete_pending: false, interrupts: InterruptManager::new(), - needs_redraw: false, reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), session_id: None, last_history_was_exec: false, + queued_user_messages: VecDeque::new(), show_welcome_banner: false, } } @@ -681,13 +686,39 @@ impl ChatWidget { self.bottom_pane.clear_ctrl_c_quit_hint(); } + // Alt+Up: Edit the most recent queued user message (if any). + if matches!( + key_event, + KeyEvent { + code: KeyCode::Up, + modifiers: KeyModifiers::ALT, + kind: KeyEventKind::Press, + .. + } + ) && !self.queued_user_messages.is_empty() + { + // Prefer the most recently queued item. + if let Some(user_message) = self.queued_user_messages.pop_back() { + self.bottom_pane.set_composer_text(user_message.text); + self.refresh_queued_user_messages(); + self.request_redraw(); + } + return; + } + match self.bottom_pane.handle_key_event(key_event) { InputResult::Submitted(text) => { - let images = self.bottom_pane.take_recent_submission_images(); - self.submit_user_message(UserMessage { + // If a task is running, queue the user input to be sent after the turn completes. + let user_message = UserMessage { text, - image_paths: images, - }); + image_paths: self.bottom_pane.take_recent_submission_images(), + }; + if self.bottom_pane.is_task_running() { + self.queued_user_messages.push_back(user_message); + self.refresh_queued_user_messages(); + } else { + self.submit_user_message(user_message); + } } InputResult::Command(cmd) => { self.dispatch_command(cmd); @@ -873,8 +904,6 @@ impl ChatWidget { } pub(crate) fn handle_codex_event(&mut self, event: Event) { - // Reset redraw flag for this dispatch - self.needs_redraw = false; let Event { id, msg } = event; match msg { @@ -938,27 +967,40 @@ impl ChatWidget { .send(crate::app_event::AppEvent::ConversationHistory(ev)); } } - // Coalesce redraws: issue at most one after handling the event - if self.needs_redraw { - self.request_redraw(); - self.needs_redraw = false; - } } fn request_redraw(&mut self) { self.frame_requester.schedule_frame(); } + // If idle and there are queued inputs, submit exactly one to start the next turn. + fn maybe_send_next_queued_input(&mut self) { + if self.bottom_pane.is_task_running() { + return; + } + if let Some(user_message) = self.queued_user_messages.pop_front() { + self.submit_user_message(user_message); + } + // Update the list to reflect the remaining queued messages (if any). + self.refresh_queued_user_messages(); + } + + /// Rebuild and update the queued user messages from the current queue. + fn refresh_queued_user_messages(&mut self) { + let messages: Vec = self + .queued_user_messages + .iter() + .map(|m| m.text.clone()) + .collect(); + self.bottom_pane.set_queued_user_messages(messages); + } + pub(crate) fn add_diff_in_progress(&mut self) { - self.bottom_pane.set_task_running(true); - self.bottom_pane - .update_status_text("computing diff".to_string()); self.request_redraw(); } pub(crate) fn on_diff_complete(&mut self) { - self.bottom_pane.set_task_running(false); - self.mark_needs_redraw(); + self.request_redraw(); } pub(crate) fn add_status_output(&mut self) { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap new file mode 100644 index 0000000000..9a205be1d2 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: terminal.backend() +--- +"? Codex wants to run echo hello world " +" " +"Model wants to run a command " +" " +"▌Allow command? " +"▌ Yes Always No No, provide feedback " +"▌ Approve and run the command " +" " +" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap new file mode 100644 index 0000000000..2e79127faa --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/chatwidget/tests.rs +assertion_line: 690 +expression: terminal.backend() +--- +"The model wants to apply changes " +" " +"This will grant write access to /tmp for the remainder of this session. " +" " +"▌Apply changes? " +"▌ Yes No No, provide feedback " +"▌ Approve and apply the changes " +" " +" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h1.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h1.snap new file mode 100644 index 0000000000..3a6e6532e5 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h1.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: terminal.backend() +--- +"▌ Ask Codex to do anything " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap new file mode 100644 index 0000000000..48e6154eb2 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: terminal.backend() +--- +"▌ Ask Codex to do anything " +" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h3.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h3.snap new file mode 100644 index 0000000000..7ab4159ade --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h3.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: terminal.backend() +--- +"▌ Ask Codex to do anything " +" " +" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h1.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h1.snap new file mode 100644 index 0000000000..3a6e6532e5 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h1.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: terminal.backend() +--- +"▌ Ask Codex to do anything " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h2.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h2.snap new file mode 100644 index 0000000000..48e6154eb2 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h2.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: terminal.backend() +--- +"▌ Ask Codex to do anything " +" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h3.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h3.snap new file mode 100644 index 0000000000..c77477a083 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h3.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: terminal.backend() +--- +" " +"▌ Ask Codex to do anything " +" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap new file mode 100644 index 0000000000..79b55e2657 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/chatwidget/tests.rs +assertion_line: 806 +expression: terminal.backend() +--- +" " +" Analyzing (0s • Esc to interrupt) " +" " +"▌ Ask Codex to do anything " +" ⏎ send Ctrl+J newline Ctrl+T transcript Ctrl+C quit " +" " +" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap new file mode 100644 index 0000000000..5cd475248e --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: terminal.backend() +--- +"? Codex wants to run echo 'hello world' " +" " +"Codex wants to run a command " +" " +"▌Allow command? " +"▌ Yes Always No No, provide feedback " +"▌ Approve and run the command " +" " +" " diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 7e4d1c7dc5..913dd00992 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -14,6 +14,7 @@ use codex_core::protocol::AgentReasoningEvent; use codex_core::protocol::ApplyPatchApprovalRequestEvent; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; +use codex_core::protocol::ExecApprovalRequestEvent; use codex_core::protocol::ExecCommandBeginEvent; use codex_core::protocol::ExecCommandEndEvent; use codex_core::protocol::FileChange; @@ -178,13 +179,13 @@ fn make_chatwidget_manual() -> ( pending_exec_completions: Vec::new(), task_complete_pending: false, interrupts: InterruptManager::new(), - needs_redraw: false, reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), session_id: None, frame_requester: crate::tui::FrameRequester::test_dummy(), show_welcome_banner: true, last_history_was_exec: false, + queued_user_messages: std::collections::VecDeque::new(), }; (widget, rx, op_rx) } @@ -338,6 +339,36 @@ fn open_fixture(name: &str) -> std::fs::File { File::open(name).expect("open fixture file") } +#[test] +fn alt_up_edits_most_recent_queued_message() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + + // Simulate a running task so messages would normally be queued. + chat.bottom_pane.set_task_running(true); + + // Seed two queued messages. + chat.queued_user_messages + .push_back(UserMessage::from("first queued".to_string())); + chat.queued_user_messages + .push_back(UserMessage::from("second queued".to_string())); + chat.refresh_queued_user_messages(); + + // Press Alt+Up to edit the most recent (last) queued message. + chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)); + + // Composer should now contain the last queued message. + assert_eq!( + chat.bottom_pane.composer_text(), + "second queued".to_string() + ); + // And the queue should now contain only the remaining (older) item. + assert_eq!(chat.queued_user_messages.len(), 1); + assert_eq!( + chat.queued_user_messages.front().unwrap().text, + "first queued" + ); +} + #[test] fn exec_history_cell_shows_working_then_completed() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); @@ -723,6 +754,189 @@ async fn binary_size_transcript_matches_ideal_fixture() { assert_eq!(visible_after, ideal); } +// +// Snapshot test: command approval modal +// +// Synthesizes a Codex ExecApprovalRequest event to trigger the approval modal +// and snapshots the visual output using the ratatui TestBackend. +#[test] +fn approval_modal_exec_snapshot() { + // Build a chat widget with manual channels to avoid spawning the agent. + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + // Ensure policy allows surfacing approvals explicitly (not strictly required for direct event). + chat.config.approval_policy = codex_core::protocol::AskForApproval::OnRequest; + // Inject an exec approval request to display the approval modal. + let ev = ExecApprovalRequestEvent { + call_id: "call-approve-cmd".into(), + command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + reason: Some("Model wants to run a command".into()), + }; + chat.handle_codex_event(Event { + id: "sub-approve".into(), + msg: EventMsg::ExecApprovalRequest(ev), + }); + // Render to a fixed-size test terminal and snapshot. + // Call desired_height first and use that exact height for rendering. + let height = chat.desired_height(80); + let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, height)) + .expect("create terminal"); + terminal + .draw(|f| f.render_widget_ref(&chat, f.area())) + .expect("draw approval modal"); + assert_snapshot!("approval_modal_exec", terminal.backend()); +} + +// Snapshot test: patch approval modal +#[test] +fn approval_modal_patch_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + chat.config.approval_policy = codex_core::protocol::AskForApproval::OnRequest; + + // Build a small changeset and a reason/grant_root to exercise the prompt text. + let mut changes = std::collections::HashMap::new(); + changes.insert( + PathBuf::from("README.md"), + FileChange::Add { + content: "hello\nworld\n".into(), + }, + ); + let ev = ApplyPatchApprovalRequestEvent { + call_id: "call-approve-patch".into(), + changes, + reason: Some("The model wants to apply changes".into()), + grant_root: Some(PathBuf::from("/tmp")), + }; + chat.handle_codex_event(Event { + id: "sub-approve-patch".into(), + msg: EventMsg::ApplyPatchApprovalRequest(ev), + }); + + // Render at the widget's desired height and snapshot. + let height = chat.desired_height(80); + let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, height)) + .expect("create terminal"); + terminal + .draw(|f| f.render_widget_ref(&chat, f.area())) + .expect("draw patch approval modal"); + assert_snapshot!("approval_modal_patch", terminal.backend()); +} + +// Snapshot test: ChatWidget at very small heights (idle) +// Ensures overall layout behaves when terminal height is extremely constrained. +#[test] +fn ui_snapshots_small_heights_idle() { + use ratatui::Terminal; + use ratatui::backend::TestBackend; + let (chat, _rx, _op_rx) = make_chatwidget_manual(); + for h in [1u16, 2, 3] { + let name = format!("chat_small_idle_h{h}"); + let mut terminal = Terminal::new(TestBackend::new(40, h)).expect("create terminal"); + terminal + .draw(|f| f.render_widget_ref(&chat, f.area())) + .expect("draw chat idle"); + assert_snapshot!(name, terminal.backend()); + } +} + +// Snapshot test: ChatWidget at very small heights (task running) +// Validates how status + composer are presented within tight space. +#[test] +fn ui_snapshots_small_heights_task_running() { + use ratatui::Terminal; + use ratatui::backend::TestBackend; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + // Activate status line + chat.handle_codex_event(Event { + id: "task-1".into(), + msg: EventMsg::TaskStarted, + }); + chat.handle_codex_event(Event { + id: "task-1".into(), + msg: EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { + delta: "**Thinking**".into(), + }), + }); + for h in [1u16, 2, 3] { + let name = format!("chat_small_running_h{h}"); + let mut terminal = Terminal::new(TestBackend::new(40, h)).expect("create terminal"); + terminal + .draw(|f| f.render_widget_ref(&chat, f.area())) + .expect("draw chat running"); + assert_snapshot!(name, terminal.backend()); + } +} + +// Snapshot test: status widget + approval modal active together +// The modal takes precedence visually; this captures the layout with a running +// task (status indicator active) while an approval request is shown. +#[test] +fn status_widget_and_approval_modal_snapshot() { + use codex_core::protocol::ExecApprovalRequestEvent; + + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + // Begin a running task so the status indicator would be active. + chat.handle_codex_event(Event { + id: "task-1".into(), + msg: EventMsg::TaskStarted, + }); + // Provide a deterministic header for the status line. + chat.handle_codex_event(Event { + id: "task-1".into(), + msg: EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { + delta: "**Analyzing**".into(), + }), + }); + + // Now show an approval modal (e.g. exec approval). + let ev = ExecApprovalRequestEvent { + call_id: "call-approve-exec".into(), + command: vec!["echo".into(), "hello world".into()], + cwd: std::path::PathBuf::from("/tmp"), + reason: Some("Codex wants to run a command".into()), + }; + chat.handle_codex_event(Event { + id: "sub-approve-exec".into(), + msg: EventMsg::ExecApprovalRequest(ev), + }); + + // Render at the widget's desired height and snapshot. + let height = chat.desired_height(80); + let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, height)) + .expect("create terminal"); + terminal + .draw(|f| f.render_widget_ref(&chat, f.area())) + .expect("draw status + approval modal"); + assert_snapshot!("status_widget_and_approval_modal", terminal.backend()); +} + +// Snapshot test: status widget active (StatusIndicatorView) +// Ensures the VT100 rendering of the status indicator is stable when active. +#[test] +fn status_widget_active_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + // Activate the status indicator by simulating a task start. + chat.handle_codex_event(Event { + id: "task-1".into(), + msg: EventMsg::TaskStarted, + }); + // Provide a deterministic header via a bold reasoning chunk. + chat.handle_codex_event(Event { + id: "task-1".into(), + msg: EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { + delta: "**Analyzing**".into(), + }), + }); + // Render and snapshot. + let height = chat.desired_height(80); + let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, height)) + .expect("create terminal"); + terminal + .draw(|f| f.render_widget_ref(&chat, f.area())) + .expect("draw status widget"); + assert_snapshot!("status_widget_active", terminal.backend()); +} + #[test] fn apply_patch_events_emit_history_cells() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index b6488ebffc..5ea193650b 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -237,7 +237,10 @@ pub(crate) fn new_session_info( } = event; if is_first_event { let cwd_str = match relativize_to_home(&config.cwd) { - Some(rel) if !rel.as_os_str().is_empty() => format!("~/{}", rel.display()), + Some(rel) if !rel.as_os_str().is_empty() => { + let sep = std::path::MAIN_SEPARATOR; + format!("~{sep}{}", rel.display()) + } Some(_) => "~".to_string(), None => config.cwd.display().to_string(), }; @@ -746,7 +749,10 @@ pub(crate) fn new_status_output( lines.push(Line::from(vec!["📂 ".into(), "Workspace".bold()])); // Path (home-relative, e.g., ~/code/project) let cwd_str = match relativize_to_home(&config.cwd) { - Some(rel) if !rel.as_os_str().is_empty() => format!("~/{}", rel.display()), + Some(rel) if !rel.as_os_str().is_empty() => { + let sep = std::path::MAIN_SEPARATOR; + format!("~{sep}{}", rel.display()) + } Some(_) => "~".to_string(), None => config.cwd.display().to_string(), }; @@ -789,7 +795,8 @@ pub(crate) fn new_status_output( ups += 1; } if reached { - format!("{}AGENTS.md", "../".repeat(ups)) + let up = format!("..{}", std::path::MAIN_SEPARATOR); + format!("{}AGENTS.md", up.repeat(ups)) } else if let Ok(stripped) = p.strip_prefix(&config.cwd) { stripped.display().to_string() } else { diff --git a/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_truncated.snap b/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_truncated.snap new file mode 100644 index 0000000000..19dc5d3137 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_truncated.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/status_indicator_widget.rs +expression: terminal.backend() +--- +" Working (0s • Esc t" +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_with_queued_messages.snap b/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_with_queued_messages.snap new file mode 100644 index 0000000000..cb51754962 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_with_queued_messages.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/status_indicator_widget.rs +expression: terminal.backend() +--- +" Working (0s • Esc to interrupt) " +" ↳ first " +" ↳ second " +" Alt+↑ edit " +" " +" " +" " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_with_working_header.snap b/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_with_working_header.snap new file mode 100644 index 0000000000..fe9eebedd6 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_with_working_header.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/status_indicator_widget.rs +expression: terminal.backend() +--- +" Working (0s • Esc to interrupt) " +" " diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index dbdd230da7..d7fe24db0e 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -7,39 +7,24 @@ use std::time::Instant; use codex_core::protocol::Op; use ratatui::buffer::Buffer; use ratatui::layout::Rect; -use ratatui::style::Color; -use ratatui::style::Modifier; -use ratatui::style::Style; +use ratatui::style::Stylize; use ratatui::text::Line; -use ratatui::text::Span; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; -use unicode_width::UnicodeWidthStr; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::shimmer::shimmer_spans; use crate::tui::FrameRequester; - -// We render the live text using markdown so it visually matches the history -// cells. Before rendering we strip any ANSI escape sequences to avoid writing -// raw control bytes into the back buffer. -use codex_ansi_escape::ansi_escape_line; +use textwrap::Options as TwOptions; +use textwrap::WordSplitter; pub(crate) struct StatusIndicatorWidget { - /// Latest text to display (truncated to the available width at render - /// time). - text: String, /// Animated header text (defaults to "Working"). header: String, + /// Queued user messages to display under the status line. + queued_messages: Vec, - /// Animation state: reveal target `text` progressively like a typewriter. - /// We compute the currently visible prefix length based on the current - /// frame index and a constant typing speed. The `base_frame` and - /// `reveal_len_at_base` form the anchor from which we advance. - last_target_len: usize, - base_frame: usize, - reveal_len_at_base: usize, start_time: Instant, app_event_tx: AppEventSender, frame_requester: FrameRequester, @@ -48,11 +33,8 @@ pub(crate) struct StatusIndicatorWidget { impl StatusIndicatorWidget { pub(crate) fn new(app_event_tx: AppEventSender, frame_requester: FrameRequester) -> Self { Self { - text: String::from("waiting for model"), header: String::from("Working"), - last_target_len: 0, - base_frame: 0, - reveal_len_at_base: 0, + queued_messages: Vec::new(), start_time: Instant::now(), app_event_tx, @@ -60,38 +42,32 @@ impl StatusIndicatorWidget { } } - pub fn desired_height(&self, _width: u16) -> u16 { - 1 - } - - /// Update the line that is displayed in the widget. - pub(crate) fn update_text(&mut self, text: String) { - // If the text hasn't changed, don't reset the baseline; let the - // animation continue advancing naturally. - if text == self.text { - return; + pub fn desired_height(&self, width: u16) -> u16 { + // Status line + wrapped queued messages (up to 3 lines per message) + // + optional ellipsis line per truncated message + 1 spacer line + let inner_width = width.max(1) as usize; + let mut total: u16 = 1; // status line + let text_width = inner_width.saturating_sub(3); // account for " ↳ " prefix + if text_width > 0 { + let opts = TwOptions::new(text_width) + .break_words(false) + .word_splitter(WordSplitter::NoHyphenation); + for q in &self.queued_messages { + let wrapped = textwrap::wrap(q, &opts); + let lines = wrapped.len().min(3) as u16; + total = total.saturating_add(lines); + if wrapped.len() > 3 { + total = total.saturating_add(1); // ellipsis line + } + } + if !self.queued_messages.is_empty() { + total = total.saturating_add(1); // keybind hint line + } + } else { + // At least one line per message if width is extremely narrow + total = total.saturating_add(self.queued_messages.len() as u16); } - // Update the target text, preserving newlines so wrapping matches history cells. - // Strip ANSI escapes for the character count so the typewriter animation speed is stable. - let stripped = { - let line = ansi_escape_line(&text); - line.spans - .iter() - .map(|s| s.content.as_ref()) - .collect::>() - .join("") - }; - let new_len = stripped.chars().count(); - - // Compute how many characters are currently revealed so we can carry - // this forward as the new baseline when target text changes. - let current_frame = self.current_frame(); - let shown_now = self.current_shown_len(current_frame); - - self.text = text; - self.last_target_len = new_len; - self.base_frame = current_frame; - self.reveal_len_at_base = shown_now.min(new_len); + total.saturating_add(1) // spacer line } pub(crate) fn interrupt(&self) { @@ -105,125 +81,57 @@ impl StatusIndicatorWidget { } } - /// Reset the animation and start revealing `text` from the beginning. - #[cfg(test)] - pub(crate) fn restart_with_text(&mut self, text: String) { - let sanitized = text.replace(['\n', '\r'], " "); - let stripped = { - let line = ansi_escape_line(&sanitized); - line.spans - .iter() - .map(|s| s.content.as_ref()) - .collect::>() - .join("") - }; - - let new_len = stripped.chars().count(); - let current_frame = self.current_frame(); - - self.text = sanitized; - self.last_target_len = new_len; - self.base_frame = current_frame; - // Start from zero revealed characters for a fresh typewriter cycle. - self.reveal_len_at_base = 0; - } - - /// Calculate how many characters should currently be visible given the - /// animation baseline and frame counter. - fn current_shown_len(&self, current_frame: usize) -> usize { - // Increase typewriter speed (~5x): reveal more characters per frame. - const TYPING_CHARS_PER_FRAME: usize = 7; - let frames = current_frame.saturating_sub(self.base_frame); - let advanced = self - .reveal_len_at_base - .saturating_add(frames.saturating_mul(TYPING_CHARS_PER_FRAME)); - advanced.min(self.last_target_len) - } - - fn current_frame(&self) -> usize { - // Derive frame index from wall-clock time. 100ms per frame to match - // the previous ticker cadence. - let since_start = self.start_time.elapsed(); - (since_start.as_millis() / 100) as usize - } - - /// Test-only helper to fast-forward the internal clock so animations - /// advance without sleeping. - #[cfg(test)] - pub(crate) fn test_fast_forward_frames(&mut self, frames: usize) { - let advance_ms = (frames as u64).saturating_mul(100); - // Move the start time into the past so `current_frame()` advances. - self.start_time = std::time::Instant::now() - std::time::Duration::from_millis(advance_ms); + /// Replace the queued messages displayed beneath the header. + pub(crate) fn set_queued_messages(&mut self, queued: Vec) { + self.queued_messages = queued; + // Ensure a redraw so changes are visible. + self.frame_requester.schedule_frame(); } } impl WidgetRef for StatusIndicatorWidget { fn render_ref(&self, area: Rect, buf: &mut Buffer) { - // Ensure minimal height - if area.height == 0 || area.width == 0 { + if area.is_empty() { return; } // Schedule next animation frame. self.frame_requester .schedule_frame_in(Duration::from_millis(32)); - let idx = self.current_frame(); let elapsed = self.start_time.elapsed().as_secs(); - let shown_now = self.current_shown_len(idx); - let status_prefix: String = self.text.chars().take(shown_now).collect(); - let animated_spans = shimmer_spans(&self.header); // Plain rendering: no borders or padding so the live cell is visually indistinguishable from terminal scrollback. - let inner_width = area.width as usize; + let mut spans = vec![" ".into()]; + spans.extend(shimmer_spans(&self.header)); + spans.extend(vec![ + " ".into(), + format!("({elapsed}s • ").dim(), + "Esc".dim().bold(), + " to interrupt)".dim(), + ]); - let mut spans: Vec> = Vec::new(); - spans.push(Span::styled("▌ ", Style::default().fg(Color::Cyan))); - - // Animated header after the left bar - spans.extend(animated_spans); - // Space between header and bracket block - spans.push(Span::raw(" ")); - // Non-animated, dim bracket content, with keys bold - let bracket_prefix = format!("({elapsed}s • "); - spans.push(Span::styled( - bracket_prefix, - Style::default().add_modifier(Modifier::DIM), - )); - spans.push(Span::styled( - "Esc", - Style::default().add_modifier(Modifier::DIM | Modifier::BOLD), - )); - spans.push(Span::styled( - " to interrupt)", - Style::default().add_modifier(Modifier::DIM), - )); - // Add a space and then the log text (not animated by the gradient) - if !status_prefix.is_empty() { - spans.push(Span::styled( - " ", - Style::default().add_modifier(Modifier::DIM), - )); - spans.push(Span::styled( - status_prefix, - Style::default().add_modifier(Modifier::DIM), - )); - } - - // Truncate spans to fit the width. - let mut acc: Vec> = Vec::new(); - let mut used = 0usize; - for s in spans { - let w = s.content.width(); - if used + w <= inner_width { - acc.push(s); - used += w; - } else { - break; + // Build lines: status, then queued messages, then spacer. + let mut lines: Vec> = Vec::new(); + lines.push(Line::from(spans)); + // Wrap queued messages using textwrap and show up to the first 3 lines per message. + let text_width = area.width.saturating_sub(3); // " ↳ " prefix + let opts = TwOptions::new(text_width as usize) + .break_words(false) + .word_splitter(WordSplitter::NoHyphenation); + for q in &self.queued_messages { + let wrapped = textwrap::wrap(q, &opts); + for (i, piece) in wrapped.iter().take(3).enumerate() { + let prefix = if i == 0 { " ↳ " } else { " " }; + let content = format!("{prefix}{piece}"); + lines.push(Line::from(content.dim())); + } + if wrapped.len() > 3 { + lines.push(Line::from(" …".dim())); } } - let lines = vec![Line::from(acc)]; - - // No-op once full text is revealed; the app no longer reacts to a completion event. + if !self.queued_messages.is_empty() { + lines.push(Line::from(vec![" ".into(), "Alt+↑".cyan(), " edit".into()]).dim()); + } let paragraph = Paragraph::new(lines); paragraph.render_ref(area, buf); @@ -235,60 +143,51 @@ mod tests { use super::*; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; + use insta::assert_snapshot; + use ratatui::Terminal; + use ratatui::backend::TestBackend; use tokio::sync::mpsc::unbounded_channel; #[test] - fn renders_without_left_border_or_padding() { + fn renders_with_working_header() { let (tx_raw, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); - let mut w = StatusIndicatorWidget::new(tx, crate::tui::FrameRequester::test_dummy()); - w.restart_with_text("Hello".to_string()); + let w = StatusIndicatorWidget::new(tx, crate::tui::FrameRequester::test_dummy()); - let area = ratatui::layout::Rect::new(0, 0, 30, 1); - // Advance animation without sleeping. - w.test_fast_forward_frames(2); - let mut buf = ratatui::buffer::Buffer::empty(area); - w.render_ref(area, &mut buf); - - // Leftmost column has the left bar - let ch0 = buf[(0, 0)].symbol().chars().next().unwrap_or(' '); - assert_eq!(ch0, '▌', "expected left bar at col 0: {ch0:?}"); + // Render into a fixed-size test terminal and snapshot the backend. + let mut terminal = Terminal::new(TestBackend::new(80, 2)).expect("terminal"); + terminal + .draw(|f| w.render_ref(f.area(), f.buffer_mut())) + .expect("draw"); + assert_snapshot!(terminal.backend()); } #[test] - fn working_header_is_present_on_last_line() { + fn renders_truncated() { let (tx_raw, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); - let mut w = StatusIndicatorWidget::new(tx, crate::tui::FrameRequester::test_dummy()); - w.restart_with_text("Hi".to_string()); - // Advance animation without sleeping. - w.test_fast_forward_frames(2); + let w = StatusIndicatorWidget::new(tx, crate::tui::FrameRequester::test_dummy()); - let area = ratatui::layout::Rect::new(0, 0, 30, 1); - let mut buf = ratatui::buffer::Buffer::empty(area); - w.render_ref(area, &mut buf); - - // Single line; it should contain the animated "Working" header. - let mut row = String::new(); - for x in 0..area.width { - row.push(buf[(x, 0)].symbol().chars().next().unwrap_or(' ')); - } - assert!(row.contains("Working"), "expected Working header: {row:?}"); + // Render into a fixed-size test terminal and snapshot the backend. + let mut terminal = Terminal::new(TestBackend::new(20, 2)).expect("terminal"); + terminal + .draw(|f| w.render_ref(f.area(), f.buffer_mut())) + .expect("draw"); + assert_snapshot!(terminal.backend()); } #[test] - fn header_starts_at_expected_position() { + fn renders_with_queued_messages() { let (tx_raw, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); let mut w = StatusIndicatorWidget::new(tx, crate::tui::FrameRequester::test_dummy()); - w.restart_with_text("Hello".to_string()); - w.test_fast_forward_frames(2); + w.set_queued_messages(vec!["first".to_string(), "second".to_string()]); - let area = ratatui::layout::Rect::new(0, 0, 30, 1); - let mut buf = ratatui::buffer::Buffer::empty(area); - w.render_ref(area, &mut buf); - - let ch = buf[(2, 0)].symbol().chars().next().unwrap_or(' '); - assert_eq!(ch, 'W', "expected Working header at col 2: {ch:?}"); + // Render into a fixed-size test terminal and snapshot the backend. + let mut terminal = Terminal::new(TestBackend::new(80, 8)).expect("terminal"); + terminal + .draw(|f| w.render_ref(f.area(), f.buffer_mut())) + .expect("draw"); + assert_snapshot!(terminal.backend()); } } diff --git a/codex-rs/tui/src/user_approval_widget.rs b/codex-rs/tui/src/user_approval_widget.rs index 69ee3524e8..39c9af084a 100644 --- a/codex-rs/tui/src/user_approval_widget.rs +++ b/codex-rs/tui/src/user_approval_widget.rs @@ -373,7 +373,11 @@ impl UserApprovalWidget { } pub(crate) fn desired_height(&self, width: u16) -> u16 { - self.get_confirmation_prompt_height(width) + self.select_options.len() as u16 + // Reserve space for: + // - 1 title line ("Allow command?" or "Apply changes?") + // - 1 buttons line (options rendered horizontally on a single row) + // - 1 description line (context for the currently selected option) + self.get_confirmation_prompt_height(width) + 3 } }