From 6d747db7d8ae51a20aaf06529aab6563b0cb4168 Mon Sep 17 00:00:00 2001 From: Abhinav Date: Sat, 9 May 2026 17:17:30 -0400 Subject: [PATCH] Improve hooks trust flow in TUI (#21755) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Why Hooks that need trust review were easy to miss, and the existing TUI flow made users discover `/hooks` manually before they could decide whether to inspect or trust them. # What - add a startup review prompt for new or changed hooks before normal composer use - add a top-level `t` shortcut in `/hooks` to trust every review-needed hook at once - make pending-review rows and helper copy use warning styling ## TUI ### Startup review interstitial ```text Hooks need review 2 hooks are new or changed. Hooks can run outside the sandbox after you trust them. › 1. Review hooks 2. Trust all and continue 3. Continue without trusting (hooks won't run) ``` ### Top-level `/hooks` page when review is needed ```text Hooks Lifecycle hooks from config and enabled plugins. ⚠ 1 hook needs review before it can run. Event Installed Active Review Description PreToolUse 1 0 1 Before a tool executes ... Press t to trust all; enter to review hooks; esc to close ``` --- codex-rs/tui/src/app.rs | 9 +- codex-rs/tui/src/app/background_requests.rs | 104 +---- codex-rs/tui/src/app/event_dispatch.rs | 3 + codex-rs/tui/src/app/startup_prompts.rs | 10 - codex-rs/tui/src/app/tests.rs | 11 - codex-rs/tui/src/app_event.rs | 5 + .../tui/src/bottom_pane/hooks_browser_view.rs | 363 +++++++++++++---- codex-rs/tui/src/bottom_pane/mod.rs | 3 +- ...oks_browser_events_with_review_column.snap | 4 +- codex-rs/tui/src/chatwidget/hooks.rs | 25 +- codex-rs/tui/src/hooks_rpc.rs | 100 +++++ codex-rs/tui/src/lib.rs | 13 +- ..._hooks_needing_review_startup_warning.snap | 5 - ...w__tests__startup_hooks_review_prompt.snap | 14 + ..._hooks_review_prompt_with_trust_error.snap | 15 + codex-rs/tui/src/startup_hooks_review.rs | 371 ++++++++++++++++++ 16 files changed, 846 insertions(+), 209 deletions(-) create mode 100644 codex-rs/tui/src/hooks_rpc.rs delete mode 100644 codex-rs/tui/src/snapshots/codex_tui__app__tests__hooks_needing_review_startup_warning.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__startup_hooks_review__tests__startup_hooks_review_prompt.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__startup_hooks_review__tests__startup_hooks_review_prompt_with_trust_error.snap create mode 100644 codex-rs/tui/src/startup_hooks_review.rs diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index dff288493..9b738a619 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -40,6 +40,7 @@ use crate::history_cell; use crate::history_cell::HistoryCell; #[cfg(not(debug_assertions))] use crate::history_cell::UpdateAvailableHistoryCell; +use crate::hooks_rpc::HookTrustUpdate; use crate::key_hint::KeyBindingListExt; use crate::keymap::RuntimeKeymap; use crate::legacy_core::config::Config; @@ -91,8 +92,7 @@ use codex_app_server_protocol::ConfigWriteResponse; use codex_app_server_protocol::FeedbackUploadParams; use codex_app_server_protocol::FeedbackUploadResponse; use codex_app_server_protocol::GetAccountRateLimitsResponse; -use codex_app_server_protocol::HooksListParams; -use codex_app_server_protocol::HooksListResponse; +use codex_app_server_protocol::HooksListEntry; use codex_app_server_protocol::ListMcpServerStatusParams; use codex_app_server_protocol::ListMcpServerStatusResponse; #[cfg(test)] @@ -618,6 +618,7 @@ impl App { remote_app_server_auth_token: Option, state_db: Option, environment_manager: Arc, + startup_hooks_browser: Option, ) -> Result { use tokio_stream::StreamExt; let (app_event_tx, mut app_event_rx) = unbounded_channel(); @@ -914,6 +915,9 @@ See the Codex keymap documentation for supported actions and examples." pending_plugin_enabled_writes: HashMap::new(), pending_hook_enabled_writes: HashMap::new(), }; + if let Some(entry) = startup_hooks_browser { + app.chat_widget.open_hooks_browser(entry); + } if let Some(started) = initial_started_thread { let thread_id = started.session.thread_id; app.enqueue_primary_thread_session(started.session, started.turns) @@ -957,7 +961,6 @@ See the Codex keymap documentation for supported actions and examples." tui.frame_requester().schedule_frame(); app.refresh_startup_skills(&app_server); - app.refresh_startup_hooks(&app_server); // Kick off a non-blocking rate-limit prefetch so the first `/status` // already has data, without delaying the initial frame render. if requires_openai_auth && has_chatgpt_account { diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index 83819349b..57c82cb7f 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -5,7 +5,6 @@ //! the main event loop remains single-threaded. use super::*; -use codex_app_server_protocol::HookTrustStatus; use codex_app_server_protocol::MarketplaceAddParams; use codex_app_server_protocol::MarketplaceAddResponse; use codex_app_server_protocol::MarketplaceRemoveParams; @@ -15,6 +14,9 @@ use codex_app_server_protocol::MarketplaceUpgradeResponse; use codex_app_server_protocol::RequestId; +use crate::hooks_rpc::fetch_hooks_list; +use crate::hooks_rpc::write_hook_trust; +use crate::hooks_rpc::write_hook_trusts; use codex_utils_absolute_path::AbsolutePathBuf; impl App { @@ -89,47 +91,6 @@ impl App { }); } - /// Emits the initial hook review warning without delaying the first interactive frame. - pub(super) fn refresh_startup_hooks(&mut self, app_server: &AppServerSession) { - let request_handle = app_server.request_handle(); - let app_event_tx = self.app_event_tx.clone(); - let cwd = self.config.cwd.to_path_buf(); - tokio::spawn(async move { - let result = fetch_hooks_list(request_handle, cwd.clone()).await; - let response = match result { - Ok(response) => response, - Err(err) => { - tracing::warn!("failed to load startup hook review state: {err:#}"); - return; - } - }; - let hooks_needing_review = response - .data - .into_iter() - .find(|entry| entry.cwd.as_path() == cwd.as_path()) - .map(|entry| { - entry - .hooks - .into_iter() - .filter(|hook| { - matches!( - hook.trust_status, - HookTrustStatus::Untrusted | HookTrustStatus::Modified - ) - }) - .count() - }) - .unwrap_or_default(); - if let Some(message) = - startup_prompts::hooks_needing_review_warning(hooks_needing_review) - { - app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( - history_cell::new_warning_event(message), - ))); - } - }); - } - pub(super) fn fetch_plugins_list(&mut self, app_server: &AppServerSession, cwd: PathBuf) { let request_handle = app_server.request_handle(); let app_event_tx = self.app_event_tx.clone(); @@ -381,6 +342,22 @@ impl App { }); } + pub(super) fn trust_hooks( + &mut self, + app_server: &AppServerSession, + updates: Vec, + ) { + let request_handle = app_server.request_handle(); + let app_event_tx = self.app_event_tx.clone(); + tokio::spawn(async move { + let result = write_hook_trusts(request_handle, updates) + .await + .map(|_| ()) + .map_err(|err| format!("Failed to trust hooks: {err}")); + app_event_tx.send(AppEvent::HookTrusted { result }); + }); + } + pub(super) fn refresh_plugin_mentions(&mut self) { let config = self.config.clone(); let app_event_tx = self.app_event_tx.clone(); @@ -674,20 +651,6 @@ pub(super) async fn fetch_plugins_list( Ok(response) } -pub(super) async fn fetch_hooks_list( - request_handle: AppServerRequestHandle, - cwd: PathBuf, -) -> Result { - let request_id = RequestId::String(format!("hooks-list-{}", Uuid::new_v4())); - request_handle - .request_typed(ClientRequest::HooksList { - request_id, - params: HooksListParams { cwds: vec![cwd] }, - }) - .await - .wrap_err("hooks/list failed in TUI") -} - const CLI_HIDDEN_PLUGIN_MARKETPLACES: &[&str] = &["openai-bundled"]; pub(super) fn hide_cli_only_plugin_marketplaces(response: &mut PluginListResponse) { @@ -864,35 +827,6 @@ pub(super) async fn write_hook_enabled( .wrap_err("config/batchWrite failed while updating hook enablement in TUI") } -pub(super) async fn write_hook_trust( - request_handle: AppServerRequestHandle, - key: String, - current_hash: String, -) -> Result { - let request_id = RequestId::String(format!("hooks-config-write-{}", Uuid::new_v4())); - let value = serde_json::json!({ - key: { - "trusted_hash": current_hash, - } - }); - request_handle - .request_typed(ClientRequest::ConfigBatchWrite { - request_id, - params: ConfigBatchWriteParams { - edits: vec![codex_app_server_protocol::ConfigEdit { - key_path: "hooks.state".to_string(), - value, - merge_strategy: MergeStrategy::Upsert, - }], - file_path: None, - expected_version: None, - reload_user_config: true, - }, - }) - .await - .wrap_err("config/batchWrite failed while updating hook trust in TUI") -} - pub(super) fn build_feedback_upload_params( origin_thread_id: Option, rollout_path: Option, diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index b2261f155..565ee6ba1 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1713,6 +1713,9 @@ impl App { AppEvent::TrustHook { key, current_hash } => { self.trust_hook(app_server, key, current_hash); } + AppEvent::TrustHooks { updates } => { + self.trust_hooks(app_server, updates); + } AppEvent::HookEnabledSet { key, enabled, diff --git a/codex-rs/tui/src/app/startup_prompts.rs b/codex-rs/tui/src/app/startup_prompts.rs index 482c75b3f..41972e675 100644 --- a/codex-rs/tui/src/app/startup_prompts.rs +++ b/codex-rs/tui/src/app/startup_prompts.rs @@ -77,16 +77,6 @@ pub(super) fn emit_system_bwrap_warning(app_event_tx: &AppEventSender, config: & ))); } -pub(super) fn hooks_needing_review_warning(count: usize) -> Option { - match count { - 0 => None, - 1 => Some("1 hook needs review before it can run. Open /hooks to review it.".to_string()), - count => Some(format!( - "{count} hooks need review before they can run. Open /hooks to review them." - )), - } -} - pub(super) fn should_show_model_migration_prompt( current_model: &str, target_model: &str, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index b74bfab8f..300b85209 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -301,17 +301,6 @@ async fn ignore_same_thread_resume_allows_reattaching_displayed_inactive_thread( assert!(app.transcript_cells.is_empty()); } -#[test] -fn hooks_needing_review_startup_warning_snapshot() { - let message = startup_prompts::hooks_needing_review_warning(/*count*/ 2) - .expect("review-needed hooks should produce a startup warning"); - let rendered = lines_to_single_string( - &history_cell::new_warning_event(message).display_lines(/*width*/ 80), - ); - - assert_app_snapshot!("hooks_needing_review_startup_warning", rendered); -} - #[tokio::test] async fn enqueue_primary_thread_session_replays_buffered_approval_after_attach() -> Result<()> { let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index b72e90929..664ddf12d 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -775,6 +775,11 @@ pub(crate) enum AppEvent { current_hash: String, }, + /// Trust the current definitions for one or more hooks by stable hook key. + TrustHooks { + updates: Vec, + }, + /// Result of persisting hook enabled state. HookEnabledSet { key: String, diff --git a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs index 2ce5b3304..c9c7a8c61 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -1,8 +1,8 @@ -use codex_app_server_protocol::HookErrorInfo; use codex_app_server_protocol::HookEventName; use codex_app_server_protocol::HookMetadata; use codex_app_server_protocol::HookSource; use codex_app_server_protocol::HookTrustStatus; +use codex_app_server_protocol::HooksListEntry; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; @@ -26,6 +26,8 @@ use super::scroll_state::ScrollState; use super::selection_popup_common::render_menu_surface; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; +use crate::hooks_rpc::HookTrustUpdate; +use crate::hooks_rpc::hook_needs_review; use crate::key_hint; use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::render::renderable::Renderable; @@ -43,9 +45,7 @@ enum HooksBrowserPage { } pub(crate) struct HooksBrowserView { - hooks: Vec, - warnings: Vec, - errors: Vec, + entry: HooksListEntry, page: HooksBrowserPage, state: ScrollState, complete: bool, @@ -53,17 +53,28 @@ pub(crate) struct HooksBrowserView { } impl HooksBrowserView { + #[cfg(test)] pub(crate) fn new( - mut hooks: Vec, + hooks: Vec, warnings: Vec, - errors: Vec, + errors: Vec, app_event_tx: AppEventSender, ) -> Self { - hooks.sort_by_key(|hook| hook.display_order); + Self::from_entry( + HooksListEntry { + cwd: std::path::PathBuf::new(), + hooks, + warnings, + errors, + }, + app_event_tx, + ) + } + + pub(crate) fn from_entry(mut entry: HooksListEntry, app_event_tx: AppEventSender) -> Self { + entry.hooks.sort_by_key(|hook| hook.display_order); let mut view = Self { - hooks, - warnings, - errors, + entry, page: HooksBrowserPage::Events, state: ScrollState::new(), complete: false, @@ -85,16 +96,19 @@ impl HooksBrowserView { .map(|event_name| { let event_name: HookEventName = event_name.into(); let installed = self + .entry .hooks .iter() .filter(|hook| hook.event_name == event_name) .count(); let active = self + .entry .hooks .iter() .filter(|hook| hook.event_name == event_name && hook_is_active(hook)) .count(); let needs_review = self + .entry .hooks .iter() .filter(|hook| hook.event_name == event_name && hook_needs_review(hook)) @@ -110,7 +124,8 @@ impl HooksBrowserView { } fn handlers_for_event(&self, event_name: HookEventName) -> impl Iterator { - self.hooks + self.entry + .hooks .iter() .filter(move |hook| hook.event_name == event_name) } @@ -124,7 +139,8 @@ impl HooksBrowserView { fn selected_hook_index(&self, event_name: HookEventName) -> Option { let selected_visible_idx = self.state.selected_idx?; - self.hooks + self.entry + .hooks .iter() .enumerate() .filter(|(_, hook)| hook.event_name == event_name) @@ -134,7 +150,7 @@ impl HooksBrowserView { fn selected_hook(&self, event_name: HookEventName) -> Option<&HookMetadata> { self.selected_hook_index(event_name) - .and_then(|idx| self.hooks.get(idx)) + .and_then(|idx| self.entry.hooks.get(idx)) } fn move_up(&mut self) { @@ -175,7 +191,7 @@ impl HooksBrowserView { let Some(idx) = self.selected_hook_index(event_name) else { return; }; - let Some(hook) = self.hooks.get_mut(idx) else { + let Some(hook) = self.entry.hooks.get_mut(idx) else { return; }; if hook.is_managed { @@ -196,7 +212,7 @@ impl HooksBrowserView { let Some(idx) = self.selected_hook_index(event_name) else { return; }; - let Some(hook) = self.hooks.get_mut(idx) else { + let Some(hook) = self.entry.hooks.get_mut(idx) else { return; }; if !hook_needs_review(hook) { @@ -210,6 +226,24 @@ impl HooksBrowserView { }); } + fn trust_all_hooks(&mut self) { + let mut updates = Vec::new(); + for hook in &mut self.entry.hooks { + if !hook_needs_review(hook) { + continue; + } + + hook.trust_status = HookTrustStatus::Trusted; + updates.push(HookTrustUpdate { + key: hook.key.clone(), + current_hash: hook.current_hash.clone(), + }); + } + if !updates.is_empty() { + self.app_event_tx.send(AppEvent::TrustHooks { updates }); + } + } + fn close(&mut self) { self.complete = true; } @@ -238,23 +272,27 @@ impl HooksBrowserView { ] } + fn review_needed_total_count(&self) -> usize { + self.entry + .hooks + .iter() + .filter(|hook| hook_needs_review(hook)) + .count() + } + + #[allow(clippy::disallowed_methods)] fn handler_header_lines( event_name: HookEventName, review_needed_count: usize, ) -> Vec> { let mut lines = vec![format!("{} hooks", event_label(event_name)).bold().into()]; - match review_needed_count { - 0 => lines.push( + match review_needed_message(review_needed_count) { + None => lines.push( "Turn hooks on or off. Your changes are saved automatically." .dim() .into(), ), - 1 => lines.push("1 hook needs review before it can run.".dim().into()), - count => lines.push( - format!("{count} hooks need review before they can run.") - .dim() - .into(), - ), + Some(message) => lines.push(message.yellow().into()), } lines } @@ -265,6 +303,7 @@ impl HooksBrowserView { .count() } + #[allow(clippy::disallowed_methods)] fn event_table_lines(&self) -> Vec> { let rows = self.event_rows(); let show_review = rows.iter().any(|row| row.needs_review > 0); @@ -280,59 +319,59 @@ impl HooksBrowserView { header.push("Description".into()); lines.push(Line::from(header)); for (idx, row) in rows.into_iter().enumerate() { - if self.state.selected_idx == Some(idx) { - let style = accent_style(); - let mut row_line = vec![ - Span::from(format!( - "{: 0; + let mut row_line = vec![ + Span::from(format!( + "{: Vec> { let mut lines = Vec::new(); - if self.warnings.is_empty() && self.errors.is_empty() { + if self.entry.warnings.is_empty() && self.entry.errors.is_empty() { return lines; } lines.push("Issues".bold().into()); lines.extend( - self.warnings + self.entry + .warnings .iter() .map(|warning| format!("⚠ {warning}").into()), ); - lines.extend(self.errors.iter().map(|error| { + lines.extend(self.entry.errors.iter().map(|error| { format!("■ {}: {}", error.path.display(), error.message) .red() .into() @@ -340,10 +379,16 @@ impl HooksBrowserView { lines } + #[allow(clippy::disallowed_methods)] fn event_page_lines(&self) -> Vec> { let mut lines = Self::event_header_lines(); lines.push(Line::default()); + if let Some(message) = review_needed_message(self.review_needed_total_count()) { + lines.push(format!("⚠ {message}").yellow().into()); + lines.push(Line::default()); + } + let issue_lines = self.event_issue_lines(); if !issue_lines.is_empty() { lines.extend(issue_lines); @@ -354,6 +399,7 @@ impl HooksBrowserView { lines } + #[allow(clippy::disallowed_methods)] fn handler_row_lines(&self, event_name: HookEventName, width: usize) -> Vec> { self.handlers_for_event(event_name) .enumerate() @@ -376,11 +422,17 @@ impl HooksBrowserView { }; let mut line = Line::from(row); line = truncate_line_with_ellipsis_if_overflow(line, width); - if hook.is_managed { - line = line.dim(); - } + let needs_review = hook_needs_review(hook); if self.state.selected_idx == Some(idx) { - line = line.patch_style(accent_style()); + if needs_review { + line = line.yellow().bold(); + } else { + line = line.patch_style(accent_style()); + } + } else if needs_review { + line = line.yellow(); + } else if hook.is_managed { + line = line.dim(); } line }) @@ -423,6 +475,15 @@ impl HooksBrowserView { height: area.height, }; let footer = match self.page { + HooksBrowserPage::Events if self.review_needed_total_count() > 0 => Line::from(vec![ + "Press ".into(), + key_hint::plain(KeyCode::Char('t')).into(), + " to trust all; ".into(), + key_hint::plain(KeyCode::Enter).into(), + " to review hooks; ".into(), + key_hint::plain(KeyCode::Esc).into(), + " to close".into(), + ]), HooksBrowserPage::Events => Line::from(vec![ "Press ".into(), key_hint::plain(KeyCode::Enter).into(), @@ -516,11 +577,10 @@ impl BottomPaneView for HooksBrowserView { code: KeyCode::Char('t'), modifiers: KeyModifiers::NONE, .. - } => { - if let HooksBrowserPage::Handlers(event_name) = self.page { - self.trust_selected_hook(event_name); - } - } + } => match self.page { + HooksBrowserPage::Events => self.trust_all_hooks(), + HooksBrowserPage::Handlers(event_name) => self.trust_selected_hook(event_name), + }, KeyEvent { code: KeyCode::Esc, .. } => match self.page { @@ -631,6 +691,14 @@ fn hook_is_active(hook: &HookMetadata) -> bool { ) } +fn review_needed_message(count: usize) -> Option { + match count { + 0 => None, + 1 => Some("1 hook needs review before it can run.".to_string()), + count => Some(format!("{count} hooks need review before they can run.")), + } +} + struct EventRow { event_name: HookEventName, installed: usize, @@ -638,13 +706,6 @@ struct EventRow { needs_review: usize, } -fn hook_needs_review(hook: &HookMetadata) -> bool { - matches!( - hook.trust_status, - HookTrustStatus::Untrusted | HookTrustStatus::Modified - ) -} - fn hook_trust_label(status: HookTrustStatus) -> &'static str { match status { HookTrustStatus::Managed => "Managed", @@ -784,6 +845,7 @@ mod tests { use crate::test_support::PathBufExt; use crate::test_support::test_path_buf; use crate::test_support::test_path_display; + use codex_app_server_protocol::HookErrorInfo; use codex_app_server_protocol::HookEventName; use codex_app_server_protocol::HookHandlerType; use codex_app_server_protocol::HookMetadata; @@ -794,6 +856,7 @@ mod tests { use insta::assert_snapshot; use ratatui::buffer::Buffer; use ratatui::layout::Rect; + use ratatui::style::Color; use ratatui::style::Modifier; use tokio::sync::mpsc::unbounded_channel; @@ -959,6 +1022,16 @@ mod tests { "hooks_browser_events_with_review_column", render_lines(&view, /*width*/ 112) ); + assert_eq!( + view.event_table_lines()[1].spans[3].style.fg, + Some(Color::Cyan) + ); + assert!( + view.event_table_lines()[1].spans[3] + .style + .add_modifier + .contains(ratatui::style::Modifier::BOLD) + ); } #[test] @@ -1015,6 +1088,62 @@ mod tests { ); } + #[test] + fn review_needed_handler_rows_use_warning_color() { + let (tx_raw, _rx) = unbounded_channel::(); + let mut untrusted_hook = hook( + "path:untrusted", + HookEventName::PreToolUse, + HookSource::User, + /*plugin_id*/ None, + "~/bin/untrusted.sh", + /*enabled*/ false, + /*is_managed*/ false, + /*display_order*/ 1, + ); + untrusted_hook.trust_status = HookTrustStatus::Untrusted; + let mut view = HooksBrowserView::new( + vec![ + hook( + "path:trusted", + HookEventName::PreToolUse, + HookSource::User, + /*plugin_id*/ None, + "~/bin/trusted.sh", + /*enabled*/ true, + /*is_managed*/ false, + /*display_order*/ 0, + ), + untrusted_hook, + ], + Vec::new(), + Vec::new(), + AppEventSender::new(tx_raw), + ); + view.handle_key_event(KeyEvent::from(KeyCode::Enter)); + + assert_eq!( + view.handler_row_lines(HookEventName::PreToolUse, /*width*/ 112)[1] + .style + .fg, + Some(Color::Yellow) + ); + } + + #[test] + fn review_needed_handler_header_uses_warning_color() { + assert_eq!( + HooksBrowserView::handler_header_lines( + HookEventName::PreToolUse, + /*review_needed_count*/ 1, + )[1] + .spans[0] + .style + .fg, + Some(Color::Yellow) + ); + } + #[test] fn renders_managed_handler_without_toggle_hint() { let mut view = view(); @@ -1373,7 +1502,7 @@ mod tests { view.handle_key_event(KeyEvent::from(KeyCode::Enter)); view.handle_key_event(KeyEvent::from(KeyCode::Char('t'))); - let hook = view.hooks.first().expect("trusted hook"); + let hook = view.entry.hooks.first().expect("trusted hook"); assert!(!hook.enabled); assert_eq!(hook.trust_status, HookTrustStatus::Trusted); match rx.try_recv().expect("trust event") { @@ -1388,6 +1517,84 @@ mod tests { } } + #[test] + fn trust_key_on_event_page_trusts_all_review_needed_hooks() { + let (tx_raw, mut rx) = unbounded_channel::(); + let mut untrusted_hook = hook( + "path:untrusted", + HookEventName::PreToolUse, + HookSource::User, + /*plugin_id*/ None, + "/tmp/pre-tool-use-check.sh", + /*enabled*/ false, + /*is_managed*/ false, + /*display_order*/ 0, + ); + untrusted_hook.trust_status = HookTrustStatus::Untrusted; + let mut modified_hook = hook( + "path:modified", + HookEventName::Stop, + HookSource::User, + /*plugin_id*/ None, + "/tmp/stop-check.sh", + /*enabled*/ false, + /*is_managed*/ false, + /*display_order*/ 1, + ); + modified_hook.trust_status = HookTrustStatus::Modified; + let mut view = HooksBrowserView::new( + vec![ + untrusted_hook, + modified_hook, + hook( + "path:trusted", + HookEventName::PreToolUse, + HookSource::User, + /*plugin_id*/ None, + "/tmp/trusted.sh", + /*enabled*/ true, + /*is_managed*/ false, + /*display_order*/ 2, + ), + ], + Vec::new(), + Vec::new(), + AppEventSender::new(tx_raw), + ); + + view.handle_key_event(KeyEvent::from(KeyCode::Char('t'))); + + assert_eq!( + view.entry + .hooks + .iter() + .map(|hook| hook.trust_status) + .collect::>(), + vec![ + HookTrustStatus::Trusted, + HookTrustStatus::Trusted, + HookTrustStatus::Trusted, + ] + ); + match rx.try_recv().expect("trust event") { + AppEvent::TrustHooks { updates } => assert_eq!( + updates, + vec![ + HookTrustUpdate { + key: "path:untrusted".to_string(), + current_hash: "sha256:current".to_string(), + }, + HookTrustUpdate { + key: "path:modified".to_string(), + current_hash: "sha256:current".to_string(), + }, + ] + ), + other => panic!("expected hook trust event, got {other:?}"), + } + assert!(rx.try_recv().is_err()); + } + #[test] fn escape_returns_to_the_selected_event() { let mut view = view(); diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 5746816b2..d28a272cf 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -31,7 +31,7 @@ use crate::render::renderable::Renderable; use crate::render::renderable::RenderableItem; use crate::tui::FrameRequester; pub(crate) use bottom_pane_view::BottomPaneView; -use bottom_pane_view::ViewCompletion; +pub(crate) use bottom_pane_view::ViewCompletion; use codex_app_server_protocol::ToolRequestUserInputParams; use codex_core_skills::model::SkillMetadata; use codex_features::Features; @@ -104,7 +104,6 @@ pub(crate) use footer::GoalStatusIndicator; #[cfg(test)] pub(crate) use footer::goal_status_indicator_line; pub(crate) use list_selection_view::ColumnWidthMode; -#[cfg(test)] pub(crate) use list_selection_view::ListSelectionView; pub(crate) use list_selection_view::SelectionRowDisplay; pub(crate) use list_selection_view::SelectionToggle; diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_review_column.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_review_column.snap index d6768118c..07a39ebc9 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_review_column.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_review_column.snap @@ -6,6 +6,8 @@ expression: "render_lines(&view, 112)" Hooks Lifecycle hooks from config and enabled plugins. + ⚠ 1 hook needs review before it can run. + Event Installed Active Review Description PreToolUse 1 0 1 Before a tool executes PermissionRequest 0 0 0 When permission is requested @@ -16,4 +18,4 @@ expression: "render_lines(&view, 112)" UserPromptSubmit 0 0 0 When the user submits a prompt Stop 0 0 0 Right before Codex ends its turn - Press enter to view hooks; esc to close + Press t to trust all; enter to review hooks; esc to close diff --git a/codex-rs/tui/src/chatwidget/hooks.rs b/codex-rs/tui/src/chatwidget/hooks.rs index 5e2748d3a..f8c52f74e 100644 --- a/codex-rs/tui/src/chatwidget/hooks.rs +++ b/codex-rs/tui/src/chatwidget/hooks.rs @@ -3,6 +3,8 @@ use std::path::PathBuf; use super::ChatWidget; use crate::app_event::AppEvent; use crate::bottom_pane::HooksBrowserView; +use crate::hooks_rpc::hooks_list_entry_for_cwd; +use codex_app_server_protocol::HooksListEntry; use codex_app_server_protocol::HooksListResponse; impl ChatWidget { @@ -23,21 +25,18 @@ impl ChatWidget { match result { Ok(response) => { - let (hooks, warnings, errors) = response - .data - .into_iter() - .find(|entry| entry.cwd.as_path() == cwd.as_path()) - .map(|entry| (entry.hooks, entry.warnings, entry.errors)) - .unwrap_or_default(); - self.bottom_pane.show_view(Box::new(HooksBrowserView::new( - hooks, - warnings, - errors, - self.app_event_tx.clone(), - ))); - self.request_redraw(); + self.open_hooks_browser(hooks_list_entry_for_cwd(response, &cwd)); } Err(err) => self.add_error_message(format!("Failed to load hooks: {err}")), } } + + pub(crate) fn open_hooks_browser(&mut self, entry: HooksListEntry) { + self.bottom_pane + .show_view(Box::new(HooksBrowserView::from_entry( + entry, + self.app_event_tx.clone(), + ))); + self.request_redraw(); + } } diff --git a/codex-rs/tui/src/hooks_rpc.rs b/codex-rs/tui/src/hooks_rpc.rs new file mode 100644 index 000000000..52be4da0e --- /dev/null +++ b/codex-rs/tui/src/hooks_rpc.rs @@ -0,0 +1,100 @@ +use codex_app_server_client::AppServerRequestHandle; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::ConfigBatchWriteParams; +use codex_app_server_protocol::ConfigWriteResponse; +use codex_app_server_protocol::HookMetadata; +use codex_app_server_protocol::HookTrustStatus; +use codex_app_server_protocol::HooksListEntry; +use codex_app_server_protocol::HooksListParams; +use codex_app_server_protocol::HooksListResponse; +use codex_app_server_protocol::MergeStrategy; +use codex_app_server_protocol::RequestId; +use color_eyre::eyre::Result; +use color_eyre::eyre::WrapErr; +use std::path::Path; +use std::path::PathBuf; +use uuid::Uuid; + +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct HookTrustUpdate { + pub(crate) key: String, + pub(crate) current_hash: String, +} + +pub(crate) async fn fetch_hooks_list( + request_handle: AppServerRequestHandle, + cwd: PathBuf, +) -> Result { + let request_id = RequestId::String(format!("hooks-list-{}", Uuid::new_v4())); + request_handle + .request_typed(ClientRequest::HooksList { + request_id, + params: HooksListParams { cwds: vec![cwd] }, + }) + .await + .wrap_err("hooks/list failed in TUI") +} + +pub(crate) fn hooks_list_entry_for_cwd(response: HooksListResponse, cwd: &Path) -> HooksListEntry { + response + .data + .into_iter() + .find(|entry| entry.cwd.as_path() == cwd) + .unwrap_or_else(|| HooksListEntry { + cwd: cwd.to_path_buf(), + hooks: Vec::new(), + warnings: Vec::new(), + errors: Vec::new(), + }) +} + +pub(crate) fn hook_needs_review(hook: &HookMetadata) -> bool { + matches!( + hook.trust_status, + HookTrustStatus::Untrusted | HookTrustStatus::Modified + ) +} + +pub(crate) async fn write_hook_trusts( + request_handle: AppServerRequestHandle, + trust_updates: Vec, +) -> Result { + let request_id = RequestId::String(format!("hooks-config-write-{}", Uuid::new_v4())); + let value = serde_json::Value::Object( + trust_updates + .into_iter() + .map(|update| { + ( + update.key, + serde_json::json!({ + "trusted_hash": update.current_hash, + }), + ) + }) + .collect(), + ); + request_handle + .request_typed(ClientRequest::ConfigBatchWrite { + request_id, + params: ConfigBatchWriteParams { + edits: vec![codex_app_server_protocol::ConfigEdit { + key_path: "hooks.state".to_string(), + value, + merge_strategy: MergeStrategy::Upsert, + }], + file_path: None, + expected_version: None, + reload_user_config: true, + }, + }) + .await + .wrap_err("config/batchWrite failed while updating hook trust in TUI") +} + +pub(crate) async fn write_hook_trust( + request_handle: AppServerRequestHandle, + key: String, + current_hash: String, +) -> Result { + write_hook_trusts(request_handle, vec![HookTrustUpdate { key, current_hash }]).await +} diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index ac5b489af..0e507dd78 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -126,6 +126,7 @@ mod frames; mod get_git_diff; mod goal_display; mod history_cell; +mod hooks_rpc; mod ide_context; pub(crate) mod insert_history; pub use insert_history::insert_history_lines; @@ -162,6 +163,7 @@ mod session_state; mod shimmer; mod skills_helpers; mod slash_command; +mod startup_hooks_review; mod status; mod status_indicator_widget; mod streaming; @@ -256,6 +258,8 @@ pub(crate) mod test_support; use crate::onboarding::onboarding_screen::OnboardingScreenArgs; use crate::onboarding::onboarding_screen::run_onboarding_app; +use crate::startup_hooks_review::StartupHooksReviewOutcome; +use crate::startup_hooks_review::maybe_run_startup_hooks_review; use crate::tui::Tui; pub use cli::Cli; use codex_arg0::Arg0DispatchPaths; @@ -1494,7 +1498,7 @@ async fn run_ratatui_app( let use_alt_screen = determine_alt_screen_mode(no_alt_screen, config.tui_alternate_screen); tui.set_alt_screen_enabled(use_alt_screen); - let app_server = match app_server { + let mut app_server = match app_server { Some(app_server) => app_server, None => match start_app_server( &app_server_target, @@ -1520,6 +1524,12 @@ async fn run_ratatui_app( }, }; + let startup_hooks_browser = + match maybe_run_startup_hooks_review(&mut app_server, &mut tui, &config).await? { + StartupHooksReviewOutcome::Continue => None, + StartupHooksReviewOutcome::OpenHooksBrowser(data) => Some(data), + }; + let app_result = App::run( &mut tui, app_server, @@ -1538,6 +1548,7 @@ async fn run_ratatui_app( remote_auth_token, state_db, environment_manager, + startup_hooks_browser, ) .await; diff --git a/codex-rs/tui/src/snapshots/codex_tui__app__tests__hooks_needing_review_startup_warning.snap b/codex-rs/tui/src/snapshots/codex_tui__app__tests__hooks_needing_review_startup_warning.snap deleted file mode 100644 index f044b95e8..000000000 --- a/codex-rs/tui/src/snapshots/codex_tui__app__tests__hooks_needing_review_startup_warning.snap +++ /dev/null @@ -1,5 +0,0 @@ ---- -source: tui/src/app/tests.rs -expression: rendered ---- -⚠ 2 hooks need review before they can run. Open /hooks to review them. diff --git a/codex-rs/tui/src/snapshots/codex_tui__startup_hooks_review__tests__startup_hooks_review_prompt.snap b/codex-rs/tui/src/snapshots/codex_tui__startup_hooks_review__tests__startup_hooks_review_prompt.snap new file mode 100644 index 000000000..038534e98 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__startup_hooks_review__tests__startup_hooks_review_prompt.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/startup_hooks_review.rs +expression: "render_lines(&view, 80)" +--- + + Hooks need review + 2 hooks are new or changed. + Hooks can run outside the sandbox after you trust them. + +› 1. Review hooks + 2. Trust all and continue + 3. Continue without trusting (hooks won't run) + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/snapshots/codex_tui__startup_hooks_review__tests__startup_hooks_review_prompt_with_trust_error.snap b/codex-rs/tui/src/snapshots/codex_tui__startup_hooks_review__tests__startup_hooks_review_prompt_with_trust_error.snap new file mode 100644 index 000000000..340c0d233 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__startup_hooks_review__tests__startup_hooks_review_prompt_with_trust_error.snap @@ -0,0 +1,15 @@ +--- +source: tui/src/startup_hooks_review.rs +expression: "render_lines(&view, 80)" +--- + + Hooks need review + 2 hooks are new or changed. + Hooks can run outside the sandbox after you trust them. + Failed to trust hooks: disk full + +› 1. Review hooks + 2. Trust all and continue + 3. Continue without trusting (hooks won't run) + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/startup_hooks_review.rs b/codex-rs/tui/src/startup_hooks_review.rs new file mode 100644 index 000000000..7c1252489 --- /dev/null +++ b/codex-rs/tui/src/startup_hooks_review.rs @@ -0,0 +1,371 @@ +use color_eyre::eyre::Result; +use crossterm::event::KeyEventKind; +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use ratatui::style::Stylize; +use ratatui::text::Line; +use ratatui::widgets::Clear; +use ratatui::widgets::WidgetRef; +use tokio::sync::mpsc::unbounded_channel; +use tokio_stream::StreamExt; + +use crate::app_event::AppEvent; +use crate::app_event_sender::AppEventSender; +use crate::app_server_session::AppServerSession; +use crate::bottom_pane::BottomPaneView; +use crate::bottom_pane::ListSelectionView; +use crate::bottom_pane::SelectionItem; +use crate::bottom_pane::SelectionViewParams; +use crate::bottom_pane::popup_consts::standard_popup_hint_line_for_keymap; +use crate::hooks_rpc::HookTrustUpdate; +use crate::hooks_rpc::fetch_hooks_list; +use crate::hooks_rpc::hook_needs_review; +use crate::hooks_rpc::hooks_list_entry_for_cwd; +use crate::hooks_rpc::write_hook_trusts; +use crate::keymap::RuntimeKeymap; +use crate::legacy_core::config::Config; +use crate::render::renderable::ColumnRenderable; +use crate::render::renderable::Renderable; +use crate::tui::Tui; +use crate::tui::TuiEvent; +use codex_app_server_protocol::HooksListEntry; + +pub(crate) enum StartupHooksReviewOutcome { + Continue, + OpenHooksBrowser(HooksListEntry), +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum StartupHooksReviewSelection { + ReviewHooks, + TrustAllAndContinue, + ContinueWithoutTrusting, +} + +pub(crate) async fn maybe_run_startup_hooks_review( + app_server: &mut AppServerSession, + tui: &mut Tui, + config: &Config, +) -> Result { + let cwd = config.cwd.to_path_buf(); + let response = match fetch_hooks_list(app_server.request_handle(), cwd.clone()).await { + Ok(response) => response, + Err(err) => { + tracing::warn!("failed to load startup hook review state: {err:#}"); + return Ok(StartupHooksReviewOutcome::Continue); + } + }; + let entry = hooks_list_entry_for_cwd(response, &cwd); + if review_needed_count(&entry) == 0 { + return Ok(StartupHooksReviewOutcome::Continue); + } + + run_startup_hooks_review_app(app_server, tui, config, entry).await +} + +async fn run_startup_hooks_review_app( + app_server: &mut AppServerSession, + tui: &mut Tui, + config: &Config, + entry: HooksListEntry, +) -> Result { + let keymap = RuntimeKeymap::from_config(&config.tui_keymap) + .map_err(|err| color_eyre::eyre::eyre!(err))?; + let (tx_raw, _rx) = unbounded_channel::(); + let app_event_tx = AppEventSender::new(tx_raw); + let mut trust_all_error = None; + let mut view = selection_view( + &entry, + trust_all_error.as_deref(), + /*trusting_all*/ false, + app_event_tx.clone(), + &keymap, + ); + draw_view(tui, &view)?; + + let tui_events = tui.event_stream(); + tokio::pin!(tui_events); + + loop { + let Some(event) = tui_events.next().await else { + return Ok(StartupHooksReviewOutcome::Continue); + }; + match event { + TuiEvent::Key(key_event) => { + if matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat) { + view.handle_key_event(key_event); + } + let Some(selection) = selected_choice(&mut view) else { + draw_view(tui, &view)?; + continue; + }; + match selection { + StartupHooksReviewSelection::ReviewHooks => { + return Ok(StartupHooksReviewOutcome::OpenHooksBrowser(entry)); + } + StartupHooksReviewSelection::ContinueWithoutTrusting => { + return Ok(StartupHooksReviewOutcome::Continue); + } + StartupHooksReviewSelection::TrustAllAndContinue => { + view = selection_view( + &entry, + trust_all_error.as_deref(), + /*trusting_all*/ true, + app_event_tx.clone(), + &keymap, + ); + draw_view(tui, &view)?; + let result = write_hook_trusts( + app_server.request_handle(), + entry + .hooks + .iter() + .filter(|hook| hook_needs_review(hook)) + .map(|hook| HookTrustUpdate { + key: hook.key.clone(), + current_hash: hook.current_hash.clone(), + }) + .collect(), + ) + .await + .map(|_| ()) + .map_err(|err| format!("Failed to trust hooks: {err}")); + match result { + Ok(()) => return Ok(StartupHooksReviewOutcome::Continue), + Err(err) => { + trust_all_error = Some(err); + view = selection_view( + &entry, + trust_all_error.as_deref(), + /*trusting_all*/ false, + app_event_tx.clone(), + &keymap, + ); + draw_view(tui, &view)?; + } + } + } + } + } + TuiEvent::Paste(_) => {} + TuiEvent::Draw | TuiEvent::Resize => draw_view(tui, &view)?, + } + } +} + +fn selected_choice(view: &mut ListSelectionView) -> Option { + if !view.is_complete() { + return None; + } + match view.take_last_selected_index() { + Some(0) => Some(StartupHooksReviewSelection::ReviewHooks), + Some(1) => Some(StartupHooksReviewSelection::TrustAllAndContinue), + Some(2) | None => Some(StartupHooksReviewSelection::ContinueWithoutTrusting), + Some(_) => None, + } +} + +fn selection_view( + entry: &HooksListEntry, + trust_all_error: Option<&str>, + trusting_all: bool, + app_event_tx: AppEventSender, + keymap: &RuntimeKeymap, +) -> ListSelectionView { + ListSelectionView::new( + selection_view_params(entry, trust_all_error, trusting_all, keymap), + app_event_tx, + keymap.list.clone(), + ) +} + +#[allow(clippy::disallowed_methods)] +fn selection_view_params( + entry: &HooksListEntry, + trust_all_error: Option<&str>, + trusting_all: bool, + keymap: &RuntimeKeymap, +) -> SelectionViewParams { + let count = review_needed_count(entry); + let count_line = match count { + 1 => "1 hook is new or changed.".to_string(), + count => format!("{count} hooks are new or changed."), + }; + let mut header = ColumnRenderable::new(); + header.push(Line::from("Hooks need review".bold())); + header.push(Line::from(count_line).yellow()); + header.push(Line::from( + "Hooks can run outside the sandbox after you trust them.".dim(), + )); + if let Some(error) = trust_all_error { + header.push(Line::from(error.to_string()).red()); + } else if trusting_all { + header.push(Line::from("Trusting hooks...".dim())); + } + + SelectionViewParams { + footer_hint: Some(standard_popup_hint_line_for_keymap(&keymap.list)), + items: vec![ + selection_item("Review hooks", trusting_all), + selection_item("Trust all and continue", trusting_all), + selection_item("Continue without trusting (hooks won't run)", trusting_all), + ], + header: Box::new(header), + ..Default::default() + } +} + +fn review_needed_count(entry: &HooksListEntry) -> usize { + entry + .hooks + .iter() + .filter(|hook| hook_needs_review(hook)) + .count() +} + +fn selection_item(name: &str, is_disabled: bool) -> SelectionItem { + SelectionItem { + name: name.to_string(), + dismiss_on_select: true, + is_disabled, + ..Default::default() + } +} + +fn draw_view(tui: &mut Tui, view: &ListSelectionView) -> Result<()> { + tui.draw(u16::MAX, |frame| { + let area = frame.area(); + frame.render_widget_ref(Clear, area); + let view_area = Rect::new( + area.x, + area.y, + area.width, + view.desired_height(area.width).min(area.height), + ); + frame.render_widget_ref(&StandaloneSelectionView { view }, view_area); + })?; + Ok(()) +} + +struct StandaloneSelectionView<'a> { + view: &'a ListSelectionView, +} + +impl WidgetRef for &StandaloneSelectionView<'_> { + fn render_ref(&self, area: Rect, buf: &mut Buffer) { + self.view.render(area, buf); + } +} + +#[cfg(test)] +mod tests { + use super::selection_view; + use crate::app_event::AppEvent; + use crate::app_event_sender::AppEventSender; + use crate::keymap::RuntimeKeymap; + use crate::render::renderable::Renderable; + use crate::test_support::PathBufExt; + use crate::test_support::test_path_buf; + use codex_app_server_protocol::HookEventName; + use codex_app_server_protocol::HookHandlerType; + use codex_app_server_protocol::HookMetadata; + use codex_app_server_protocol::HookSource; + use codex_app_server_protocol::HookTrustStatus; + use codex_app_server_protocol::HooksListEntry; + use insta::assert_snapshot; + use ratatui::buffer::Buffer; + use ratatui::layout::Rect; + use tokio::sync::mpsc::unbounded_channel; + + fn hook(key: &str, trust_status: HookTrustStatus) -> HookMetadata { + HookMetadata { + key: key.to_string(), + event_name: HookEventName::PreToolUse, + handler_type: HookHandlerType::Command, + is_managed: false, + matcher: Some("Bash".to_string()), + command: Some("/tmp/hook.sh".to_string()), + timeout_sec: 30, + status_message: None, + source_path: test_path_buf("/tmp/hooks.json").abs(), + source: HookSource::User, + plugin_id: None, + display_order: 0, + enabled: false, + current_hash: format!("sha256:{key}"), + trust_status, + } + } + + fn entry() -> HooksListEntry { + HooksListEntry { + cwd: test_path_buf("/tmp"), + hooks: vec![ + hook("path:new", HookTrustStatus::Untrusted), + hook("path:changed", HookTrustStatus::Modified), + ], + warnings: Vec::new(), + errors: Vec::new(), + } + } + + fn render_lines(view: &crate::bottom_pane::ListSelectionView, width: u16) -> String { + let height = view.desired_height(width); + let area = Rect::new(0, 0, width, height); + let mut buf = Buffer::empty(area); + view.render(area, &mut buf); + + (0..area.height) + .map(|row| { + let rendered = (0..area.width) + .map(|col| { + let symbol = buf[(area.x + col, area.y + row)].symbol(); + if symbol.is_empty() { + " ".to_string() + } else { + symbol.to_string() + } + }) + .collect::(); + format!("{rendered:width$}", width = area.width as usize) + }) + .collect::>() + .join("\n") + } + + #[test] + fn renders_prompt() { + let (tx_raw, _rx) = unbounded_channel::(); + let keymap = RuntimeKeymap::defaults(); + let view = selection_view( + &entry(), + /*trust_all_error*/ None, + /*trusting_all*/ false, + AppEventSender::new(tx_raw), + &keymap, + ); + + assert_snapshot!( + "startup_hooks_review_prompt", + render_lines(&view, /*width*/ 80) + ); + } + + #[test] + fn renders_prompt_with_trust_error() { + let (tx_raw, _rx) = unbounded_channel::(); + let keymap = RuntimeKeymap::defaults(); + let view = selection_view( + &entry(), + Some("Failed to trust hooks: disk full"), + /*trusting_all*/ false, + AppEventSender::new(tx_raw), + &keymap, + ); + + assert_snapshot!( + "startup_hooks_review_prompt_with_trust_error", + render_lines(&view, /*width*/ 80) + ); + } +}