From 6ef38eb18a05305199adb2e068995b253db34933 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 24 May 2026 01:50:38 -0700 Subject: [PATCH 1/4] Fix TUI hook trust bypass startup prompt (#24093) --- codex-rs/tui/src/startup_hooks_review.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/codex-rs/tui/src/startup_hooks_review.rs b/codex-rs/tui/src/startup_hooks_review.rs index 7c125248962d..fa782aba5b89 100644 --- a/codex-rs/tui/src/startup_hooks_review.rs +++ b/codex-rs/tui/src/startup_hooks_review.rs @@ -56,7 +56,7 @@ pub(crate) async fn maybe_run_startup_hooks_review( } }; let entry = hooks_list_entry_for_cwd(response, &cwd); - if review_needed_count(&entry) == 0 { + if !review_is_needed(config.bypass_hook_trust, &entry) { return Ok(StartupHooksReviewOutcome::Continue); } @@ -223,6 +223,10 @@ fn review_needed_count(entry: &HooksListEntry) -> usize { .count() } +fn review_is_needed(bypass_hook_trust: bool, entry: &HooksListEntry) -> bool { + !bypass_hook_trust && review_needed_count(entry) > 0 +} + fn selection_item(name: &str, is_disabled: bool) -> SelectionItem { SelectionItem { name: name.to_string(), @@ -259,6 +263,7 @@ impl WidgetRef for &StandaloneSelectionView<'_> { #[cfg(test)] mod tests { + use super::review_is_needed; use super::selection_view; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; @@ -333,6 +338,16 @@ mod tests { .join("\n") } + #[test] + fn bypass_hook_trust_suppresses_startup_review() { + assert!(!review_is_needed(/*bypass_hook_trust*/ true, &entry())); + } + + #[test] + fn untrusted_hooks_need_review_without_bypass() { + assert!(review_is_needed(/*bypass_hook_trust*/ false, &entry())); + } + #[test] fn renders_prompt() { let (tx_raw, _rx) = unbounded_channel::(); From 8f691e6c905aded6c1280838436bbbe633a51e6b Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 24 May 2026 02:03:18 -0700 Subject: [PATCH 2/4] codex: address PR review feedback (#24317) Refs #24093. --- codex-rs/app-server/src/config_manager.rs | 12 +++++++-- .../thread_processor_tests.rs | 26 +++++++++++++++++++ codex-rs/tui/src/app_server_session.rs | 20 ++++++++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/codex-rs/app-server/src/config_manager.rs b/codex-rs/app-server/src/config_manager.rs index 25fdc5c0cb84..2cfd53b130d8 100644 --- a/codex-rs/app-server/src/config_manager.rs +++ b/codex-rs/app-server/src/config_manager.rs @@ -216,15 +216,23 @@ impl ConfigManager { &self, cli_overrides: &[(String, TomlValue)], request_overrides: Option>, - typesafe_overrides: ConfigOverrides, + mut typesafe_overrides: ConfigOverrides, fallback_cwd: Option, ) -> std::io::Result { + let mut request_overrides = request_overrides.unwrap_or_default(); + if let Some(value) = request_overrides.remove("bypass_hook_trust") { + typesafe_overrides.bypass_hook_trust = Some(value.as_bool().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + "`bypass_hook_trust` override must be a boolean", + ) + })?); + } let merged_cli_overrides = cli_overrides .iter() .cloned() .chain( request_overrides - .unwrap_or_default() .into_iter() .map(|(key, value)| (key, json_to_toml(value))), ) diff --git a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs index c6b5541aa2d6..7d220abbf793 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs @@ -629,6 +629,32 @@ mod thread_processor_behavior_tests { Ok(()) } + #[tokio::test] + async fn request_overrides_forward_bypass_hook_trust() -> Result<()> { + let temp_dir = TempDir::new()?; + let config_manager = ConfigManager::new( + temp_dir.path().to_path_buf(), + Vec::new(), + LoaderOverrides::default(), + /*strict_config*/ false, + CloudRequirementsLoader::default(), + Arg0DispatchPaths::default(), + Arc::new(StaticThreadConfigLoader::new(Vec::new())), + ); + let config = config_manager + .load_with_overrides( + Some(HashMap::from([( + "bypass_hook_trust".to_string(), + json!(true), + )])), + ConfigOverrides::default(), + ) + .await?; + + assert!(config.bypass_hook_trust); + Ok(()) + } + #[test] fn collect_resume_override_mismatches_includes_service_tier() { let cwd = test_path_buf("/tmp").abs(); diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 59d56ca56eb1..fc42c7e76f16 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -1252,6 +1252,12 @@ fn config_request_overrides_from_config( "web_search", Some(config.web_search_mode.value().to_string()), ); + if config.bypass_hook_trust { + overrides.insert( + "bypass_hook_trust".to_string(), + serde_json::Value::Bool(true), + ); + } Some(overrides) } @@ -2195,6 +2201,20 @@ mod tests { ); } + #[tokio::test] + async fn config_request_overrides_forward_bypass_hook_trust() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let mut config = build_config(&temp_dir).await; + config.bypass_hook_trust = true; + + let overrides = config_request_overrides_from_config(&config).expect("config overrides"); + + assert_eq!( + overrides.get("bypass_hook_trust"), + Some(&serde_json::Value::Bool(true)) + ); + } + #[tokio::test] async fn thread_fork_params_forward_instruction_overrides() { let temp_dir = tempfile::tempdir().expect("tempdir"); From 35b5ab1279ae78abc795cb6de1369b4022fa78ca Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 24 May 2026 02:26:44 -0700 Subject: [PATCH 3/4] codex: simplify PR review fix (#24317) Refs #24093. --- .../thread_processor_tests.rs | 26 +------------------ codex-rs/tui/src/app_server_session.rs | 23 +++------------- 2 files changed, 5 insertions(+), 44 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs index 7d220abbf793..ab8f52ffc028 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs @@ -610,6 +610,7 @@ mod thread_processor_behavior_tests { Some(HashMap::from([ ("model_provider".to_string(), json!("request")), ("features.plugins".to_string(), json!(true)), + ("bypass_hook_trust".to_string(), json!(true)), ( "model_providers.session".to_string(), json!({ @@ -626,31 +627,6 @@ mod thread_processor_behavior_tests { assert_eq!(config.model_provider_id, "session"); assert_eq!(config.model_provider, session_provider); assert!(!config.features.enabled(Feature::Plugins)); - Ok(()) - } - - #[tokio::test] - async fn request_overrides_forward_bypass_hook_trust() -> Result<()> { - let temp_dir = TempDir::new()?; - let config_manager = ConfigManager::new( - temp_dir.path().to_path_buf(), - Vec::new(), - LoaderOverrides::default(), - /*strict_config*/ false, - CloudRequirementsLoader::default(), - Arg0DispatchPaths::default(), - Arc::new(StaticThreadConfigLoader::new(Vec::new())), - ); - let config = config_manager - .load_with_overrides( - Some(HashMap::from([( - "bypass_hook_trust".to_string(), - json!(true), - )])), - ConfigOverrides::default(), - ) - .await?; - assert!(config.bypass_hook_trust); Ok(()) } diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index fc42c7e76f16..e61d815cab2f 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -1253,10 +1253,7 @@ fn config_request_overrides_from_config( Some(config.web_search_mode.value().to_string()), ); if config.bypass_hook_trust { - overrides.insert( - "bypass_hook_trust".to_string(), - serde_json::Value::Bool(true), - ); + overrides.insert("bypass_hook_trust".to_string(), true.into()); } Some(overrides) } @@ -2130,7 +2127,7 @@ mod tests { } #[tokio::test] - async fn thread_lifecycle_params_forward_model_reasoning_and_service_tier() { + async fn thread_lifecycle_params_forward_config_overrides_and_service_tier() { let temp_dir = tempfile::tempdir().expect("tempdir"); let mut config = build_config(&temp_dir).await; config.model_reasoning_effort = Some(ReasoningEffort::High); @@ -2141,6 +2138,7 @@ mod tests { .web_search_mode .set(WebSearchMode::Disabled) .expect("test web search mode should be allowed"); + config.bypass_hook_trust = true; config.service_tier = Some(ServiceTier::Fast.request_value().to_string()); let thread_id = ThreadId::new(); @@ -2174,6 +2172,7 @@ mod tests { ("model_verbosity".to_string(), string("low")), ("personality".to_string(), string("pragmatic")), ("web_search".to_string(), string("disabled")), + ("bypass_hook_trust".to_string(), true.into()), ]); assert_eq!(start.config, Some(expected_config.clone())); assert_eq!(resume.config, Some(expected_config.clone())); @@ -2201,20 +2200,6 @@ mod tests { ); } - #[tokio::test] - async fn config_request_overrides_forward_bypass_hook_trust() { - let temp_dir = tempfile::tempdir().expect("tempdir"); - let mut config = build_config(&temp_dir).await; - config.bypass_hook_trust = true; - - let overrides = config_request_overrides_from_config(&config).expect("config overrides"); - - assert_eq!( - overrides.get("bypass_hook_trust"), - Some(&serde_json::Value::Bool(true)) - ); - } - #[tokio::test] async fn thread_fork_params_forward_instruction_overrides() { let temp_dir = tempfile::tempdir().expect("tempdir"); From e07de3d26d2ee41d413796e9dd208d47cc5edc79 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 24 May 2026 02:34:52 -0700 Subject: [PATCH 4/4] codex: keep hook review for persistent resumes (#24317) Refs #24093. --- codex-rs/tui/src/lib.rs | 24 +++++++++++++++++++----- codex-rs/tui/src/startup_hooks_review.rs | 3 ++- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 7348e3b452ff..2683d3796a87 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -1706,11 +1706,25 @@ 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), - }; + // Persistent app-server resumes may attach to an already-running thread, + // where resume config overrides are ignored. + let is_persistent_resume = !matches!(&app_server_target, AppServerTarget::Embedded) + && matches!( + &session_selection, + resume_picker::SessionSelection::Resume(_) + ); + let bypass_hook_trust_for_startup_review = config.bypass_hook_trust && !is_persistent_resume; + let startup_hooks_browser = match maybe_run_startup_hooks_review( + &mut app_server, + &mut tui, + &config, + bypass_hook_trust_for_startup_review, + ) + .await? + { + StartupHooksReviewOutcome::Continue => None, + StartupHooksReviewOutcome::OpenHooksBrowser(data) => Some(data), + }; let app_result = App::run( &mut tui, diff --git a/codex-rs/tui/src/startup_hooks_review.rs b/codex-rs/tui/src/startup_hooks_review.rs index fa782aba5b89..559c796aa0d7 100644 --- a/codex-rs/tui/src/startup_hooks_review.rs +++ b/codex-rs/tui/src/startup_hooks_review.rs @@ -46,6 +46,7 @@ pub(crate) async fn maybe_run_startup_hooks_review( app_server: &mut AppServerSession, tui: &mut Tui, config: &Config, + bypass_hook_trust: bool, ) -> Result { let cwd = config.cwd.to_path_buf(); let response = match fetch_hooks_list(app_server.request_handle(), cwd.clone()).await { @@ -56,7 +57,7 @@ pub(crate) async fn maybe_run_startup_hooks_review( } }; let entry = hooks_list_entry_for_cwd(response, &cwd); - if !review_is_needed(config.bypass_hook_trust, &entry) { + if !review_is_needed(bypass_hook_trust, &entry) { return Ok(StartupHooksReviewOutcome::Continue); }