From f7ef9599eddade2c4d1763fcc04581315994dbc6 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 28 Mar 2026 16:01:04 -0700 Subject: [PATCH] exec: make review-policy tests hermetic (#16137) ## Why `thread_start_params_from_config()` is supposed to forward the effective `approvals_reviewer` into the app-server request, but these tests were constructing that config through `ConfigBuilder::build()`, which also loads ambient system and managed config layers. On machines with an admin or host-level reviewer override, the manual-only case could inherit `guardian_subagent` and fail even though the exec-side mapping was correct. ## What changed - Set `approvals_reviewer` explicitly via `harness_overrides` in the two `thread_start_params_*review_policy*` tests in `codex-rs/exec/src/lib.rs`. - Removed the dependence on default config resolution and temp `config.toml` writes so the tests exercise only the reviewer-to-request mapping in `codex-exec`. ## Testing - `cargo test -p codex-exec` --- codex-rs/exec/src/lib.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 84f1685d9..71a37cc6a 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -1975,10 +1975,14 @@ mod tests { let cwd = tempdir().expect("create temp cwd"); let config = ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) + .harness_overrides(ConfigOverrides { + approvals_reviewer: Some(ApprovalsReviewer::User), + ..Default::default() + }) .fallback_cwd(Some(cwd.path().to_path_buf())) .build() .await - .expect("build default config"); + .expect("build config with manual-only review policy"); let params = thread_start_params_from_config(&config); @@ -1992,17 +1996,16 @@ mod tests { async fn thread_start_params_include_review_policy_when_auto_review_is_enabled() { let codex_home = tempdir().expect("create temp codex home"); let cwd = tempdir().expect("create temp cwd"); - std::fs::write( - codex_home.path().join("config.toml"), - "approvals_reviewer = \"guardian_subagent\"\n", - ) - .expect("write auto-review config"); let config = ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) + .harness_overrides(ConfigOverrides { + approvals_reviewer: Some(ApprovalsReviewer::GuardianSubagent), + ..Default::default() + }) .fallback_cwd(Some(cwd.path().to_path_buf())) .build() .await - .expect("build auto-review config"); + .expect("build config with guardian review policy"); let params = thread_start_params_from_config(&config);