Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions codex-rs/app-server/src/config_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,15 +216,23 @@ impl ConfigManager {
&self,
cli_overrides: &[(String, TomlValue)],
request_overrides: Option<HashMap<String, serde_json::Value>>,
typesafe_overrides: ConfigOverrides,
mut typesafe_overrides: ConfigOverrides,
fallback_cwd: Option<PathBuf>,
) -> std::io::Result<Config> {
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))),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!({
Expand All @@ -626,6 +627,7 @@ 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));
assert!(config.bypass_hook_trust);
Ok(())
}

Expand Down
7 changes: 6 additions & 1 deletion codex-rs/tui/src/app_server_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1252,6 +1252,9 @@ 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(), true.into());
}
Some(overrides)
}

Expand Down Expand Up @@ -2124,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);
Expand All @@ -2135,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();

Expand Down Expand Up @@ -2168,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()));
Expand Down
24 changes: 19 additions & 5 deletions codex-rs/tui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 17 additions & 1 deletion codex-rs/tui/src/startup_hooks_review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<StartupHooksReviewOutcome> {
let cwd = config.cwd.to_path_buf();
let response = match fetch_hooks_list(app_server.request_handle(), cwd.clone()).await {
Expand All @@ -56,7 +57,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(bypass_hook_trust, &entry) {
return Ok(StartupHooksReviewOutcome::Continue);
}

Expand Down Expand Up @@ -223,6 +224,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(),
Expand Down Expand Up @@ -259,6 +264,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;
Expand Down Expand Up @@ -333,6 +339,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::<AppEvent>();
Expand Down
Loading