From b8dbc57d6f41cec2c3c489a2ba6195f9de287aa3 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Thu, 30 Apr 2026 14:10:58 -0700 Subject: [PATCH 1/8] Add hook trust review flow to TUI --- codex-rs/tui/src/app.rs | 1 + codex-rs/tui/src/app/background_requests.rs | 99 +++++++++ codex-rs/tui/src/app/event_dispatch.rs | 12 ++ codex-rs/tui/src/app/startup_prompts.rs | 10 + codex-rs/tui/src/app/tests.rs | 10 + codex-rs/tui/src/app_event.rs | 12 ++ .../tui/src/bottom_pane/hooks_browser_view.rs | 197 +++++++++++++++++- ..._hooks_browser_capped_command_details.snap | 3 +- ...__tests__hooks_browser_empty_handlers.snap | 2 +- ...ser_view__tests__hooks_browser_events.snap | 14 +- ...sts__hooks_browser_events_with_issues.snap | 14 +- ...r_view__tests__hooks_browser_handlers.snap | 3 +- ..._tests__hooks_browser_managed_handler.snap | 3 +- ...__hooks_browser_review_needed_handler.snap | 18 ++ ...ests__hooks_browser_scrolled_handlers.snap | 3 +- ...ooks_browser_selected_managed_handler.snap | 3 +- ...s__hooks_popup_shows_list_diagnostics.snap | 14 +- ..._hooks_needing_review_startup_warning.snap | 5 + 18 files changed, 390 insertions(+), 33 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__app__tests__hooks_needing_review_startup_warning.snap diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 1f1f2d70865c..e80ada43c233 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -926,6 +926,7 @@ 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 36155fb33985..c66a40e7f7bc 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -5,6 +5,7 @@ //! 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; @@ -88,6 +89,47 @@ 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::Changed + ) + }) + .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(); @@ -322,6 +364,24 @@ impl App { }); } + pub(super) fn trust_hook( + &mut self, + app_server: &AppServerSession, + key: String, + trusted_hash: String, + enable: bool, + ) { + let request_handle = app_server.request_handle(); + let app_event_tx = self.app_event_tx.clone(); + tokio::spawn(async move { + let result = write_hook_trust(request_handle, key, trusted_hash, enable) + .await + .map(|_| ()) + .map_err(|err| format!("Failed to trust hook: {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(); @@ -805,6 +865,45 @@ 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, + trusted_hash: String, + enable: bool, +) -> Result { + let request_id = RequestId::String(format!("hooks-config-write-{}", Uuid::new_v4())); + let value = if enable { + serde_json::json!({ + key: { + "enabled": true, + "trusted_hash": trusted_hash, + } + }) + } else { + serde_json::json!({ + key: { + "trusted_hash": trusted_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 6bdc413725ff..6b658fe819cf 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1692,6 +1692,13 @@ impl App { AppEvent::SetHookEnabled { key, enabled } => { self.set_hook_enabled(app_server, key, enabled); } + AppEvent::TrustHook { + key, + trusted_hash, + enable, + } => { + self.trust_hook(app_server, key, trusted_hash, enable); + } AppEvent::HookEnabledSet { key, enabled, @@ -1716,6 +1723,11 @@ impl App { } } } + AppEvent::HookTrusted { result } => { + if let Err(err) = result { + self.chat_widget.add_error_message(err); + } + } AppEvent::OpenPermissionsPopup => { self.chat_widget.open_permissions_popup(); } diff --git a/codex-rs/tui/src/app/startup_prompts.rs b/codex-rs/tui/src/app/startup_prompts.rs index 41972e6751ab..482c75b3fad4 100644 --- a/codex-rs/tui/src/app/startup_prompts.rs +++ b/codex-rs/tui/src/app/startup_prompts.rs @@ -77,6 +77,16 @@ 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 84500e3edfe0..9a81db67665e 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -263,6 +263,16 @@ 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(2) + .expect("review-needed hooks should produce a startup warning"); + let rendered = + lines_to_single_string(&history_cell::new_warning_event(message).display_lines(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 5e45bf38e0ad..46a62bd0d09d 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -749,6 +749,13 @@ pub(crate) enum AppEvent { enabled: bool, }, + /// Trust the current definition for a hook by stable hook key. + TrustHook { + key: String, + trusted_hash: String, + enable: bool, + }, + /// Result of persisting hook enabled state. HookEnabledSet { key: String, @@ -756,6 +763,11 @@ pub(crate) enum AppEvent { result: Result<(), String>, }, + /// Result of persisting hook trust state. + HookTrusted { + result: Result<(), String>, + }, + /// Notify that the manage skills popup was closed. ManageSkillsClosed, 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 78fca8e745c5..22abd331e4e9 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -68,7 +68,12 @@ impl HooksBrowserView { app_event_tx, }; if view.page_len() > 0 { - view.state.selected_idx = Some(0); + view.state.selected_idx = Some( + view.event_rows() + .iter() + .position(|row| row.needs_review > 0) + .unwrap_or(0), + ); } view } @@ -87,10 +92,16 @@ impl HooksBrowserView { .iter() .filter(|hook| hook.event_name == event_name && hook_is_active(hook)) .count(); + let needs_review = self + .hooks + .iter() + .filter(|hook| hook.event_name == event_name && hook_needs_review(hook)) + .count(); EventRow { event_name, installed, active, + needs_review, } }) .collect() @@ -168,6 +179,9 @@ impl HooksBrowserView { if hook.is_managed { return; } + if hook_needs_review(hook) { + return; + } hook.enabled = !hook.enabled; self.app_event_tx.send(AppEvent::SetHookEnabled { @@ -176,6 +190,29 @@ impl HooksBrowserView { }); } + fn trust_selected_hook(&mut self, event_name: HookEventName) { + let Some(idx) = self.selected_hook_index(event_name) else { + return; + }; + let Some(hook) = self.hooks.get_mut(idx) else { + return; + }; + if !hook_needs_review(hook) { + return; + } + + let enable = !hook.enabled; + hook.trust_status = HookTrustStatus::Trusted; + if enable { + hook.enabled = true; + } + self.app_event_tx.send(AppEvent::TrustHook { + key: hook.key.clone(), + current_hash: hook.current_hash.clone(), + enable, + }); + } + fn close(&mut self) { self.complete = true; } @@ -207,7 +244,7 @@ impl HooksBrowserView { fn handler_header_lines(event_name: HookEventName) -> Vec> { vec![ format!("{} hooks", event_label(event_name)).bold().into(), - "Turn hooks on or off. Your changes are saved automatically." + "Review new or changed hooks, then turn them on or off." .dim() .into(), ] @@ -219,6 +256,7 @@ impl HooksBrowserView { format!("{: { + format!("[{marker}] {} · modified", hook_title(idx)) + } + HookTrustStatus::Untrusted => format!("[{marker}] {} · new", hook_title(idx)), + HookTrustStatus::Managed | HookTrustStatus::Trusted => { + format!("[{marker}] {}", hook_title(idx)) + } + }; let mut line = Line::from(row); line = truncate_line_with_ellipsis_if_overflow(line, width); if hook.is_managed { @@ -330,6 +386,7 @@ impl HooksBrowserView { Some(MAX_COMMAND_DETAIL_LINES), )); lines.push(detail_line("Timeout", &format!("{}s", hook.timeout_sec))); + lines.push(detail_line("Trust", hook_trust_label(hook.trust_status))); lines } @@ -362,6 +419,14 @@ impl HooksBrowserView { key_hint::plain(KeyCode::Esc).into(), " to go back".into(), ]) + } else if selected_hook.is_some_and(hook_needs_review) { + Line::from(vec![ + "Press ".into(), + key_hint::plain(KeyCode::Char('t')).into(), + " to trust; ".into(), + key_hint::plain(KeyCode::Esc).into(), + " to go back".into(), + ]) } else { Line::from(vec![ "Press ".into(), @@ -422,6 +487,15 @@ impl BottomPaneView for HooksBrowserView { self.toggle_selected_hook(event_name); } } + KeyEvent { + code: KeyCode::Char('t'), + modifiers: KeyModifiers::NONE, + .. + } => { + if let HooksBrowserPage::Handlers(event_name) = self.page { + self.trust_selected_hook(event_name); + } + } KeyEvent { code: KeyCode::Esc, .. } => match self.page { @@ -532,6 +606,23 @@ struct EventRow { event_name: HookEventName, installed: usize, active: usize, + 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", + HookTrustStatus::Trusted => "Trusted", + HookTrustStatus::Untrusted => "New hook - review required", + HookTrustStatus::Modified => "Modified since last trusted - review required", + } } fn event_label(event_name: HookEventName) -> &'static str { @@ -987,14 +1078,14 @@ mod tests { } #[test] - fn untrusted_enabled_hooks_do_not_count_as_active() { + fn review_needed_hooks_are_not_active() { let (tx_raw, _rx) = unbounded_channel::(); let mut untrusted_hook = hook( "path:untrusted", HookEventName::PreToolUse, HookSource::User, /*plugin_id*/ None, - "~/bin/untrusted.sh", + "/tmp/pre-tool-use-check.sh", /*enabled*/ true, /*is_managed*/ false, /*display_order*/ 0, @@ -1015,6 +1106,62 @@ mod tests { assert_eq!(pre_tool_use.installed, 1); assert_eq!(pre_tool_use.active, 0); + assert_eq!(pre_tool_use.needs_review, 1); + } + + #[test] + fn review_needed_event_is_selected_by_default() { + let (tx_raw, _rx) = unbounded_channel::(); + let mut untrusted_hook = hook( + "path:untrusted", + HookEventName::PermissionRequest, + HookSource::User, + /*plugin_id*/ None, + "/tmp/permission-request-check.sh", + /*enabled*/ false, + /*is_managed*/ false, + /*display_order*/ 0, + ); + untrusted_hook.trust_status = HookTrustStatus::Untrusted; + let view = HooksBrowserView::new( + vec![untrusted_hook], + Vec::new(), + Vec::new(), + AppEventSender::new(tx_raw), + ); + + assert_eq!( + view.selected_event(), + Some(HookEventName::PermissionRequest) + ); + } + + #[test] + fn renders_review_needed_handler() { + let (tx_raw, _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 view = HooksBrowserView::new( + vec![untrusted_hook], + Vec::new(), + Vec::new(), + AppEventSender::new(tx_raw), + ); + view.handle_key_event(KeyEvent::from(KeyCode::Enter)); + + assert_snapshot!( + "hooks_browser_review_needed_handler", + render_lines(&view, /*width*/ 112) + ); } fn assert_unmanaged_toggle_key(key_code: KeyCode) { @@ -1077,6 +1224,44 @@ mod tests { assert!(rx.try_recv().is_err()); } + #[test] + fn trust_key_trusts_review_needed_handler() { + 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 current_hash = untrusted_hook.current_hash.clone(); + let mut view = HooksBrowserView::new( + vec![untrusted_hook], + Vec::new(), + Vec::new(), + AppEventSender::new(tx_raw), + ); + view.handle_key_event(KeyEvent::from(KeyCode::Enter)); + view.handle_key_event(KeyEvent::from(KeyCode::Char('t'))); + + match rx.try_recv().expect("trust event") { + AppEvent::TrustHook { + key, + current_hash: trusted_hash, + enable, + } => { + assert_eq!(key, "path:untrusted"); + assert_eq!(trusted_hash, current_hash); + assert!(enable); + } + other => panic!("expected hook trust event, got {other:?}"), + } + } + #[test] fn escape_returns_to_the_selected_event() { let mut view = view(); diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap index 7af93e3c5a8a..f6efb4995a3b 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap @@ -4,7 +4,7 @@ expression: "render_lines(&view, 44)" --- PreToolUse hooks - Turn hooks on or off. Your changes are s + Review new or changed hooks, then turn t [x] Hook 1 @@ -15,5 +15,6 @@ expression: "render_lines(&view, 44)" seven eight nine ten eleven twelve thirteen fourteen… Timeout 30s + Trust Trusted Press space or enter to toggle; esc to go diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap index 33321eec2be3..753ee2331de3 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap @@ -4,7 +4,7 @@ expression: "render_lines(&view, 112)" --- PermissionRequest hooks - Turn hooks on or off. Your changes are saved automatically. + Review new or changed hooks, then turn them on or off. No hooks installed for this event. diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events.snap index 522105c30d9f..096fb562f876 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events.snap @@ -6,12 +6,12 @@ expression: "render_lines(&view, 112)" Hooks Lifecycle hooks from config and enabled plugins. - Event Installed Active Description - PreToolUse 2 1 Before a tool executes - PermissionRequest 1 1 When permission is requested - PostToolUse 0 0 After a tool executes - SessionStart 0 0 When a new session starts - UserPromptSubmit 0 0 When the user submits a prompt - Stop 0 0 Right before Codex ends its turn + Event Installed Active Review Description + PreToolUse 2 1 0 Before a tool executes + PermissionRequest 1 1 0 When permission is requested + PostToolUse 0 0 0 After a tool executes + SessionStart 0 0 0 When a new session starts + 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 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_issues.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_issues.snap index 18e3b9f849ab..8d10909d377e 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_issues.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_issues.snap @@ -10,12 +10,12 @@ expression: "render_lines(&view, 112)" ⚠ skipped invalid matcher for PreToolUse ■ /tmp/hooks.json: failed to parse hooks config - Event Installed Active Description - PreToolUse 0 0 Before a tool executes - PermissionRequest 0 0 When permission is requested - PostToolUse 0 0 After a tool executes - SessionStart 0 0 When a new session starts - UserPromptSubmit 0 0 When the user submits a prompt - Stop 0 0 Right before Codex ends its turn + Event Installed Active Review Description + PreToolUse 0 0 0 Before a tool executes + PermissionRequest 0 0 0 When permission is requested + PostToolUse 0 0 0 After a tool executes + SessionStart 0 0 0 When a new session starts + 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 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap index c44f4b866a39..7e5fcf627ff5 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap @@ -4,7 +4,7 @@ expression: "render_lines(&view, 112)" --- PreToolUse hooks - Turn hooks on or off. Your changes are saved automatically. + Review new or changed hooks, then turn them on or off. [x] Hook 1 [ ] Hook 2 @@ -14,5 +14,6 @@ expression: "render_lines(&view, 112)" Source Plugin - superpowers@openai-curated Command ${CODEX_PLUGIN_ROOT}/hooks/pre-tool-use-check.sh Timeout 30s + Trust Trusted Press space or enter to toggle; esc to go back diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap index 21c59065f5dd..48bbd9cd0156 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap @@ -4,7 +4,7 @@ expression: "render_lines(&view, 112)" --- PermissionRequest hooks - Turn hooks on or off. Your changes are saved automatically. + Review new or changed hooks, then turn them on or off. [x] Hook 1 @@ -13,5 +13,6 @@ expression: "render_lines(&view, 112)" Source Admin config Command /enterprise/hooks/permission-check.sh Timeout 30s + Trust Managed Managed hooks are always on; press esc to go back diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap new file mode 100644 index 000000000000..1b7c45701b3f --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap @@ -0,0 +1,18 @@ +--- +source: tui/src/bottom_pane/hooks_browser_view.rs +expression: "render_lines(&view, 112)" +--- + + PreToolUse hooks + Review new or changed hooks, then turn them on or off. + + [!] Hook 1 · new + + Event PreToolUse + Matcher Bash + Source User config - /tmp/hooks.json + Command /tmp/pre-tool-use-check.sh + Timeout 30s + Trust New hook - review required + + Press t to trust; esc to go back diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap index 4f4a4377c6a4..fe896ea59cb0 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap @@ -4,7 +4,7 @@ expression: "render_lines(&view, 112)" --- PreToolUse hooks - Turn hooks on or off. Your changes are saved automatically. + Review new or changed hooks, then turn them on or off. [x] Hook 2 [x] Hook 3 @@ -20,5 +20,6 @@ expression: "render_lines(&view, 112)" Source User config - /tmp/hooks.json Command /tmp/hook-8.sh Timeout 30s + Trust Trusted Press space or enter to toggle; esc to go back diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap index 9a53b95d6d29..0c5a7ccf7ad6 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap @@ -4,7 +4,7 @@ expression: "render_lines(&view, 112)" --- PreToolUse hooks - Turn hooks on or off. Your changes are saved automatically. + Review new or changed hooks, then turn them on or off. [x] Hook 1 [x] Hook 2 @@ -14,5 +14,6 @@ expression: "render_lines(&view, 112)" Source Admin config Command /enterprise/hooks/pre-tool-use-2.sh Timeout 30s + Trust Managed Managed hooks are always on; press esc to go back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hooks_popup_shows_list_diagnostics.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hooks_popup_shows_list_diagnostics.snap index 865d19031fbd..1db52419a1a0 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hooks_popup_shows_list_diagnostics.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hooks_popup_shows_list_diagnostics.snap @@ -9,12 +9,12 @@ expression: popup ⚠ skipped invalid matcher for PreToolUse ■ /tmp/hooks.json: failed to parse hooks config - Event Installed Active Description - PreToolUse 0 0 Before a tool executes - PermissionRequest 0 0 When permission is requested - PostToolUse 0 0 After a tool executes - SessionStart 0 0 When a new session starts - UserPromptSubmit 0 0 When the user submits a prompt - Stop 0 0 Right before Codex ends its turn + Event Installed Active Review Description + PreToolUse 0 0 0 Before a tool executes + PermissionRequest 0 0 0 When permission is requested + PostToolUse 0 0 0 After a tool executes + SessionStart 0 0 0 When a new session starts + 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 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 new file mode 100644 index 000000000000..f044b95e8645 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__app__tests__hooks_needing_review_startup_warning.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/app/tests.rs +expression: rendered +--- +⚠ 2 hooks need review before they can run. Open /hooks to review them. From 1370c133b56dcd8eef91715775328eca43a2d696 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Thu, 30 Apr 2026 15:23:35 -0700 Subject: [PATCH 2/8] Fix argument comment lint in TUI test --- codex-rs/tui/src/app/tests.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 9a81db67665e..9f421d6558e0 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -265,10 +265,11 @@ async fn ignore_same_thread_resume_allows_reattaching_displayed_inactive_thread( #[test] fn hooks_needing_review_startup_warning_snapshot() { - let message = startup_prompts::hooks_needing_review_warning(2) + 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(80)); + 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); } From 20fdb433991214f4668749d7e6118210d6b819e5 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Thu, 30 Apr 2026 15:30:33 -0700 Subject: [PATCH 3/8] Address hook browser review feedback --- .../tui/src/bottom_pane/hooks_browser_view.rs | 111 ++++++++++++++---- ..._hooks_browser_capped_command_details.snap | 1 - ...__tests__hooks_browser_empty_handlers.snap | 1 - ...ser_view__tests__hooks_browser_events.snap | 14 +-- ...sts__hooks_browser_events_with_issues.snap | 14 +-- ...oks_browser_events_with_review_column.snap | 17 +++ ...r_view__tests__hooks_browser_handlers.snap | 1 - ..._tests__hooks_browser_managed_handler.snap | 1 - ...__hooks_browser_review_needed_handler.snap | 2 +- ...ests__hooks_browser_scrolled_handlers.snap | 1 - ...ooks_browser_selected_managed_handler.snap | 1 - ...s__hooks_popup_shows_list_diagnostics.snap | 14 +-- 12 files changed, 125 insertions(+), 53 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_review_column.snap 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 22abd331e4e9..26e762e3b52b 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -241,27 +241,46 @@ impl HooksBrowserView { ] } - fn handler_header_lines(event_name: HookEventName) -> Vec> { - vec![ - format!("{} hooks", event_label(event_name)).bold().into(), - "Review new or changed hooks, then turn them on or off." - .dim() - .into(), - ] + 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 => {} + 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(), + ), + } + lines + } + + fn review_needed_count(&self, event_name: HookEventName) -> usize { + self.handlers_for_event(event_name) + .filter(|hook| hook_needs_review(hook)) + .count() } fn event_table_lines(&self) -> Vec> { + let rows = self.event_rows(); + let show_review = rows.iter().any(|row| row.needs_review > 0); let mut lines = Vec::new(); - lines.push(Line::from(vec![ + let mut header = vec![ format!("{: self.event_page_lines().len(), HooksBrowserPage::Handlers(event_name) => { let row_count = self.handler_row_lines(event_name, content_width).len(); + let header_line_count = + Self::handler_header_lines(event_name, self.review_needed_count(event_name)) + .len(); if row_count == 0 { - Self::handler_header_lines(event_name).len() + 2 + header_line_count + 2 } else { let visible_row_count = row_count.min(MAX_POPUP_ROWS); - Self::handler_header_lines(event_name).len() + header_line_count + 1 + visible_row_count + 1 @@ -554,7 +586,8 @@ impl Renderable for HooksBrowserView { let lines = match self.page { HooksBrowserPage::Events => self.event_page_lines(), HooksBrowserPage::Handlers(event_name) => { - let mut lines = Self::handler_header_lines(event_name); + let mut lines = + Self::handler_header_lines(event_name, self.review_needed_count(event_name)); let rows = self.handler_row_lines(event_name, width); if rows.is_empty() { lines.push(Line::default()); @@ -872,6 +905,34 @@ mod tests { assert_snapshot!("hooks_browser_events", render_lines(&view, /*width*/ 112)); } + #[test] + fn renders_event_browser_with_review_column_when_needed() { + let (tx_raw, _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.trusted_hash = None; + untrusted_hook.trust_status = HookTrustStatus::Untrusted; + let view = HooksBrowserView::new( + vec![untrusted_hook], + Vec::new(), + Vec::new(), + AppEventSender::new(tx_raw), + ); + + assert_snapshot!( + "hooks_browser_events_with_review_column", + render_lines(&view, /*width*/ 112) + ); + } + #[test] fn renders_event_browser_with_issues() { let (tx_raw, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap index f6efb4995a3b..a747e50f2bfb 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap @@ -4,7 +4,6 @@ expression: "render_lines(&view, 44)" --- PreToolUse hooks - Review new or changed hooks, then turn t [x] Hook 1 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap index 753ee2331de3..e65f9535534c 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap @@ -4,7 +4,6 @@ expression: "render_lines(&view, 112)" --- PermissionRequest hooks - Review new or changed hooks, then turn them on or off. No hooks installed for this event. diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events.snap index 096fb562f876..522105c30d9f 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events.snap @@ -6,12 +6,12 @@ expression: "render_lines(&view, 112)" Hooks Lifecycle hooks from config and enabled plugins. - Event Installed Active Review Description - PreToolUse 2 1 0 Before a tool executes - PermissionRequest 1 1 0 When permission is requested - PostToolUse 0 0 0 After a tool executes - SessionStart 0 0 0 When a new session starts - UserPromptSubmit 0 0 0 When the user submits a prompt - Stop 0 0 0 Right before Codex ends its turn + Event Installed Active Description + PreToolUse 2 1 Before a tool executes + PermissionRequest 1 1 When permission is requested + PostToolUse 0 0 After a tool executes + SessionStart 0 0 When a new session starts + UserPromptSubmit 0 0 When the user submits a prompt + Stop 0 0 Right before Codex ends its turn Press enter to view hooks; esc to close diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_issues.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_issues.snap index 8d10909d377e..18e3b9f849ab 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_issues.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_issues.snap @@ -10,12 +10,12 @@ expression: "render_lines(&view, 112)" ⚠ skipped invalid matcher for PreToolUse ■ /tmp/hooks.json: failed to parse hooks config - Event Installed Active Review Description - PreToolUse 0 0 0 Before a tool executes - PermissionRequest 0 0 0 When permission is requested - PostToolUse 0 0 0 After a tool executes - SessionStart 0 0 0 When a new session starts - UserPromptSubmit 0 0 0 When the user submits a prompt - Stop 0 0 0 Right before Codex ends its turn + Event Installed Active Description + PreToolUse 0 0 Before a tool executes + PermissionRequest 0 0 When permission is requested + PostToolUse 0 0 After a tool executes + SessionStart 0 0 When a new session starts + UserPromptSubmit 0 0 When the user submits a prompt + Stop 0 0 Right before Codex ends its turn Press enter to view hooks; esc to close 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 new file mode 100644 index 000000000000..85e930c68c4e --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_review_column.snap @@ -0,0 +1,17 @@ +--- +source: tui/src/bottom_pane/hooks_browser_view.rs +expression: "render_lines(&view, 112)" +--- + + Hooks + Lifecycle hooks from config and enabled plugins. + + Event Installed Active Review Description + PreToolUse 1 0 1 Before a tool executes + PermissionRequest 0 0 0 When permission is requested + PostToolUse 0 0 0 After a tool executes + SessionStart 0 0 0 When a new session starts + 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 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap index 7e5fcf627ff5..62890612dade 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap @@ -4,7 +4,6 @@ expression: "render_lines(&view, 112)" --- PreToolUse hooks - Review new or changed hooks, then turn them on or off. [x] Hook 1 [ ] Hook 2 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap index 48bbd9cd0156..39ed8a6cdabc 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap @@ -4,7 +4,6 @@ expression: "render_lines(&view, 112)" --- PermissionRequest hooks - Review new or changed hooks, then turn them on or off. [x] Hook 1 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap index 1b7c45701b3f..b4a5c117e10a 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap @@ -4,7 +4,7 @@ expression: "render_lines(&view, 112)" --- PreToolUse hooks - Review new or changed hooks, then turn them on or off. + 1 hook needs review before it can run. [!] Hook 1 · new diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap index fe896ea59cb0..c4d42319da53 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap @@ -4,7 +4,6 @@ expression: "render_lines(&view, 112)" --- PreToolUse hooks - Review new or changed hooks, then turn them on or off. [x] Hook 2 [x] Hook 3 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap index 0c5a7ccf7ad6..ac23f90ff673 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap @@ -4,7 +4,6 @@ expression: "render_lines(&view, 112)" --- PreToolUse hooks - Review new or changed hooks, then turn them on or off. [x] Hook 1 [x] Hook 2 diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hooks_popup_shows_list_diagnostics.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hooks_popup_shows_list_diagnostics.snap index 1db52419a1a0..865d19031fbd 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hooks_popup_shows_list_diagnostics.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hooks_popup_shows_list_diagnostics.snap @@ -9,12 +9,12 @@ expression: popup ⚠ skipped invalid matcher for PreToolUse ■ /tmp/hooks.json: failed to parse hooks config - Event Installed Active Review Description - PreToolUse 0 0 0 Before a tool executes - PermissionRequest 0 0 0 When permission is requested - PostToolUse 0 0 0 After a tool executes - SessionStart 0 0 0 When a new session starts - UserPromptSubmit 0 0 0 When the user submits a prompt - Stop 0 0 0 Right before Codex ends its turn + Event Installed Active Description + PreToolUse 0 0 Before a tool executes + PermissionRequest 0 0 When permission is requested + PostToolUse 0 0 After a tool executes + SessionStart 0 0 When a new session starts + UserPromptSubmit 0 0 When the user submits a prompt + Stop 0 0 Right before Codex ends its turn Press enter to view hooks; esc to close From c959b049aec5fad405ae5e8914009cb041974792 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Thu, 30 Apr 2026 15:35:29 -0700 Subject: [PATCH 4/8] Restore base hooks browser guidance --- codex-rs/tui/src/bottom_pane/hooks_browser_view.rs | 6 +++++- ...r_view__tests__hooks_browser_capped_command_details.snap | 1 + ...s_browser_view__tests__hooks_browser_empty_handlers.snap | 1 + ...__hooks_browser_view__tests__hooks_browser_handlers.snap | 1 + ..._browser_view__tests__hooks_browser_managed_handler.snap | 1 + ...rowser_view__tests__hooks_browser_scrolled_handlers.snap | 1 + ...view__tests__hooks_browser_selected_managed_handler.snap | 1 + 7 files changed, 11 insertions(+), 1 deletion(-) 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 26e762e3b52b..84f47d10788a 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -247,7 +247,11 @@ impl HooksBrowserView { ) -> Vec> { let mut lines = vec![format!("{} hooks", event_label(event_name)).bold().into()]; match review_needed_count { - 0 => {} + 0 => 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.") diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap index a747e50f2bfb..808b9dedbf67 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap @@ -4,6 +4,7 @@ expression: "render_lines(&view, 44)" --- PreToolUse hooks + Turn hooks on or off. Your changes are s [x] Hook 1 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap index e65f9535534c..33321eec2be3 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap @@ -4,6 +4,7 @@ expression: "render_lines(&view, 112)" --- PermissionRequest hooks + Turn hooks on or off. Your changes are saved automatically. No hooks installed for this event. diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap index 62890612dade..6e8873498062 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap @@ -4,6 +4,7 @@ expression: "render_lines(&view, 112)" --- PreToolUse hooks + Turn hooks on or off. Your changes are saved automatically. [x] Hook 1 [ ] Hook 2 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap index 39ed8a6cdabc..d073b11b3c2d 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap @@ -4,6 +4,7 @@ expression: "render_lines(&view, 112)" --- PermissionRequest hooks + Turn hooks on or off. Your changes are saved automatically. [x] Hook 1 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap index c4d42319da53..efeb0b240543 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap @@ -4,6 +4,7 @@ expression: "render_lines(&view, 112)" --- PreToolUse hooks + Turn hooks on or off. Your changes are saved automatically. [x] Hook 2 [x] Hook 3 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap index ac23f90ff673..514a8917a440 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap @@ -4,6 +4,7 @@ expression: "render_lines(&view, 112)" --- PreToolUse hooks + Turn hooks on or off. Your changes are saved automatically. [x] Hook 1 [x] Hook 2 From 6c05cc5babae312a533b65774fe9a8bc85340a4c Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Fri, 1 May 2026 14:15:23 -0700 Subject: [PATCH 5/8] Rename changed hook trust state --- codex-rs/tui/src/app/background_requests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index c66a40e7f7bc..570847bd097b 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -114,7 +114,7 @@ impl App { .filter(|hook| { matches!( hook.trust_status, - HookTrustStatus::Untrusted | HookTrustStatus::Changed + HookTrustStatus::Untrusted | HookTrustStatus::Modified ) }) .count() From 5bab3608cf17f39242e79351a3458fa7682f0994 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Fri, 1 May 2026 18:52:08 -0700 Subject: [PATCH 6/8] Align TUI hook trust flow with current hash --- codex-rs/tui/src/app/background_requests.rs | 10 +++++----- codex-rs/tui/src/app/event_dispatch.rs | 4 ++-- codex-rs/tui/src/app_event.rs | 2 +- codex-rs/tui/src/bottom_pane/hooks_browser_view.rs | 5 ++--- ...tests__hooks_browser_untrusted_enabled_handler.snap | 7 ++++--- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index 570847bd097b..73432499340b 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -368,13 +368,13 @@ impl App { &mut self, app_server: &AppServerSession, key: String, - trusted_hash: String, + current_hash: String, enable: bool, ) { let request_handle = app_server.request_handle(); let app_event_tx = self.app_event_tx.clone(); tokio::spawn(async move { - let result = write_hook_trust(request_handle, key, trusted_hash, enable) + let result = write_hook_trust(request_handle, key, current_hash, enable) .await .map(|_| ()) .map_err(|err| format!("Failed to trust hook: {err}")); @@ -868,7 +868,7 @@ pub(super) async fn write_hook_enabled( pub(super) async fn write_hook_trust( request_handle: AppServerRequestHandle, key: String, - trusted_hash: String, + current_hash: String, enable: bool, ) -> Result { let request_id = RequestId::String(format!("hooks-config-write-{}", Uuid::new_v4())); @@ -876,13 +876,13 @@ pub(super) async fn write_hook_trust( serde_json::json!({ key: { "enabled": true, - "trusted_hash": trusted_hash, + "trusted_hash": current_hash, } }) } else { serde_json::json!({ key: { - "trusted_hash": trusted_hash, + "trusted_hash": current_hash, } }) }; diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 6b658fe819cf..35556dc11763 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1694,10 +1694,10 @@ impl App { } AppEvent::TrustHook { key, - trusted_hash, + current_hash, enable, } => { - self.trust_hook(app_server, key, trusted_hash, enable); + self.trust_hook(app_server, key, current_hash, enable); } AppEvent::HookEnabledSet { key, diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 46a62bd0d09d..d2cc30608ca6 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -752,7 +752,7 @@ pub(crate) enum AppEvent { /// Trust the current definition for a hook by stable hook key. TrustHook { key: String, - trusted_hash: String, + current_hash: String, enable: bool, }, 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 84f47d10788a..5c7a9aa6b324 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -922,7 +922,6 @@ mod tests { /*is_managed*/ false, /*display_order*/ 0, ); - untrusted_hook.trusted_hash = None; untrusted_hook.trust_status = HookTrustStatus::Untrusted; let view = HooksBrowserView::new( vec![untrusted_hook], @@ -1316,11 +1315,11 @@ mod tests { match rx.try_recv().expect("trust event") { AppEvent::TrustHook { key, - current_hash: trusted_hash, + current_hash: hash_to_trust, enable, } => { assert_eq!(key, "path:untrusted"); - assert_eq!(trusted_hash, current_hash); + assert_eq!(hash_to_trust, current_hash); assert!(enable); } other => panic!("expected hook trust event, got {other:?}"), diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_untrusted_enabled_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_untrusted_enabled_handler.snap index 76d99758dc89..4fa01776f691 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_untrusted_enabled_handler.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_untrusted_enabled_handler.snap @@ -4,14 +4,15 @@ expression: "render_lines(&view, 112)" --- PreToolUse hooks - Turn hooks on or off. Your changes are saved automatically. + 1 hook needs review before it can run. - [ ] Hook 1 + [!] Hook 1 · new Event PreToolUse Matcher Bash Source User config - /tmp/hooks.json Command ~/bin/untrusted.sh Timeout 30s + Trust New hook - review required - Press space or enter to toggle; esc to go back + Press t to trust; esc to go back From 83db2d30fb9fc46871d896b690f430f52caecd00 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Mon, 4 May 2026 10:54:45 -0700 Subject: [PATCH 7/8] Keep hook trust separate from enablement --- codex-rs/tui/src/app/background_requests.rs | 23 +++------ codex-rs/tui/src/app/event_dispatch.rs | 8 +--- codex-rs/tui/src/app_event.rs | 1 - .../tui/src/bottom_pane/hooks_browser_view.rs | 48 +++++++++++++++---- 4 files changed, 48 insertions(+), 32 deletions(-) diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index 73432499340b..3233db8276db 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -369,12 +369,11 @@ impl App { app_server: &AppServerSession, key: String, current_hash: String, - enable: bool, ) { let request_handle = app_server.request_handle(); let app_event_tx = self.app_event_tx.clone(); tokio::spawn(async move { - let result = write_hook_trust(request_handle, key, current_hash, enable) + let result = write_hook_trust(request_handle, key, current_hash) .await .map(|_| ()) .map_err(|err| format!("Failed to trust hook: {err}")); @@ -869,23 +868,13 @@ pub(super) async fn write_hook_trust( request_handle: AppServerRequestHandle, key: String, current_hash: String, - enable: bool, ) -> Result { let request_id = RequestId::String(format!("hooks-config-write-{}", Uuid::new_v4())); - let value = if enable { - serde_json::json!({ - key: { - "enabled": true, - "trusted_hash": current_hash, - } - }) - } else { - serde_json::json!({ - key: { - "trusted_hash": current_hash, - } - }) - }; + let value = serde_json::json!({ + key: { + "trusted_hash": current_hash, + } + }); request_handle .request_typed(ClientRequest::ConfigBatchWrite { request_id, diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 6b29553e6957..596f2da9cbb1 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1695,12 +1695,8 @@ impl App { AppEvent::SetHookEnabled { key, enabled } => { self.set_hook_enabled(app_server, key, enabled); } - AppEvent::TrustHook { - key, - current_hash, - enable, - } => { - self.trust_hook(app_server, key, current_hash, enable); + AppEvent::TrustHook { key, current_hash } => { + self.trust_hook(app_server, key, current_hash); } AppEvent::HookEnabledSet { key, diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index cc58641b9726..18c2335465b0 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -757,7 +757,6 @@ pub(crate) enum AppEvent { TrustHook { key: String, current_hash: String, - enable: bool, }, /// Result of persisting hook enabled state. 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 5c7a9aa6b324..1ae7e5a2c880 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -201,15 +201,10 @@ impl HooksBrowserView { return; } - let enable = !hook.enabled; hook.trust_status = HookTrustStatus::Trusted; - if enable { - hook.enabled = true; - } self.app_event_tx.send(AppEvent::TrustHook { key: hook.key.clone(), current_hash: hook.current_hash.clone(), - enable, }); } @@ -1289,7 +1284,7 @@ mod tests { } #[test] - fn trust_key_trusts_review_needed_handler() { + fn trust_key_trusts_review_needed_handler_without_changing_enablement() { let (tx_raw, mut rx) = unbounded_channel::(); let mut untrusted_hook = hook( "path:untrusted", @@ -1316,11 +1311,48 @@ mod tests { AppEvent::TrustHook { key, current_hash: hash_to_trust, - enable, } => { assert_eq!(key, "path:untrusted"); assert_eq!(hash_to_trust, current_hash); - assert!(enable); + } + other => panic!("expected hook trust event, got {other:?}"), + } + } + + #[test] + fn trust_key_preserves_disabled_modified_handler() { + let (tx_raw, mut rx) = unbounded_channel::(); + let mut modified_hook = hook( + "path:modified", + HookEventName::PreToolUse, + HookSource::User, + /*plugin_id*/ None, + "/tmp/pre-tool-use-check.sh", + /*enabled*/ false, + /*is_managed*/ false, + /*display_order*/ 0, + ); + modified_hook.trust_status = HookTrustStatus::Modified; + let current_hash = modified_hook.current_hash.clone(); + let mut view = HooksBrowserView::new( + vec![modified_hook], + Vec::new(), + Vec::new(), + AppEventSender::new(tx_raw), + ); + 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"); + assert_eq!(hook.enabled, false); + assert_eq!(hook.trust_status, HookTrustStatus::Trusted); + match rx.try_recv().expect("trust event") { + AppEvent::TrustHook { + key, + current_hash: hash_to_trust, + } => { + assert_eq!(key, "path:modified"); + assert_eq!(hash_to_trust, current_hash); } other => panic!("expected hook trust event, got {other:?}"), } From 246243cccb72de72237d6ca157a8704273ec4c8a Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Mon, 4 May 2026 18:55:57 -0700 Subject: [PATCH 8/8] Fix bool assertion in hooks browser test --- codex-rs/tui/src/bottom_pane/hooks_browser_view.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1ae7e5a2c880..c146bae8b43d 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -1344,7 +1344,7 @@ mod tests { view.handle_key_event(KeyEvent::from(KeyCode::Char('t'))); let hook = view.hooks.first().expect("trusted hook"); - assert_eq!(hook.enabled, false); + assert!(!hook.enabled); assert_eq!(hook.trust_status, HookTrustStatus::Trusted); match rx.try_recv().expect("trust event") { AppEvent::TrustHook {