From bc39b0ed59a85c47b2e9c267cdd57bb1018bc5f2 Mon Sep 17 00:00:00 2001 From: Abhinav Date: Tue, 1 Sep 2026 18:56:40 +0000 Subject: [PATCH] Refine hook activity rendering in the TUI (#42118) ## What changed - Show revealed running hooks in the bottom activity area instead of the conversation viewport or transcript overlay. Fold the hook message into the active status row when space permits and reflow it below the row without displacing background-process controls or status details. - Keep fast hooks hidden, collapse concurrent hooks to a shared summary when needed, and support hook activity outside an active agent turn. - Simplify durable hook output into user-facing messages and actionable failure, block, and stop states. Suppress model-facing context and quiet successful completions from all TUI history surfaces. ## Testing - Add lifecycle, layout, narrow-width, multi-width Unicode, transcript, restart, and completion-output coverage for hook status and history rendering. GitOrigin-RevId: 4c52b7a736e9e466e109107eb3c04a39d089597f --- codex-rs/tui/src/bottom_pane/hook_status.rs | 32 + codex-rs/tui/src/bottom_pane/mod.rs | 38 +- codex-rs/tui/src/chatwidget.rs | 23 +- codex-rs/tui/src/chatwidget/hook_lifecycle.rs | 24 +- codex-rs/tui/src/chatwidget/rendering.rs | 12 - ..._following_assistant_message_snapshot.snap | 9 +- ...h_output_flushes_immediately_snapshot.snap | 22 +- ...hook_output_survives_restart_snapshot.snap | 17 +- ...eted_turn_clears_visible_running_hook.snap | 16 +- ...cked_failed_feedback_history_snapshot.snap | 8 +- ...pleted_without_running_flash_snapshot.snap | 10 - ...running_then_quiet_completed_snapshot.snap | 21 - ..._hook_runs_while_exec_active_snapshot.snap | 36 +- ...ning_hooks_collapse_to_count_snapshot.snap | 9 - ...interrupt_hook_events_render_snapshot.snap | 2 +- ...pted_turn_clears_visible_running_hook.snap | 16 +- ...ext_is_truncated_with_transcript_hint.snap | 10 - ...ng_hook_below_background_activity_row.snap | 22 + ...g_hooks_share_background_activity_row.snap | 21 + ...__overlapping_hook_live_cell_snapshot.snap | 60 +- ..._tool_use_hook_events_render_snapshot.snap | 3 +- ..._tool_use_hook_events_render_snapshot.snap | 3 +- ...g_hooks_share_background_activity_row.snap | 22 + ...get__tests__session_end_hook_activity.snap | 24 + ...ion_end_hook_activity_without_message.snap | 24 + ...ion_start_hook_events_render_snapshot.snap | 3 +- ...del_context_and_preserves_stop_reason.snap | 7 + ...er_hook_notifications_render_snapshot.snap | 5 +- codex-rs/tui/src/chatwidget/tests/helpers.rs | 43 +- .../src/chatwidget/tests/slash_commands.rs | 1 - .../src/chatwidget/tests/status_and_layout.rs | 344 ++++---- codex-rs/tui/src/history_cell/hook_cell.rs | 798 +++++------------- ...atus_reflows_with_background_activity.snap | 21 + ...s_reflows_without_background_activity.snap | 21 + codex-rs/tui/src/status_indicator_widget.rs | 121 ++- 35 files changed, 903 insertions(+), 945 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/hook_status.rs delete mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_completed_before_reveal_renders_completed_without_running_flash_snapshot.snap delete mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_live_running_then_quiet_completed_snapshot.snap delete mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__identical_parallel_running_hooks_collapse_to_count_snapshot.snap delete mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__long_hook_context_is_truncated_with_transcript_hint.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__long_running_hook_below_background_activity_row.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mixed_running_hooks_share_background_activity_row.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__running_hooks_share_background_activity_row.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__session_end_hook_activity.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__session_end_hook_activity_without_message.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__stopped_hook_hides_model_context_and_preserves_stop_reason.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__hook_status_reflows_with_background_activity.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__hook_status_reflows_without_background_activity.snap diff --git a/codex-rs/tui/src/bottom_pane/hook_status.rs b/codex-rs/tui/src/bottom_pane/hook_status.rs new file mode 100644 index 0000000000..2552ea3bc9 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/hook_status.rs @@ -0,0 +1,32 @@ +//! Transient hook activity, independent of whether an agent turn owns a status row. + +use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; +use crate::motion::MotionMode; +use crate::motion::shimmer_text; +use crate::render::renderable::Renderable; +use crate::text_formatting::capitalize_first; +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use ratatui::style::Stylize; +use ratatui::text::Line; + +pub(super) struct HookStatus<'a> { + pub message: &'a str, + pub animations_enabled: bool, +} + +impl Renderable for HookStatus<'_> { + fn render(&self, area: Rect, buf: &mut Buffer) { + let mut spans = vec!["• ".dim()]; + spans.extend(shimmer_text( + &capitalize_first(self.message), + MotionMode::from_animations_enabled(self.animations_enabled), + )); + truncate_line_with_ellipsis_if_overflow(Line::from(spans).dim(), usize::from(area.width)) + .render(area, buf); + } + + fn desired_height(&self, _width: u16) -> u16 { + 1 + } +} diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 4e9e0c948e..f4caf92490 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -61,6 +61,7 @@ mod actionable_banner; mod app_link_view; mod apply_patch_header; mod approval_overlay; +mod hook_status; mod mcp_server_elicitation; mod multi_select_picker; mod request_user_input; @@ -248,6 +249,8 @@ pub(crate) struct BottomPane { /// Inline status indicator shown above the composer while a task is running. status: Option, + /// Running-hook summary supplied by the lifecycle owner after its reveal delay. + hook_status_message: Option, inline_banner: Option, /// Streaming may drop the row without losing its elapsed time or modal pause. status_timer: crate::status_indicator_widget::StatusTimer, @@ -321,6 +324,7 @@ impl BottomPane { disable_paste_burst, is_task_running: false, status: None, + hook_status_message: None, inline_banner: None, status_timer: crate::status_indicator_widget::StatusTimer::default(), unified_exec_footer: UnifiedExecFooter::new(), @@ -1434,13 +1438,29 @@ impl BottomPane { } } - /// Copy unified-exec summary text into the active status row, if any. + /// Update hook activity after the lifecycle reveal delay, even outside a turn. + pub(crate) fn set_hook_status_message(&mut self, message: Option) { + if self.hook_status_message == message { + return; + } + + self.hook_status_message = message; + if self.hook_status_message.is_some() && self.status.is_none() && self.is_task_running() { + self.ensure_status_indicator(); + } else { + self.sync_status_inline_message(); + self.request_redraw(); + } + } + + /// Copy background activity and hook text into the active status row, if any. /// /// This keeps status-line inline text synchronized without forcing the /// standalone unified-exec footer row to be visible. fn sync_status_inline_message(&mut self) { if let Some(status) = self.status.as_mut() { status.update_inline_message(self.unified_exec_footer.summary_text()); + status.update_hook_status_message(self.hook_status_message.clone()); } } @@ -1877,6 +1897,17 @@ impl BottomPane { RenderableItem::Owned(Box::new(status.with_timer(&self.status_timer))), ); } + if self.status.is_none() + && let Some(message) = &self.hook_status_message + { + flex.push( + /*flex*/ 0, + RenderableItem::Owned(Box::new(hook_status::HookStatus { + message, + animations_enabled: self.animations_enabled, + })), + ); + } // Avoid double-surfacing the same summary and avoid adding an extra // row while the status line is already visible. if self.status_widget().is_none() && !self.unified_exec_footer.is_empty() { @@ -1889,8 +1920,9 @@ impl BottomPane { let has_pending_input = !self.pending_input_preview.queued_messages.is_empty() || !self.pending_input_preview.pending_steers.is_empty() || !self.pending_input_preview.rejected_steers.is_empty(); - let has_status_or_footer = - self.status_widget().is_some() || !self.unified_exec_footer.is_empty(); + let has_status_or_footer = self.status_widget().is_some() + || self.hook_status_message.is_some() + || !self.unified_exec_footer.is_empty(); let has_inline_previews = has_pending_thread_approvals || has_pending_input; if has_inline_previews && has_status_or_footer { flex.push(/*flex*/ 0, RenderableItem::Owned("".into())); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 01a1dd9b14..40eadfaf58 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1902,7 +1902,7 @@ impl ChatWidget { /// Returns a cache key describing the current in-flight cells for the transcript overlay. /// /// `Ctrl+T` renders committed transcript cells plus a render-only live tail derived from the - /// current active, hook, and asynchronous usage cells, and the overlay caches that tail; this + /// current active and asynchronous usage cells, and the overlay caches that tail; this /// key is what it uses to decide whether it must recompute. When there are no live cells, this /// returns `None` so the overlay can drop the tail entirely. /// @@ -1911,14 +1911,9 @@ impl ChatWidget { /// the main viewport updates. pub(crate) fn active_cell_transcript_key(&self) -> Option { let cell = self.transcript.active_cell.as_ref(); - let hook_cell = self.active_hook_cell.as_ref(); let token_activity_cell = self.pending_token_activity_output(); let rate_limit_reset_hint = self.pending_rate_limit_reset_hint(); - if cell.is_none() - && hook_cell.is_none() - && token_activity_cell.is_none() - && rate_limit_reset_hint.is_none() - { + if cell.is_none() && token_activity_cell.is_none() && rate_limit_reset_hint.is_none() { return None; } Some(ActiveCellTranscriptKey { @@ -1926,11 +1921,7 @@ impl ChatWidget { is_stream_continuation: cell .map(|cell| cell.is_stream_continuation()) .unwrap_or(false), - animation_tick: cell - .and_then(|cell| cell.transcript_animation_tick()) - .or_else(|| { - hook_cell.and_then(super::history_cell::HistoryCell::transcript_animation_tick) - }), + animation_tick: cell.and_then(|cell| cell.transcript_animation_tick()), }) } @@ -1948,14 +1939,6 @@ impl ChatWidget { if let Some(cell) = self.transcript.active_cell.as_ref() { lines.extend(cell.transcript_hyperlink_lines(width)); } - if let Some(hook_cell) = self.active_hook_cell.as_ref() { - // Compute hook lines first so hidden hooks do not add a separator. - let hook_lines = hook_cell.transcript_hyperlink_lines(width); - if !hook_lines.is_empty() && !lines.is_empty() { - lines.push(HyperlinkLine::from("")); - } - lines.extend(hook_lines); - } if let Some(token_activity_cell) = self.pending_token_activity_output() { let token_activity_lines = token_activity_cell.transcript_hyperlink_lines(width); if !token_activity_lines.is_empty() && !lines.is_empty() { diff --git a/codex-rs/tui/src/chatwidget/hook_lifecycle.rs b/codex-rs/tui/src/chatwidget/hook_lifecycle.rs index c5974cb3f8..3f98599846 100644 --- a/codex-rs/tui/src/chatwidget/hook_lifecycle.rs +++ b/codex-rs/tui/src/chatwidget/hook_lifecycle.rs @@ -1,13 +1,14 @@ //! Hook run lifecycle handling for `ChatWidget`. //! -//! This module keeps active hook cells, hook timers, and hook completion output -//! together. +//! This module keeps hook timers, compact activity-row progress, and durable +//! hook completion output together. use super::*; impl ChatWidget { /// Drop transient live hook status without flushing it into history. pub(super) fn clear_active_hook_cell(&mut self) { + self.bottom_pane.set_hook_status_message(/*message*/ None); if self.active_hook_cell.take().is_some() { self.bump_active_cell_revision(); self.request_pending_usage_output_insertion(); @@ -23,13 +24,11 @@ impl ChatWidget { self.bump_active_cell_revision(); } None => { - self.active_hook_cell = Some(history_cell::new_active_hook_cell( - run, - self.config.animations, - )); + self.active_hook_cell = Some(history_cell::new_active_hook_cell(run)); self.bump_active_cell_revision(); } } + self.sync_hook_status_message(); self.request_redraw(); } @@ -51,8 +50,7 @@ impl ChatWidget { self.bump_active_cell_revision(); } None => { - let cell = - history_cell::new_completed_hook_cell(completed, self.config.animations); + let cell = history_cell::new_completed_hook_cell(completed); if !cell.is_empty() { self.active_hook_cell = Some(cell); self.bump_active_cell_revision(); @@ -62,6 +60,7 @@ impl ChatWidget { } self.flush_completed_hook_output(); self.finish_active_hook_cell_if_idle(); + self.sync_hook_status_message(); self.request_redraw(); } @@ -118,6 +117,15 @@ impl ChatWidget { self.bump_active_cell_revision(); } self.finish_active_hook_cell_if_idle(); + self.sync_hook_status_message(); + } + + fn sync_hook_status_message(&mut self) { + let status_message = self + .active_hook_cell + .as_ref() + .and_then(HookCell::running_status_summary); + self.bottom_pane.set_hook_status_message(status_message); } pub(super) fn schedule_hook_timer_if_needed(&self) { diff --git a/codex-rs/tui/src/chatwidget/rendering.rs b/codex-rs/tui/src/chatwidget/rendering.rs index fd6fc7aa57..693ede4edf 100644 --- a/codex-rs/tui/src/chatwidget/rendering.rs +++ b/codex-rs/tui/src/chatwidget/rendering.rs @@ -44,20 +44,8 @@ impl ChatWidget { })), None => RenderableItem::Owned(Box::new(())), }; - let active_hook_cell_renderable = match &self.active_hook_cell { - Some(cell) if cell.should_render() => { - RenderableItem::Owned(Box::new(TranscriptAreaRenderable { - child: cell, - top: 1, - right: active_cell_right_reserve, - persistent_layout: None, - })) - } - _ => RenderableItem::Owned(Box::new(())), - }; let mut flex = FlexRenderable::new(); flex.push(/*flex*/ 1, active_cell_renderable); - flex.push(/*flex*/ 0, active_hook_cell_renderable); if let Some(cell) = self.pending_token_activity_output() { flex.push( /*flex*/ 1, diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_hook_output_precedes_following_assistant_message_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_hook_output_precedes_following_assistant_message_snapshot.snap index 5e8360f015..14d4a791cb 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_hook_output_precedes_following_assistant_message_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_hook_output_precedes_following_assistant_message_snapshot.snap @@ -1,11 +1,8 @@ --- source: tui/src/chatwidget/tests/status_and_layout.rs -expression: "format!(\"active hooks:\\n{}history:\\n{history}\", active_hook_blob(&chat))" +expression: history --- -active hooks: - -history: -• PreToolUse hook (blocked) - feedback: command blocked by policy +• Blocked by hook + └ command blocked by policy • The hook feedback was applied. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_hook_with_output_flushes_immediately_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_hook_with_output_flushes_immediately_snapshot.snap index d2673f5400..bb8be43b02 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_hook_with_output_flushes_immediately_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_hook_with_output_flushes_immediately_snapshot.snap @@ -3,14 +3,24 @@ source: tui/src/chatwidget/tests/status_and_layout.rs expression: "format!(\"{running_snapshot}\\n\\n{completed_snapshot}\")" --- running -live hooks: -• Running PreToolUse hook: checking command +activity: +" " +"• Checking command " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " history: completed -live hooks: - +activity: +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " history: -• PreToolUse hook (blocked) - feedback: command blocked by policy +• Blocked by hook + └ command blocked by policy diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_same_id_hook_output_survives_restart_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_same_id_hook_output_survives_restart_snapshot.snap index ebdca8fc75..f56015d661 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_same_id_hook_output_survives_restart_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_same_id_hook_output_survives_restart_snapshot.snap @@ -1,9 +1,16 @@ --- source: tui/src/chatwidget/tests/status_and_layout.rs -expression: "format!(\"active hooks:\\n{}history:\\n{history}\", active_hook_blob(&chat))" +expression: "hook_status_and_history_snapshot(&chat, \"restarted\", &history)" --- -active hooks: -• Running Stop hook: checking stop condition +restarted +activity: +" " +"• Checking stop condition " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " history: -• Stop hook (stopped) - stop: continue with more context +• Hook stopped + └ continue with more context diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_turn_clears_visible_running_hook.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_turn_clears_visible_running_hook.snap index e41954e415..37e8d9eb86 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_turn_clears_visible_running_hook.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_turn_clears_visible_running_hook.snap @@ -1,8 +1,18 @@ --- source: tui/src/chatwidget/tests/status_and_layout.rs -expression: "format!(\"before completion:\\n{before_completion}after completion:\\n{}\",\nactive_hook_blob(&chat))" +expression: "format!(\"before completion:\\n{before_completion}\\nafter completion:\\n{}\",\nhook_status_frame(&chat, 80))" --- before completion: -• Running PostToolUse hook +" " +"• Working (0s • esc to interrupt) · Running hook " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " after completion: - +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_blocked_failed_feedback_history_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_blocked_failed_feedback_history_snapshot.snap index e0d2e7e225..17a3b42f38 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_blocked_failed_feedback_history_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_blocked_failed_feedback_history_snapshot.snap @@ -2,8 +2,8 @@ source: tui/src/chatwidget/tests/status_and_layout.rs expression: rendered --- -• PreToolUse hook (blocked) - feedback: run tests before touching the fixture +• Blocked by hook + └ run tests before touching the fixture -• PostToolUse hook (failed) - error: hook exited with code 7 +• Hook failed + └ hook exited with code 7 diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_completed_before_reveal_renders_completed_without_running_flash_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_completed_before_reveal_renders_completed_without_running_flash_snapshot.snap deleted file mode 100644 index 0dd1aa064e..0000000000 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_completed_before_reveal_renders_completed_without_running_flash_snapshot.snap +++ /dev/null @@ -1,10 +0,0 @@ ---- -source: tui/src/chatwidget/tests/status_and_layout.rs -expression: "format!(\"started hidden:\\n{started_hidden_snapshot}\\nhistory:\\n{history}\")" ---- -started hidden: - -history: -• SessionStart hook (completed) - hook context: session context - second line diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_live_running_then_quiet_completed_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_live_running_then_quiet_completed_snapshot.snap deleted file mode 100644 index 290d093ed8..0000000000 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_live_running_then_quiet_completed_snapshot.snap +++ /dev/null @@ -1,21 +0,0 @@ ---- -source: tui/src/chatwidget/tests/status_and_layout.rs -expression: "format!(\"{running_snapshot}\\n\\n{completed_lingering_snapshot}\\n\\n{completed_snapshot}\")" ---- -running -live hooks: -• Running PostToolUse hook -history: - - -completed lingering -live hooks: -• Running PostToolUse hook -history: - - -completed after linger -live hooks: - -history: - diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_runs_while_exec_active_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_runs_while_exec_active_snapshot.snap index 2f28b4ad9a..01050a7906 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_runs_while_exec_active_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_runs_while_exec_active_snapshot.snap @@ -1,25 +1,37 @@ --- source: tui/src/chatwidget/tests/status_and_layout.rs -expression: "format!(\"exec running:\\n{exec_running}\\nexec and hook running:\\n{exec_and_hook_running}\\nhistory after exec:\\n{history_after_exec}\\nhook running after exec:\\n{hook_running_after_exec}\\nquiet hook completed lingering:\\n{quiet_hook_completed_lingering}\\nquiet hook completed:\\n{quiet_hook_completed}\")" +expression: "format!(\"exec running:\\n{exec_running}\\nexec and hook running:\\n{exec_and_hook_running}\\nhistory after exec:\\n{history_after_exec}\\nhook running after exec:\\n{hook_running_after_exec}\\nquiet hook completed:\\n{quiet_hook_completed}\")" --- exec running: • Running echo done exec and hook running: -active exec: -• Running echo done -active hooks: -• Running PostToolUse hook: checking output policy - +" " +"• Running echo done " +" " +"• Working (0s • esc to interrupt) · checking output policy " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " history after exec: • Ran echo done └ done hook running after exec: -• Running PostToolUse hook: checking output policy - -quiet hook completed lingering: -• Running PostToolUse hook: checking output policy - +" " +"• Working (0s • esc to interrupt) · checking output policy " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " quiet hook completed: - +" " +"• Working (0s • esc to interrupt) " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__identical_parallel_running_hooks_collapse_to_count_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__identical_parallel_running_hooks_collapse_to_count_snapshot.snap deleted file mode 100644 index 353d722794..0000000000 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__identical_parallel_running_hooks_collapse_to_count_snapshot.snap +++ /dev/null @@ -1,9 +0,0 @@ ---- -source: tui/src/chatwidget/tests/status_and_layout.rs -expression: "hook_live_and_history_snapshot(&chat, \"running\", \"\")" ---- -running -live hooks: -• Running 3 PreToolUse hooks: checking command policy -history: - diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupt_hook_events_render_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupt_hook_events_render_snapshot.snap index 9cb557b103..4b6474c9c2 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupt_hook_events_render_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupt_hook_events_render_snapshot.snap @@ -2,4 +2,4 @@ source: tui/src/chatwidget/tests/helpers.rs expression: combined --- -• Interrupt (completed) says: Heads up from the hook +↳ Hook · Heads up from the hook diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupted_turn_clears_visible_running_hook.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupted_turn_clears_visible_running_hook.snap index 22c70e3357..80069ba40c 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupted_turn_clears_visible_running_hook.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupted_turn_clears_visible_running_hook.snap @@ -1,8 +1,18 @@ --- source: tui/src/chatwidget/tests/status_and_layout.rs -expression: "format!(\"before interrupt:\\n{before_interrupt}after interrupt:\\n{}\",\nactive_hook_blob(&chat))" +expression: "format!(\"before interrupt:\\n{before_interrupt}\\nafter interrupt:\\n{}\",\nhook_status_frame(&chat, 80))" --- before interrupt: -• Running PreToolUse hook: checking command policy +" " +"• Working (0s • esc to interrupt) · checking command policy " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " after interrupt: - +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__long_hook_context_is_truncated_with_transcript_hint.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__long_hook_context_is_truncated_with_transcript_hint.snap deleted file mode 100644 index 4b88bc9e1b..0000000000 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__long_hook_context_is_truncated_with_transcript_hint.snap +++ /dev/null @@ -1,10 +0,0 @@ ---- -source: tui/src/chatwidget/tests/status_and_layout.rs -expression: history ---- -• SessionStart hook (stopped) - hook context: This hook context is intentionally long enough to wrap across - several terminal rows while keeping the complete value available in the - … +2 lines (ctrl + t to view transcript) - stop: The hook stopped this turn for an important reason. - This second line must remain visible in full. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__long_running_hook_below_background_activity_row.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__long_running_hook_below_background_activity_row.snap new file mode 100644 index 0000000000..c35eeb638b --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__long_running_hook_below_background_activity_row.snap @@ -0,0 +1,22 @@ +--- +source: tui/src/chatwidget/tests/status_and_layout.rs +expression: "format!(\"80 columns:\\n{}\\n120 columns:\\n{}\", hook_status_frame(&chat, 80),\nhook_status_frame(&chat, 120),)" +--- +80 columns: +" " +"• Working (0s • esc to interrupt) · 1 background terminal running · /ps to view…" +" └ checking command policy for deployment commands, production credentials, an…" +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " +120 columns: +" " +"• Working (0s • esc to interrupt) · 1 background terminal running · /ps to view · /stop to close " +" └ checking command policy for deployment commands, production credentials, and paths containing 日本語 before continu…" Hidden by multi-width symbols: [(99, " "), (101, " "), (103, " ")] +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mixed_running_hooks_share_background_activity_row.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mixed_running_hooks_share_background_activity_row.snap new file mode 100644 index 0000000000..047611adf9 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mixed_running_hooks_share_background_activity_row.snap @@ -0,0 +1,21 @@ +--- +source: tui/src/chatwidget/tests/status_and_layout.rs +expression: "format!(\"80 columns:\\n{}\\n120 columns:\\n{}\", hook_status_frame(&chat, 80),\nhook_status_frame(&chat, 120),)" +--- +80 columns: +" " +"• Working (0s • esc to interrupt) · 1 background terminal running · /ps to view…" +" └ Running hooks " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " +120 columns: +" " +"• Working (0s • esc to interrupt) · 1 background terminal running · /ps to view · /stop to close · Running hooks " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__overlapping_hook_live_cell_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__overlapping_hook_live_cell_snapshot.snap index f71909dddf..d7cf088ac6 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__overlapping_hook_live_cell_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__overlapping_hook_live_cell_snapshot.snap @@ -1,43 +1,51 @@ --- source: tui/src/chatwidget/tests/status_and_layout.rs -expression: "format!(\"{first_running_snapshot}\\n\\n{second_running_snapshot}\\n\\n{older_completed_snapshot}\\n\\n{older_completed_expired_snapshot}\\n\\n{all_completed_lingering_snapshot}\\n\\n{all_completed_snapshot}\")" +expression: "format!(\"{first_running_snapshot}\\n\\n{second_running_snapshot}\\n\\n{older_completed_snapshot}\\n\\n{all_completed_snapshot}\")" --- pre running -live hooks: -• Running PreToolUse hook: checking command policy +activity: +" " +"• Thinking (0s • esc to interrupt) · checking command policy " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " history: post running -live hooks: -• Running PreToolUse hook: checking command policy - -• Running PostToolUse hook: checking output policy +activity: +" " +"• Thinking (0s • esc to interrupt) · Running hooks " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " history: -pre completed lingering -live hooks: -• Running PreToolUse hook: checking command policy - -• Running PostToolUse hook: checking output policy -history: - - -pre completed after linger -live hooks: -• Running PostToolUse hook: checking output policy -history: - - -all completed lingering -live hooks: -• Running PostToolUse hook: checking output policy +pre completed +activity: +" " +"• Thinking (0s • esc to interrupt) · checking output policy " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " history: all completed -live hooks: - +activity: +" " +"• Thinking (0s • esc to interrupt) " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " history: diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__post_tool_use_hook_events_render_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__post_tool_use_hook_events_render_snapshot.snap index 42861562ea..4b6474c9c2 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__post_tool_use_hook_events_render_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__post_tool_use_hook_events_render_snapshot.snap @@ -2,5 +2,4 @@ source: tui/src/chatwidget/tests/helpers.rs expression: combined --- -• PostToolUse (completed) says: Heads up from the hook - hook context: Remember the startup checklist. +↳ Hook · Heads up from the hook diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__pre_tool_use_hook_events_render_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__pre_tool_use_hook_events_render_snapshot.snap index 2cd5581cd0..4b6474c9c2 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__pre_tool_use_hook_events_render_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__pre_tool_use_hook_events_render_snapshot.snap @@ -2,5 +2,4 @@ source: tui/src/chatwidget/tests/helpers.rs expression: combined --- -• PreToolUse (completed) says: Heads up from the hook - hook context: Remember the startup checklist. +↳ Hook · Heads up from the hook diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__running_hooks_share_background_activity_row.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__running_hooks_share_background_activity_row.snap new file mode 100644 index 0000000000..18cab0743d --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__running_hooks_share_background_activity_row.snap @@ -0,0 +1,22 @@ +--- +source: tui/src/chatwidget/tests/status_and_layout.rs +expression: "format!(\"80 columns:\\n{}\\n120 columns:\\n{}\", hook_status_frame(&chat, 80),\nhook_status_frame(&chat, 120),)" +--- +80 columns: +" " +"• Working (0s • esc to interrupt) · 1 background terminal running · /ps to view…" +" └ checking command policy " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " +120 columns: +" " +"• Working (0s • esc to interrupt) · 1 background terminal running · /ps to view · /stop to close " +" └ checking command policy " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__session_end_hook_activity.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__session_end_hook_activity.snap new file mode 100644 index 0000000000..144e7bf8ae --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__session_end_hook_activity.snap @@ -0,0 +1,24 @@ +--- +source: tui/src/chatwidget/tests/status_and_layout.rs +expression: "format!(\"before reveal:\\n{hidden}\\nrunning:\\n{running}\\ncompleted:\\n{}\",\nhook_status_frame(&chat, 80),)" +--- +before reveal: +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " +running: +" " +"• Saving session summary " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " +completed: +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__session_end_hook_activity_without_message.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__session_end_hook_activity_without_message.snap new file mode 100644 index 0000000000..b32dad2b4a --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__session_end_hook_activity_without_message.snap @@ -0,0 +1,24 @@ +--- +source: tui/src/chatwidget/tests/status_and_layout.rs +expression: "format!(\"before reveal:\\n{hidden}\\nrunning:\\n{running}\\ncompleted:\\n{}\",\nhook_status_frame(&chat, 80),)" +--- +before reveal: +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " +running: +" " +"• Running hook " +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " +completed: +" " +" " +"› Ask Codex to do anything " +" " +" gpt-5.6-sol default · /tmp/project " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__session_start_hook_events_render_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__session_start_hook_events_render_snapshot.snap index 49563b27f2..4b6474c9c2 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__session_start_hook_events_render_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__session_start_hook_events_render_snapshot.snap @@ -2,5 +2,4 @@ source: tui/src/chatwidget/tests/helpers.rs expression: combined --- -• SessionStart (completed) says: Heads up from the hook - hook context: Remember the startup checklist. +↳ Hook · Heads up from the hook diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__stopped_hook_hides_model_context_and_preserves_stop_reason.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__stopped_hook_hides_model_context_and_preserves_stop_reason.snap new file mode 100644 index 0000000000..008c9f5df8 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__stopped_hook_hides_model_context_and_preserves_stop_reason.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/chatwidget/tests/status_and_layout.rs +expression: history +--- +• Hook stopped + └ The hook stopped this turn for an important reason. + This second line must remain visible in full. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__user_prompt_submit_app_server_hook_notifications_render_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__user_prompt_submit_app_server_hook_notifications_render_snapshot.snap index ab6082fdea..34890a9c3b 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__user_prompt_submit_app_server_hook_notifications_render_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__user_prompt_submit_app_server_hook_notifications_render_snapshot.snap @@ -2,5 +2,6 @@ source: tui/src/chatwidget/tests/status_and_layout.rs expression: combined --- -• UserPromptSubmit (stopped) says: go-workflow must start from PlanMode - stop: prompt blocked +• Hook stopped + └ go-workflow must start from PlanMode + └ prompt blocked diff --git a/codex-rs/tui/src/chatwidget/tests/helpers.rs b/codex-rs/tui/src/chatwidget/tests/helpers.rs index 33c46c33de..6cb423661c 100644 --- a/codex-rs/tui/src/chatwidget/tests/helpers.rs +++ b/codex-rs/tui/src/chatwidget/tests/helpers.rs @@ -1127,14 +1127,6 @@ pub(super) fn active_blob(chat: &ChatWidget) -> String { lines_to_single_string(&lines) } -pub(super) fn active_hook_blob(chat: &ChatWidget) -> String { - let Some(cell) = chat.active_hook_cell.as_ref() else { - return "\n".to_string(); - }; - let lines = cell.display_lines(/*width*/ 80); - lines_to_single_string(&lines) -} - pub(super) fn expire_quiet_hook_linger(chat: &mut ChatWidget) { if let Some(cell) = chat.active_hook_cell.as_mut() { cell.expire_quiet_runs_now_for_test(); @@ -1649,6 +1641,7 @@ pub(super) async fn assert_hook_events_snapshot( snapshot_name: &str, ) { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.on_task_started(); handle_hook_started( &mut chat, @@ -1665,12 +1658,19 @@ pub(super) async fn assert_hook_events_snapshot( "hook start should update the live hook cell instead of writing history" ); reveal_running_hooks(&mut chat); + let width = 100; + let height = chat.desired_height(width); + let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(width, height)) + .expect("create terminal"); + terminal + .draw(|frame| chat.render(frame.area(), frame.buffer_mut())) + .expect("render hook activity status"); + let running = normalized_backend_snapshot(terminal.backend()); assert!( - active_hook_blob(&chat).contains(&format!( - "Running {} hook: {status_message}", - hook_event_label(event_name) - )), - "hook start should render in the live hook cell" + running + .lines() + .any(|line| line.contains("Working") && line.contains(status_message)), + "hook start should render its status in the activity row: {running}" ); let mut entries = vec![codex_app_server_protocol::HookOutputEntry { @@ -1701,20 +1701,3 @@ pub(super) async fn assert_hook_events_snapshot( .collect::(); assert_chatwidget_snapshot!(snapshot_name, combined); } - -fn hook_event_label(event_name: codex_app_server_protocol::HookEventName) -> &'static str { - match event_name { - codex_app_server_protocol::HookEventName::PreToolUse => "PreToolUse", - codex_app_server_protocol::HookEventName::PermissionRequest => "PermissionRequest", - codex_app_server_protocol::HookEventName::PostToolUse => "PostToolUse", - codex_app_server_protocol::HookEventName::PreCompact => "PreCompact", - codex_app_server_protocol::HookEventName::PostCompact => "PostCompact", - codex_app_server_protocol::HookEventName::SessionStart => "SessionStart", - codex_app_server_protocol::HookEventName::SessionEnd => "SessionEnd", - codex_app_server_protocol::HookEventName::UserPromptSubmit => "UserPromptSubmit", - codex_app_server_protocol::HookEventName::SubagentStart => "SubagentStart", - codex_app_server_protocol::HookEventName::SubagentStop => "SubagentStop", - codex_app_server_protocol::HookEventName::Stop => "Stop", - codex_app_server_protocol::HookEventName::Interrupt => "Interrupt", - } -} diff --git a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index bfd3fee686..de6eb5f405 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -1648,7 +1648,6 @@ async fn completed_token_activity_refresh_waits_for_active_hook() { ), ); - assert_matches!(rx.try_recv(), Ok(AppEvent::InsertHistoryCell(_))); assert_matches!(rx.try_recv(), Ok(AppEvent::CommitPendingUsageOutput)); } diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index 6b72898912..2a6d8738a4 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -3786,6 +3786,7 @@ impl crate::workspace_command::WorkspaceCommandExecutor for NoopWorkspaceCommand #[tokio::test] async fn interrupted_turn_clears_visible_running_hook() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.on_task_started(); handle_hook_started( &mut chat, @@ -3796,15 +3797,15 @@ async fn interrupted_turn_clears_visible_running_hook() { ), ); reveal_running_hooks(&mut chat); - let before_interrupt = active_hook_blob(&chat); + let before_interrupt = hook_status_frame(&chat, /*width*/ 80); handle_turn_interrupted(&mut chat, "turn-1"); assert_chatwidget_snapshot!( "interrupted_turn_clears_visible_running_hook", format!( - "before interrupt:\n{before_interrupt}after interrupt:\n{}", - active_hook_blob(&chat) + "before interrupt:\n{before_interrupt}\nafter interrupt:\n{}", + hook_status_frame(&chat, /*width*/ 80) ) ); } @@ -3812,6 +3813,7 @@ async fn interrupted_turn_clears_visible_running_hook() { #[tokio::test] async fn completed_turn_clears_visible_running_hook() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.on_task_started(); handle_hook_started( &mut chat, @@ -3822,15 +3824,15 @@ async fn completed_turn_clears_visible_running_hook() { ), ); reveal_running_hooks(&mut chat); - let before_completion = active_hook_blob(&chat); + let before_completion = hook_status_frame(&chat, /*width*/ 80); handle_turn_completed(&mut chat, "turn-1", /*duration_ms*/ None); assert_chatwidget_snapshot!( "completed_turn_clears_visible_running_hook", format!( - "before completion:\n{before_completion}after completion:\n{}", - active_hook_blob(&chat) + "before completion:\n{before_completion}\nafter completion:\n{}", + hook_status_frame(&chat, /*width*/ 80) ) ); } @@ -4857,74 +4859,43 @@ async fn post_tool_use_hook_events_render_snapshot() { } #[tokio::test] -async fn completed_hook_with_no_entries_stays_out_of_history() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; +async fn quiet_hook_cleanup_starts_when_the_hook_is_revealed() { + for delayed_redraw in [false, true] { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + handle_hook_started( + &mut chat, + hook_started_run( + "post-tool-use:0:/tmp/hooks.json", + codex_app_server_protocol::HookEventName::PostToolUse, + Some("checking output policy"), + ), + ); + if delayed_redraw { + reveal_running_hooks_after_delayed_redraw(&mut chat); + } else { + reveal_running_hooks(&mut chat); + } + handle_hook_completed( + &mut chat, + hook_completed_run( + "post-tool-use:0:/tmp/hooks.json", + codex_app_server_protocol::HookEventName::PostToolUse, + codex_app_server_protocol::HookRunStatus::Completed, + Vec::new(), + ), + ); - handle_hook_started( - &mut chat, - hook_started_run( - "post-tool-use:0:/tmp/hooks.json", - codex_app_server_protocol::HookEventName::PostToolUse, - /*status_message*/ None, - ), - ); - assert!(drain_insert_history(&mut rx).is_empty()); - reveal_running_hooks(&mut chat); - let running_snapshot = hook_live_and_history_snapshot(&chat, "running", ""); - - handle_hook_completed( - &mut chat, - hook_completed_run( - "post-tool-use:0:/tmp/hooks.json", - codex_app_server_protocol::HookEventName::PostToolUse, - codex_app_server_protocol::HookRunStatus::Completed, - Vec::new(), - ), - ); - - assert!(drain_insert_history(&mut rx).is_empty()); - let completed_lingering_snapshot = - hook_live_and_history_snapshot(&chat, "completed lingering", ""); - expire_quiet_hook_linger(&mut chat); - let completed_snapshot = hook_live_and_history_snapshot(&chat, "completed after linger", ""); - assert_chatwidget_snapshot!( - "hook_live_running_then_quiet_completed_snapshot", - format!("{running_snapshot}\n\n{completed_lingering_snapshot}\n\n{completed_snapshot}") - ); -} - -#[tokio::test] -async fn quiet_hook_linger_starts_when_delayed_redraw_reveals_hook() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - - handle_hook_started( - &mut chat, - hook_started_run( - "post-tool-use:0:/tmp/hooks.json", - codex_app_server_protocol::HookEventName::PostToolUse, - Some("checking output policy"), - ), - ); - assert!(drain_insert_history(&mut rx).is_empty()); - - reveal_running_hooks_after_delayed_redraw(&mut chat); - handle_hook_completed( - &mut chat, - hook_completed_run( - "post-tool-use:0:/tmp/hooks.json", - codex_app_server_protocol::HookEventName::PostToolUse, - codex_app_server_protocol::HookRunStatus::Completed, - Vec::new(), - ), - ); - - assert!(drain_insert_history(&mut rx).is_empty()); - assert!( - active_hook_blob(&chat).contains("Running PostToolUse hook"), - "quiet hook should linger after the row becomes visible" - ); - expire_quiet_hook_linger(&mut chat); - assert_eq!(active_hook_blob(&chat), "\n"); + let cell = chat + .active_hook_cell + .as_ref() + .expect("quiet cleanup should linger"); + assert!(cell.is_active()); + assert!(cell.next_timer_deadline().is_some()); + assert_eq!(cell.running_status_summary(), None); + expire_quiet_hook_linger(&mut chat); + assert!(chat.active_hook_cell.is_none()); + assert!(drain_insert_history(&mut rx).is_empty()); + } } #[tokio::test] @@ -4962,13 +4933,11 @@ async fn blocked_and_failed_hooks_render_feedback_and_errors() { .collect::(); assert_chatwidget_snapshot!("hook_blocked_failed_feedback_history_snapshot", rendered); assert!( - rendered.contains( - "PreToolUse hook (blocked)\n feedback: run tests before touching the fixture" - ), + rendered.contains("Blocked by hook\n └ run tests before touching the fixture"), "expected blocked hook feedback: {rendered:?}" ); assert!( - rendered.contains("PostToolUse hook (failed)\n error: hook exited with code 7"), + rendered.contains("Hook failed\n └ hook exited with code 7"), "expected failed hook error: {rendered:?}" ); } @@ -4986,7 +4955,7 @@ async fn completed_hook_with_output_flushes_immediately() { ), ); reveal_running_hooks(&mut chat); - let running_snapshot = hook_live_and_history_snapshot(&chat, "running", ""); + let running_snapshot = hook_status_and_history_snapshot(&chat, "running", ""); handle_hook_completed( &mut chat, @@ -5004,7 +4973,7 @@ async fn completed_hook_with_output_flushes_immediately() { .iter() .map(|lines| lines_to_single_string(lines)) .collect::(); - let completed_snapshot = hook_live_and_history_snapshot(&chat, "completed", &history); + let completed_snapshot = hook_status_and_history_snapshot(&chat, "completed", &history); assert_chatwidget_snapshot!( "completed_hook_with_output_flushes_immediately_snapshot", @@ -5052,13 +5021,10 @@ async fn completed_hook_output_precedes_following_assistant_message() { .collect::(); assert_chatwidget_snapshot!( "completed_hook_output_precedes_following_assistant_message_snapshot", - format!( - "active hooks:\n{}history:\n{history}", - active_hook_blob(&chat) - ) + history ); let hook_index = history - .find("PreToolUse hook (blocked)") + .find("Blocked by hook") .expect("hook feedback should be in history"); let assistant_index = history .find("The hook feedback was applied.") @@ -5111,45 +5077,125 @@ async fn completed_same_id_hook_output_survives_restart() { .collect::(); assert_chatwidget_snapshot!( "completed_same_id_hook_output_survives_restart_snapshot", - format!( - "active hooks:\n{}history:\n{history}", - active_hook_blob(&chat) - ) + hook_status_and_history_snapshot(&chat, "restarted", &history) ); assert!( - history.contains("Stop hook (stopped)\n stop: continue with more context"), + history.contains("Hook stopped\n └ continue with more context"), "first hook output should not be overwritten: {history:?}" ); } #[tokio::test] -async fn identical_parallel_running_hooks_collapse_to_count() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; +async fn running_hooks_fit_around_background_activity_and_finish_without_history_snapshot() { + for (messages, snapshot) in [ + ( + vec!["checking command policy"; 3], + "running_hooks_share_background_activity_row", + ), + ( + vec![ + "checking command policy", + "scanning for secrets", + "loading project context", + ], + "mixed_running_hooks_share_background_activity_row", + ), + ( + vec![ + "checking command policy for deployment commands, production credentials, and paths containing 日本語 before continuing", + ], + "long_running_hook_below_background_activity_row", + ), + ] { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.on_task_started(); + begin_unified_exec_startup(&mut chat, "call-1", "proc-1", "sleep 2"); + chat.bottom_pane.hide_status_indicator(); - for tool_call_id in ["tool-call-1", "tool-call-2", "tool-call-3"] { + for (index, message) in messages.iter().enumerate() { + handle_hook_started( + &mut chat, + hook_started_run( + &format!("hook-{index}"), + codex_app_server_protocol::HookEventName::PreToolUse, + Some(message), + ), + ); + } + reveal_running_hooks(&mut chat); + assert_chatwidget_snapshot!( + snapshot, + format!( + "80 columns:\n{}\n120 columns:\n{}", + hook_status_frame(&chat, /*width*/ 80), + hook_status_frame(&chat, /*width*/ 120), + ) + ); + + // Match production delivery: finish the batch before drawing another frame. + for index in 0..messages.len() { + handle_hook_completed( + &mut chat, + hook_completed_run( + &format!("hook-{index}"), + codex_app_server_protocol::HookEventName::PreToolUse, + codex_app_server_protocol::HookRunStatus::Completed, + Vec::new(), + ), + ); + } + let rendered = hook_status_frame(&chat, /*width*/ 120); + assert!(rendered.contains("1 background terminal running")); + assert!(!rendered.contains("Running hooks")); + assert!(!rendered.contains("checking command policy")); + assert!(drain_insert_history(&mut rx).is_empty()); + } +} + +#[tokio::test] +async fn session_end_hook_has_standalone_activity_row() { + for (message, snapshot) in [ + (Some("saving session summary"), "session_end_hook_activity"), + (None, "session_end_hook_activity_without_message"), + ] { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; handle_hook_started( &mut chat, hook_started_run( - &format!("pre-tool-use:0:/tmp/hooks.json:{tool_call_id}"), - codex_app_server_protocol::HookEventName::PreToolUse, - Some("checking command policy"), + "session-end", + codex_app_server_protocol::HookEventName::SessionEnd, + message, ), ); + let hidden = hook_status_frame(&chat, /*width*/ 80); + reveal_running_hooks(&mut chat); + let running = hook_status_frame(&chat, /*width*/ 80); + handle_hook_completed( + &mut chat, + hook_completed_run( + "session-end", + codex_app_server_protocol::HookEventName::SessionEnd, + codex_app_server_protocol::HookRunStatus::Completed, + Vec::new(), + ), + ); + assert_chatwidget_snapshot!( + snapshot, + format!( + "before reveal:\n{hidden}\nrunning:\n{running}\ncompleted:\n{}", + hook_status_frame(&chat, /*width*/ 80), + ) + ); + assert!(drain_insert_history(&mut rx).is_empty()); } - reveal_running_hooks(&mut chat); - - assert_chatwidget_snapshot!( - "identical_parallel_running_hooks_collapse_to_count_snapshot", - hook_live_and_history_snapshot(&chat, "running", "") - ); } #[tokio::test] async fn overlapping_hook_live_cell_tracks_parallel_quiet_hooks() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.on_task_started(); chat.set_status_header("Thinking".to_string()); - chat.bottom_pane.ensure_status_indicator(); handle_hook_started( &mut chat, @@ -5161,7 +5207,7 @@ async fn overlapping_hook_live_cell_tracks_parallel_quiet_hooks() { ); assert_eq!(chat.status_state.current_status.header, "Thinking"); reveal_running_hooks(&mut chat); - let first_running_snapshot = hook_live_and_history_snapshot(&chat, "pre running", ""); + let first_running_snapshot = hook_status_and_history_snapshot(&chat, "pre running", ""); handle_hook_started( &mut chat, @@ -5173,7 +5219,7 @@ async fn overlapping_hook_live_cell_tracks_parallel_quiet_hooks() { ); assert_eq!(chat.status_state.current_status.header, "Thinking"); reveal_running_hooks(&mut chat); - let second_running_snapshot = hook_live_and_history_snapshot(&chat, "post running", ""); + let second_running_snapshot = hook_status_and_history_snapshot(&chat, "post running", ""); handle_hook_completed( &mut chat, @@ -5185,11 +5231,15 @@ async fn overlapping_hook_live_cell_tracks_parallel_quiet_hooks() { ), ); assert_eq!(chat.status_state.current_status.header, "Thinking"); - let older_completed_snapshot = - hook_live_and_history_snapshot(&chat, "pre completed lingering", ""); + let older_completed_snapshot = hook_status_and_history_snapshot(&chat, "pre completed", ""); expire_quiet_hook_linger(&mut chat); - let older_completed_expired_snapshot = - hook_live_and_history_snapshot(&chat, "pre completed after linger", ""); + assert_eq!( + chat.active_hook_cell + .as_ref() + .and_then(HookCell::running_status_summary) + .as_deref(), + Some("checking output policy"), + ); handle_hook_completed( &mut chat, @@ -5203,14 +5253,13 @@ async fn overlapping_hook_live_cell_tracks_parallel_quiet_hooks() { assert_eq!(chat.status_state.current_status.header, "Thinking"); assert!(chat.bottom_pane.status_indicator_visible()); assert!(drain_insert_history(&mut rx).is_empty()); - let all_completed_lingering_snapshot = - hook_live_and_history_snapshot(&chat, "all completed lingering", ""); + let all_completed_snapshot = hook_status_and_history_snapshot(&chat, "all completed", ""); expire_quiet_hook_linger(&mut chat); - let all_completed_snapshot = hook_live_and_history_snapshot(&chat, "all completed", ""); + assert!(chat.active_hook_cell.is_none()); assert_chatwidget_snapshot!( "overlapping_hook_live_cell_snapshot", format!( - "{first_running_snapshot}\n\n{second_running_snapshot}\n\n{older_completed_snapshot}\n\n{older_completed_expired_snapshot}\n\n{all_completed_lingering_snapshot}\n\n{all_completed_snapshot}" + "{first_running_snapshot}\n\n{second_running_snapshot}\n\n{older_completed_snapshot}\n\n{all_completed_snapshot}" ) ); } @@ -5231,18 +5280,14 @@ async fn running_hook_does_not_displace_active_exec_cell() { ), ); reveal_running_hooks(&mut chat); - let exec_and_hook_running = format!( - "active exec:\n{}active hooks:\n{}", - active_blob(&chat), - active_hook_blob(&chat) - ); + let exec_and_hook_running = hook_status_frame(&chat, /*width*/ 80); end_exec(&mut chat, begin, "done", "", /*exit_code*/ 0); let history_after_exec = drain_insert_history(&mut rx) .iter() .map(|lines| lines_to_single_string(lines)) .collect::(); - let hook_running_after_exec = active_hook_blob(&chat); + let hook_running_after_exec = hook_status_frame(&chat, /*width*/ 80); handle_hook_completed( &mut chat, @@ -5254,20 +5299,20 @@ async fn running_hook_does_not_displace_active_exec_cell() { ), ); assert!(drain_insert_history(&mut rx).is_empty()); - let quiet_hook_completed_lingering = active_hook_blob(&chat); + let quiet_hook_completed = hook_status_frame(&chat, /*width*/ 80); expire_quiet_hook_linger(&mut chat); - let quiet_hook_completed = active_hook_blob(&chat); + assert!(chat.active_hook_cell.is_none()); assert_chatwidget_snapshot!( "hook_runs_while_exec_active_snapshot", format!( - "exec running:\n{exec_running}\nexec and hook running:\n{exec_and_hook_running}\nhistory after exec:\n{history_after_exec}\nhook running after exec:\n{hook_running_after_exec}\nquiet hook completed lingering:\n{quiet_hook_completed_lingering}\nquiet hook completed:\n{quiet_hook_completed}" + "exec running:\n{exec_running}\nexec and hook running:\n{exec_and_hook_running}\nhistory after exec:\n{history_after_exec}\nhook running after exec:\n{hook_running_after_exec}\nquiet hook completed:\n{quiet_hook_completed}" ) ); } #[tokio::test] -async fn hidden_active_hook_does_not_add_transcript_separator() { +async fn running_hook_never_adds_active_transcript_lines() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; begin_exec(&mut chat, "call-1", "echo done"); @@ -5290,28 +5335,14 @@ async fn hidden_active_hook_does_not_add_transcript_separator() { assert_eq!(hidden_hook_transcript.len(), exec_only_line_count); reveal_running_hooks(&mut chat); - let visible_hook_lines = chat - .active_hook_cell - .as_ref() - .expect("active hook cell") - .transcript_lines(/*width*/ 80); let visible_hook_transcript = chat .active_cell_transcript_lines(/*width*/ 80) - .expect("active exec and hook transcript lines"); - assert_eq!( - visible_hook_transcript.len(), - exec_only_line_count + 1 + visible_hook_lines.len() - ); - assert_eq!( - lines_to_single_string( - &visible_hook_transcript[exec_only_line_count..exec_only_line_count + 1], - ), - "\n" - ); + .expect("active exec transcript lines"); + assert_eq!(visible_hook_transcript.len(), exec_only_line_count); } #[tokio::test] -async fn hook_completed_before_reveal_renders_completed_without_running_flash() { +async fn context_only_hook_completed_before_reveal_stays_out_of_history() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; handle_hook_started( @@ -5322,7 +5353,6 @@ async fn hook_completed_before_reveal_renders_completed_without_running_flash() Some("warming the shell"), ), ); - let started_hidden_snapshot = active_hook_blob(&chat); handle_hook_completed( &mut chat, @@ -5337,18 +5367,12 @@ async fn hook_completed_before_reveal_renders_completed_without_running_flash() ), ); - let history = drain_insert_history(&mut rx) - .iter() - .map(|lines| lines_to_single_string(lines)) - .collect::(); - assert_chatwidget_snapshot!( - "hook_completed_before_reveal_renders_completed_without_running_flash_snapshot", - format!("started hidden:\n{started_hidden_snapshot}\nhistory:\n{history}") - ); + assert!(drain_insert_history(&mut rx).is_empty()); + assert!(chat.active_hook_cell.is_none()); } #[tokio::test] -async fn long_hook_context_is_truncated_with_transcript_hint_snapshot() { +async fn stopped_hook_hides_model_context_and_preserves_stop_reason_snapshot() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; handle_hook_completed( @@ -5377,7 +5401,7 @@ async fn long_hook_context_is_truncated_with_transcript_hint_snapshot() { .map(|lines| lines_to_single_string(lines)) .collect::(); assert_chatwidget_snapshot!( - "long_hook_context_is_truncated_with_transcript_hint", + "stopped_hook_hides_model_context_and_preserves_stop_reason", history ); } @@ -5443,18 +5467,32 @@ fn hook_run_summary( } } -fn hook_live_and_history_snapshot(chat: &ChatWidget, phase: &str, history: &str) -> String { +fn hook_status_and_history_snapshot(chat: &ChatWidget, phase: &str, history: &str) -> String { let history = if history.is_empty() { "" } else { history }; format!( - "{phase}\nlive hooks:\n{}history:\n{history}", - active_hook_blob(chat), + "{phase}\nactivity:\n{}\nhistory:\n{history}", + hook_status_frame(chat, /*width*/ 80), ) } +fn hook_status_frame(chat: &ChatWidget, width: u16) -> String { + let height = chat.desired_height(width); + let mut terminal = ratatui::Terminal::new(TestBackend::new(width, height)) + .expect("create terminal for hook activity"); + terminal + .draw(|frame| chat.render(frame.area(), frame.buffer_mut())) + .expect("render hook activity frame"); + // Windows path normalization drops Display's final newline; make composed + // frames independent of that platform difference. + normalized_backend_snapshot(terminal.backend()) + .trim_end() + .to_string() +} + // Combined visual snapshot using vt100 for history + direct buffer overlay for UI. // This renders the final visual as seen in a terminal: history above, then a blank line, // then the exec block, another blank line, the status line, a blank line, and the composer. diff --git a/codex-rs/tui/src/history_cell/hook_cell.rs b/codex-rs/tui/src/history_cell/hook_cell.rs index 97567b18bf..04c0946750 100644 --- a/codex-rs/tui/src/history_cell/hook_cell.rs +++ b/codex-rs/tui/src/history_cell/hook_cell.rs @@ -1,36 +1,23 @@ //! History cell for hook execution. //! //! Hooks are intentionally quieter than normal tool calls. A hook that starts and finishes -//! successfully without output should not leave a transcript artifact, and very fast hooks should -//! not flash in the viewport. This cell keeps that policy local by treating each hook run as a -//! small rendering state machine: +//! successfully without user-facing output should not leave a transcript artifact, and very fast +//! hooks should not flash in the viewport. Model-facing hook context never appears in the TUI. This +//! cell keeps that policy local by treating each hook run as a small rendering state machine: //! //! 1. New runs begin hidden in `PendingReveal`. -//! 2. Runs that outlive the reveal delay become visible and may be coalesced with adjacent runs. -//! 3. Visible quiet successes linger briefly so they do not disappear in the same frame they were -//! first drawn. -//! 4. Completed runs only persist when they have output or a non-success status. +//! 2. Revealed runs contribute to the compact activity summary, never transcript lines. +//! 3. Quiet completions leave the summary immediately but retain their existing cleanup timer +//! for exec-flush and usage-output ordering. +//! 4. Completed runs only persist when they have user-facing output or a non-success status. use super::HistoryCell; use super::plain_lines; -use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; -use crate::motion::MotionMode; -use crate::motion::ReducedMotionIndicator; -use crate::motion::activity_indicator; -use crate::motion::shimmer_text; -use crate::render::line_utils::push_owned_lines; -use crate::render::renderable::Renderable; -use crate::ui_consts::TRANSCRIPT_HINT; -use crate::wrapping::RtOptions; -use crate::wrapping::word_wrap_line; -use codex_app_server_protocol::HookEventName; use codex_app_server_protocol::HookOutputEntry; use codex_app_server_protocol::HookOutputEntryKind; use codex_app_server_protocol::HookRunStatus; use codex_app_server_protocol::HookRunSummary; use ratatui::prelude::*; use ratatui::style::Stylize; -use ratatui::widgets::Paragraph; -use ratatui::widgets::Wrap; use std::time::Duration; use std::time::Instant; @@ -38,31 +25,26 @@ use std::time::Instant; pub(crate) struct HookCell { /// Hook runs that are active, lingering, or have persistent output to render. runs: Vec, - /// Mirrors the global animation setting so transcript rendering and viewport rendering agree. - animations_enabled: bool, } /// Minimum runtime before a hook is allowed to draw. /// -/// Helps avoids a flash of text forwork that was effectively instant. +/// Avoids flashing activity for work that was effectively instant. const HOOK_RUN_REVEAL_DELAY: Duration = Duration::from_millis(300); -/// Minimum time a quiet success remains on screen after becoming visible. +/// Minimum interval after reveal before quiet-completion bookkeeping is removed. /// -/// This pairs with `HOOK_RUN_REVEAL_DELAY`: once the user has seen a hook row, keep it stable long -/// enough to read instead of removing it immediately when the success event arrives. +/// The activity summary clears on completion; retain the existing cleanup timing +/// used by exec flushing and deferred usage output. const QUIET_HOOK_MIN_VISIBLE: Duration = Duration::from_millis(600); const HOOK_OUTPUT_INDENT: &str = " "; const HOOK_OUTPUT_BODY_INDENT: &str = " "; -const HOOK_CONTEXT_MAX_DISPLAY_ROWS: usize = 3; #[derive(Debug)] struct HookRunCell { /// Stable protocol id used to match begin/end updates for the same hook invocation. id: String, - /// Hook event kind, kept outside `state` so a begin update can refresh metadata in place. - event_name: HookEventName, /// Optional hook-supplied detail shown next to the running header. status_message: Option, /// Rendering lifecycle for this run. @@ -73,22 +55,16 @@ struct HookRunCell { enum HookRunState { /// A newly-started run that is active but deliberately hidden until `reveal_deadline`. PendingReveal { - /// The original start time, used for spinner phase and grouping once revealed. - start_time: Instant, /// First instant at which the run may become visible. reveal_deadline: Instant, }, /// A run that survived the reveal delay and is currently shown as running. VisibleRunning { - /// The original start time, used to keep animation timing stable across transitions. - start_time: Instant, /// First instant the run was actually rendered, used by quiet-success linger. visible_since: Instant, }, /// A visible run that completed successfully without output but is still lingering briefly. QuietLinger { - /// The original start time, retained so the spinner does not jump during the linger frame. - start_time: Instant, /// Instant after which the quiet success can be removed entirely. removal_deadline: Instant, }, @@ -101,42 +77,17 @@ enum HookRunState { }, } -#[derive(Debug, PartialEq, Eq)] -struct RunningHookGroupKey { - event_name: HookEventName, - status_message: Option, -} - -/// Accumulator for adjacent running hooks that can share one status line. -/// -/// Grouping happens only while building display lines, the underlying runs stay separate so their -/// protocol ids and completion transitions remain independent. -struct RunningHookGroup { - /// Shared event/status pair for every run in this display group. - key: RunningHookGroupKey, - /// Earliest start time in the group, so the combined spinner reflects the oldest work. - start_time: Option, - /// Number of adjacent runs represented by the group line. - count: usize, -} - impl HookCell { /// Creates a cell around a hook that has just started. - fn new_active(run: HookRunSummary, animations_enabled: bool) -> Self { - let mut cell = Self { - runs: Vec::new(), - animations_enabled, - }; + fn new_active(run: HookRunSummary) -> Self { + let mut cell = Self { runs: Vec::new() }; cell.start_run(run); cell } /// Creates a cell around an already-completed hook from transcript/history data. - fn new_completed(run: HookRunSummary, animations_enabled: bool) -> Self { - let mut cell = Self { - runs: Vec::new(), - animations_enabled, - }; + fn new_completed(run: HookRunSummary) -> Self { + let mut cell = Self { runs: Vec::new() }; cell.add_completed_run(run); cell } @@ -155,11 +106,6 @@ impl HookCell { !self.is_active() && !self.is_empty() } - /// Returns whether this cell has at least one line worth drawing right now. - pub(crate) fn should_render(&self) -> bool { - self.runs.iter().any(|run| run.state.should_render()) - } - /// Splits durable completed runs from ephemeral active-cell bookkeeping. /// /// Quiet successes are left behind so they can disappear from the active cell, while failures, @@ -175,17 +121,43 @@ impl HookCell { } } self.runs = remaining; - (!completed.is_empty()).then_some(Self { - runs: completed, - animations_enabled: self.animations_enabled, - }) + (!completed.is_empty()).then_some(Self { runs: completed }) } - /// Used by callers that need to know whether the active cell currently occupies viewport space. + /// Returns true for revealed runs still participating in lifecycle bookkeeping. pub(crate) fn has_visible_running_run(&self) -> bool { self.runs.iter().any(|run| run.state.is_running_visible()) } + /// Describes revealed hooks without implying per-handler completion progress. + /// Synchronous completions can arrive together after the whole batch finishes. + pub(crate) fn running_status_summary(&self) -> Option { + let mut messages = self + .runs + .iter() + .filter(|run| matches!(run.state, HookRunState::VisibleRunning { .. })) + .map(|run| { + run.status_message + .as_deref() + .map(str::trim) + .filter(|message| !message.is_empty()) + }) + .peekable(); + let message = messages.next()?; + let multiple = messages.peek().is_some(); + Some( + if let Some(message) = message + && messages.all(|other| other == Some(message)) + { + message.to_string() + } else if multiple { + "Running hooks".to_string() + } else { + "Running hook".to_string() + }, + ) + } + /// Advances reveal/removal timers and reports whether rendering should be refreshed. pub(crate) fn advance_time(&mut self, now: Instant) -> bool { let old_len = self.runs.len(); @@ -204,14 +176,12 @@ impl HookCell { pub(crate) fn start_run(&mut self, run: HookRunSummary) { let now = Instant::now(); if let Some(existing) = self.runs.iter_mut().find(|existing| existing.id == run.id) { - existing.event_name = run.event_name; existing.status_message = run.status_message; existing.state = HookRunState::pending(now); return; } self.runs.push(HookRunCell { id: run.id, - event_name: run.event_name, status_message: run.status_message, state: HookRunState::pending(now), }); @@ -235,14 +205,12 @@ impl HookCell { return true; } let HookRunSummary { - event_name, status_message, status, entries, .. } = run; let existing = &mut self.runs[index]; - existing.event_name = event_name; existing.status_message = status_message; existing.state = HookRunState::completed(status, entries); true @@ -257,7 +225,6 @@ impl HookCell { } let HookRunSummary { id, - event_name, status_message, status, entries, @@ -265,7 +232,6 @@ impl HookCell { } = run; self.runs.push(HookRunCell { id, - event_name, status_message, state: HookRunState::completed(status, entries), }); @@ -301,93 +267,74 @@ impl HookCell { } } - /// Builds hook lines for either the bounded main viewport or the full transcript overlay. - fn output_lines(&self, width: u16, render_full_context: bool) -> Vec> { + /// Builds only durable completion output; active hooks use the activity summary. + fn output_lines(&self) -> Vec> { let mut lines = Vec::new(); - let mut running_group: Option = None; for run in &self.runs { - if !run.state.should_render() { - continue; - } - - let Some(key) = run.running_group_key() else { - // Completed runs keep their own output lines, so any pending running group must be - // emitted before drawing the completed run. - if let Some(group) = running_group.take() { - push_running_hook_group(&mut lines, &group, self.animations_enabled); - } - push_hook_line_separator(&mut lines); - run.push_display_lines( - &mut lines, - self.animations_enabled, - width, - render_full_context, - ); + let HookRunState::Completed { status, entries } = &run.state else { continue; }; - - if let Some(group) = running_group.as_mut() - && group.key == key - { - group.count += 1; - // Preserve the earliest start time so grouped spinners do not reset when a later - // adjacent hook is folded into the same line. - group.start_time = earliest_instant(group.start_time, run.state.start_time()); - continue; + if !lines.is_empty() { + lines.push("".into()); } - - if let Some(group) = - running_group.replace(RunningHookGroup::new(key, run.state.start_time())) + let system_message = entries + .iter() + .find(|entry| entry.kind == HookOutputEntryKind::Warning); + let mut system_message_lines = system_message.map(|entry| entry.text.split('\n')); + if *status == HookRunStatus::Completed + && let Some(first_line) = system_message_lines.as_mut().and_then(Iterator::next) { - push_running_hook_group(&mut lines, &group, self.animations_enabled); + lines.push(vec!["↳ Hook · ".dim(), first_line.to_string().into()].into()); + } else { + let header_text = match status { + HookRunStatus::Completed => "Hook completed", + HookRunStatus::Failed => "Hook failed", + HookRunStatus::Blocked => "Blocked by hook", + HookRunStatus::Stopped => "Hook stopped", + HookRunStatus::Running => "Hook running", + }; + lines.push( + vec![ + hook_completed_bullet(*status), + " ".into(), + header_text.into(), + ] + .into(), + ); + if let Some(first_line) = system_message_lines.as_mut().and_then(Iterator::next) { + lines.push(format!("{HOOK_OUTPUT_INDENT}└ {first_line}").into()); + } + } + if let Some(system_message_lines) = system_message_lines { + for line in system_message_lines { + if line.is_empty() { + lines.push("".into()); + } else { + lines.push(format!("{HOOK_OUTPUT_BODY_INDENT}{line}").into()); + } + } + } + for entry in entries { + if matches!( + entry.kind, + HookOutputEntryKind::Warning | HookOutputEntryKind::Context + ) { + continue; + } + push_full_hook_output_entry(&mut lines, entry); } - } - if let Some(group) = running_group { - push_running_hook_group(&mut lines, &group, self.animations_enabled); } lines } } impl HistoryCell for HookCell { - /// Builds viewport lines while coalescing adjacent visible-running hooks. - fn display_lines(&self, width: u16) -> Vec> { - self.output_lines(width, /*render_full_context*/ false) - } - - /// The transcript overlay preserves complete hook context hidden by the viewport preview. - fn transcript_lines(&self, width: u16) -> Vec> { - self.output_lines(width, /*render_full_context*/ true) + fn display_lines(&self, _width: u16) -> Vec> { + self.output_lines() } fn raw_lines(&self) -> Vec> { - plain_lines(self.output_lines(u16::MAX, /*render_full_context*/ true)) - } - - /// Produces a coarse cache key for transcript overlays while hook animations are active. - fn transcript_animation_tick(&self) -> Option { - if !self.animations_enabled { - return None; - } - let elapsed = self - .runs - .iter() - .filter(|run| run.state.is_running_visible()) - .find_map(|run| run.state.start_time())? - .elapsed(); - Some(elapsed.as_millis() as u64 / 600) - } -} - -impl Renderable for HookCell { - fn render(&self, area: Rect, buf: &mut Buffer) { - let lines = self.display_lines(area.width); - let paragraph = Paragraph::new(Text::from(lines)).wrap(Wrap { trim: false }); - paragraph.render(area, buf); - } - - fn desired_height(&self, width: u16) -> u16 { - HistoryCell::desired_height(self, width) + plain_lines(self.output_lines()) } } @@ -424,89 +371,12 @@ impl HookRunCell { *reveal_deadline = delayed_deadline; } } - - /// Returns the grouping key only for states that render as running. - fn running_group_key(&self) -> Option { - self.state - .is_running_visible() - .then(|| RunningHookGroupKey { - event_name: self.event_name, - status_message: self.status_message.clone(), - }) - } - - /// Appends the lines for a single, ungrouped hook run. - fn push_display_lines( - &self, - lines: &mut Vec>, - animations_enabled: bool, - width: u16, - render_full_context: bool, - ) { - let label = hook_event_label(self.event_name); - match &self.state { - HookRunState::VisibleRunning { start_time, .. } - | HookRunState::QuietLinger { start_time, .. } => { - let hook_text = format!("Running {label} hook"); - push_running_hook_header( - lines, - &hook_text, - Some(*start_time), - self.status_message.as_deref(), - animations_enabled, - ); - } - HookRunState::Completed { status, entries } => { - let system_message = entries - .iter() - .find(|entry| entry.kind == HookOutputEntryKind::Warning); - let mut system_message_lines = system_message.map(|entry| entry.text.split('\n')); - let status_text = format!("{status:?}").to_lowercase(); - let header_text = if let Some(first_line) = - system_message_lines.as_mut().and_then(Iterator::next) - { - format!("{label} ({status_text}) says: {first_line}") - } else { - format!("{label} hook ({status_text})") - }; - lines.push( - vec![ - hook_completed_bullet(*status, entries), - " ".into(), - header_text.into(), - ] - .into(), - ); - if let Some(system_message_lines) = system_message_lines { - for line in system_message_lines { - if line.is_empty() { - lines.push("".into()); - } else { - lines.push(format!("{HOOK_OUTPUT_BODY_INDENT}{line}").into()); - } - } - } - for entry in entries { - if entry.kind == HookOutputEntryKind::Warning { - continue; - } - if !render_full_context && entry.kind == HookOutputEntryKind::Context { - lines.extend(hook_context_preview_lines(&entry.text, width)); - } else { - push_full_hook_output_entry(lines, entry); - } - } - } - HookRunState::PendingReveal { .. } => {} - } - } } fn push_full_hook_output_entry(lines: &mut Vec>, entry: &HookOutputEntry) { - let prefix = hook_output_prefix(entry.kind); let mut output_lines = entry.text.split('\n'); if let Some(first_line) = output_lines.next() { - lines.push(format!("{HOOK_OUTPUT_INDENT}{prefix}{first_line}").into()); + lines.push(format!("{HOOK_OUTPUT_INDENT}└ {first_line}").into()); } for line in output_lines { if line.is_empty() { @@ -517,70 +387,10 @@ fn push_full_hook_output_entry(lines: &mut Vec>, entry: &HookOutpu } } -fn hook_context_preview_lines(text: &str, width: u16) -> Vec> { - let width = usize::from(width.max(1)); - let mut wrapped = Vec::new(); - let mut source_lines = text.split('\n'); - let first_line = source_lines.next().unwrap_or_default(); - push_wrapped_hook_context_line( - &mut wrapped, - first_line, - width, - Line::from(format!( - "{HOOK_OUTPUT_INDENT}{}", - hook_output_prefix(HookOutputEntryKind::Context) - )), - ); - for line in source_lines { - if line.is_empty() { - wrapped.push("".into()); - } else { - push_wrapped_hook_context_line( - &mut wrapped, - line, - width, - Line::from(HOOK_OUTPUT_BODY_INDENT), - ); - } - } - - if wrapped.len() <= HOOK_CONTEXT_MAX_DISPLAY_ROWS { - return wrapped; - } - - let retained_rows = HOOK_CONTEXT_MAX_DISPLAY_ROWS - 1; - let omitted_rows = wrapped.len() - retained_rows; - wrapped.truncate(retained_rows); - let hint = vec![ - HOOK_OUTPUT_BODY_INDENT.into(), - format!("… +{omitted_rows} lines ({TRANSCRIPT_HINT})").dim(), - ] - .into(); - wrapped.push(truncate_line_with_ellipsis_if_overflow(hint, width)); - wrapped -} - -fn push_wrapped_hook_context_line( - output: &mut Vec>, - text: &str, - width: usize, - initial_indent: Line<'static>, -) { - let line = Line::from(text.to_string()); - let wrapped = word_wrap_line( - &line, - RtOptions::new(width) - .initial_indent(initial_indent) - .subsequent_indent(Line::from(HOOK_OUTPUT_BODY_INDENT)), - ); - push_owned_lines(&wrapped, output); -} - impl HookRunState { /// Creates the hidden initial state for a live hook run. fn pending(start_time: Instant) -> Self { Self::PendingReveal { - start_time, reveal_deadline: start_time + HOOK_RUN_REVEAL_DELAY, } } @@ -600,21 +410,14 @@ impl HookRunState { } } - /// Returns true when this run contributes at least one line to the current render. - fn should_render(&self) -> bool { - match self { - HookRunState::VisibleRunning { .. } - | HookRunState::QuietLinger { .. } - | HookRunState::Completed { .. } => true, - HookRunState::PendingReveal { .. } => false, - } - } - /// Returns true for completed runs that should survive outside the active cell. fn has_persistent_output(&self) -> bool { match self { HookRunState::Completed { status, entries } => { - *status != HookRunStatus::Completed || !entries.is_empty() + *status != HookRunStatus::Completed + || entries + .iter() + .any(|entry| entry.kind != HookOutputEntryKind::Context) } HookRunState::PendingReveal { .. } | HookRunState::VisibleRunning { .. } @@ -622,19 +425,7 @@ impl HookRunState { } } - /// Returns the original start time for active states. - /// - /// Completed runs no longer animate, so they intentionally have no start time. - fn start_time(&self) -> Option { - match self { - HookRunState::PendingReveal { start_time, .. } - | HookRunState::VisibleRunning { start_time, .. } - | HookRunState::QuietLinger { start_time, .. } => Some(*start_time), - HookRunState::Completed { .. } => None, - } - } - - /// Returns true when the run should be treated as an in-progress row. + /// Returns true for revealed runs whose lifecycle bookkeeping is still active. fn is_running_visible(&self) -> bool { matches!( self, @@ -647,20 +438,13 @@ impl HookRunState { /// Returns true only when this call changes the state, allowing timer callbacks to avoid /// unnecessary redraws. fn reveal_if_due(&mut self, now: Instant) -> bool { - let HookRunState::PendingReveal { - start_time, - reveal_deadline, - } = self - else { + let HookRunState::PendingReveal { reveal_deadline } = self else { return false; }; if now < *reveal_deadline { return false; } - *self = HookRunState::VisibleRunning { - start_time: *start_time, - visible_since: now, - }; + *self = HookRunState::VisibleRunning { visible_since: now }; true } @@ -694,186 +478,83 @@ impl HookRunState { /// Returns false when the success should be removed immediately: either it was never visible or /// it has already stayed visible for the minimum duration. fn complete_quiet_success(&mut self, now: Instant) -> bool { - let HookRunState::VisibleRunning { - start_time, - visible_since, - .. - } = self - else { + let HookRunState::VisibleRunning { visible_since, .. } = self else { return false; }; - let start_time = *start_time; let minimum_deadline = *visible_since + QUIET_HOOK_MIN_VISIBLE; if now >= minimum_deadline { return false; } *self = HookRunState::QuietLinger { - start_time, removal_deadline: minimum_deadline, }; true } } -impl RunningHookGroup { - fn new(key: RunningHookGroupKey, start_time: Option) -> Self { - Self { - key, - start_time, - count: 1, - } - } +pub(crate) fn new_active_hook_cell(run: HookRunSummary) -> HookCell { + HookCell::new_active(run) } -/// Emits one grouped running-hook status row. -fn push_running_hook_group( - lines: &mut Vec>, - group: &RunningHookGroup, - animations_enabled: bool, -) { - push_hook_line_separator(lines); - let label = hook_event_label(group.key.event_name); - let hook_text = if group.count == 1 { - format!("Running {label} hook") - } else { - format!("Running {} {label} hooks", group.count) - }; - push_running_hook_header( - lines, - &hook_text, - group.start_time, - group.key.status_message.as_deref(), - animations_enabled, - ); -} - -/// Emits the animated or static header used by all running hook rows. -fn push_running_hook_header( - lines: &mut Vec>, - hook_text: &str, - start_time: Option, - status_message: Option<&str>, - animations_enabled: bool, -) { - let mut header = Vec::new(); - let motion_mode = MotionMode::from_animations_enabled(animations_enabled); - if let Some(indicator) = - activity_indicator(start_time, motion_mode, ReducedMotionIndicator::Hidden) - { - header.push(indicator); - header.push(" ".into()); - } - header.extend(shimmer_text(hook_text, motion_mode)); - if !animations_enabled && let Some(span) = header.last_mut() { - span.style = span.style.patch(Style::default().bold()); - } - if let Some(status_message) = status_message - && !status_message.is_empty() - { - header.push(": ".into()); - header.push(status_message.to_string().dim()); - } - lines.push(header.into()); -} - -/// Adds a blank separator between hook blocks without leaving a leading blank line. -fn push_hook_line_separator(lines: &mut Vec>) { - if !lines.is_empty() { - lines.push("".into()); - } -} - -/// Combines optional instants while preserving the earliest known start time. -fn earliest_instant(left: Option, right: Option) -> Option { - match (left, right) { - (Some(left), Some(right)) => Some(left.min(right)), - (Some(left), None) => Some(left), - (None, Some(right)) => Some(right), - (None, None) => None, - } -} - -pub(crate) fn new_active_hook_cell(run: HookRunSummary, animations_enabled: bool) -> HookCell { - HookCell::new_active(run, animations_enabled) -} - -pub(crate) fn new_completed_hook_cell(run: HookRunSummary, animations_enabled: bool) -> HookCell { - HookCell::new_completed(run, animations_enabled) +pub(crate) fn new_completed_hook_cell(run: HookRunSummary) -> HookCell { + HookCell::new_completed(run) } /// Returns true for hook completions that should be invisible in history. fn hook_run_is_quiet_success(run: &HookRunSummary) -> bool { - run.status == HookRunStatus::Completed && run.entries.is_empty() + run.status == HookRunStatus::Completed + && run + .entries + .iter() + .all(|entry| entry.kind == HookOutputEntryKind::Context) } -fn hook_completed_bullet(status: HookRunStatus, entries: &[HookOutputEntry]) -> Span<'static> { +fn hook_completed_bullet(status: HookRunStatus) -> Span<'static> { match status { - HookRunStatus::Completed => { - if entries - .iter() - .any(|entry| entry.kind == HookOutputEntryKind::Warning) - { - "•".bold() - } else { - "•".green().bold() - } - } + HookRunStatus::Completed => "•".green().bold(), HookRunStatus::Blocked | HookRunStatus::Failed | HookRunStatus::Stopped => "•".red().bold(), HookRunStatus::Running => "•".into(), } } -fn hook_output_prefix(kind: HookOutputEntryKind) -> &'static str { - match kind { - HookOutputEntryKind::Warning => "warning: ", - HookOutputEntryKind::Stop => "stop: ", - HookOutputEntryKind::Feedback => "feedback: ", - HookOutputEntryKind::Context => "hook context: ", - HookOutputEntryKind::Error => "error: ", - } -} - -fn hook_event_label(event_name: HookEventName) -> &'static str { - match event_name { - HookEventName::PreToolUse => "PreToolUse", - HookEventName::PermissionRequest => "PermissionRequest", - HookEventName::PostToolUse => "PostToolUse", - HookEventName::PreCompact => "PreCompact", - HookEventName::PostCompact => "PostCompact", - HookEventName::SessionStart => "SessionStart", - HookEventName::SessionEnd => "SessionEnd", - HookEventName::UserPromptSubmit => "UserPromptSubmit", - HookEventName::SubagentStart => "SubagentStart", - HookEventName::SubagentStop => "SubagentStop", - HookEventName::Stop => "Stop", - HookEventName::Interrupt => "Interrupt", - } -} - #[cfg(test)] mod tests { use super::*; use crate::test_support::PathBufExt; use crate::test_support::test_path_buf; + use codex_app_server_protocol::HookEventName; use pretty_assertions::assert_eq; + use ratatui::style::Color; use ratatui::style::Modifier; #[test] - fn completed_hook_with_warning_uses_default_bold_bullet() { - let entries = vec![HookOutputEntry { - kind: HookOutputEntryKind::Warning, - text: "Heads up from the hook".to_string(), - }]; + fn completed_hook_system_message_uses_dim_provenance_and_hides_context() { + let cell = completed_hook_cell( + HookEventName::SessionStart, + HookRunStatus::Completed, + vec![ + HookOutputEntry { + kind: HookOutputEntryKind::Warning, + text: "Heads up from the hook".to_string(), + }, + HookOutputEntry { + kind: HookOutputEntryKind::Context, + text: "Private model-facing project instructions".to_string(), + }, + ], + ); + let lines = cell.display_lines(/*width*/ 80); + let expected = vec!["↳ Hook · Heads up from the hook".to_string()]; - let bullet = hook_completed_bullet(HookRunStatus::Completed, &entries); - - assert_eq!(bullet.content.as_ref(), "•"); - assert_eq!(bullet.style.fg, None); - assert!(bullet.style.add_modifier.contains(Modifier::BOLD)); + assert_eq!(line_texts(&lines), expected); + assert_eq!(line_texts(&cell.transcript_lines(/*width*/ 80)), expected); + assert_eq!(line_texts(&cell.raw_lines()), expected); + assert!(lines[0].spans[0].style.add_modifier.contains(Modifier::DIM)); + assert_eq!(lines[0].spans[1].style, Style::default()); } #[test] - fn completed_hook_short_multiline_context_preserves_display_transcript_and_raw_lines() { + fn completed_hook_with_only_context_is_quiet_on_every_tui_surface() { let cell = completed_hook_cell( HookEventName::SessionStart, HookRunStatus::Completed, @@ -882,59 +563,61 @@ mod tests { text: "## Working Memory Recall\n\nSource: Codex compaction".to_string(), }], ); - let expected = vec![ - "• SessionStart hook (completed)".to_string(), - " hook context: ## Working Memory Recall".to_string(), - "".to_string(), - " Source: Codex compaction".to_string(), - ]; - assert_eq!(line_texts(&cell.display_lines(/*width*/ 80)), expected); - assert_eq!(line_texts(&cell.transcript_lines(/*width*/ 80)), expected); - assert_eq!(line_texts(&cell.raw_lines()), expected); + assert!(cell.is_empty()); + assert!(cell.display_lines(/*width*/ 80).is_empty()); + assert!(cell.transcript_lines(/*width*/ 80).is_empty()); + assert!(cell.raw_lines().is_empty()); } #[test] - fn completed_hook_long_single_line_context_is_truncated_only_in_display() { - let full_context = format!( - "{}tail-marker", - "context words that should wrap across the terminal width ".repeat(8) - ); - let cell = completed_hook_cell( - HookEventName::SessionStart, - HookRunStatus::Completed, - vec![HookOutputEntry { - kind: HookOutputEntryKind::Context, - text: full_context.clone(), - }], - ); + fn running_hook_summary_uses_only_revealed_messages() { + for (messages, expected) in [ + (vec![None], "Running hook"), + (vec![Some(" ")], "Running hook"), + (vec![None, None], "Running hooks"), + (vec![Some("checking policy")], "checking policy"), + ( + vec![Some(" checking policy "), Some("checking policy")], + "checking policy", + ), + (vec![Some("checking policy"), None], "Running hooks"), + (vec![None, Some("checking policy")], "Running hooks"), + ( + vec![Some("checking policy"), Some("scanning secrets")], + "Running hooks", + ), + ] { + let mut first = hook_run_summary("0"); + first.status_message = messages[0].map(str::to_string); + let mut cell = HookCell::new_active(first); + for (index, message) in messages.iter().enumerate().skip(1) { + let mut run = hook_run_summary(&index.to_string()); + run.status_message = message.map(str::to_string); + cell.start_run(run); + } + assert_eq!(cell.running_status_summary(), None); + cell.reveal_running_runs_now_for_test(); + cell.advance_time(Instant::now()); + assert_eq!(cell.running_status_summary().as_deref(), Some(expected)); - let display_lines = cell.display_lines(/*width*/ 80); - let display = line_texts(&display_lines); - assert_eq!(display.len(), 4); - assert_eq!( - Paragraph::new(Text::from(display_lines[1..].to_vec())) - .wrap(Wrap { trim: false }) - .line_count(/*width*/ 80), - HOOK_CONTEXT_MAX_DISPLAY_ROWS - ); - assert!( - display - .iter() - .any(|line| line.contains("ctrl + t to view transcript")), - "expected truncated context to advertise the transcript: {display:?}" - ); - assert!(display.iter().all(|line| !line.contains("tail-marker"))); + // A new hook must not change the summary before its own reveal delay. + let mut fast = hook_run_summary("fast"); + fast.status_message = Some("another message".to_string()); + cell.start_run(fast.clone()); + assert_eq!(cell.running_status_summary().as_deref(), Some(expected)); + fast.status = HookRunStatus::Completed; + cell.complete_run(fast); + assert_eq!(cell.running_status_summary().as_deref(), Some(expected)); - let expected_full = vec![ - "• SessionStart hook (completed)".to_string(), - format!(" hook context: {full_context}"), - ]; - assert_eq!( - line_texts(&cell.transcript_lines(/*width*/ 80)), - expected_full - ); - assert_eq!(line_texts(&cell.raw_lines()), expected_full); + // Synchronous completion notifications arrive after the batch finishes. + for index in 0..messages.len() { + let mut completed = hook_run_summary(&index.to_string()); + completed.status = HookRunStatus::Completed; + cell.complete_run(completed); + } + assert_eq!(cell.running_status_summary(), None); + } } #[test] @@ -954,16 +637,42 @@ mod tests { }], ); - let display = line_texts(&cell.display_lines(/*width*/ 20)); - assert!( - display.iter().any(|line| line == " fifth"), - "expected {kind:?} output to remain complete: {display:?}" + assert_eq!( + line_texts(&cell.display_lines(/*width*/ 20)).join("\n"), + "• Hook stopped\n └ first\n second\n third\n fourth\n fifth", + "expected {kind:?} output to remain complete", ); + } + } + + #[test] + fn unsuccessful_hooks_use_bold_red_bullets_and_actionable_details() { + for (status, expected_header) in [ + (HookRunStatus::Failed, "• Hook failed"), + (HookRunStatus::Blocked, "• Blocked by hook"), + (HookRunStatus::Stopped, "• Hook stopped"), + ] { + let detail = "Policy prevented this action."; + let cell = completed_hook_cell( + HookEventName::PreToolUse, + status, + vec![HookOutputEntry { + kind: HookOutputEntryKind::Error, + text: detail.to_string(), + }], + ); + let lines = cell.display_lines(/*width*/ 80); + + assert_eq!( + line_texts(&lines), + vec![expected_header.to_string(), format!(" └ {detail}")], + ); + assert_eq!(lines[0].spans[0].style.fg, Some(Color::Red)); assert!( - display - .iter() - .all(|line| !line.contains("ctrl + t to view transcript")), - "did not expect a transcript hint for {kind:?}: {display:?}" + lines[0].spans[0] + .style + .add_modifier + .contains(Modifier::BOLD) ); } } @@ -982,63 +691,12 @@ mod tests { assert_eq!( line_texts(&cell.display_lines(/*width*/ 80)), vec![ - "• Stop (completed) says: Heads up".to_string(), + "↳ Hook · Heads up".to_string(), " Review generated files".to_string(), ] ); } - #[test] - fn pending_hook_does_not_animate_transcript() { - let cell = - HookCell::new_active(hook_run_summary("hook-1"), /*animations_enabled*/ true); - - assert_eq!(cell.transcript_animation_tick(), None); - } - - #[test] - fn visible_hook_animates_transcript_when_animations_enabled() { - let mut cell = - HookCell::new_active(hook_run_summary("hook-1"), /*animations_enabled*/ true); - cell.reveal_running_runs_now_for_test(); - cell.advance_time(Instant::now()); - - assert_eq!(cell.transcript_animation_tick(), Some(0)); - } - - #[test] - fn visible_hook_does_not_animate_transcript_when_animations_disabled() { - let mut cell = HookCell::new_active( - hook_run_summary("hook-1"), - /*animations_enabled*/ false, - ); - cell.reveal_running_runs_now_for_test(); - cell.advance_time(Instant::now()); - - assert_eq!(cell.transcript_animation_tick(), None); - } - - #[test] - fn visible_hook_without_animations_omits_spinner() { - let mut cell = HookCell::new_active( - hook_run_summary("hook-1"), - /*animations_enabled*/ false, - ); - cell.reveal_running_runs_now_for_test(); - cell.advance_time(Instant::now()); - - let rendered: Vec = cell - .display_lines(/*width*/ 80) - .iter() - .map(line_text) - .collect(); - - assert_eq!( - rendered, - vec!["Running PostToolUse hook: checking output policy".to_string()] - ); - } - fn completed_hook_cell( event_name: HookEventName, status: HookRunStatus, @@ -1051,7 +709,7 @@ mod tests { run.completed_at = Some(2); run.duration_ms = Some(1); run.entries = entries; - HookCell::new_completed(run, /*animations_enabled*/ false) + HookCell::new_completed(run) } fn line_texts(lines: &[Line<'_>]) -> Vec { diff --git a/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__hook_status_reflows_with_background_activity.snap b/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__hook_status_reflows_with_background_activity.snap new file mode 100644 index 0000000000..0528ac1820 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__hook_status_reflows_with_background_activity.snap @@ -0,0 +1,21 @@ +--- +source: tui/src/status_indicator_widget.rs +expression: "frames.join(\"\\n\")" +--- +124 columns: +"Working (0s • esc to interrupt) · 1 background terminal running · /ps to view · /stop to close · checking 日本語 ガパ policy" Hidden by multi-width symbols: [(107, " "), (109, " "), (111, " "), (114, " "), (116, " ")] +" └ existing details " + +123 columns: +"Working (0s • esc to interrupt) · 1 background terminal running · /ps to view · /stop to close " +" └ checking 日本語 ガパ policy " Hidden by multi-width symbols: [(14, " "), (16, " "), (18, " "), (21, " "), (23, " ")] +" └ existing details " + +24 columns: +"Working (0s • esc to in…" +" └ checking 日本語 ガ… " Hidden by multi-width symbols: [(14, " "), (16, " "), (18, " "), (21, " ")] +" └ existing details " + +124 columns: +"Working (0s • esc to interrupt) · 1 background terminal running · /ps to view · /stop to close · checking 日本語 ガパ policy" Hidden by multi-width symbols: [(107, " "), (109, " "), (111, " "), (114, " "), (116, " ")] +" └ existing details " diff --git a/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__hook_status_reflows_without_background_activity.snap b/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__hook_status_reflows_without_background_activity.snap new file mode 100644 index 0000000000..d15a97e336 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__hook_status_reflows_without_background_activity.snap @@ -0,0 +1,21 @@ +--- +source: tui/src/status_indicator_widget.rs +expression: "frames.join(\"\\n\")" +--- +61 columns: +"Working (0s • esc to interrupt) · checking 日本語 ガパ policy" Hidden by multi-width symbols: [(44, " "), (46, " "), (48, " "), (51, " "), (53, " ")] +" └ existing details " + +60 columns: +"Working (0s • esc to interrupt) " +" └ checking 日本語 ガパ policy " Hidden by multi-width symbols: [(14, " "), (16, " "), (18, " "), (21, " "), (23, " ")] +" └ existing details " + +24 columns: +"Working (0s • esc to in…" +" └ checking 日本語 ガ… " Hidden by multi-width symbols: [(14, " "), (16, " "), (18, " "), (21, " ")] +" └ existing details " + +61 columns: +"Working (0s • esc to interrupt) · checking 日本語 ガパ policy" Hidden by multi-width symbols: [(44, " "), (46, " "), (48, " "), (51, " "), (53, " ")] +" └ existing details " diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index 8f3e8c8977..893bdef5c4 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -3,6 +3,8 @@ //! The row renders a separately owned clock, the optional interrupt hint, and short inline //! context (for example, the unified-exec background-process summary). Keeping //! these pieces on one line avoids vertical layout churn in the bottom pane. +//! Hook activity uses the remaining space or its own line on overflow, so it +//! never displaces background-process controls. use std::time::Duration; use std::time::Instant; @@ -21,6 +23,7 @@ use unicode_width::UnicodeWidthStr; use crate::app_event_sender::AppEventSender; use crate::key_hint; use crate::key_hint::ShortcutHint; +use crate::line_truncation::line_width; use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::motion::MotionMode; use crate::motion::ReducedMotionIndicator; @@ -29,6 +32,7 @@ use crate::motion::shimmer_text; use crate::render::renderable::Renderable; use crate::text_formatting::capitalize_first; use crate::tui::FrameRequester; +use crate::width::display_width; use crate::wrapping::RtOptions; use crate::wrapping::word_wrap_lines; @@ -52,6 +56,8 @@ pub(crate) struct StatusIndicatorWidget { details_max_lines: usize, /// Optional suffix rendered after the elapsed/interrupt segment. inline_message: Option, + /// Hook activity may move below the status row when it cannot fit in full. + hook_status_message: Option, show_interrupt_hint: bool, interrupt_binding: Option, @@ -88,6 +94,7 @@ impl StatusIndicatorWidget { details: None, details_max_lines: STATUS_DETAILS_DEFAULT_MAX_LINES, inline_message: None, + hook_status_message: None, show_interrupt_hint: true, interrupt_binding: Some(key_hint::plain(KeyCode::Esc).into()), app_event_tx, @@ -135,6 +142,10 @@ impl StatusIndicatorWidget { .filter(|message| !message.is_empty()); } + pub(crate) fn update_hook_status_message(&mut self, message: Option) { + self.hook_status_message = message; + } + pub(crate) fn header(&self) -> &str { &self.header } @@ -194,22 +205,11 @@ struct StatusIndicator<'a> { timer: &'a StatusTimer, } -impl Renderable for StatusIndicator<'_> { - fn desired_height(&self, width: u16) -> u16 { - 1 + u16::try_from(self.row.wrapped_details_lines(width).len()).unwrap_or(0) - } - - fn render(&self, area: Rect, buf: &mut Buffer) { - if area.is_empty() { - return; - } - +impl StatusIndicator<'_> { + // Share width decisions between height measurement and rendering, including + // wide Unicode characters, remapped interrupt hints, and elapsed-time text. + fn lines(&self, width: u16) -> Vec> { let row = self.row; - if row.animations_enabled { - // Schedule next animation frame. - row.frame_requester - .schedule_frame_in(Duration::from_millis(32)); - } let now = Instant::now(); let elapsed_duration = self.timer.elapsed_at(now); let pretty_elapsed = fmt_elapsed_compact(elapsed_duration.as_secs()); @@ -246,19 +246,46 @@ impl Renderable for StatusIndicator<'_> { spans.push(message.clone().dim()); } + let mut header = Line::from(spans); + let mut hook_overflow = None; + if let Some(message) = &row.hook_status_message { + if line_width(&header) + display_width(" · ") + display_width(message) + <= usize::from(width) + { + header.spans.extend([" · ".dim(), message.clone().dim()]); + } else { + hook_overflow = Some(truncate_line_with_ellipsis_if_overflow( + Line::from(vec![DETAILS_PREFIX.dim(), message.clone().dim()]), + usize::from(width), + )); + } + } let mut lines = Vec::new(); lines.push(truncate_line_with_ellipsis_if_overflow( - Line::from(spans), - usize::from(area.width), + header, + usize::from(width), )); - if area.height > 1 { - // If there is enough space, add the details lines below the header. - let details = row.wrapped_details_lines(area.width); - let max_details = usize::from(area.height.saturating_sub(1)); - lines.extend(details.into_iter().take(max_details)); - } + lines.extend(hook_overflow); + lines.extend(row.wrapped_details_lines(width)); + lines + } +} - Paragraph::new(Text::from(lines)).render(area, buf); +impl Renderable for StatusIndicator<'_> { + fn desired_height(&self, width: u16) -> u16 { + self.lines(width).len() as u16 + } + + fn render(&self, area: Rect, buf: &mut Buffer) { + if area.is_empty() { + return; + } + if self.row.animations_enabled { + self.row + .frame_requester + .schedule_frame_in(Duration::from_millis(32)); + } + Paragraph::new(Text::from(self.lines(area.width))).render(area, buf); } } @@ -398,6 +425,52 @@ mod tests { insta::assert_snapshot!(terminal.backend()); } + #[test] + fn hook_status_reflows_without_displacing_controls_or_details() { + let (tx, _rx) = unbounded_channel::(); + let mut w = StatusIndicatorWidget::new( + AppEventSender::new(tx), + crate::tui::FrameRequester::test_dummy(), + /*animations_enabled*/ false, + ); + let mut timer = StatusTimer::default(); + timer.pause_at(timer.last_resume_at); + w.update_hook_status_message(Some("checking 日本語 ガパ policy".to_string())); + w.update_details( + Some("existing details".to_string()), + StatusDetailsCapitalization::Preserve, + STATUS_DETAILS_DEFAULT_MAX_LINES, + ); + + for (background, snapshot) in [ + (None, "hook_status_reflows_without_background_activity"), + ( + Some("1 background terminal running · /ps to view · /stop to close"), + "hook_status_reflows_with_background_activity", + ), + ] { + w.update_inline_message(background.map(str::to_string)); + let mut expected = "Working (0s • esc to interrupt)".to_string(); + if let Some(background) = background { + expected.push_str(&format!(" · {background}")); + } + expected.push_str(" · checking 日本語 ガパ policy"); + let fit_width = display_width(&expected) as u16; + let mut frames = Vec::new(); + for width in [fit_width, fit_width - 1, 24, fit_width] { + let height = w.with_timer(&timer).desired_height(width); + assert_eq!(height, if width >= fit_width { 2 } else { 3 }); + let mut terminal = + Terminal::new(TestBackend::new(width, height)).expect("terminal"); + terminal + .draw(|f| w.with_timer(&timer).render(f.area(), f.buffer_mut())) + .expect("draw"); + frames.push(format!("{width} columns:\n{}", terminal.backend())); + } + insta::assert_snapshot!(snapshot, frames.join("\n")); + } + } + #[test] fn details_overflow_adds_ellipsis() { let (tx_raw, _rx) = unbounded_channel::();