diff --git a/codex-rs/hooks/src/events/common.rs b/codex-rs/hooks/src/events/common.rs index 8b6ba138ee4e..305a1214adb5 100644 --- a/codex-rs/hooks/src/events/common.rs +++ b/codex-rs/hooks/src/events/common.rs @@ -3,6 +3,7 @@ use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::HookOutputEntry; use codex_protocol::protocol::HookOutputEntryKind; use codex_protocol::protocol::HookRunStatus; +use codex_protocol::protocol::HookRunSummary; use crate::engine::ConfiguredHandler; use crate::engine::dispatcher; @@ -69,6 +70,31 @@ pub(crate) fn serialization_failure_hook_events( .collect() } +pub(crate) fn serialization_failure_hook_events_for_tool_use( + handlers: Vec, + turn_id: Option, + error_message: String, + tool_use_id: &str, +) -> Vec { + serialization_failure_hook_events(handlers, turn_id, error_message) + .into_iter() + .map(|event| hook_completed_for_tool_use(event, tool_use_id)) + .collect() +} + +pub(crate) fn hook_completed_for_tool_use( + mut event: HookCompletedEvent, + tool_use_id: &str, +) -> HookCompletedEvent { + event.run = hook_run_for_tool_use(event.run, tool_use_id); + event +} + +pub(crate) fn hook_run_for_tool_use(mut run: HookRunSummary, tool_use_id: &str) -> HookRunSummary { + run.id = format!("{}:{tool_use_id}", run.id); + run +} + pub(crate) fn matcher_pattern_for_event( event_name: HookEventName, matcher: Option<&str>, diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index 3af9bef5e562..9df05338e66a 100644 --- a/codex-rs/hooks/src/events/post_tool_use.rs +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -59,7 +59,9 @@ pub(crate) fn preview( Some(&request.tool_name), ) .into_iter() - .map(|handler| dispatcher::running_summary(&handler)) + .map(|handler| { + common::hook_run_for_tool_use(dispatcher::running_summary(&handler), &request.tool_use_id) + }) .collect() } @@ -100,11 +102,13 @@ pub(crate) async fn run( }) { Ok(input_json) => input_json, Err(error) => { - return serialization_failure_outcome(common::serialization_failure_hook_events( + let hook_events = common::serialization_failure_hook_events_for_tool_use( matched, - Some(request.turn_id), + Some(request.turn_id.clone()), format!("failed to serialize post tool use hook input: {error}"), - )); + &request.tool_use_id, + ); + return serialization_failure_outcome(hook_events); } }; @@ -113,7 +117,7 @@ pub(crate) async fn run( matched, input_json, request.cwd.as_path(), - Some(request.turn_id), + Some(request.turn_id.clone()), parse_completed, ) .await; @@ -135,7 +139,12 @@ pub(crate) async fn run( ); PostToolUseOutcome { - hook_events: results.into_iter().map(|result| result.completed).collect(), + hook_events: results + .into_iter() + .map(|result| { + common::hook_completed_for_tool_use(result.completed, &request.tool_use_id) + }) + .collect(), should_stop, stop_reason, additional_contexts, @@ -295,16 +304,20 @@ fn serialization_failure_outcome(hook_events: Vec) -> PostTo mod tests { use std::path::PathBuf; + use codex_protocol::ThreadId; use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::HookOutputEntry; use codex_protocol::protocol::HookOutputEntryKind; use codex_protocol::protocol::HookRunStatus; use pretty_assertions::assert_eq; + use serde_json::json; use super::PostToolUseHandlerData; use super::parse_completed; + use super::preview; use crate::engine::ConfiguredHandler; use crate::engine::command_runner::CommandRunResult; + use crate::events::common; #[test] fn block_decision_stops_normal_processing() { @@ -463,6 +476,40 @@ mod tests { assert_eq!(parsed.completed.run.entries, Vec::::new()); } + #[test] + fn preview_and_completed_run_ids_include_tool_use_id() { + let request = request_for_tool_use("tool-call-456"); + let runs = preview(&[handler()], &request); + + assert_eq!(runs.len(), 1); + assert_eq!(runs[0].id, "post-tool-use:0:/tmp/hooks.json:tool-call-456"); + + let parsed = parse_completed( + &handler(), + run_result(Some(0), "", ""), + Some("turn-1".to_string()), + ); + let completed = common::hook_completed_for_tool_use(parsed.completed, &request.tool_use_id); + + assert_eq!(completed.run.id, runs[0].id); + } + + #[test] + fn serialization_failure_run_ids_include_tool_use_id() { + let request = request_for_tool_use("tool-call-456"); + let runs = preview(&[handler()], &request); + + let completed = common::serialization_failure_hook_events_for_tool_use( + vec![handler()], + Some(request.turn_id.clone()), + "serialize failed".into(), + &request.tool_use_id, + ); + + assert_eq!(completed.len(), 1); + assert_eq!(completed[0].run.id, runs[0].id); + } + fn handler() -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::PostToolUse, @@ -486,4 +533,19 @@ mod tests { error: None, } } + + fn request_for_tool_use(tool_use_id: &str) -> super::PostToolUseRequest { + super::PostToolUseRequest { + session_id: ThreadId::new(), + turn_id: "turn-1".to_string(), + cwd: PathBuf::from("/tmp"), + transcript_path: None, + model: "gpt-test".to_string(), + permission_mode: "default".to_string(), + tool_name: "Bash".to_string(), + tool_use_id: tool_use_id.to_string(), + command: "echo hello".to_string(), + tool_response: json!({"ok": true}), + } + } } diff --git a/codex-rs/hooks/src/events/pre_tool_use.rs b/codex-rs/hooks/src/events/pre_tool_use.rs index 8366bb632cb0..44c9021b998f 100644 --- a/codex-rs/hooks/src/events/pre_tool_use.rs +++ b/codex-rs/hooks/src/events/pre_tool_use.rs @@ -52,7 +52,9 @@ pub(crate) fn preview( Some(&request.tool_name), ) .into_iter() - .map(|handler| dispatcher::running_summary(&handler)) + .map(|handler| { + common::hook_run_for_tool_use(dispatcher::running_summary(&handler), &request.tool_use_id) + }) .collect() } @@ -90,11 +92,13 @@ pub(crate) async fn run( }) { Ok(input_json) => input_json, Err(error) => { - return serialization_failure_outcome(common::serialization_failure_hook_events( + let hook_events = common::serialization_failure_hook_events_for_tool_use( matched, - Some(request.turn_id), + Some(request.turn_id.clone()), format!("failed to serialize pre tool use hook input: {error}"), - )); + &request.tool_use_id, + ); + return serialization_failure_outcome(hook_events); } }; @@ -103,7 +107,7 @@ pub(crate) async fn run( matched, input_json, request.cwd.as_path(), - Some(request.turn_id), + Some(request.turn_id.clone()), parse_completed, ) .await; @@ -114,7 +118,12 @@ pub(crate) async fn run( .find_map(|result| result.data.block_reason.clone()); PreToolUseOutcome { - hook_events: results.into_iter().map(|result| result.completed).collect(), + hook_events: results + .into_iter() + .map(|result| { + common::hook_completed_for_tool_use(result.completed, &request.tool_use_id) + }) + .collect(), should_block, block_reason, } @@ -232,6 +241,7 @@ fn serialization_failure_outcome(hook_events: Vec) -> PreToo mod tests { use std::path::PathBuf; + use codex_protocol::ThreadId; use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::HookOutputEntry; use codex_protocol::protocol::HookOutputEntryKind; @@ -240,8 +250,10 @@ mod tests { use super::PreToolUseHandlerData; use super::parse_completed; + use super::preview; use crate::engine::ConfiguredHandler; use crate::engine::command_runner::CommandRunResult; + use crate::events::common; #[test] fn permission_decision_deny_blocks_processing() { @@ -453,6 +465,40 @@ mod tests { ); } + #[test] + fn preview_and_completed_run_ids_include_tool_use_id() { + let request = request_for_tool_use("tool-call-123"); + let runs = preview(&[handler()], &request); + + assert_eq!(runs.len(), 1); + assert_eq!(runs[0].id, "pre-tool-use:0:/tmp/hooks.json:tool-call-123"); + + let parsed = parse_completed( + &handler(), + run_result(Some(0), "", ""), + Some("turn-1".to_string()), + ); + let completed = common::hook_completed_for_tool_use(parsed.completed, &request.tool_use_id); + + assert_eq!(completed.run.id, runs[0].id); + } + + #[test] + fn serialization_failure_run_ids_include_tool_use_id() { + let request = request_for_tool_use("tool-call-123"); + let runs = preview(&[handler()], &request); + + let completed = common::serialization_failure_hook_events_for_tool_use( + vec![handler()], + Some(request.turn_id.clone()), + "serialize failed".into(), + &request.tool_use_id, + ); + + assert_eq!(completed.len(), 1); + assert_eq!(completed[0].run.id, runs[0].id); + } + fn handler() -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::PreToolUse, @@ -476,4 +522,18 @@ mod tests { error: None, } } + + fn request_for_tool_use(tool_use_id: &str) -> super::PreToolUseRequest { + super::PreToolUseRequest { + session_id: ThreadId::new(), + turn_id: "turn-1".to_string(), + cwd: PathBuf::from("/tmp"), + transcript_path: None, + model: "gpt-test".to_string(), + permission_mode: "default".to_string(), + tool_name: "Bash".to_string(), + tool_use_id: tool_use_id.to_string(), + command: "echo hello".to_string(), + } + } } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index c110660549be..e567ac652347 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -330,6 +330,7 @@ use crate::history_cell; #[cfg(test)] use crate::history_cell::AgentMessageCell; use crate::history_cell::HistoryCell; +use crate::history_cell::HookCell; use crate::history_cell::McpToolCallCell; use crate::history_cell::PlainHistoryCell; use crate::history_cell::WebSearchCell; @@ -832,6 +833,8 @@ pub(crate) struct ChatWidget { // Guardian review keeps its own pending set so it can derive a single // footer summary from one or more in-flight review events. pending_guardian_review_status: PendingGuardianReviewStatus, + // Active hook runs render in a dedicated live cell so they can run alongside tools. + active_hook_cell: Option, // Semantic status used for terminal-title status rendering. terminal_title_status_kind: TerminalTitleStatusKind, // Previous status header to restore after a transient stream retry. @@ -2293,6 +2296,9 @@ impl ChatWidget { self.quit_shortcut_key = None; self.update_task_running_state(); self.retry_status_header = None; + if self.active_hook_cell.take().is_some() { + self.bump_active_cell_revision(); + } self.pending_status_indicator_restore = false; self.bottom_pane .set_interrupt_hint_visible(/*visible*/ true); @@ -3946,36 +3952,127 @@ impl ChatWidget { } fn on_hook_started(&mut self, event: codex_protocol::protocol::HookStartedEvent) { - let label = hook_event_label(event.run.event_name); - let mut message = format!("Running {label} hook"); - if let Some(status_message) = event.run.status_message - && !status_message.is_empty() - { - message.push_str(": "); - message.push_str(&status_message); + self.flush_answer_stream_with_separator(); + self.flush_completed_hook_output(); + match self.active_hook_cell.as_mut() { + Some(cell) => { + cell.start_run(event.run); + self.bump_active_cell_revision(); + } + None => { + self.active_hook_cell = Some(history_cell::new_active_hook_cell( + event.run, + self.config.animations, + )); + self.bump_active_cell_revision(); + } } - self.add_to_history(history_cell::new_info_event(message, /*hint*/ None)); self.request_redraw(); } fn on_hook_completed(&mut self, event: codex_protocol::protocol::HookCompletedEvent) { - let status = format!("{:?}", event.run.status).to_lowercase(); - let header = format!("{} hook ({status})", hook_event_label(event.run.event_name)); - let mut lines: Vec> = vec![header.into()]; - for entry in event.run.entries { - let prefix = match entry.kind { - codex_protocol::protocol::HookOutputEntryKind::Warning => "warning: ", - codex_protocol::protocol::HookOutputEntryKind::Stop => "stop: ", - codex_protocol::protocol::HookOutputEntryKind::Feedback => "feedback: ", - codex_protocol::protocol::HookOutputEntryKind::Context => "hook context: ", - codex_protocol::protocol::HookOutputEntryKind::Error => "error: ", - }; - lines.push(format!(" {prefix}{}", entry.text).into()); + let completed = event.run; + let completed_existing_run = self + .active_hook_cell + .as_mut() + .map(|cell| cell.complete_run(completed.clone())) + .unwrap_or(false); + if completed_existing_run { + self.bump_active_cell_revision(); + } else { + match self.active_hook_cell.as_mut() { + Some(cell) => { + cell.add_completed_run(completed); + self.bump_active_cell_revision(); + } + None => { + let cell = + history_cell::new_completed_hook_cell(completed, self.config.animations); + if !cell.is_empty() { + self.active_hook_cell = Some(cell); + self.bump_active_cell_revision(); + } + } + } } - self.add_to_history(PlainHistoryCell::new(lines)); + self.flush_completed_hook_output(); + self.finish_active_hook_cell_if_idle(); self.request_redraw(); } + fn flush_completed_hook_output(&mut self) { + let Some(completed_cell) = self + .active_hook_cell + .as_mut() + .and_then(HookCell::take_completed_persistent_runs) + else { + return; + }; + let active_cell_is_empty = self + .active_hook_cell + .as_ref() + .is_some_and(HookCell::is_empty); + if active_cell_is_empty { + self.active_hook_cell = None; + } + self.bump_active_cell_revision(); + self.needs_final_message_separator = true; + self.app_event_tx + .send(AppEvent::InsertHistoryCell(Box::new(completed_cell))); + } + + fn finish_active_hook_cell_if_idle(&mut self) { + let Some(cell) = self.active_hook_cell.as_ref() else { + return; + }; + if cell.is_empty() { + self.active_hook_cell = None; + self.bump_active_cell_revision(); + return; + } + if cell.should_flush() + && let Some(cell) = self.active_hook_cell.take() + { + self.bump_active_cell_revision(); + self.needs_final_message_separator = true; + self.app_event_tx + .send(AppEvent::InsertHistoryCell(Box::new(cell))); + } + } + + fn update_due_hook_visibility(&mut self) { + let Some(cell) = self.active_hook_cell.as_mut() else { + return; + }; + let now = Instant::now(); + if cell.advance_time(now) { + self.bump_active_cell_revision(); + } + self.finish_active_hook_cell_if_idle(); + } + + fn schedule_hook_timer_if_needed(&self) { + if self.config.animations + && self + .active_hook_cell + .as_ref() + .is_some_and(HookCell::has_visible_running_run) + { + self.frame_requester + .schedule_frame_in(Duration::from_millis(50)); + } + + let Some(deadline) = self + .active_hook_cell + .as_ref() + .and_then(HookCell::next_timer_deadline) + else { + return; + }; + let delay = deadline.saturating_duration_since(Instant::now()); + self.frame_requester.schedule_frame_in(delay); + } + #[cfg(test)] fn on_undo_started(&mut self, event: UndoStartedEvent) { self.bottom_pane.ensure_status_indicator(); @@ -4023,6 +4120,8 @@ impl ChatWidget { } pub(crate) fn pre_draw_tick(&mut self) { + self.update_due_hook_visibility(); + self.schedule_hook_timer_if_needed(); self.bottom_pane.pre_draw_tick(); if self.should_animate_terminal_title_spinner() { self.refresh_terminal_title(); @@ -4670,6 +4769,7 @@ impl ChatWidget { full_reasoning_buffer: String::new(), current_status: StatusIndicatorState::working(), pending_guardian_review_status: PendingGuardianReviewStatus::default(), + active_hook_cell: None, terminal_title_status_kind: TerminalTitleStatusKind::Working, retry_status_header: None, pending_status_indicator_restore: false, @@ -10810,11 +10910,21 @@ impl ChatWidget { /// providing an appropriate animation tick), the overlay will keep showing a stale tail while /// the main viewport updates. pub(crate) fn active_cell_transcript_key(&self) -> Option { - let cell = self.active_cell.as_ref()?; + let cell = self.active_cell.as_ref(); + let hook_cell = self.active_hook_cell.as_ref(); + if cell.is_none() && hook_cell.is_none() { + return None; + } Some(ActiveCellTranscriptKey { revision: self.active_cell_revision, - is_stream_continuation: cell.is_stream_continuation(), - animation_tick: cell.transcript_animation_tick(), + 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) + }), }) } @@ -10825,8 +10935,18 @@ impl ChatWidget { /// should pass the same width the overlay uses; using a different width will cause wrapping /// mismatches between the main viewport and the transcript overlay. pub(crate) fn active_cell_transcript_lines(&self, width: u16) -> Option>> { - let cell = self.active_cell.as_ref()?; - let lines = cell.transcript_lines(width); + let mut lines = Vec::new(); + if let Some(cell) = self.active_cell.as_ref() { + lines.extend(cell.transcript_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_lines(width); + if !hook_lines.is_empty() && !lines.is_empty() { + lines.push("".into()); + } + lines.extend(hook_lines); + } (!lines.is_empty()).then_some(lines) } @@ -10852,8 +10972,17 @@ impl ChatWidget { )), None => RenderableItem::Owned(Box::new(())), }; + let active_hook_cell_renderable = match &self.active_hook_cell { + Some(cell) if cell.should_render() => { + RenderableItem::Borrowed(cell).inset(Insets::tlbr( + /*top*/ 1, /*left*/ 0, /*bottom*/ 0, /*right*/ 0, + )) + } + _ => RenderableItem::Owned(Box::new(())), + }; let mut flex = FlexRenderable::new(); flex.push(/*flex*/ 1, active_cell_renderable); + flex.push(/*flex*/ 0, active_hook_cell_renderable); flex.push( /*flex*/ 0, RenderableItem::Borrowed(&self.bottom_pane).inset(Insets::tlbr( @@ -11082,16 +11211,6 @@ fn extract_first_bold(s: &str) -> Option { None } -fn hook_event_label(event_name: codex_protocol::protocol::HookEventName) -> &'static str { - match event_name { - codex_protocol::protocol::HookEventName::PreToolUse => "PreToolUse", - codex_protocol::protocol::HookEventName::PostToolUse => "PostToolUse", - codex_protocol::protocol::HookEventName::SessionStart => "SessionStart", - codex_protocol::protocol::HookEventName::UserPromptSubmit => "UserPromptSubmit", - codex_protocol::protocol::HookEventName::Stop => "Stop", - } -} - #[cfg(test)] pub(crate) fn show_review_commit_picker_with_entries( chat: &mut ChatWidget, 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 new file mode 100644 index 000000000000..5e8360f015cd --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_hook_output_precedes_following_assistant_message_snapshot.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/chatwidget/tests/status_and_layout.rs +expression: "format!(\"active hooks:\\n{}history:\\n{history}\", active_hook_blob(&chat))" +--- +active hooks: + +history: +• PreToolUse hook (blocked) + feedback: 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 new file mode 100644 index 000000000000..d2673f54006c --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_hook_with_output_flushes_immediately_snapshot.snap @@ -0,0 +1,16 @@ +--- +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 +history: + + +completed +live hooks: + +history: +• PreToolUse hook (blocked) + feedback: 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 new file mode 100644 index 000000000000..ebdca8fc7523 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__completed_same_id_hook_output_survives_restart_snapshot.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/chatwidget/tests/status_and_layout.rs +expression: "format!(\"active hooks:\\n{}history:\\n{history}\", active_hook_blob(&chat))" +--- +active hooks: +• Running Stop hook: checking stop condition +history: +• Stop hook (stopped) + stop: continue with more context 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 new file mode 100644 index 000000000000..e0d2e7e2252a --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_blocked_failed_feedback_history_snapshot.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/chatwidget/tests/status_and_layout.rs +expression: rendered +--- +• PreToolUse hook (blocked) + feedback: run tests before touching the fixture + +• PostToolUse hook (failed) + error: 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 new file mode 100644 index 000000000000..b38d4e260af7 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_completed_before_reveal_renders_completed_without_running_flash_snapshot.snap @@ -0,0 +1,9 @@ +--- +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 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 new file mode 100644 index 000000000000..290d093ed870 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_live_running_then_quiet_completed_snapshot.snap @@ -0,0 +1,21 @@ +--- +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 new file mode 100644 index 000000000000..2f28b4ad9a0f --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hook_runs_while_exec_active_snapshot.snap @@ -0,0 +1,25 @@ +--- +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}\")" +--- +exec running: +• Running echo done + +exec and hook running: +active exec: +• Running echo done +active hooks: +• Running PostToolUse hook: checking output policy + +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 + +quiet hook completed: + 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 new file mode 100644 index 000000000000..353d7227944c --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__identical_parallel_running_hooks_collapse_to_count_snapshot.snap @@ -0,0 +1,9 @@ +--- +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__overlapping_hook_live_cell_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__overlapping_hook_live_cell_snapshot.snap new file mode 100644 index 000000000000..f71909dddf39 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__overlapping_hook_live_cell_snapshot.snap @@ -0,0 +1,43 @@ +--- +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}\")" +--- +pre running +live hooks: +• Running PreToolUse hook: checking command policy +history: + + +post running +live hooks: +• Running PreToolUse hook: checking command policy + +• Running PostToolUse hook: checking output policy +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 +history: + + +all completed +live hooks: + +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 722aa89e47c8..b00225f1cd5a 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 @@ -1,9 +1,7 @@ --- -source: tui/src/chatwidget/tests.rs +source: tui/src/chatwidget/tests/helpers.rs expression: combined --- -• Running PostToolUse hook: warming the shell - -PostToolUse hook (completed) +• PostToolUse hook (completed) warning: Heads up from the hook hook context: Remember the startup checklist. 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 11fa8ab8e349..311be2edf5ad 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 @@ -1,9 +1,7 @@ --- -source: tui/src/chatwidget/tests.rs +source: tui/src/chatwidget/tests/helpers.rs expression: combined --- -• Running PreToolUse hook: warming the shell - -PreToolUse hook (completed) +• PreToolUse hook (completed) warning: Heads up from the hook hook context: Remember the startup checklist. 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 30dfa78e5b03..3d08e82937a9 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 @@ -1,9 +1,7 @@ --- -source: tui/src/chatwidget/tests.rs +source: tui/src/chatwidget/tests/helpers.rs expression: combined --- -• Running SessionStart hook: warming the shell - -SessionStart hook (completed) +• SessionStart hook (completed) warning: Heads up from the hook hook context: Remember the startup checklist. 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 c7e23942304d..43b91941416f 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 @@ -1,10 +1,7 @@ --- -source: tui/src/chatwidget/tests.rs -assertion_line: 12789 +source: tui/src/chatwidget/tests/status_and_layout.rs expression: combined --- -• Running UserPromptSubmit hook: checking go-workflow input policy - -UserPromptSubmit hook (stopped) +• UserPromptSubmit hook (stopped) warning: go-workflow must start from PlanMode stop: prompt blocked diff --git a/codex-rs/tui/src/chatwidget/tests/helpers.rs b/codex-rs/tui/src/chatwidget/tests/helpers.rs index d5b30bc042b5..9aea70000460 100644 --- a/codex-rs/tui/src/chatwidget/tests/helpers.rs +++ b/codex-rs/tui/src/chatwidget/tests/helpers.rs @@ -238,6 +238,7 @@ pub(super) async fn make_chatwidget_manual( reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), current_status: StatusIndicatorState::working(), + active_hook_cell: None, retry_status_header: None, pending_status_indicator_restore: false, suppress_queue_autosend: false, @@ -664,6 +665,35 @@ 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(); + } + chat.pre_draw_tick(); +} + +pub(super) fn reveal_running_hooks(chat: &mut ChatWidget) { + if let Some(cell) = chat.active_hook_cell.as_mut() { + cell.reveal_running_runs_now_for_test(); + } + chat.pre_draw_tick(); +} + +pub(super) fn reveal_running_hooks_after_delayed_redraw(chat: &mut ChatWidget) { + if let Some(cell) = chat.active_hook_cell.as_mut() { + cell.reveal_running_runs_after_delayed_redraw_for_test(); + } + chat.pre_draw_tick(); +} + pub(super) fn get_available_model(chat: &ChatWidget, model: &str) -> ModelPreset { let models = chat .model_catalog @@ -977,6 +1007,18 @@ pub(super) async fn assert_hook_events_snapshot( }, }), }); + assert!( + drain_insert_history(&mut rx).is_empty(), + "hook start should update the live hook cell instead of writing history" + ); + reveal_running_hooks(&mut chat); + 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" + ); chat.handle_codex_event(Event { id: "hook-1".into(), @@ -1016,3 +1058,13 @@ pub(super) async fn assert_hook_events_snapshot( .collect::(); assert_chatwidget_snapshot!(snapshot_name, combined); } + +fn hook_event_label(event_name: codex_protocol::protocol::HookEventName) -> &'static str { + match event_name { + codex_protocol::protocol::HookEventName::PreToolUse => "PreToolUse", + codex_protocol::protocol::HookEventName::PostToolUse => "PostToolUse", + codex_protocol::protocol::HookEventName::SessionStart => "SessionStart", + codex_protocol::protocol::HookEventName::UserPromptSubmit => "UserPromptSubmit", + codex_protocol::protocol::HookEventName::Stop => "Stop", + } +} 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 b04ce74a1466..2d1183ddd6ff 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -1389,6 +1389,7 @@ async fn user_prompt_submit_app_server_hook_notifications_render_snapshot() { "user_prompt_submit_app_server_hook_notifications_render_snapshot", combined ); + assert!(!chat.bottom_pane.status_indicator_visible()); } #[tokio::test] @@ -1413,6 +1414,459 @@ async fn post_tool_use_hook_events_render_snapshot() { .await; } +#[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; + + chat.handle_codex_event(Event { + id: "hook-1".into(), + msg: EventMsg::HookStarted(codex_protocol::protocol::HookStartedEvent { + turn_id: None, + run: codex_protocol::protocol::HookRunSummary { + id: "post-tool-use:0:/tmp/hooks.json".to_string(), + event_name: codex_protocol::protocol::HookEventName::PostToolUse, + handler_type: codex_protocol::protocol::HookHandlerType::Command, + execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, + scope: codex_protocol::protocol::HookScope::Turn, + source_path: PathBuf::from("/tmp/hooks.json"), + display_order: 0, + status: codex_protocol::protocol::HookRunStatus::Running, + status_message: None, + started_at: 1, + completed_at: None, + duration_ms: None, + entries: Vec::new(), + }, + }), + }); + assert!(drain_insert_history(&mut rx).is_empty()); + reveal_running_hooks(&mut chat); + let running_snapshot = hook_live_and_history_snapshot(&chat, "running", ""); + + chat.handle_codex_event(Event { + id: "hook-1".into(), + msg: EventMsg::HookCompleted(codex_protocol::protocol::HookCompletedEvent { + turn_id: None, + run: codex_protocol::protocol::HookRunSummary { + id: "post-tool-use:0:/tmp/hooks.json".to_string(), + event_name: codex_protocol::protocol::HookEventName::PostToolUse, + handler_type: codex_protocol::protocol::HookHandlerType::Command, + execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, + scope: codex_protocol::protocol::HookScope::Turn, + source_path: PathBuf::from("/tmp/hooks.json"), + display_order: 0, + status: codex_protocol::protocol::HookRunStatus::Completed, + status_message: None, + started_at: 1, + completed_at: Some(2), + duration_ms: Some(1), + entries: 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; + + chat.handle_codex_event(hook_started_event( + "post-tool-use:0:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::PostToolUse, + Some("checking output policy"), + )); + assert!(drain_insert_history(&mut rx).is_empty()); + + reveal_running_hooks_after_delayed_redraw(&mut chat); + chat.handle_codex_event(hook_completed_event( + "post-tool-use:0:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::PostToolUse, + codex_protocol::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"); +} + +#[tokio::test] +async fn blocked_and_failed_hooks_render_feedback_and_errors() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + chat.handle_codex_event(hook_completed_event( + "pre-tool-use:0:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::PreToolUse, + codex_protocol::protocol::HookRunStatus::Blocked, + vec![codex_protocol::protocol::HookOutputEntry { + kind: codex_protocol::protocol::HookOutputEntryKind::Feedback, + text: "run tests before touching the fixture".to_string(), + }], + )); + chat.handle_codex_event(hook_completed_event( + "post-tool-use:1:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::PostToolUse, + codex_protocol::protocol::HookRunStatus::Failed, + vec![codex_protocol::protocol::HookOutputEntry { + kind: codex_protocol::protocol::HookOutputEntryKind::Error, + text: "hook exited with code 7".to_string(), + }], + )); + + let rendered = drain_insert_history(&mut rx) + .iter() + .map(|lines| lines_to_single_string(lines)) + .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" + ), + "expected blocked hook feedback: {rendered:?}" + ); + assert!( + rendered.contains("PostToolUse hook (failed)\n error: hook exited with code 7"), + "expected failed hook error: {rendered:?}" + ); +} + +#[tokio::test] +async fn completed_hook_with_output_flushes_immediately() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + chat.handle_codex_event(hook_started_event( + "pre-tool-use:0:/tmp/hooks.json:tool-call-1", + codex_protocol::protocol::HookEventName::PreToolUse, + Some("checking command"), + )); + reveal_running_hooks(&mut chat); + let running_snapshot = hook_live_and_history_snapshot(&chat, "running", ""); + + chat.handle_codex_event(hook_completed_event( + "pre-tool-use:0:/tmp/hooks.json:tool-call-1", + codex_protocol::protocol::HookEventName::PreToolUse, + codex_protocol::protocol::HookRunStatus::Blocked, + vec![codex_protocol::protocol::HookOutputEntry { + kind: codex_protocol::protocol::HookOutputEntryKind::Feedback, + text: "command blocked by policy".to_string(), + }], + )); + let history = drain_insert_history(&mut rx) + .iter() + .map(|lines| lines_to_single_string(lines)) + .collect::(); + let completed_snapshot = hook_live_and_history_snapshot(&chat, "completed", &history); + + assert_chatwidget_snapshot!( + "completed_hook_with_output_flushes_immediately_snapshot", + format!("{running_snapshot}\n\n{completed_snapshot}") + ); +} + +#[tokio::test] +async fn completed_hook_output_precedes_following_assistant_message() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + chat.handle_codex_event(hook_started_event( + "pre-tool-use:0:/tmp/hooks.json:tool-call-1", + codex_protocol::protocol::HookEventName::PreToolUse, + Some("checking command"), + )); + reveal_running_hooks(&mut chat); + + chat.handle_codex_event(hook_completed_event( + "pre-tool-use:0:/tmp/hooks.json:tool-call-1", + codex_protocol::protocol::HookEventName::PreToolUse, + codex_protocol::protocol::HookRunStatus::Blocked, + vec![codex_protocol::protocol::HookOutputEntry { + kind: codex_protocol::protocol::HookOutputEntryKind::Feedback, + text: "command blocked by policy".to_string(), + }], + )); + + complete_assistant_message( + &mut chat, + "msg-after-hook", + "The hook feedback was applied.", + /*phase*/ None, + ); + + let history = drain_insert_history(&mut rx) + .iter() + .map(|lines| lines_to_single_string(lines)) + .collect::(); + assert_chatwidget_snapshot!( + "completed_hook_output_precedes_following_assistant_message_snapshot", + format!( + "active hooks:\n{}history:\n{history}", + active_hook_blob(&chat) + ) + ); + let hook_index = history + .find("PreToolUse hook (blocked)") + .expect("hook feedback should be in history"); + let assistant_index = history + .find("The hook feedback was applied.") + .expect("assistant message should be in history"); + assert!( + hook_index < assistant_index, + "hook output should precede later assistant text: {history:?}" + ); +} + +#[tokio::test] +async fn completed_same_id_hook_output_survives_restart() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let hook_id = "stop:0:/tmp/hooks.json"; + + chat.handle_codex_event(hook_started_event( + hook_id, + codex_protocol::protocol::HookEventName::Stop, + Some("checking stop condition"), + )); + reveal_running_hooks(&mut chat); + chat.handle_codex_event(hook_completed_event( + hook_id, + codex_protocol::protocol::HookEventName::Stop, + codex_protocol::protocol::HookRunStatus::Stopped, + vec![codex_protocol::protocol::HookOutputEntry { + kind: codex_protocol::protocol::HookOutputEntryKind::Stop, + text: "continue with more context".to_string(), + }], + )); + chat.handle_codex_event(hook_started_event( + hook_id, + codex_protocol::protocol::HookEventName::Stop, + Some("checking stop condition"), + )); + reveal_running_hooks(&mut chat); + + let history = drain_insert_history(&mut rx) + .iter() + .map(|lines| lines_to_single_string(lines)) + .collect::(); + assert_chatwidget_snapshot!( + "completed_same_id_hook_output_survives_restart_snapshot", + format!( + "active hooks:\n{}history:\n{history}", + active_hook_blob(&chat) + ) + ); + assert!( + history.contains("Stop hook (stopped)\n stop: 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; + + for tool_call_id in ["tool-call-1", "tool-call-2", "tool-call-3"] { + chat.handle_codex_event(hook_started_event( + &format!("pre-tool-use:0:/tmp/hooks.json:{tool_call_id}"), + codex_protocol::protocol::HookEventName::PreToolUse, + Some("checking command policy"), + )); + } + 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.set_status_header("Thinking".to_string()); + chat.bottom_pane.ensure_status_indicator(); + + chat.handle_codex_event(hook_started_event( + "pre-tool-use:0:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::PreToolUse, + Some("checking command policy"), + )); + assert_eq!(chat.current_status.header, "Thinking"); + reveal_running_hooks(&mut chat); + let first_running_snapshot = hook_live_and_history_snapshot(&chat, "pre running", ""); + + chat.handle_codex_event(hook_started_event( + "post-tool-use:1:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::PostToolUse, + Some("checking output policy"), + )); + assert_eq!(chat.current_status.header, "Thinking"); + reveal_running_hooks(&mut chat); + let second_running_snapshot = hook_live_and_history_snapshot(&chat, "post running", ""); + + chat.handle_codex_event(hook_completed_event( + "pre-tool-use:0:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::PreToolUse, + codex_protocol::protocol::HookRunStatus::Completed, + Vec::new(), + )); + assert_eq!(chat.current_status.header, "Thinking"); + let older_completed_snapshot = + hook_live_and_history_snapshot(&chat, "pre completed lingering", ""); + expire_quiet_hook_linger(&mut chat); + let older_completed_expired_snapshot = + hook_live_and_history_snapshot(&chat, "pre completed after linger", ""); + + chat.handle_codex_event(hook_completed_event( + "post-tool-use:1:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::PostToolUse, + codex_protocol::protocol::HookRunStatus::Completed, + Vec::new(), + )); + assert_eq!(chat.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", ""); + expire_quiet_hook_linger(&mut chat); + let all_completed_snapshot = hook_live_and_history_snapshot(&chat, "all completed", ""); + 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}" + ) + ); +} + +#[tokio::test] +async fn running_hook_does_not_displace_active_exec_cell() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + let begin = begin_exec(&mut chat, "call-1", "echo done"); + let exec_running = active_blob(&chat); + + chat.handle_codex_event(hook_started_event( + "post-tool-use:0:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::PostToolUse, + Some("checking output policy"), + )); + reveal_running_hooks(&mut chat); + let exec_and_hook_running = format!( + "active exec:\n{}active hooks:\n{}", + active_blob(&chat), + active_hook_blob(&chat) + ); + + 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); + + chat.handle_codex_event(hook_completed_event( + "post-tool-use:0:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::PostToolUse, + codex_protocol::protocol::HookRunStatus::Completed, + Vec::new(), + )); + assert!(drain_insert_history(&mut rx).is_empty()); + let quiet_hook_completed_lingering = active_hook_blob(&chat); + expire_quiet_hook_linger(&mut chat); + let quiet_hook_completed = active_hook_blob(&chat); + + 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}" + ) + ); +} + +#[tokio::test] +async fn hidden_active_hook_does_not_add_transcript_separator() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + begin_exec(&mut chat, "call-1", "echo done"); + let exec_only_line_count = chat + .active_cell_transcript_lines(/*width*/ 80) + .expect("active exec transcript lines") + .len(); + + chat.handle_codex_event(hook_started_event( + "post-tool-use:0:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::PostToolUse, + Some("checking output policy"), + )); + let hidden_hook_transcript = chat + .active_cell_transcript_lines(/*width*/ 80) + .expect("active exec transcript lines"); + 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" + ); +} + +#[tokio::test] +async fn hook_completed_before_reveal_renders_completed_without_running_flash() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + chat.handle_codex_event(hook_started_event( + "session-start:0:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::SessionStart, + Some("warming the shell"), + )); + let started_hidden_snapshot = active_hook_blob(&chat); + + chat.handle_codex_event(hook_completed_event( + "session-start:0:/tmp/hooks.json", + codex_protocol::protocol::HookEventName::SessionStart, + codex_protocol::protocol::HookRunStatus::Completed, + vec![codex_protocol::protocol::HookOutputEntry { + kind: codex_protocol::protocol::HookOutputEntryKind::Context, + text: "session context".to_string(), + }], + )); + + 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}") + ); +} + #[tokio::test] async fn session_start_hook_events_render_snapshot() { assert_hook_events_snapshot( @@ -1424,6 +1878,79 @@ async fn session_start_hook_events_render_snapshot() { .await; } +fn hook_started_event( + id: &str, + event_name: codex_protocol::protocol::HookEventName, + status_message: Option<&str>, +) -> Event { + Event { + id: id.to_string(), + msg: EventMsg::HookStarted(codex_protocol::protocol::HookStartedEvent { + turn_id: None, + run: hook_run_summary( + id, + event_name, + codex_protocol::protocol::HookRunStatus::Running, + status_message, + Vec::new(), + ), + }), + } +} + +fn hook_completed_event( + id: &str, + event_name: codex_protocol::protocol::HookEventName, + status: codex_protocol::protocol::HookRunStatus, + entries: Vec, +) -> Event { + Event { + id: id.to_string(), + msg: EventMsg::HookCompleted(codex_protocol::protocol::HookCompletedEvent { + turn_id: None, + run: hook_run_summary( + id, event_name, status, /*status_message*/ None, entries, + ), + }), + } +} + +fn hook_run_summary( + id: &str, + event_name: codex_protocol::protocol::HookEventName, + status: codex_protocol::protocol::HookRunStatus, + status_message: Option<&str>, + entries: Vec, +) -> codex_protocol::protocol::HookRunSummary { + codex_protocol::protocol::HookRunSummary { + id: id.to_string(), + event_name, + handler_type: codex_protocol::protocol::HookHandlerType::Command, + execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, + scope: codex_protocol::protocol::HookScope::Turn, + source_path: PathBuf::from("/tmp/hooks.json"), + display_order: 0, + status, + status_message: status_message.map(str::to_string), + started_at: 1, + completed_at: (status != codex_protocol::protocol::HookRunStatus::Running).then_some(2), + duration_ms: (status != codex_protocol::protocol::HookRunStatus::Running).then_some(1), + entries, + } +} + +fn hook_live_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), + ) +} + // 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.rs b/codex-rs/tui/src/history_cell.rs index 67c7e9f98b57..67bde227d9c5 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -94,6 +94,12 @@ use tracing::error; use unicode_segmentation::UnicodeSegmentation; use unicode_width::UnicodeWidthStr; +mod hook_cell; + +pub(crate) use hook_cell::HookCell; +pub(crate) use hook_cell::new_active_hook_cell; +pub(crate) use hook_cell::new_completed_hook_cell; + /// Represents an event to display in the conversation history. Returns its /// `Vec>` representation to make it easier to display in a /// scrollable list. diff --git a/codex-rs/tui/src/history_cell/hook_cell.rs b/codex-rs/tui/src/history_cell/hook_cell.rs new file mode 100644 index 000000000000..bedf80cc45e9 --- /dev/null +++ b/codex-rs/tui/src/history_cell/hook_cell.rs @@ -0,0 +1,779 @@ +//! 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: +//! +//! 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. +use super::HistoryCell; +use crate::exec_cell::spinner; +use crate::render::renderable::Renderable; +use crate::shimmer::shimmer_spans; +use codex_protocol::protocol::HookEventName; +use codex_protocol::protocol::HookOutputEntry; +use codex_protocol::protocol::HookOutputEntryKind; +use codex_protocol::protocol::HookRunStatus; +use codex_protocol::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; + +#[derive(Debug)] +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. +const HOOK_RUN_REVEAL_DELAY: Duration = Duration::from_millis(300); + +/// Minimum time a quiet success remains on screen after becoming visible. +/// +/// 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. +const QUIET_HOOK_MIN_VISIBLE: Duration = Duration::from_millis(600); + +#[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. + state: HookRunState, +} + +#[derive(Debug)] +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, + }, + /// A completed run with output or a status worth preserving in history. + Completed { + /// Final protocol status for the hook invocation. + status: HookRunStatus, + /// Hook output entries rendered below the completed header. + entries: Vec, + }, +} + +#[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, + }; + 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, + }; + cell.add_completed_run(run); + cell + } + + pub(crate) fn is_empty(&self) -> bool { + self.runs.is_empty() + } + + /// Returns true while any run can still change due to an end event or timer. + pub(crate) fn is_active(&self) -> bool { + self.runs.iter().any(|run| run.state.is_active()) + } + + /// Completed hook cells are flushed out of the active slot once no timers remain. + pub(crate) fn should_flush(&self) -> bool { + !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, + /// blocked/stopped hooks, and hooks with emitted output become a persistent history cell. + pub(crate) fn take_completed_persistent_runs(&mut self) -> Option { + let mut completed = Vec::new(); + let mut remaining = Vec::new(); + for run in self.runs.drain(..) { + if run.state.has_persistent_output() { + completed.push(run); + } else { + remaining.push(run); + } + } + self.runs = remaining; + (!completed.is_empty()).then_some(Self { + runs: completed, + animations_enabled: self.animations_enabled, + }) + } + + /// Used by callers that need to know whether the active cell currently occupies viewport space. + pub(crate) fn has_visible_running_run(&self) -> bool { + self.runs.iter().any(|run| run.state.is_running_visible()) + } + + /// 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(); + let mut changed = false; + for run in &mut self.runs { + changed |= run.state.reveal_if_due(now); + } + self.runs.retain(|run| !run.state.quiet_linger_expired(now)); + changed || self.runs.len() != old_len + } + + /// Inserts or refreshes a started hook run. + /// + /// A duplicate begin event resets the reveal timer rather than adding a second row, because + /// matching by id is the invariant that keeps begin/end events paired. + 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), + }); + } + + /// Completes a run and returns whether the run was already present in this cell. + /// + /// Quiet successes intentionally avoid persistent output. If they were never visible, they + /// disappear immediately; if they had already drawn, they move into `QuietLinger`. + pub(crate) fn complete_run(&mut self, run: HookRunSummary) -> bool { + let Some(index) = self.runs.iter().position(|existing| existing.id == run.id) else { + return false; + }; + if hook_run_is_quiet_success(&run) { + if !self.runs[index] + .state + .complete_quiet_success(Instant::now()) + { + self.runs.remove(index); + } + 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 + } + + /// Adds a completed hook that did not pass through this live cell. + /// + /// This is used for replay/restoration paths where the final run summary is already known. + pub(crate) fn add_completed_run(&mut self, run: HookRunSummary) { + if hook_run_is_quiet_success(&run) { + return; + } + let HookRunSummary { + id, + event_name, + status_message, + status, + entries, + .. + } = run; + self.runs.push(HookRunCell { + id, + event_name, + status_message, + state: HookRunState::completed(status, entries), + }); + } + + pub(crate) fn next_timer_deadline(&self) -> Option { + self.runs + .iter() + .filter_map(|run| run.state.next_timer_deadline()) + .min() + } + + #[cfg(test)] + pub(crate) fn expire_quiet_runs_now_for_test(&mut self) { + for run in &mut self.runs { + run.expire_quiet_linger_now_for_test(); + } + } + + #[cfg(test)] + pub(crate) fn reveal_running_runs_now_for_test(&mut self) { + let now = Instant::now(); + for run in &mut self.runs { + run.reveal_running_now_for_test(now); + } + } + + #[cfg(test)] + pub(crate) fn reveal_running_runs_after_delayed_redraw_for_test(&mut self) { + let now = Instant::now(); + for run in &mut self.runs { + run.reveal_running_after_delayed_redraw_for_test(now); + } + } +} + +impl HistoryCell for HookCell { + /// Builds viewport lines while coalescing adjacent visible-running hooks. + fn display_lines(&self, _width: u16) -> 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); + 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 let Some(group) = + running_group.replace(RunningHookGroup::new(key, run.state.start_time())) + { + push_running_hook_group(&mut lines, &group, self.animations_enabled); + } + } + if let Some(group) = running_group { + push_running_hook_group(&mut lines, &group, self.animations_enabled); + } + lines + } + + /// Hook transcript output matches viewport output. + fn transcript_lines(&self, width: u16) -> Vec> { + self.display_lines(width) + } + + /// 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) + } +} + +impl HookRunCell { + #[cfg(test)] + fn expire_quiet_linger_now_for_test(&mut self) { + if let HookRunState::QuietLinger { + removal_deadline, .. + } = &mut self.state + { + *removal_deadline = Instant::now(); + } + } + + #[cfg(test)] + fn reveal_running_now_for_test(&mut self, now: Instant) { + if let HookRunState::PendingReveal { + reveal_deadline, .. + } = &mut self.state + { + *reveal_deadline = now; + } + } + + #[cfg(test)] + fn reveal_running_after_delayed_redraw_for_test(&mut self, now: Instant) { + if let HookRunState::PendingReveal { + reveal_deadline, .. + } = &mut self.state + { + let delayed_deadline = now + .checked_sub(QUIET_HOOK_MIN_VISIBLE + Duration::from_millis(100)) + .unwrap_or(now); + *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) { + 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 status_text = format!("{status:?}").to_lowercase(); + let bullet = hook_completed_bullet(*status, entries); + lines.push( + vec![ + bullet, + " ".into(), + format!("{label} hook ({status_text})").into(), + ] + .into(), + ); + for entry in entries { + // Output entries are already short hook-authored strings; keep their prefixes + // explicit so warnings/stops/errors remain easy to scan in history. + lines + .push(format!(" {}{}", hook_output_prefix(entry.kind), entry.text).into()); + } + } + HookRunState::PendingReveal { .. } => {} + } + } +} + +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, + } + } + + /// Creates the persistent final state for a hook with visible output or a notable status. + fn completed(status: HookRunStatus, entries: Vec) -> Self { + Self::Completed { status, entries } + } + + /// Returns true while the run is still waiting for a completion event or timer cleanup. + fn is_active(&self) -> bool { + match self { + HookRunState::PendingReveal { .. } + | HookRunState::VisibleRunning { .. } + | HookRunState::QuietLinger { .. } => true, + HookRunState::Completed { .. } => false, + } + } + + /// 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() + } + HookRunState::PendingReveal { .. } + | HookRunState::VisibleRunning { .. } + | HookRunState::QuietLinger { .. } => false, + } + } + + /// 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. + fn is_running_visible(&self) -> bool { + matches!( + self, + HookRunState::VisibleRunning { .. } | HookRunState::QuietLinger { .. } + ) + } + + /// Reveals a pending run once its deadline has passed. + /// + /// 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 { + return false; + }; + if now < *reveal_deadline { + return false; + } + *self = HookRunState::VisibleRunning { + start_time: *start_time, + visible_since: now, + }; + true + } + + /// Returns the next state-machine deadline owned by this run. + fn next_timer_deadline(&self) -> Option { + match self { + HookRunState::PendingReveal { + reveal_deadline, .. + } => Some(*reveal_deadline), + HookRunState::QuietLinger { + removal_deadline, .. + } => Some(*removal_deadline), + HookRunState::VisibleRunning { .. } | HookRunState::Completed { .. } => None, + } + } + + /// Returns true once a quiet success has lingered for long enough. + fn quiet_linger_expired(&self, now: Instant) -> bool { + match self { + HookRunState::QuietLinger { + removal_deadline, .. + } => now >= *removal_deadline, + HookRunState::PendingReveal { .. } + | HookRunState::VisibleRunning { .. } + | HookRunState::Completed { .. } => false, + } + } + + /// Converts a visible quiet success into a temporary linger state. + /// + /// 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 { + 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, + } + } +} + +/// 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![spinner(start_time, animations_enabled), " ".into()]; + if animations_enabled { + header.extend(shimmer_spans(hook_text)); + } else { + header.push(hook_text.to_string().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) +} + +/// 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() +} + +fn hook_completed_bullet(status: HookRunStatus, entries: &[HookOutputEntry]) -> Span<'static> { + match status { + HookRunStatus::Completed => { + if entries + .iter() + .any(|entry| entry.kind == HookOutputEntryKind::Warning) + { + "•".bold() + } else { + "•".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::PostToolUse => "PostToolUse", + HookEventName::SessionStart => "SessionStart", + HookEventName::UserPromptSubmit => "UserPromptSubmit", + HookEventName::Stop => "Stop", + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use ratatui::style::Modifier; + use std::path::PathBuf; + + #[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(), + }]; + + 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)); + } + + #[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); + } + + fn hook_run_summary(id: &str) -> HookRunSummary { + HookRunSummary { + id: id.to_string(), + event_name: HookEventName::PostToolUse, + handler_type: codex_protocol::protocol::HookHandlerType::Command, + execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, + scope: codex_protocol::protocol::HookScope::Turn, + source_path: PathBuf::from("/tmp/hooks.json"), + display_order: 0, + status: HookRunStatus::Running, + status_message: Some("checking output policy".to_string()), + started_at: 1, + completed_at: None, + duration_ms: None, + entries: Vec::new(), + } + } +}