diff --git a/codex-rs/core/src/tasks/review.rs b/codex-rs/core/src/tasks/review.rs index aca1d259f..c58af2678 100644 --- a/codex-rs/core/src/tasks/review.rs +++ b/codex-rs/core/src/tasks/review.rs @@ -7,6 +7,7 @@ use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::AgentMessageContentDeltaEvent; use codex_protocol::protocol::AgentMessageDeltaEvent; +use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExitedReviewModeEvent; @@ -95,6 +96,7 @@ async fn start_review_conversation( // Set explicit review rubric for the sub-agent sub_agent_config.base_instructions = Some(crate::REVIEW_PROMPT.to_string()); + sub_agent_config.approval_policy = Constrained::allow_only(AskForApproval::Never); let model = config .review_model diff --git a/codex-rs/core/src/tools/handlers/collab.rs b/codex-rs/core/src/tools/handlers/collab.rs index 92780eee8..aef8e3cc7 100644 --- a/codex-rs/core/src/tools/handlers/collab.rs +++ b/codex-rs/core/src/tools/handlers/collab.rs @@ -3,6 +3,7 @@ use crate::agent::exceeds_thread_spawn_depth_limit; use crate::codex::Session; use crate::codex::TurnContext; use crate::config::Config; +use crate::config::Constrained; use crate::error::CodexErr; use crate::features::Feature; use crate::function_tool::FunctionCallError; @@ -16,6 +17,7 @@ use async_trait::async_trait; use codex_protocol::ThreadId; use codex_protocol::models::BaseInstructions; use codex_protocol::models::FunctionCallOutputBody; +use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::CollabAgentInteractionBeginEvent; use codex_protocol::protocol::CollabAgentInteractionEndEvent; use codex_protocol::protocol::CollabAgentSpawnBeginEvent; @@ -819,12 +821,7 @@ fn build_agent_shared_config( config.shell_environment_policy = turn.shell_environment_policy.clone(); config.codex_linux_sandbox_exe = turn.codex_linux_sandbox_exe.clone(); config.cwd = turn.cwd.clone(); - config - .approval_policy - .set(turn.approval_policy) - .map_err(|err| { - FunctionCallError::RespondToModel(format!("approval_policy is invalid: {err}")) - })?; + config.approval_policy = Constrained::allow_only(AskForApproval::Never); config .sandbox_policy .set(turn.sandbox_policy.clone()) @@ -1690,22 +1687,6 @@ mod tests { #[tokio::test] async fn build_agent_spawn_config_uses_turn_context_values() { - fn pick_allowed_approval_policy( - constraint: &crate::config::Constrained, - base: AskForApproval, - ) -> AskForApproval { - let candidates = [ - AskForApproval::Never, - AskForApproval::UnlessTrusted, - AskForApproval::OnRequest, - AskForApproval::OnFailure, - ]; - candidates - .into_iter() - .find(|candidate| *candidate != base && constraint.can_set(candidate).is_ok()) - .unwrap_or(base) - } - fn pick_allowed_sandbox_policy( constraint: &crate::config::Constrained, base: SandboxPolicy, @@ -1734,10 +1715,6 @@ mod tests { let temp_dir = tempfile::tempdir().expect("temp dir"); turn.cwd = temp_dir.path().to_path_buf(); turn.codex_linux_sandbox_exe = Some(PathBuf::from("/bin/echo")); - turn.approval_policy = pick_allowed_approval_policy( - &turn.config.approval_policy, - *turn.config.approval_policy.get(), - ); turn.sandbox_policy = pick_allowed_sandbox_policy( &turn.config.sandbox_policy, turn.config.sandbox_policy.get().clone(), @@ -1757,7 +1734,7 @@ mod tests { expected.cwd = turn.cwd.clone(); expected .approval_policy - .set(turn.approval_policy) + .set(AskForApproval::Never) .expect("approval policy set"); expected .sandbox_policy @@ -1804,7 +1781,7 @@ mod tests { expected.cwd = turn.cwd.clone(); expected .approval_policy - .set(turn.approval_policy) + .set(AskForApproval::Never) .expect("approval policy set"); expected .sandbox_policy diff --git a/codex-rs/core/tests/suite/codex_delegate.rs b/codex-rs/core/tests/suite/codex_delegate.rs index ea759ba5f..994556814 100644 --- a/codex-rs/core/tests/suite/codex_delegate.rs +++ b/codex-rs/core/tests/suite/codex_delegate.rs @@ -24,6 +24,7 @@ use pretty_assertions::assert_eq; /// Delegate should surface ExecApprovalRequest from sub-agent and proceed /// after parent submits an approval decision. +#[ignore = "TODO once we have a delegate that can ask for approvals"] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() { skip_if_no_network!(); @@ -114,6 +115,7 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() { /// Delegate should surface ApplyPatchApprovalRequest and honor parent decision /// so the sub-agent can proceed to completion. +#[ignore = "TODO once we have a delegate that can ask for approvals"] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn codex_delegate_forwards_patch_approval_and_proceeds_on_decision() { skip_if_no_network!();